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

endpoint_cnt store updates should not create an object #2004

Merged
merged 1 commit into from Nov 6, 2017

Conversation

Projects
None yet
3 participants
@mavenugo
Contributor

mavenugo commented Oct 27, 2017

endpoint_cnt object is created during network create and destroyed when
network is deleted. But the updateToStore function creates an object
when it is not present in the store. endpoint_cnt is a mutable object
and is updated during endpoint create and delete events. If endpoint
create or delete happens after the network is deleted, it can
incorrectly create an endpoint_cnt object in the store and that can
cause problems when the same network is created again later.

The fix is to not create the endpoint_cnt object when endpoint_cnt is
incremented or decremented

This handles the error seen in moby/moby#35310.
But I think moby/moby#35310 needs a more complete fix which is not handled in this PR

Signed-off-by: Madhu Venugopal madhu@docker.com

endpoint_cnt store updates should not create an object
endpoint_cnt object is created during network create and destroyed when
network is deleted. But the updateToStore function creates an object
when it is not present in the store. endpoint_cnt is a mutable object
and is updated during endpoint create and delete events. If endpoint
create or delete happens after the network is deleted, it can
incorrectly create an endpoint_cnt object in the store and that can
cause problems when the same network is created again later.

The fix is to not create the endpoint_cnt object when endpoint_cnt is
incremented or decremented

Signed-off-by: Madhu Venugopal <madhu@docker.com>
@mavenugo

This comment has been minimized.

Show comment
Hide comment
@mavenugo
Contributor

mavenugo commented Oct 27, 2017

@abhi

This comment has been minimized.

Show comment
Hide comment
@abhi

abhi Oct 27, 2017

Member

LGTM. Fix has been verified locally.

Member

abhi commented Oct 27, 2017

LGTM. Fix has been verified locally.

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Oct 27, 2017

Codecov Report

❗️ No coverage uploaded for pull request base (master@dd20549). Click here to learn what that means.
The diff coverage is 22.22%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2004   +/-   ##
=========================================
  Coverage          ?   38.05%           
=========================================
  Files             ?      137           
  Lines             ?    27367           
  Branches          ?        0           
=========================================
  Hits              ?    10415           
  Misses            ?    15680           
  Partials          ?     1272
Impacted Files Coverage Δ
store.go 59.28% <0%> (ø)
endpoint_cnt.go 81.51% <25%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd20549...f13e15b. Read the comment docs.

codecov-io commented Oct 27, 2017

Codecov Report

❗️ No coverage uploaded for pull request base (master@dd20549). Click here to learn what that means.
The diff coverage is 22.22%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2004   +/-   ##
=========================================
  Coverage          ?   38.05%           
=========================================
  Files             ?      137           
  Lines             ?    27367           
  Branches          ?        0           
=========================================
  Hits              ?    10415           
  Misses            ?    15680           
  Partials          ?     1272
Impacted Files Coverage Δ
store.go 59.28% <0%> (ø)
endpoint_cnt.go 81.51% <25%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd20549...f13e15b. Read the comment docs.

@mavenugo mavenugo merged commit 6bbcd1b into docker:master Nov 6, 2017

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
dco-signed All commits are signed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment