Skip to content

Commit

Permalink
engine: metadata updates through agent restarts
Browse files Browse the repository at this point in the history
This change adds logic in the agent to update the metadata file after
the agent restarts and the related unit test.

Note that we added a MetadataFileUpdated flag in the api.Container
struct and this is included with state file's ECSDataVersion 6 bump.
  • Loading branch information
adnxn committed Sep 13, 2017
1 parent 16f9a03 commit 2feb1fa
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 5 deletions.
18 changes: 18 additions & 0 deletions agent/api/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ type Container struct {
// handled properly so that the state storage continues to work.
SentStatusUnsafe ContainerStatus `json:"SentStatus"`

// MetadataUpdated is set to true when we have completed updating the
// metadata file
MetadataFileUpdated bool `json:"metadataFileUpdated"`

knownExitCode *int
KnownPortBindings []PortBinding

Expand Down Expand Up @@ -306,3 +310,17 @@ func (c *Container) IsInternal() bool {
func (c *Container) IsRunning() bool {
return c.GetKnownStatus().IsRunning()
}

func (c *Container) IsMetadataFileUpdated() bool {
c.lock.RLock()
defer c.lock.RUnlock()

return c.MetadataFileUpdated
}

func (c *Container) SetMetadataFileUpdated() {
c.lock.Lock()
defer c.lock.Unlock()

c.MetadataFileUpdated = true
}
15 changes: 15 additions & 0 deletions agent/engine/docker_task_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,10 @@ func (engine *DockerTaskEngine) synchronizeState() {
}
} else {
engine.imageManager.RecordContainerReference(cont.Container)
if !cont.Container.IsMetadataFileUpdated() {
go engine.updateMetadataFile(task, cont)
}

}
if currentState > cont.Container.GetKnownStatus() {
cont.Container.SetKnownStatus(currentState)
Expand Down Expand Up @@ -697,6 +701,7 @@ func (engine *DockerTaskEngine) startContainer(task *api.Task, container *api.Co
if err != nil {
seelog.Errorf("Update metadata file failed for container %s of task %s: %v", container, task, err)
} else {
container.SetMetadataFileUpdated()
seelog.Debugf("Updated metadata file for container %s of task %s", container, task)
}
}()
Expand Down Expand Up @@ -925,3 +930,13 @@ func (engine *DockerTaskEngine) isParallelPullCompatible() bool {

return false
}

func (engine *DockerTaskEngine) updateMetadataFile(task *api.Task, cont *api.DockerContainer) {
err := engine.metadataManager.Update(cont.DockerID, task.Arn, cont.Container.Name)
if err != nil {
seelog.Errorf("Update metadata file failed for container %s of task %s: %v", cont.Container, task, err)
} else {
cont.Container.SetMetadataFileUpdated()
seelog.Debugf("Updated metadata file for container %s of task %s", cont.Container, task)
}
}
63 changes: 62 additions & 1 deletion agent/engine/docker_task_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1833,7 +1833,7 @@ func TestTaskWithCircularDependency(t *testing.T) {
// TestCreateContainerOnAgentRestart tests when agent restarts it should use the
// docker container name restored from agent state file to create the container
func TestCreateContainerOnAgentRestart(t *testing.T) {
ctrl, client, _, privateTaskEngine, _, _ := mocks(t, &config.Config{})
ctrl, client, _, privateTaskEngine, _, _, _ := mocks(t, &config.Config{})
saver := mock_statemanager.NewMockStateManager(ctrl)
defer ctrl.Finish()

Expand All @@ -1855,3 +1855,64 @@ func TestCreateContainerOnAgentRestart(t *testing.T) {
t.Error("Unexpected error", metadata.Error)
}
}

// TestMetadataFileUpdatedAgentRestart checks whether metadataManager.Update(...) is
// invoked in the path DockerTaskEngine.Init() -> .synchronizeState() -> .updateMetadataFile(...)
// for the following case:
// agent starts, container created, metadata file created, agent restarted, container recovered
// during task engine init, metadata file updated
func TestMetadataFileUpdatedAgentRestart(t *testing.T) {

conf := &defaultConfig
conf.ContainerMetadataEnabled = true

ctrl, client, _, privateTaskEngine, _, imageManager, metadataManager := mocks(t, conf)
saver := mock_statemanager.NewMockStateManager(ctrl)
defer ctrl.Finish()

var wg sync.WaitGroup
wg.Add(1)

taskEngine, _ := privateTaskEngine.(*DockerTaskEngine)
assert.True(t, taskEngine.cfg.ContainerMetadataEnabled, "ContainerMetadataEnabled set to false.")

taskEngine.SetSaver(saver)
state := taskEngine.State()
task := testdata.LoadTask("sleep5")

container, _ := task.ContainerByName("sleep5")
assert.False(t, container.MetadataFileUpdated)

dockerContainer := &api.DockerContainer{DockerID: containerID, Container: container}

expectedTaskARN := task.Arn
expectedDockerID := dockerContainer.DockerID
expectedContainerName := container.Name

state.AddContainer(dockerContainer, task)

client.EXPECT().Version()
eventStream := make(chan DockerContainerChangeEvent)
client.EXPECT().ContainerEvents(gomock.Any()).Return(eventStream, nil)
imageManager.EXPECT().AddAllImageStates(gomock.Any()).AnyTimes()
imageManager.EXPECT().RecordContainerReference(gomock.Any()).AnyTimes()
imageManager.EXPECT().GetImageStateFromImageName(gomock.Any()).AnyTimes()
client.EXPECT().PullImage(gomock.Any(), gomock.Any()).AnyTimes()
client.EXPECT().DescribeContainer(gomock.Any()).AnyTimes()
saver.EXPECT().Save().AnyTimes()

metadataManager.EXPECT().Update(gomock.Any(), gomock.Any(), gomock.Any()).Do(func(dockerID string, taskARN string, containerName string) {
assert.Equal(t, expectedTaskARN, taskARN)
assert.Equal(t, expectedContainerName, containerName)
assert.Equal(t, expectedDockerID, dockerID)
wg.Done()
})

ctx, cancel := context.WithCancel(context.TODO())
err := taskEngine.Init(ctx)
assert.NoError(t, err)
defer cancel()
defer taskEngine.Disable()

wg.Wait()
}
9 changes: 5 additions & 4 deletions agent/engine/engine_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions agent/statemanager/state_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const (
// b) Add 'ContainerResourcesProvisioned' as a new 'ContainerStatus' enum
// c) Add 'SteadyStateStatus' field to 'Container' struct
// d) Add 'ENIAttachments' struct
// e) Add 'MetadataUpdated' field to 'api.Container'
ECSDataVersion = 6

// ecsDataFile specifies the filename in the ECS_DATADIR
Expand Down

0 comments on commit 2feb1fa

Please sign in to comment.