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.

Sign up for GitHub

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

bugfix: fix remove all endpoints when network disconnect #1284

Merged
merged 1 commit into from
May 11, 2018
Merged

bugfix: fix remove all endpoints when network disconnect #1284

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 48 additions & 13 deletions daemon/mgr/network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ func (nm *NetworkManager) EndpointCreate(ctx context.Context, endpoint *types.En
defer func() {
if err != nil {
if err := ep.Delete(true); err != nil {
logrus.Errorf("could not delete endpoint %s after failing to create endpoint: %v", ep.Name(), err)
logrus.Errorf("could not delete endpoint %s after failing to create endpoint(%v)", ep.Name(), err)
}
}
}()
Expand All @@ -282,12 +282,12 @@ func (nm *NetworkManager) EndpointCreate(ctx context.Context, endpoint *types.En
if sb == nil {
sandboxOptions, err := nm.sandboxOptions(endpoint)
if err != nil {
return "", fmt.Errorf("failed to build sandbox options: %v", err)
return "", fmt.Errorf("failed to build sandbox options(%v)", err)
}

sb, err = nm.controller.NewSandbox(containerID, sandboxOptions...)
if err != nil {
return "", fmt.Errorf("failed to create sandbox: %v", err)
return "", fmt.Errorf("failed to create sandbox(%v)", err)
}
}
networkConfig.SandboxID = sb.ID()
Expand All @@ -299,7 +299,7 @@ func (nm *NetworkManager) EndpointCreate(ctx context.Context, endpoint *types.En
return "", err
}
if err := ep.Join(sb, joinOptions...); err != nil {
return "", fmt.Errorf("failed to join sandbox: %v", err)
return "", fmt.Errorf("failed to join sandbox(%v)", err)
}

// update endpoint settings
Expand Down Expand Up @@ -332,29 +332,64 @@ func (nm *NetworkManager) EndpointCreate(ctx context.Context, endpoint *types.En

// EndpointRemove is used to remove network endpoint.
func (nm *NetworkManager) EndpointRemove(ctx context.Context, endpoint *types.Endpoint) error {
var (
ep libnetwork.Endpoint
)

sid := endpoint.NetworkConfig.SandboxID
epConfig := endpoint.EndpointConfig

logrus.Debugf("remove endpoint: %s on network: %s", epConfig.EndpointID, endpoint.Name)
logrus.Debugf("remove endpoint(%s) on network(%s)", epConfig.EndpointID, endpoint.Name)

if sid == "" {
return nil
}

// find endpoint in network and delete it.
sb, err := nm.controller.SandboxByID(sid)
if err == nil {
if err := sb.Delete(); err != nil {
logrus.Errorf("failed to delete sandbox id: %s, err: %v", sid, err)
return err
if err != nil {
return errors.Wrapf(err, "failed to get sandbox by id(%s)", sid)
}
if sb == nil {
return errors.Errorf("failed to get sandbox by id(%s)", sid)
}

eplist := sb.Endpoints()
if len(eplist) == 0 {
return errors.Errorf("no endpoint in sandbox(%s)", sid)
}

for _, e := range eplist {
if e.ID() == epConfig.EndpointID {
ep = e
break
}
} else if _, ok := err.(networktypes.NotFoundError); !ok {
logrus.Errorf("failed to get sandbox id: %s, err: %v", sid, err)
return fmt.Errorf("failed to get sandbox id: %s, err: %v", sid, err)
}

if ep == nil {
return errors.Errorf("not connected to the network(%s)", endpoint.Name)
}

if err := ep.Leave(sb); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

In this pr, you said remove all endpoints when network disconnect,

However, I found that it only deletes one endpoint. Is it correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I mean the bug is remove all endpoints when network disconnect, the correct result is just remove the endpoint in this network.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So I think the pr title is some kind of misleading. 😃

return errors.Wrapf(err, "failed to leave network(%s)", endpoint.Name)
}

if err := ep.Delete(false); err != nil {
return errors.Wrapf(err, "failed to delete endpoint(%s)", endpoint.ID)
}

// clean endpoint configure data
nm.cleanEndpointConfig(epConfig)

// check sandbox has endpoint or not.
eplist = sb.Endpoints()
if len(eplist) == 0 {
if err := sb.Delete(); err != nil {
logrus.Errorf("failed to delete sandbox id(%s), err(%v)", sid, err)
return errors.Wrapf(err, "failed to delete sandbox id(%s)", sid)
}
}

return nil
}

Expand Down Expand Up @@ -456,7 +491,7 @@ func getIpamConfig(data []apitypes.IPAMConfig) ([]*libnetwork.IpamConf, []*libne
iCfg.AuxAddresses = d.AuxAddress
ip, _, err := net.ParseCIDR(d.Subnet)
if err != nil {
return nil, nil, fmt.Errorf("Invalid subnet %s : %v", d.Subnet, err)
return nil, nil, fmt.Errorf("Invalid subnet(%s), err(%v)", d.Subnet, err)
}
if ip.To4() != nil {
ipamV4Cfg = append(ipamV4Cfg, &iCfg)
Expand Down