Skip to content
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

Merged
merged 1 commit into from
Jun 9, 2017

Conversation

aaronlehmann
Copy link
Collaborator

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.

…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>
Copy link
Contributor

@cyli cyli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@codecov
Copy link

codecov bot commented Jun 9, 2017

Codecov Report

Merging #2233 into master will increase coverage by 0.49%.
The diff coverage is n/a.

@@            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

Copy link
Member

@cpuguy83 cpuguy83 left a 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

👍

Copy link
Contributor

@nishanttotla nishanttotla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nishanttotla nishanttotla merged commit 42cab91 into moby:master Jun 9, 2017
@aaronlehmann aaronlehmann deleted the spec-version-sanity branch June 9, 2017 20:18
@cyli cyli added this to the 17.06 milestone Jun 9, 2017
silvin-lubecki pushed a commit to silvin-lubecki/docker-ce that referenced this pull request Feb 3, 2020
silvin-lubecki pushed a commit to silvin-lubecki/engine-extract that referenced this pull request Feb 3, 2020
silvin-lubecki pushed a commit to silvin-lubecki/engine-extract that referenced this pull request Mar 10, 2020
silvin-lubecki pushed a commit to silvin-lubecki/engine-extract that referenced this pull request Mar 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants