Skip to content

orchestrator: Check that service's SpecVersion exists before dereferencing#2233

Merged
nishanttotla merged 1 commit intomoby:masterfrom
aaronlehmann:spec-version-sanity
Jun 9, 2017
Merged

orchestrator: Check that service's SpecVersion exists before dereferencing#2233
nishanttotla merged 1 commit intomoby:masterfrom
aaronlehmann:spec-version-sanity

Conversation

@aaronlehmann
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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