Skip to content

Commit

Permalink
Add container error message to ContainerState
Browse files Browse the repository at this point in the history 
This change aims to store an error message to the ContainerState struct
when an error comes out of the OCI Runtime.

Fixes: containers#13729

Signed-off-by: Jake Correnti <jakecorrenti+github@proton.me>
  • Loading branch information
@jakecorrenti
jakecorrenti committed Dec 11, 2022
1 parent 47bcd10 commit 9920290
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 9 deletions.
2 changes: 2 additions & 0 deletions libpod/container.go
View file
Expand Up @@ -150,6 +150,8 @@ type ContainerState struct {
ExitCode int32 `json:"exitCode,omitempty"`
// Exited is whether the container has exited
Exited bool `json:"exited,omitempty"`
// The error message produced by a failed container operation
Error string `json:"error,omitempty"`
// OOMKilled indicates that the container was killed as it ran out of
// memory
OOMKilled bool `json:"oomKilled,omitempty"`
Expand Down
62 changes: 57 additions & 5 deletions libpod/container_api.go
View file
Expand Up @@ -92,6 +92,9 @@ func (c *Container) Start(ctx context.Context, recursive bool) error {
}
}
if err := c.prepareToStart(ctx, recursive); err != nil {
if e := saveContainerError(c, err); e != nil {
return e
}
return err
}

Expand Down Expand Up @@ -125,6 +128,9 @@ func (c *Container) StartAndAttach(ctx context.Context, streams *define.AttachSt
}

if err := c.prepareToStart(ctx, recursive); err != nil {
if e := saveContainerError(c, err); e != nil {
return nil, e
}
return nil, err
}
attachChan := make(chan error)
Expand All @@ -147,7 +153,11 @@ func (c *Container) StartAndAttach(ctx context.Context, streams *define.AttachSt
opts.Started = startedChan

if err := c.ociRuntime.Attach(c, opts); err != nil {
attachChan <- err
if e := saveContainerError(c, err); e != nil {
attachChan <- e
} else {
attachChan <- err
}
}
close(attachChan)
}()
Expand Down Expand Up @@ -237,6 +247,9 @@ func (c *Container) Kill(signal uint) error {

// Hardcode all = false, we only use all when removing.
if err := c.ociRuntime.KillContainer(c, signal, false); err != nil {
if e := saveContainerError(c, err); e != nil {
return e
}
return err
}

Expand Down Expand Up @@ -283,7 +296,13 @@ func (c *Container) Attach(streams *define.AttachStreams, keys string, resize <-
go func() {
<-attachRdy
if err := c.ociRuntime.KillContainer(c, uint(signal.SIGWINCH), false); err != nil {
logrus.Warnf("Unable to send SIGWINCH to container %s after attach: %v", c.ID(), err)
killErr := fmt.Errorf("unable to send SIGWINCH to container %s after attach: %v", c.ID(), err)

logrus.Warn(killErr.Error())

if e := saveContainerError(c, killErr); e != nil {
logrus.Warn(e.Error())
}
}
}()
}
Expand All @@ -299,7 +318,16 @@ func (c *Container) Attach(streams *define.AttachStreams, keys string, resize <-
opts.AttachReady = attachRdy

c.newContainerEvent(events.Attach)
return c.ociRuntime.Attach(c, opts)

err := c.ociRuntime.Attach(c, opts)
if err != nil {
if e := saveContainerError(c, err); e != nil {
return e
}
return err
}

return nil
}

// HTTPAttach forwards an attach session over a hijacked HTTP session.
Expand Down Expand Up @@ -346,7 +374,16 @@ func (c *Container) HTTPAttach(r *http.Request, w http.ResponseWriter, streams *
logrus.Infof("Performing HTTP Hijack attach to container %s", c.ID())

c.newContainerEvent(events.Attach)
return c.ociRuntime.HTTPAttach(c, r, w, streams, detachKeys, cancel, hijackDone, streamAttach, streamLogs)

err := c.ociRuntime.HTTPAttach(c, r, w, streams, detachKeys, cancel, hijackDone, streamAttach, streamLogs)
if err != nil {
if e := saveContainerError(c, err); e != nil {
return e
}
return err
}

return nil
}

// AttachResize resizes the container's terminal, which is displayed by Attach
Expand All @@ -367,7 +404,14 @@ func (c *Container) AttachResize(newSize resize.TerminalSize) error {

logrus.Infof("Resizing TTY of container %s", c.ID())

return c.ociRuntime.AttachResize(c, newSize)
err := c.ociRuntime.AttachResize(c, newSize)
if err != nil {
if e := saveContainerError(c, err); e != nil {
return e
}
return err
}
return nil
}

// Mount mounts a container's filesystem on the host
Expand Down Expand Up @@ -549,6 +593,9 @@ func (c *Container) WaitForExit(ctx context.Context, pollInterval time.Duration)
case errors.Is(err, define.ErrNoSuchCtr):
containerRemoved = true
case err != nil:
if e := saveContainerError(c, err); e != nil {
return false, -1, e
}
return false, -1, err
case !conmonAlive:
timerDuration := time.Second * 20
Expand Down Expand Up @@ -1035,3 +1082,8 @@ func (c *Container) Stat(ctx context.Context, containerPath string) (*define.Fil
info, _, _, err := c.stat(mountPoint, containerPath)
return info, err
}

func saveContainerError(c *Container, err error) error {
c.state.Error = err.Error()
return c.save()
}
2 changes: 1 addition & 1 deletion libpod/container_inspect.go
View file
Expand Up @@ -129,7 +129,7 @@ func (c *Container) getContainerInspectData(size bool, driverData *define.Driver
Pid: runtimeInfo.PID,
ConmonPid: runtimeInfo.ConmonPID,
ExitCode: runtimeInfo.ExitCode,
Error: "", // can't get yet
Error: runtimeInfo.Error,
StartedAt: runtimeInfo.StartedTime,
FinishedAt: runtimeInfo.FinishedTime,
Checkpointed: runtimeInfo.Checkpointed,
Expand Down
46 changes: 43 additions & 3 deletions libpod/container_internal.go
View file
Expand Up @@ -154,12 +154,26 @@ func (c *Container) PreCheckPointPath() string {

// AttachSocketPath retrieves the path of the container's attach socket
func (c *Container) AttachSocketPath() (string, error) {
return c.ociRuntime.AttachSocketPath(c)
path, err := c.ociRuntime.AttachSocketPath(c)
if err != nil {
if e := saveContainerError(c, err); e != nil {
return "", e
}
return "", err
}
return path, nil
}

// exitFilePath gets the path to the container's exit file
func (c *Container) exitFilePath() (string, error) {
return c.ociRuntime.ExitFilePath(c)
path, err := c.ociRuntime.ExitFilePath(c)
if err != nil {
if e := saveContainerError(c, err); e != nil {
return "", e
}
return "", err
}
return path, nil
}

// Wait for the container's exit file to appear.
Expand Down Expand Up @@ -1209,6 +1223,10 @@ func (c *Container) start() error {
}

if err := c.ociRuntime.StartContainer(c); err != nil {
c.state.Error = err.Error()
if e := saveContainerError(c, err); e != nil {
return e
}
return err
}
logrus.Debugf("Started container %s", c.ID())
Expand Down Expand Up @@ -1299,6 +1317,9 @@ func (c *Container) stop(timeout uint) error {

if stopErr != nil {
logrus.Errorf("Syncing container %s status: %v", c.ID(), err)
if e := saveContainerError(c, stopErr); e != nil {
return e
}
return stopErr
}
return err
Expand All @@ -1308,6 +1329,9 @@ func (c *Container) stop(timeout uint) error {
// We have to check stopErr *after* we lock again - otherwise, we have a
// change of panicking on a double-unlock. Ref: GH Issue 9615
if stopErr != nil {
if e := saveContainerError(c, stopErr); e != nil {
return e
}
return stopErr
}

Expand All @@ -1331,6 +1355,9 @@ func (c *Container) stop(timeout uint) error {
func (c *Container) waitForConmonToExitAndSave() error {
conmonAlive, err := c.ociRuntime.CheckConmonRunning(c)
if err != nil {
if e := saveContainerError(c, err); e != nil {
return e
}
return err
}
if !conmonAlive {
Expand Down Expand Up @@ -1370,6 +1397,9 @@ func (c *Container) pause() error {
}

if err := c.ociRuntime.PauseContainer(c); err != nil {
if e := saveContainerError(c, err); e != nil {
return e
}
// TODO when using docker-py there is some sort of race/incompatibility here
return err
}
Expand All @@ -1388,6 +1418,9 @@ func (c *Container) unpause() error {
}

if err := c.ociRuntime.UnpauseContainer(c); err != nil {
if e := saveContainerError(c, err); e != nil {
return e
}
// TODO when using docker-py there is some sort of race/incompatibility here
return err
}
Expand Down Expand Up @@ -1979,7 +2012,11 @@ func (c *Container) stopPodIfNeeded(ctx context.Context) error {
// hooks.
func (c *Container) delete(ctx context.Context) error {
if err := c.ociRuntime.DeleteContainer(c); err != nil {
return fmt.Errorf("removing container %s from runtime: %w", c.ID(), err)
err := fmt.Errorf("removing container %s from runtime: %w", c.ID(), err)
if e := saveContainerError(c, err); e != nil {
return e
}
return err
}

if err := c.postDeleteHooks(ctx); err != nil {
Expand Down Expand Up @@ -2363,6 +2400,9 @@ func (c *Container) extractSecretToCtrStorage(secr *ContainerSecret) error {
// update calls the ociRuntime update function to modify a cgroup config after container creation
func (c *Container) update(resources *spec.LinuxResources) error {
if err := c.ociRuntime.UpdateContainer(c, resources); err != nil {
if e := saveContainerError(c, err); e != nil {
return e
}
return err
}
logrus.Debugf("updated container %s", c.ID())
Expand Down
15 changes: 15 additions & 0 deletions test/e2e/start_test.go
View file
Expand Up @@ -249,4 +249,19 @@ var _ = Describe("Podman start", func() {
Expect(session1).Should(Exit(0))
Expect(session1.OutputToString()).To(BeEquivalentTo(cid2))
})

It("podman start container with bad args", func() {
session := podmanTest.Podman([]string{"container", "create", ALPINE, "efcho", "Hello World"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))
cid := session.OutputToString()

session = podmanTest.Podman([]string{"start", cid})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(125))
session = podmanTest.Podman([]string{"container", "inspect", cid, "-f", "'{{ .State.Error }}"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))
Expect(session.OutputToString()).Should(ContainSubstring("crun: executable file `efcho` not found in $PATH: No such file or directory: OCI runtime attempted to invoke a command that was not found"))
})
})

0 comments on commit 9920290

Please sign in to comment.