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

Remove driver endpoints on network deleting #1333

Merged
merged 1 commit into from
Oct 26, 2016

Conversation

coolljt0725
Copy link
Contributor

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

On driver deleting network, we should also remove the endpoints belong to it if
there are endpoints remain, or else the endpoints in the store would never been removed.
ping @aboch

@aboch
Copy link
Contributor

aboch commented Jul 19, 2016

Agree with the idea, but this covers only a corner case where the drivers' endpoints are out of sync with libnetwork endpoints. During normal operation there should never be stale driver endpoints on Deletenetwork().

The stale endpoints would happen on ungraceful shutdown of the daemon. And the code to get rid of the stale endpoints in already in place when the drivers initialize.

I am not against the change, but I would not include it in docker 1.12 yet.

@coolljt0725
Copy link
Contributor Author

@aboch I'm ok with this not included in docker 1.12

@aboch
Copy link
Contributor

aboch commented Sep 7, 2016

LGTM
I requested one more change

@@ -753,6 +753,17 @@ func (d *driver) DeleteNetwork(nid string) error {
config := n.config
n.Unlock()

// delele endpoints belong to this network
for _, ep := range n.endpoints {
Copy link
Contributor

Choose a reason for hiding this comment

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

@coolljt0725
Just to be safe, can you please also add the following:

if err := n.releasePorts(endpoint); err != nil {
        logrus.Warn(err)
    }

@coolljt0725 coolljt0725 force-pushed the cleanup_driver_endpoint branch from cac00d7 to 045f49e Compare October 14, 2016 01:38
@coolljt0725
Copy link
Contributor Author

@aboch updated

@@ -753,6 +753,20 @@ func (d *driver) DeleteNetwork(nid string) error {
config := n.config
n.Unlock()

// delele endpoints belong to this network
for _, ep := range n.endpoints {
if err := n.releasePorts(endpoint); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

endpoint -> ep

@aboch
Copy link
Contributor

aboch commented Oct 14, 2016

Thanks, also pls rebase to latest so that CI would get a chance to run.

@coolljt0725 coolljt0725 force-pushed the cleanup_driver_endpoint branch from 045f49e to 2027525 Compare October 14, 2016 02:00
Signed-off-by: Lei Jitang <leijitang@huawei.com>
@coolljt0725 coolljt0725 force-pushed the cleanup_driver_endpoint branch from 2027525 to b757f57 Compare October 14, 2016 02:06
@coolljt0725
Copy link
Contributor Author

@aboch rebased

@aboch
Copy link
Contributor

aboch commented Oct 14, 2016

Thanks @coolljt0725

LGTM

@aboch aboch merged commit 0bcd2d6 into moby:master Oct 26, 2016
@coolljt0725 coolljt0725 deleted the cleanup_driver_endpoint branch October 27, 2016 07:42
luzhipeng-zte pushed a commit to luzhipeng-zte/libnetwork that referenced this pull request Feb 28, 2018
… of LinkByName is NULL

Signed-off-by: ZhiPeng Lu <lu.zhipeng@zte.com.cn>
fcrisciani pushed a commit that referenced this pull request Feb 28, 2018
fix for #1333, calling LinkDel to delete link device when the err is NULL
luzhipeng-zte pushed a commit to luzhipeng-zte/libnetwork that referenced this pull request Feb 28, 2018
… of LinkByName is NULL

Signed-off-by: ZhiPeng Lu <lu.zhipeng@zte.com.cn>
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.

3 participants