Ensuring endpoint resources are freed even on delete failures#1853
Ensuring endpoint resources are freed even on delete failures#1853abhi wants to merge 1 commit intomoby:masterfrom
Conversation
|
LGTM |
endpoint.go
Outdated
| } | ||
| } | ||
|
|
||
| ep.releaseAddress() |
There was a problem hiding this comment.
The current code makes sure that the resources are not freed up if deleteEndpoint fails.
I think the better fix is to make sandbox Delete() to call sb.delete(true), which will forcefully remove the endpoint.
|
@mavenugo @abhinandanpb is this ready to go in? |
|
Considering that now the |
|
@fcrisciani I agree we should still log the error even on force. |
6950c76 to
745cc37
Compare
| continue | ||
| } | ||
|
|
||
| if !force { |
There was a problem hiding this comment.
Just wondering, this was never done then?
There was a problem hiding this comment.
This would be done during delete sandbox.
On the the contrary this would have been skipped during sandboxCleanup.
There was a problem hiding this comment.
actually we should be skipping this by default now. Let me check this again.
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 <abhi@docker.com>
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 abhi@docker.com