Skip to content

Commit

Permalink
Change ContainerExitCode from int to int pointer
Browse files Browse the repository at this point in the history
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 containers#19124

Signed-off-by: Chetan Giradkar <cgiradka@redhat.com>
  • Loading branch information
cgiradkar committed Nov 17, 2023
1 parent bf162ff commit ced4a4b
Show file tree
Hide file tree
Showing 16 changed files with 67 additions and 41 deletions.
14 changes: 7 additions & 7 deletions libpod/boltdb_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -1379,24 +1379,23 @@ func (s *BoltState) AddContainerExitCode(id string, exitCode int32) error {
}

// GetContainerExitCode returns the exit code for the specified container.
func (s *BoltState) GetContainerExitCode(id string) (int32, error) {
func (s *BoltState) GetContainerExitCode(id string, ec *int32) error {
if len(id) == 0 {
return -1, define.ErrEmptyID
return define.ErrEmptyID
}

if !s.valid {
return -1, define.ErrDBClosed
return define.ErrDBClosed
}

db, err := s.getDBCon()
if err != nil {
return -1, err
return err
}
defer s.deferredCloseDBCon(db)

rawID := []byte(id)
result := int32(-1)
return result, db.View(func(tx *bolt.Tx) error {
return db.View(func(tx *bolt.Tx) error {
exitCodeBucket, err := getExitCodeBucket(tx)
if err != nil {
return err
Expand All @@ -1412,7 +1411,8 @@ func (s *BoltState) GetContainerExitCode(id string) (int32, error) {
return fmt.Errorf("converting raw exit code %v of container %s: %w", rawExitCode, id, err)
}

result = int32(exitCode)
intExitCode := int32(exitCode)
ec = &intExitCode
return nil
})
}
Expand Down
10 changes: 7 additions & 3 deletions libpod/container_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -638,8 +638,8 @@ func (c *Container) WaitForExit(ctx context.Context, pollInterval time.Duration)
}
}
}

exitCode, err := c.runtime.state.GetContainerExitCode(id)
var exitCode *int32
err := c.runtime.state.GetContainerExitCode(id, exitCode)
if err != nil {
if errors.Is(err, define.ErrNoSuchExitCode) {
// If the container is configured or created, we must assume it never ran.
Expand All @@ -650,7 +650,11 @@ func (c *Container) WaitForExit(ctx context.Context, pollInterval time.Duration)
return true, -1, fmt.Errorf("%w (container in state %s)%s", err, c.state.State, timedout)
}

return true, exitCode, nil
if exitCode == nil {
return c.state.Exited, -1, nil
}

return true, *exitCode, nil
}

for {
Expand Down
2 changes: 1 addition & 1 deletion libpod/container_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
13 changes: 9 additions & 4 deletions libpod/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,11 @@ func (c *Container) newContainerEventWithInspectData(status events.Status, inspe
}

if status == events.Remove {
exitCode, _ := c.runtime.state.GetContainerExitCode(c.ID())
e.ContainerExitCode = int(exitCode)
exitCode := &c.state.ExitCode
if exitCode != nil {
intExitCode := int(*exitCode)
e.ContainerExitCode = &intExitCode
}
}

return c.runtime.eventer.Write(e)
Expand All @@ -93,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,
Expand All @@ -112,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

Expand Down
2 changes: 1 addition & 1 deletion libpod/events/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions libpod/events/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions libpod/events/journal_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}

Expand Down
5 changes: 3 additions & 2 deletions libpod/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,12 @@ func (c *Container) canStopServiceContainer() (*serviceContainerReport, error) {
if pc.IsInfra() {
continue // ignore infra containers
}
exitCode, err := c.runtime.state.GetContainerExitCode(pc.ID())
var exitCode *int32
err := c.runtime.state.GetContainerExitCode(pc.ID(), exitCode)
if err != nil {
return nil, err
}
if exitCode != 0 {
if exitCode != nil {
report.failedContainers++
}
report.numContainers++
Expand Down
21 changes: 13 additions & 8 deletions libpod/sqlite_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -925,26 +925,31 @@ func (s *SQLiteState) AddContainerExitCode(id string, exitCode int32) (defErr er
}

// GetContainerExitCode returns the exit code for the specified container.
func (s *SQLiteState) GetContainerExitCode(id string) (int32, error) {
//
//nolint:staticcheck
func (s *SQLiteState) GetContainerExitCode(id string, ec *int32) error {
if len(id) == 0 {
return -1, define.ErrEmptyID
return define.ErrEmptyID
}

if !s.valid {
return -1, define.ErrDBClosed
return define.ErrDBClosed
}

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)
return fmt.Errorf("getting exit code of container %s from DB: %w", id, define.ErrNoSuchExitCode)
}
return -1, fmt.Errorf("scanning exit code of container %s: %w", id, err)
return fmt.Errorf("scanning exit code of container %s: %w", id, err)
}

return exitCode, nil
if exitCode != -1 {
ec = &exitCode
}

return nil
}

// GetContainerExitCodeTimeStamp returns the time stamp when the exit code of
Expand Down
2 changes: 1 addition & 1 deletion libpod/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ type State interface { //nolint:interfacebloat
// Add the exit code for the specified container to the database.
AddContainerExitCode(id string, exitCode int32) error
// Return the exit code for the specified container.
GetContainerExitCode(id string) (int32, error)
GetContainerExitCode(id string, exitCode *int32) error
// Remove exit codes older than 5 minutes.
PruneContainerExitCodes() error

Expand Down
2 changes: 1 addition & 1 deletion pkg/api/handlers/utils/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ func waitNextExit(ctx context.Context, containerName string) (int32, error) {

evt, ok := <-eventChannel
if ok {
return int32(evt.ContainerExitCode), nil
return int32(*evt.ContainerExitCode), nil
}
// if ok == false then containerEngine.Events() has exited
// it may happen if request was canceled (e.g. client closed connection prematurely) or
Expand Down
12 changes: 9 additions & 3 deletions pkg/domain/entities/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ 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"])
ec, ok := e.Actor.Attributes["containerExitCode"]
if !ok {
return nil
}
exitCode, err := strconv.Atoi(ec)
if err != nil {
return nil
}
Expand All @@ -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,
Expand All @@ -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
Expand Down
6 changes: 4 additions & 2 deletions pkg/domain/infra/abi/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -1192,9 +1192,11 @@ 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)
intExitCode := int(exitCode)
return intExitCode
}

func (ic *ContainerEngine) ContainerLogs(ctx context.Context, namesOrIds []string, options entities.ContainerLogsOptions) error {
Expand Down
4 changes: 2 additions & 2 deletions pkg/domain/infra/tunnel/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions test/apiv2/27-containersEvents.at
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ t GET "events?stream=false&since=$START" 200 \
'select(.status | contains("die")).Action=die' \
'select(.status | contains("die")).Actor.Attributes.exitCode=1'

# status=remove (#19124)
t GET "events?stream=false&since=$START" 200 \
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
2 changes: 1 addition & 1 deletion test/system/090-events.bats
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ EOF
assert "${#lines[@]}" = 52 "Number of events returned"
is "${lines[0]}" "{\"Name\":\"$eventsFile\",\"Status\":\"log-rotation\",\"Time\":\".*\",\"Type\":\"system\",\"Attributes\":{\"io.podman.event.rotate\":\"begin\"}}"
is "${lines[-2]}" "{\"Name\":\"$eventsFile\",\"Status\":\"log-rotation\",\"Time\":\".*\",\"Type\":\"system\",\"Attributes\":{\"io.podman.event.rotate\":\"end\"}}"
is "${lines[-1]}" "{\"ContainerExitCode\":-1,\"ID\":\"$ctrID\",\"Image\":\"$IMAGE\",\"Name\":\".*\",\"Status\":\"remove\",\"Time\":\".*\",\"Type\":\"container\",\"Attributes\":{.*}}"
is "${lines[-1]}" "{\"ID\":\"$ctrID\",\"Image\":\"$IMAGE\",\"Name\":\".*\",\"Status\":\"remove\",\"Time\":\".*\",\"Type\":\"container\",\"Attributes\":{.*}}"
}

@test "events log-file no duplicates" {
Expand Down

0 comments on commit ced4a4b

Please sign in to comment.