From 572f38c0db705bb03a64052f3ac9fc9f393a40be Mon Sep 17 00:00:00 2001 From: Chetan Giradkar Date: Mon, 6 Nov 2023 15:44:57 +0000 Subject: [PATCH] Set correct exitcode in remove events and change ContainerExitCode from int to int ptr Added additional check for event type to be remove and set the correct exitcode. While it was getting difficult to maintain the omitempty notation for Event->ContainerExitCode, changing the type from int to int ptr gives us the ability to check for ContainerExitCode to be not nil and continue operations from there. closes #19124 Signed-off-by: Chetan Giradkar --- libpod/container_exec.go | 2 +- libpod/events.go | 14 ++++++++++++-- libpod/events/config.go | 2 +- libpod/events/events.go | 3 +++ libpod/events/journal_linux.go | 6 +++--- libpod/sqlite_state.go | 3 +-- pkg/api/handlers/utils/containers.go | 5 ++++- pkg/domain/entities/events.go | 16 +++++++++++----- pkg/domain/infra/abi/containers.go | 3 ++- pkg/domain/infra/tunnel/containers.go | 4 ++-- test/apiv2/27-containersEvents.at | 6 +++++- 11 files changed, 45 insertions(+), 19 deletions(-) diff --git a/libpod/container_exec.go b/libpod/container_exec.go index df52993c19..f04f369d60 100644 --- a/libpod/container_exec.go +++ b/libpod/container_exec.go @@ -820,7 +820,7 @@ func (c *Container) exec(config *ExecConfig, streams *define.AttachStreams, resi if err != nil { return -1, fmt.Errorf("retrieving exec session %s exit code: %w", sessionID, err) } - return diedEvent.ContainerExitCode, nil + return *diedEvent.ContainerExitCode, nil } return -1, err } diff --git a/libpod/events.go b/libpod/events.go index 6e8c409e3a..8af4f19d1a 100644 --- a/libpod/events.go +++ b/libpod/events.go @@ -77,6 +77,14 @@ func (c *Container) newContainerEventWithInspectData(status events.Status, inspe e.HealthStatus = containerHealthStatus } + if status == events.Remove { + exitCode, err := c.runtime.state.GetContainerExitCode(c.ID()) + if err == nil { + intExitCode := int(exitCode) + e.ContainerExitCode = &intExitCode + } + } + return c.runtime.eventer.Write(e) } @@ -88,7 +96,8 @@ func (c *Container) newContainerExitedEvent(exitCode int32) { e.Image = c.config.RootfsImageName e.Type = events.Container e.PodID = c.PodID() - e.ContainerExitCode = int(exitCode) + intExitCode := int(exitCode) + e.ContainerExitCode = &intExitCode e.Details = events.Details{ ID: e.ID, @@ -107,7 +116,8 @@ func (c *Container) newExecDiedEvent(sessionID string, exitCode int) { e.Name = c.Name() e.Image = c.config.RootfsImageName e.Type = events.Container - e.ContainerExitCode = exitCode + intExitCode := exitCode + e.ContainerExitCode = &intExitCode e.Attributes = make(map[string]string) e.Attributes["execID"] = sessionID diff --git a/libpod/events/config.go b/libpod/events/config.go index 309a495744..7b31842097 100644 --- a/libpod/events/config.go +++ b/libpod/events/config.go @@ -24,7 +24,7 @@ const ( type Event struct { // ContainerExitCode is for storing the exit code of a container which can // be used for "internal" event notification - ContainerExitCode int `json:",omitempty"` + ContainerExitCode *int `json:",omitempty"` // ID can be for the container, image, volume, etc ID string `json:",omitempty"` // Image used where applicable diff --git a/libpod/events/events.go b/libpod/events/events.go index 2105a3b89f..bcda07e06e 100644 --- a/libpod/events/events.go +++ b/libpod/events/events.go @@ -69,6 +69,9 @@ func (e *Event) ToJSONString() (string, error) { // ToHumanReadable returns human-readable event as a formatted string func (e *Event) ToHumanReadable(truncate bool) string { + if e == nil { + return "" + } var humanFormat string id := e.ID if truncate { diff --git a/libpod/events/journal_linux.go b/libpod/events/journal_linux.go index debb49a8df..72c8352202 100644 --- a/libpod/events/journal_linux.go +++ b/libpod/events/journal_linux.go @@ -48,8 +48,8 @@ func (e EventJournalD) Write(ee Event) error { m["PODMAN_IMAGE"] = ee.Image m["PODMAN_NAME"] = ee.Name m["PODMAN_ID"] = ee.ID - if ee.ContainerExitCode != 0 { - m["PODMAN_EXIT_CODE"] = strconv.Itoa(ee.ContainerExitCode) + if ee.ContainerExitCode != nil { + m["PODMAN_EXIT_CODE"] = strconv.Itoa(*ee.ContainerExitCode) } if ee.PodID != "" { m["PODMAN_POD_ID"] = ee.PodID @@ -206,7 +206,7 @@ func newEventFromJournalEntry(entry *sdjournal.JournalEntry) (*Event, error) { if err != nil { logrus.Errorf("Parsing event exit code %s", code) } else { - newEvent.ContainerExitCode = intCode + newEvent.ContainerExitCode = &intCode } } diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go index cdd0d25c88..76d0b88194 100644 --- a/libpod/sqlite_state.go +++ b/libpod/sqlite_state.go @@ -934,8 +934,7 @@ func (s *SQLiteState) GetContainerExitCode(id string) (int32, error) { } row := s.conn.QueryRow("SELECT ExitCode FROM ContainerExitCode WHERE ID=?;", id) - - var exitCode int32 + var exitCode int32 = -1 if err := row.Scan(&exitCode); err != nil { if errors.Is(err, sql.ErrNoRows) { return -1, fmt.Errorf("getting exit code of container %s from DB: %w", id, define.ErrNoSuchExitCode) diff --git a/pkg/api/handlers/utils/containers.go b/pkg/api/handlers/utils/containers.go index 6491dc4029..00f7242f66 100644 --- a/pkg/api/handlers/utils/containers.go +++ b/pkg/api/handlers/utils/containers.go @@ -238,7 +238,10 @@ func waitNextExit(ctx context.Context, containerName string) (int32, error) { evt, ok := <-eventChannel if ok { - return int32(evt.ContainerExitCode), nil + if evt.ContainerExitCode != nil { + return int32(*evt.ContainerExitCode), nil + } + return -1, nil } // if ok == false then containerEngine.Events() has exited // it may happen if request was canceled (e.g. client closed connection prematurely) or diff --git a/pkg/domain/entities/events.go b/pkg/domain/entities/events.go index 34a6fe0489..5a747e1179 100644 --- a/pkg/domain/entities/events.go +++ b/pkg/domain/entities/events.go @@ -19,9 +19,13 @@ type Event struct { // ConvertToLibpodEvent converts an entities event to a libpod one. func ConvertToLibpodEvent(e Event) *libpodEvents.Event { - exitCode, err := strconv.Atoi(e.Actor.Attributes["containerExitCode"]) - if err != nil { - return nil + var exitCode int + if ec, ok := e.Actor.Attributes["containerExitCode"]; ok { + var err error + exitCode, err = strconv.Atoi(ec) + if err != nil { + return nil + } } status, err := libpodEvents.StringToStatus(e.Action) if err != nil { @@ -39,7 +43,7 @@ func ConvertToLibpodEvent(e Event) *libpodEvents.Event { delete(details, "name") delete(details, "containerExitCode") return &libpodEvents.Event{ - ContainerExitCode: exitCode, + ContainerExitCode: &exitCode, ID: e.Actor.ID, Image: image, Name: name, @@ -62,7 +66,9 @@ func ConvertToEntitiesEvent(e libpodEvents.Event) *Event { } attributes["image"] = e.Image attributes["name"] = e.Name - attributes["containerExitCode"] = strconv.Itoa(e.ContainerExitCode) + if e.ContainerExitCode != nil { + attributes["containerExitCode"] = strconv.Itoa(*e.ContainerExitCode) + } attributes["podId"] = e.PodID message := dockerEvents.Message{ // Compatibility with clients that still look for deprecated API elements diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index b495bc91b7..e8cb8e346e 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -1192,7 +1192,8 @@ func (ic *ContainerEngine) GetContainerExitCode(ctx context.Context, ctr *libpod exitCode, err := ctr.Wait(ctx) if err != nil { logrus.Errorf("Waiting for container %s: %v", ctr.ID(), err) - return define.ExecErrorCodeNotFound + intExitCode := int(define.ExecErrorCodeNotFound) + return intExitCode } return int(exitCode) } diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 55dc6a0dfb..81d092ab4e 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -773,7 +773,7 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri logrus.Errorf("Cannot get exit code: %v", err) report.ExitCode = define.ExecErrorCodeNotFound } else { - report.ExitCode = event.ContainerExitCode + report.ExitCode = *event.ContainerExitCode } } else { report.ExitCode = int(exitCode) @@ -962,7 +962,7 @@ func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.Conta return &report, nil //nolint: nilerr } - report.ExitCode = lastEvent.ContainerExitCode + report.ExitCode = *lastEvent.ContainerExitCode return &report, err } diff --git a/test/apiv2/27-containersEvents.at b/test/apiv2/27-containersEvents.at index a5b5b24a31..aac426218d 100644 --- a/test/apiv2/27-containersEvents.at +++ b/test/apiv2/27-containersEvents.at @@ -10,7 +10,7 @@ podman rm -a -f &>/dev/null START=$(date +%s) -podman run $IMAGE false || true +podman run --rm $IMAGE false || true # libpod api t GET "libpod/events?stream=false&since=$START" 200 \ @@ -28,4 +28,8 @@ t GET "events?stream=false&since=$START" 200 \ 'select(.status | contains("die")).Action=die' \ 'select(.status | contains("die")).Actor.Attributes.exitCode=1' +t GET "events?stream=false&since=$START&type=remove" 200 \ + 'select(.status| contains("remove")).Action=remove' \ + 'select(.status | contains("remove")).Actor.Attributes.containerExitCode=1' + # vim: filetype=sh