Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Persist container exit code in agent state file #1125

Merged
merged 1 commit into from
Dec 4, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Unreleased
* Bug - Fixed a bug where `-version` fails due to its dependency on docker client. [#1118](https://github.com/aws/amazon-ecs-agent/pull/1118)
* Bug - Persist container exit code in agent state file [#1125](https://github.com/aws/amazon-ecs-agent/pull/1125)

## 1.16.0
* Feature - Support pulling from Amazon ECR with specified IAM role in task definition
Expand Down
17 changes: 13 additions & 4 deletions agent/api/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,14 @@ type Container struct {
// metadata file
MetadataFileUpdated bool `json:"metadataFileUpdated"`

knownExitCode *int
// KnownExitCodeUnsafe specifies the exit code for the container.
// It is exposed outside of the package so that it's marshalled/unmarshalled in
// the JSON body while saving the state.
// NOTE: Do not access KnownExitCodeUnsafe directly. Instead, use `GetKnownExitCode`
// and `SetKnownExitCode`.
KnownExitCodeUnsafe *int `json:"KnownExitCode"`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may also bump the state file version here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the KnownExitCode was part of the state file since the beginning. It was recently accidentally removed, this change is just putting it back in. So I thought I shouldn't bump the version?


// KnownPortBindings is an array of port bindings for the container.
KnownPortBindings []PortBinding

// SteadyStateStatusUnsafe specifies the steady state status for the container
Expand Down Expand Up @@ -237,14 +244,16 @@ func (c *Container) SetSentStatus(status ContainerStatus) {
func (c *Container) SetKnownExitCode(i *int) {
c.lock.Lock()
defer c.lock.Unlock()
c.knownExitCode = i

c.KnownExitCodeUnsafe = i
}

// GetKnownExitCode returns the container exit code
func (c *Container) GetKnownExitCode() *int {
c.lock.RLock()
defer c.lock.RUnlock()
return c.knownExitCode

return c.KnownExitCodeUnsafe
}

// SetRegistryAuthCredentials sets the credentials for pulling image from ECR
Expand Down Expand Up @@ -363,7 +372,7 @@ func (c *Container) IsEssential() bool {
return c.Essential
}

// LogAuthExecutionRole returns true if the auth is by exectution role
// AWSLogAuthExecutionRole returns true if the auth is by execution role
func (c *Container) AWSLogAuthExecutionRole() bool {
return c.LogsAuthStrategy == awslogsAuthExecutionRole
}
11 changes: 9 additions & 2 deletions agent/statemanager/state_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/aws/amazon-ecs-agent/agent/engine/dockerstate"
"github.com/aws/amazon-ecs-agent/agent/statemanager"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestStateManagerNonexistantDirectory(t *testing.T) {
Expand All @@ -34,7 +35,7 @@ func TestStateManagerNonexistantDirectory(t *testing.T) {

func TestLoadsV1DataCorrectly(t *testing.T) {
cleanup, err := setupWindowsTest(filepath.Join(".", "testdata", "v1", "1", "ecs_agent_data.json"))
assert.Nil(t, err, "Failed to set up test")
require.Nil(t, err, "Failed to set up test")
defer cleanup()
cfg := &config.Config{DataDir: filepath.Join(".", "testdata", "v1", "1")}

Expand Down Expand Up @@ -62,11 +63,17 @@ func TestLoadsV1DataCorrectly(t *testing.T) {
deadTask = task
}
}
assert.NotNil(t, deadTask)

require.NotNil(t, deadTask)
assert.Equal(t, deadTask.GetSentStatus(), api.TaskStopped)
assert.Equal(t, deadTask.Containers[0].SentStatusUnsafe, api.ContainerStopped)
assert.Equal(t, deadTask.Containers[0].DesiredStatusUnsafe, api.ContainerStopped)
assert.Equal(t, deadTask.Containers[0].KnownStatusUnsafe, api.ContainerStopped)

exitCode := deadTask.Containers[0].KnownExitCodeUnsafe
require.NotNil(t, exitCode)
assert.Equal(t, *exitCode, 128)

expected, _ := time.Parse(time.RFC3339, "2015-04-28T17:29:48.129140193Z")
assert.Equal(t, deadTask.GetKnownStatusTime(), expected)
}