-
Notifications
You must be signed in to change notification settings - Fork 612
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
orchestrator: Check that service's SpecVersion exists before dereferencing #2233
Conversation
…ncing IsTaskDirty assumed that if a task had a SpecVersion, the corresponding service must also have a SpecVersion, since that is where the task's field is copied from. However, there are mixed-version scenarios where a service could get updated by a manager running an older version of swarmkit than the manager that created the task. Add a check to avoid a crash in this case. Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Codecov Report
@@ Coverage Diff @@
## master #2233 +/- ##
==========================================
+ Coverage 60.23% 60.72% +0.49%
==========================================
Files 124 124
Lines 20156 20156
==========================================
+ Hits 12141 12240 +99
+ Misses 6658 6543 -115
- Partials 1357 1373 +16 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Cool, I didn't want to just submit a PR to do this since I wasn't sure if the bug is that it sent a nil version or not
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
- moby/swarmkit#2218 - moby/swarmkit#2215 - moby/swarmkit#2233 Signed-off-by: Ying <ying.li@docker.com>
- moby/swarmkit#2218 - moby/swarmkit#2215 - moby/swarmkit#2233 Signed-off-by: Ying <ying.li@docker.com>
- moby/swarmkit#2218 - moby/swarmkit#2215 - moby/swarmkit#2233 Signed-off-by: Ying <ying.li@docker.com>
- moby/swarmkit#2218 - moby/swarmkit#2215 - moby/swarmkit#2233 Signed-off-by: Ying <ying.li@docker.com>
IsTaskDirty
assumed that if a task had aSpecVersion
, the corresponding service must also have aSpecVersion
, since that is where the task's field is copied from. However, there are mixed-version scenarios where a service could get updated by a manager running an older version of swarmkit than the manager that created the task. Add a check to avoid a crash in this case.