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
with the last known error from the Start, StartAndAttach, and Stop OCI
Runtime functions.

The goal was to act in accordance with Docker's behavior.

Fixes: containers#13729

Signed-off-by: Jake Correnti <jakecorrenti+github@proton.me>
  • Loading branch information
jakecorrenti committed Dec 13, 2022
1 parent 47bcd10 commit c5a5d09
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 21 deletions.
2 changes: 2 additions & 0 deletions libpod/container.go
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
61 changes: 41 additions & 20 deletions libpod/container_api.go
Expand Up @@ -82,21 +82,28 @@ func (c *Container) Init(ctx context.Context, recursive bool) error {
// Start requites that all dependency containers (e.g. pod infra containers) be
// running before being run. The recursive parameter, if set, will start all
// dependencies before starting this container.
func (c *Container) Start(ctx context.Context, recursive bool) error {
func (c *Container) Start(ctx context.Context, recursive bool) (err error) {
defer func() {
if err != nil {
_ = saveContainerError(c, err)
}
}()

if !c.batched {
c.lock.Lock()
defer c.lock.Unlock()

if err := c.syncContainer(); err != nil {
return err
if err = c.syncContainer(); err != nil {
return
}
}
if err := c.prepareToStart(ctx, recursive); err != nil {
return err
if err = c.prepareToStart(ctx, recursive); err != nil {
return
}

// Start the container
return c.start()
err = c.start()
return
}

// Update updates the given container.
Expand All @@ -114,18 +121,24 @@ func (c *Container) Update(res *spec.LinuxResources) error {
// Attach call occurs before Start).
// In overall functionality, it is identical to the Start call, with the added
// side effect that an attach session will also be started.
func (c *Container) StartAndAttach(ctx context.Context, streams *define.AttachStreams, keys string, resize <-chan resize.TerminalSize, recursive bool) (<-chan error, error) {
func (c *Container) StartAndAttach(ctx context.Context, streams *define.AttachStreams, keys string, resize <-chan resize.TerminalSize, recursive bool) (retChan <-chan error, err error) {
defer func() {
if err != nil {
_ = saveContainerError(c, err)
}
}()

if !c.batched {
c.lock.Lock()
defer c.lock.Unlock()

if err := c.syncContainer(); err != nil {
return nil, err
if err = c.syncContainer(); err != nil {
return
}
}

if err := c.prepareToStart(ctx, recursive); err != nil {
return nil, err
if err = c.prepareToStart(ctx, recursive); err != nil {
return
}
attachChan := make(chan error)

Expand All @@ -146,15 +159,15 @@ func (c *Container) StartAndAttach(ctx context.Context, streams *define.AttachSt
opts.Start = true
opts.Started = startedChan

if err := c.ociRuntime.Attach(c, opts); err != nil {
if err = c.ociRuntime.Attach(c, opts); err != nil {
attachChan <- err
}
close(attachChan)
}()

select {
case err := <-attachChan:
return nil, err
case err = <-attachChan:
return
case <-startedChan:
c.newContainerEvent(events.Attach)
}
Expand Down Expand Up @@ -193,25 +206,28 @@ func (c *Container) Stop() error {
// StopWithTimeout is a version of Stop that allows a timeout to be specified
// manually. If timeout is 0, SIGKILL will be used immediately to kill the
// container.
func (c *Container) StopWithTimeout(timeout uint) error {
func (c *Container) StopWithTimeout(timeout uint) (err error) {
if !c.batched {
c.lock.Lock()
defer c.lock.Unlock()

if err := c.syncContainer(); err != nil {
return err
if err = c.syncContainer(); err != nil {
return
}
}

if c.ensureState(define.ContainerStateStopped, define.ContainerStateExited) {
return define.ErrCtrStopped
err = define.ErrCtrStopped
return
}

if !c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning, define.ContainerStateStopping) {
return fmt.Errorf("can only stop created or running containers. %s is in state %s: %w", c.ID(), c.state.State.String(), define.ErrCtrStateInvalid)
err = fmt.Errorf("can only stop created or running containers. %s is in state %s: %w", c.ID(), c.state.State.String(), define.ErrCtrStateInvalid)
return
}

return c.stop(timeout)
err = c.stop(timeout)
return
}

// Kill sends a signal to a container
Expand Down Expand Up @@ -1035,3 +1051,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
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
16 changes: 16 additions & 0 deletions test/e2e/inspect_test.go
Expand Up @@ -540,4 +540,20 @@ var _ = Describe("Podman inspect", func() {
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))
})

It("podman inspect container with bad create 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()).To(Not(HaveLen(0)))
})

})

0 comments on commit c5a5d09

Please sign in to comment.