Skip to content

[release/v0.9] Handle cleanup DNS for attachable container - #1999

Merged
mavenugo merged 2 commits into
moby:release/v0.9from
andrewhsu:att
Oct 24, 2017
Merged

mavenugo merged 2 commits into
moby:release/v0.9from
andrewhsu:att

Conversation

@andrewhsu

@andrewhsu andrewhsu commented Oct 23, 2017 •

Copy link
Copy Markdown
Contributor

backport:

with cherry-pick of git commits 1c04e19 52a9ab5:

$ git cherry-pick -s -x 1c04e19 52a9ab5

conflict with file service_common.go resolved by adding functions:

  • func (c *controller) getLBIndex(sid, nid string, ingressPorts []*PortConfig) int
  • func (c *controller) cleanupServiceDiscovery(cleanupNID string)

@andrewhsu

Copy link
Copy Markdown
Contributor Author

@fcrisciani @abhi @mavenugo

@andrewhsu andrewhsu changed the title [17.03] Handle cleanup DNS for attachable container [release/v0.9] Handle cleanup DNS for attachable container Oct 23, 2017
Comment thread service_common.go Outdated
}
}

func (c *controller) getLBIndex(sid, nid string, ingressPorts []*PortConfig) int {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This function is not used by this commit. I dont think adding this new function to resolve the conflict is appropriate.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i've updated the commit a75ba12 to not include getLBIndex in service_common.go

@mavenugo mavenugo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The conflict resolution needs some care.

Flavio Crisciani added 2 commits October 23, 2017 17:03
Attachable containers they are tasks with no service associated
their cleanup was not done properly so it was possible to have
a leak of their name resolution if that was the last container
on the network.
Cleanupservicebindings was not able to do the cleanup because there
is no service, while also the notification of the delete arrives
after that the network is already being cleaned

Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
(cherry picked from commit 1c04e19)

Conflicts:
service_common.go

Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
Unit test for the cleanupServiceDiscovery,
follow up of PR: #1985

Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
(cherry picked from commit 52a9ab5)
Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
@fcrisciani

Copy link
Copy Markdown

looks good now, thanks

@mavenugo mavenugo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@mavenugo
mavenugo merged commit 1dea4e0 into moby:release/v0.9 Oct 24, 2017
@andrewhsu
andrewhsu deleted the att branch October 24, 2017 18:20
@mghazizadeh

Copy link
Copy Markdown

I believe we have e2e tests for this as well that can now be run automated using testkit. @antonybichon17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants