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 leftover autoremove containers during refresh #21541

Merged
merged 2 commits into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 13 additions & 1 deletion libpod/container_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,19 @@ func resetContainerState(state *ContainerState) {
state.ConmonPID = 0
state.Mountpoint = ""
state.Mounted = false
if state.State != define.ContainerStateExited {
// Reset state.
// Almost all states are reset to either Configured or Exited,
// except ContainerStateRemoving which is preserved.
switch state.State {
case define.ContainerStateStopped, define.ContainerStateExited, define.ContainerStateStopping, define.ContainerStateRunning, define.ContainerStatePaused:
// All containers that ran at any point during the last boot
// must be placed in the Exited state.
state.State = define.ContainerStateExited
case define.ContainerStateConfigured, define.ContainerStateCreated:
state.State = define.ContainerStateConfigured
case define.ContainerStateUnknown:
// Something really strange must have happened to get us here.
// Reset to configured, maybe the reboot cleared things up?
state.State = define.ContainerStateConfigured
}
state.ExecSessions = make(map[string]*ExecSession)
Expand Down
41 changes: 30 additions & 11 deletions libpod/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func NewRuntime(ctx context.Context, options ...RuntimeOption) (*Runtime, error)
if err != nil {
return nil, err
}
return newRuntimeFromConfig(conf, options...)
return newRuntimeFromConfig(ctx, conf, options...)
}

// NewRuntimeFromConfig creates a new container runtime using the given
Expand All @@ -175,10 +175,10 @@ func NewRuntime(ctx context.Context, options ...RuntimeOption) (*Runtime, error)
// An error will be returned if the configuration file at the given path does
// not exist or cannot be loaded
func NewRuntimeFromConfig(ctx context.Context, userConfig *config.Config, options ...RuntimeOption) (*Runtime, error) {
return newRuntimeFromConfig(userConfig, options...)
return newRuntimeFromConfig(ctx, userConfig, options...)
}

func newRuntimeFromConfig(conf *config.Config, options ...RuntimeOption) (*Runtime, error) {
func newRuntimeFromConfig(ctx context.Context, conf *config.Config, options ...RuntimeOption) (*Runtime, error) {
runtime := new(Runtime)

if conf.Engine.OCIRuntime == "" {
Expand Down Expand Up @@ -223,7 +223,7 @@ func newRuntimeFromConfig(conf *config.Config, options ...RuntimeOption) (*Runti
return nil, fmt.Errorf("starting shutdown signal handler: %w", err)
}

if err := makeRuntime(runtime); err != nil {
if err := makeRuntime(ctx, runtime); err != nil {
return nil, err
}

Expand Down Expand Up @@ -333,7 +333,7 @@ func getDBState(runtime *Runtime) (State, error) {

// Make a new runtime based on the given configuration
// Sets up containers/storage, state store, OCI runtime
func makeRuntime(runtime *Runtime) (retErr error) {
func makeRuntime(ctx context.Context, runtime *Runtime) (retErr error) {
// Find a working conmon binary
cPath, err := runtime.config.FindConmon()
if err != nil {
Expand Down Expand Up @@ -629,6 +629,13 @@ func makeRuntime(runtime *Runtime) (retErr error) {
return err
}

// Mark the runtime as valid - ready to be used, cannot be modified
// further.
// Need to do this *before* refresh as we can remove containers there.
// Should not be a big deal as we don't return it to users until after
// refresh runs.
runtime.valid = true

// If we need to refresh the state, do it now - things are guaranteed to
// be set up by now.
if doRefresh {
Expand All @@ -639,17 +646,13 @@ func makeRuntime(runtime *Runtime) (retErr error) {
}
}

if err2 := runtime.refresh(runtimeAliveFile); err2 != nil {
if err2 := runtime.refresh(ctx, runtimeAliveFile); err2 != nil {
return err2
}
}

runtime.startWorker()

// Mark the runtime as valid - ready to be used, cannot be modified
// further
runtime.valid = true

return nil
}

Expand Down Expand Up @@ -819,7 +822,7 @@ func (r *Runtime) Shutdown(force bool) error {
// Reconfigures the runtime after a reboot
// Refreshes the state, recreating temporary files
// Does not check validity as the runtime is not valid until after this has run
func (r *Runtime) refresh(alivePath string) error {
func (r *Runtime) refresh(ctx context.Context, alivePath string) error {
logrus.Debugf("Podman detected system restart - performing state refresh")

// Clear state of database if not running in container
Expand Down Expand Up @@ -856,6 +859,22 @@ func (r *Runtime) refresh(alivePath string) error {
if err := ctr.refresh(); err != nil {
logrus.Errorf("Refreshing container %s: %v", ctr.ID(), err)
}
// This is the only place it's safe to use ctr.state.State unlocked
// We're holding the alive lock, guaranteed to be the only Libpod on the system right now.
if (ctr.AutoRemove() && ctr.state.State == define.ContainerStateExited) || ctr.state.State == define.ContainerStateRemoving {
opts := ctrRmOpts{
// Don't force-remove, we're supposed to be fresh off a reboot
// If we have to force something is seriously wrong
Force: false,
RemoveVolume: true,
}
// This container should have autoremoved before the
// reboot but did not.
// Get rid of it.
if _, _, err := r.removeContainer(ctx, ctr, opts); err != nil {
logrus.Errorf("Unable to remove container %s which should have autoremoved: %v", ctr.ID(), err)
}
}
}
for _, pod := range pods {
if err := pod.refresh(); err != nil {
Expand Down
Loading