From 90e177fd8da719d8b6099af287ad67e386b1a348 Mon Sep 17 00:00:00 2001 From: Fabricio Voznika Date: Tue, 23 Apr 2024 10:39:06 -0700 Subject: [PATCH] Add container runtime state Before making RPC calls to the sandbox, kill(0) is sent to the container's init process to ensure the container is running. Apart from being more costly than needed, this doesn't work when the container is restoring. For multi-container restore, containerd checks the state of containers while the sandbox is being restored (containerd restores 1 container at a time, but the sandbox can only be restored after the last containers is restored). kill(0) fails because the container's init process isn't running yet. This change makes an explicit RPC call to check on the state of a container, which succeeds when the container is being restored. Updates #1956 PiperOrigin-RevId: 627435351 --- runsc/boot/controller.go | 10 ++++++++++ runsc/boot/loader.go | 36 +++++++++++++++++++++++++++++++++++ runsc/cmd/state.go | 13 +++++-------- runsc/container/container.go | 20 ++++++++++++++++--- runsc/container/state_file.go | 14 ++------------ runsc/sandbox/sandbox.go | 23 +++++++++++++++------- 6 files changed, 86 insertions(+), 30 deletions(-) diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go index a31ac0d29..dc4f56d71 100644 --- a/runsc/boot/controller.go +++ b/runsc/boot/controller.go @@ -97,6 +97,9 @@ const ( // ContMgrMount mounts a filesystem in a container. ContMgrMount = "containerManager.Mount" + + // ContMgrContainerRuntimeState returns the runtime state of a container. + ContMgrContainerRuntimeState = "containerManager.ContainerRuntimeState" ) const ( @@ -745,3 +748,10 @@ func (cm *containerManager) Mount(args *MountArgs, _ *struct{}) error { cu.Release() return nil } + +// ContainerRuntimeState returns the runtime state of a container. +func (cm *containerManager) ContainerRuntimeState(cid *string, state *ContainerRuntimeState) error { + log.Debugf("containerManager.ContainerRuntimeState: cid: %s", cid) + *state = cm.l.containerRuntimeState(*cid) + return nil +} diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index e5e1da555..1d55ce52d 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -91,6 +91,21 @@ import ( _ "gvisor.dev/gvisor/pkg/sentry/socket/unix" ) +// ContainerRuntimeState is the runtime state of a container. +type ContainerRuntimeState int + +const ( + // RuntimeStateInvalid used just in case of error. + RuntimeStateInvalid ContainerRuntimeState = iota + // RuntimeStateCreating indicates that the container is being + // created, but has not started running yet. + RuntimeStateCreating + // RuntimeStateRunning indicates that the container is running. + RuntimeStateRunning + // RuntimeStateStopped indicates that the container has stopped. + RuntimeStateStopped +) + type containerInfo struct { cid string @@ -1741,3 +1756,24 @@ func (l *Loader) networkStats() ([]*NetworkInterface, error) { } return stats, nil } + +func (l *Loader) containerRuntimeState(cid string) ContainerRuntimeState { + l.mu.Lock() + defer l.mu.Unlock() + exec, ok := l.processes[execID{cid: cid}] + if !ok { + // Can't distinguish between invalid CID and stopped container, assume that + // CID is valid. + return RuntimeStateStopped + } + if exec.tg == nil { + // Container has no thread group assigned, so it has started yet. + return RuntimeStateCreating + } + if exec.tg.Leader().ExitState() == kernel.TaskExitNone { + // Init process is still running. + return RuntimeStateRunning + } + // Init process has stopped, but no one has called wait on it yet. + return RuntimeStateStopped +} diff --git a/runsc/cmd/state.go b/runsc/cmd/state.go index 606987d5a..253b7314a 100644 --- a/runsc/cmd/state.go +++ b/runsc/cmd/state.go @@ -62,18 +62,15 @@ func (*State) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcomman if err != nil { util.Fatalf("loading container: %v", err) } - log.Debugf("Returning state for container %+v", c) state := c.State() - log.Debugf("State: %+v", state) + log.Debugf("Returning state for container %q: %+v", c.ID, state) // Write json-encoded state directly to stdout. - b, err := json.MarshalIndent(state, "", " ") - if err != nil { - util.Fatalf("marshaling container state: %v", err) - } - if _, err := os.Stdout.Write(b); err != nil { - util.Fatalf("Error writing to stdout: %v", err) + encoder := json.NewEncoder(os.Stdout) + encoder.SetIndent("", " ") + if err := encoder.Encode(state); err != nil { + util.Fatalf("error marshaling container state: %v", err) } return subcommands.ExitSuccess } diff --git a/runsc/container/container.go b/runsc/container/container.go index a81dc9f13..87d8cb9bb 100644 --- a/runsc/container/container.go +++ b/runsc/container/container.go @@ -1404,9 +1404,7 @@ func (c *Container) changeStatus(s Status) { } case Stopped: - if c.Status != Creating && c.Status != Created && c.Status != Running && c.Status != Stopped { - panic(fmt.Sprintf("invalid state transition: %v => %v", c.Status, s)) - } + // All states can transition to Stopped. default: panic(fmt.Sprintf("invalid new state: %v", s)) @@ -1994,3 +1992,19 @@ func nvproxySetupAfterGoferUserns(spec *specs.Spec, conf *config.Config, goferCm return nil }, nil } + +// CheckStopped checks if the container is stopped and updates its status. +func (c *Container) CheckStopped() { + if state, err := c.Sandbox.ContainerRuntimeState(c.ID); err != nil { + log.Warningf("Cannot find if container %v exists, checking if sandbox %v is running, err: %v", c.ID, c.Sandbox.ID, err) + if !c.IsSandboxRunning() { + log.Warningf("Sandbox isn't running anymore, marking container %v as stopped:", c.ID) + c.changeStatus(Stopped) + } + } else { + if state == boot.RuntimeStateStopped { + log.Warningf("Container %v is stopped", c.ID) + c.changeStatus(Stopped) + } + } +} diff --git a/runsc/container/state_file.go b/runsc/container/state_file.go index 76faf1f0a..a935b655a 100644 --- a/runsc/container/state_file.go +++ b/runsc/container/state_file.go @@ -25,7 +25,6 @@ import ( "strings" "github.com/gofrs/flock" - "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/sync" ) @@ -113,17 +112,8 @@ func Load(rootDir string, id FullID, opts LoadOpts) (*Container, error) { // // This is inherently racy. switch c.Status { - case Created: - if !c.IsSandboxRunning() { - // Sandbox no longer exists, so this container definitely does not exist. - log.Warningf("Process for sandbox %v is no longer running; assuming container is in stopped state", c.Sandbox.ID) - c.changeStatus(Stopped) - } - case Running: - if err := c.SignalContainer(unix.Signal(0), false); err != nil { - log.Warningf("Cannot signal container %v for sandbox %v (err: %v); assuming container is in stopped state", c.ID, c.Sandbox.ID, err) - c.changeStatus(Stopped) - } + case Created, Running: + c.CheckStopped() } } diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index aaf9ccc0e..930e6bd86 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -1381,14 +1381,12 @@ func (s *Sandbox) ExportMetrics(opts control.MetricsExportOpts) (*prometheus.Sna // IsRunning returns true if the sandbox or gofer process is running. func (s *Sandbox) IsRunning() bool { pid := s.Pid.load() - if pid != 0 { - // Send a signal 0 to the sandbox process. - if err := unix.Kill(pid, 0); err == nil { - // Succeeded, process is running. - return true - } + if pid == 0 { + return false } - return false + // Send a signal 0 to the sandbox process. If it succeeds, the sandbox + // process is running. + return unix.Kill(pid, 0) == nil } // Stacks collects and returns all stacks for the sandbox. @@ -1762,6 +1760,17 @@ func (s *Sandbox) Mount(cid, fstype, src, dest string) error { return s.call(boot.ContMgrMount, &args, nil) } +// ContainerRuntimeState returns the runtime state of a container. +func (s *Sandbox) ContainerRuntimeState(cid string) (boot.ContainerRuntimeState, error) { + log.Debugf("ContainerRuntimeState, sandbox: %q, cid: %q", s.ID, cid) + var state boot.ContainerRuntimeState + if err := s.call(boot.ContMgrContainerRuntimeState, &cid, &state); err != nil { + return boot.RuntimeStateInvalid, fmt.Errorf("getting container state (CID: %q): %w", cid, err) + } + log.Debugf("ContainerRuntimeState, sandbox: %q, cid: %q, state: %v", s.ID, cid, state) + return state, nil +} + func setCloExeOnAllFDs() error { f, err := os.Open("/proc/self/fd") if err != nil {