Apply update strategies app-wide #2230
Conversation
cywang117
force-pushed
the
multi-container-state-apply
branch
2 times, most recently
from
c83496d to
369c358
Compare
cywang117
changed the title
pipex
force-pushed
the
multi-container-state-apply
branch
from
369c358 to
a908942
Compare
cywang117
force-pushed
the
multi-container-state-apply
branch
from
a908942 to
565f373
Compare
|
Could you add a test for this too? #2095 |
That should be covered under The test case defines current services A & B, and target services A & C. While C is still downloading, the test expects 2 noop steps, one for each of service A & B. When C has finished downloading, only then does the test expect the kill step. |
cywang117
force-pushed
the
multi-container-state-apply
branch
from
565f373 to
9eb75ef
Compare
|
@pipex I made some changes following your feedback. The new changes make it so that the Supervisor fetches new target services even if the update strategy is |
src/compose/app.ts
Outdated
| // if kill|delete-then-download, don't kill any services | ||
| // before new images in target have been downloaded. | ||
| // Services are safe to kill if they are in both current and target | ||
| // even if the target image isn't yet ready. |
There was a problem hiding this comment.
I don't understand why this is needed. Result of dependenciesMetForServiceKill is not used by getStepsForStrategy if the strategy is kill-then-download or delete-then-download so this code would be redundant. Also this doesn't seem like the most efficient alternative considering this function is called for every service.
There was a problem hiding this comment.
Oh, wait, sorry, you changed the getStepsForStrategy but function, but I still don't understand why
cywang117
force-pushed
the
multi-container-state-apply
branch
from
96291b9 to
e0d5a5f
Compare
Fixes behavior for release updates which removes a service in current state and adds a new service in target state. Change-type: patch Closes: #2095 Signed-off-by: Christina Ying Wang <[email protected]>
cywang117
force-pushed
the
multi-container-state-apply
branch
from
e0d5a5f to
3afcef2
Compare
The Supervisor's state engine has some edge cases where it does not respect the update strategy of the app. These edge cases relate to a release update which removes an old service and also adds a new service. This PR fixes those edge cases.
See: https://balena.fibery.io/Work/Improvement/Supervisor-improved-multi-container-updates-1329
Change-type: patch
Closes: #2095
Signed-off-by: Christina Ying Wang [email protected]