Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix #19477, clean up the ports when release network #19483

Merged
merged 1 commit into from
Jan 21, 2016

Conversation

coolljt0725
Copy link
Contributor

Signed-off-by: Lei Jitang leijitang@huawei.com

Closes #19477

ping @ibuildthecloud @aboch @mavenugo

@vdemeester
Copy link
Member

LGTM 🐹

@@ -999,6 +999,9 @@ func (daemon *Daemon) releaseNetwork(container *container.Container) {

sid := container.NetworkSettings.SandboxID
settings := container.NetworkSettings.Networks
if container.NetworkSettings.Ports != nil {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe just set container.NetworkSettings.Ports = nil no matter what and avoid the if branch.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@cpuguy83
Copy link
Member

Seems ok to me but pinging @mavenugo @mrjana

@tiborvass
Copy link
Contributor

LGTM apart from minor nit, but if we need to merge as-is i'm fine too.

@mavenugo
Copy link
Contributor

LGTM. @tiborvass +1.

@coolljt0725
Copy link
Contributor Author

updated as @cpuguy83 's suggestion

@cpuguy83
Copy link
Member

LGTM

Signed-off-by: Lei Jitang <leijitang@huawei.com>
vdemeester added a commit that referenced this pull request Jan 21, 2016
Fix #19477, clean up the ports when release network
@vdemeester vdemeester merged commit 476edba into moby:master Jan 21, 2016
@coolljt0725 coolljt0725 deleted the fix_19477 branch January 22, 2016 09:25
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants