Skip to content

Commit

Permalink
Ensuring endpoint resources are freed even on delete failures
Browse files Browse the repository at this point in the history
Came across a code path where we might not be releasing ip
address assigned to an endpoint if we have a failure with
deleteEndpoint. Even if there is a failure it is better to
release the resource rather than holding them. This might
lead to issues where ip never gets released even though
the container has exited and the only way of recovery is a
reload.

Signed-off-by: Abhinandan Prativadi <[email protected]>
  • Loading branch information
abhi committed Aug 15, 2017
1 parent 6426d1e commit 6950c76
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 11 deletions.
2 changes: 1 addition & 1 deletion endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,7 @@ func (ep *endpoint) Delete(force bool) error {
}
}

if err = n.getController().deleteFromStore(ep); err != nil {
if err = n.getController().deleteFromStore(ep); err != nil && !force {
return err
}

Expand Down
16 changes: 7 additions & 9 deletions sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,10 @@ func (sb *sandbox) Statistics() (map[string]*types.InterfaceStatistics, error) {
}

func (sb *sandbox) Delete() error {
return sb.delete(false)
return sb.delete()
}

func (sb *sandbox) delete(force bool) error {
func (sb *sandbox) delete() error {
sb.Lock()
if sb.inDelete {
sb.Unlock()
Expand All @@ -208,10 +208,10 @@ func (sb *sandbox) delete(force bool) error {
retain := false
for _, ep := range sb.getConnectedEndpoints() {
// gw network endpoint detach and removal are automatic
if ep.endpointInGWNetwork() && !force {
if ep.endpointInGWNetwork() {
continue
}
// Retain the sanbdox if we can't obtain the network from store.
// Retain the sandbox if we can't obtain the network from store.
if _, err := c.getNetworkFromStore(ep.getNetwork().ID()); err != nil {
if c.isDistributedControl() {
retain = true
Expand All @@ -220,13 +220,11 @@ func (sb *sandbox) delete(force bool) error {
continue
}

if !force {
if err := ep.Leave(sb); err != nil {
logrus.Warnf("Failed detaching sandbox %s from endpoint %s: %v\n", sb.ID(), ep.ID(), err)
}
if err := ep.Leave(sb); err != nil {
logrus.Warnf("Failed detaching sandbox %s from endpoint %s: %v\n", sb.ID(), ep.ID(), err)
}

if err := ep.Delete(force); err != nil {
if err := ep.Delete(true); err != nil {
logrus.Warnf("Failed deleting endpoint %s: %v\n", ep.ID(), err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion sandbox_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ func (c *controller) sandboxCleanup(activeSandboxes map[string]interface{}) {

if _, ok := activeSandboxes[sb.ID()]; !ok {
logrus.Infof("Removing stale sandbox %s (%s)", sb.id, sb.containerID)
if err := sb.delete(true); err != nil {
if err := sb.delete(); err != nil {
logrus.Errorf("Failed to delete sandbox %s while trying to cleanup: %v", sb.id, err)
}
continue
Expand Down

0 comments on commit 6950c76

Please sign in to comment.