-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
sandbox: Stop and clean up containers that fail to create #434
sandbox: Stop and clean up containers that fail to create #434
Conversation
/test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @evanfoster.
lgtm
@@ -1201,6 +1202,16 @@ func (s *Sandbox) CreateContainer(contConfig ContainerConfig) (VCContainer, erro | |||
defer func() { | |||
// Rollback if error happens. | |||
if err != nil { | |||
logger := s.Logger().WithFields(logrus.Fields{"container-id": c.id, "sandox-id": s.id, "rollback": true}) | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I would get rid of the extra empty line here.
} | ||
|
||
logger.Debug("Removing stopped container from sandbox store") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here. Could get rid of this extra line.
@evanfoster Thanks for raising. LGTM. Few minor nits. |
da588b5
to
dbfad6f
Compare
/test |
@evanfoster Could you please rebase to include the sob/wip checks? Thanks! |
A container that is created and added to a sandbox can still fail the final creation steps. In this case, the container must be stopped and have its resources cleaned up to prevent leaking sandbox mounts. Forward port of kata-containers/runtime#2826 Fixes kata-containers#2816 Signed-off-by: Evan Foster <[email protected]>
dbfad6f
to
e5910c9
Compare
/test |
A container that is created and added to a sandbox can still fail
the final creation steps. In this case, the container must be stopped
and have its resources cleaned up to prevent leaking sandbox mounts.
This PR is a port of kata-containers/runtime#2826
CC @fidencio
Fixes #2816
Signed-off-by: Evan Foster [email protected]