engine: Make the agent reuse the generated container name on restart

    Save the generated docker container name and when agent restart
    restore the agent state, when agent try to create the container it
    will reuse the name so that no duplicate container will be created.

Author: richardpen

agent/engine/docker_container_engine.go

@@ -468,10 +468,20 @@ func (dg *dockerGoClient) startContainer(ctx context.Context, id string) DockerC
 	return metadata
 }
 
+// https://github.com/fsouza/go-dockerclient/blob/fd53184a1439b6d7b82ca54c1cd9adac9a5278f2/container.go#L197
 func dockerStateToState(state docker.State) api.ContainerStatus {
 	if state.Running {
 		return api.ContainerRunning
 	}
+
+	if state.Dead {
+		return api.ContainerStopped
+	}
+
+	if state.StartedAt.IsZero() {
+		return api.ContainerCreated
+	}
+
 	return api.ContainerStopped
 }
 

agent/engine/docker_task_engine.go

@@ -236,6 +236,7 @@ func (engine *DockerTaskEngine) synchronizeState() {
 					log.Warn("Could not find matching container for expected", "name", cont.DockerName)
 				} else {
 					cont.DockerID = describedCont.ID
+					cont.Container.SetKnownStatus(dockerStateToState(describedCont.State))
 					// update mappings that need dockerid
 					engine.state.AddContainer(cont, task)
 					engine.imageManager.RecordContainerReference(cont.Container)
@@ -558,15 +559,24 @@ func (engine *DockerTaskEngine) createContainer(task *api.Task, container *api.C
 		client = client.WithVersion(dockerclient.DockerVersion(*container.DockerConfig.Version))
 	}
 
-	// Resolve HostConfig
-	// we have to do this in create, not start, because docker no longer handles
-	// merging create config with start hostconfig the same; e.g. memory limits
-	// get lost
+	dockerContainerName := ""
 	containerMap, ok := engine.state.ContainerMapByArn(task.Arn)
 	if !ok {
 		containerMap = make(map[string]*api.DockerContainer)
+	} else {
+		// looking for container that has docker name but not created
+		for _, v := range containerMap {
+			if v.Container.Name == container.Name {
+				dockerContainerName = v.DockerName
+				break
+			}
+		}
 	}
 
+	// Resolve HostConfig
+	// we have to do this in create, not start, because docker no longer handles
+	// merging create config with start hostconfig the same; e.g. memory limits
+	// get lost
 	hostConfig, hcerr := task.DockerHostConfig(container, containerMap)
 	if hcerr != nil {
 		return DockerContainerMetadata{Error: api.NamedError(hcerr)}
@@ -586,28 +596,30 @@ func (engine *DockerTaskEngine) createContainer(task *api.Task, container *api.C
 	config.Labels[labelPrefix+"task-definition-version"] = task.Version
 	config.Labels[labelPrefix+"cluster"] = engine.cfg.Cluster
 
-	name := ""
-	for i := 0; i < len(container.Name); i++ {
-		c := container.Name[i]
-		if !((c <= '9' && c >= '0') || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c == '-')) {
-			continue
+	if dockerContainerName == "" {
+		name := ""
+		for i := 0; i < len(container.Name); i++ {
+			c := container.Name[i]
+			if !((c <= '9' && c >= '0') || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c == '-')) {
+				continue
+			}
+			name += string(c)
 		}
-		name += string(c)
-	}
 
-	containerName := "ecs-" + task.Family + "-" + task.Version + "-" + name + "-" + utils.RandHex()
+		dockerContainerName = "ecs-" + task.Family + "-" + task.Version + "-" + name + "-" + utils.RandHex()
 
-	// Pre-add the container in case we stop before the next, more useful,
-	// AddContainer call. This ensures we have a way to get the container if
-	// we die before 'createContainer' returns because we can inspect by
-	// name
-	engine.state.AddContainer(&api.DockerContainer{DockerName: containerName, Container: container}, task)
-	seelog.Infof("Created container name mapping for task %s - %s -> %s", task, container, containerName)
-	engine.saver.ForceSave()
+		// Pre-add the container in case we stop before the next, more useful,
+		// AddContainer call. This ensures we have a way to get the container if
+		// we die before 'createContainer' returns because we can inspect by
+		// name
+		engine.state.AddContainer(&api.DockerContainer{DockerName: dockerContainerName, Container: container}, task)
+		seelog.Infof("Created container name mapping for task %s - %s -> %s", task, container, dockerContainerName)
+		engine.saver.ForceSave()
+	}
 
-	metadata := client.CreateContainer(config, hostConfig, containerName, createContainerTimeout)
+	metadata := client.CreateContainer(config, hostConfig, dockerContainerName, createContainerTimeout)
 	if metadata.DockerID != "" {
-		engine.state.AddContainer(&api.DockerContainer{DockerID: metadata.DockerID, DockerName: containerName, Container: container}, task)
+		engine.state.AddContainer(&api.DockerContainer{DockerID: metadata.DockerID, DockerName: dockerContainerName, Container: container}, task)
 	}
 	seelog.Infof("Created docker container for task %s: %s -> %s", task, container, metadata.DockerID)
 	return metadata

agent/engine/docker_task_engine_test.go

@@ -1203,3 +1203,29 @@ func TestTaskWithCircularDependency(t *testing.T) {
 	_, ok = taskEngine.(*DockerTaskEngine).managedTasks[task.Arn]
 	assert.False(t, ok, "Task should not be added to task manager for processing")
 }
+
+// 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{})
+	saver := mock_statemanager.NewMockStateManager(ctrl)
+	defer ctrl.Finish()
+
+	taskEngine, _ := privateTaskEngine.(*DockerTaskEngine)
+	taskEngine.SetSaver(saver)
+	state := taskEngine.State()
+
+	sleepTask := testdata.LoadTask("sleep5")
+	sleepContainer, _ := sleepTask.ContainerByName("sleep5")
+	// Store the generated container name to state
+	state.AddContainer(&api.DockerContainer{DockerName: "docker_container_name", Container: sleepContainer}, sleepTask)
+
+	gomock.InOrder(
+		client.EXPECT().CreateContainer(gomock.Any(), gomock.Any(), "docker_container_name", gomock.Any()),
+	)
+
+	metadata := taskEngine.createContainer(sleepTask, sleepContainer)
+	if metadata.Error != nil {
+		t.Error("Unexpected error", metadata.Error)
+	}
+}

agent/engine/dockerstate/docker_task_engine_state.go

@@ -250,18 +250,25 @@ func (state *DockerTaskEngineState) AddContainer(container *api.DockerContainer,
 	}
 
 	if container.DockerID != "" {
+		// Update the container id to the state
+		state.idToContainer[container.DockerID] = container
 		state.idToTask[container.DockerID] = task.Arn
+
+		// Remove the previously added name mapping
+		delete(state.idToContainer, container.DockerName)
+		delete(state.idToTask, container.DockerName)
+	} else if container.DockerName != "" {
+		// Update the container name mapping to the state when the ID isn't available
+		state.idToContainer[container.DockerName] = container
+		state.idToTask[container.DockerName] = task.Arn
 	}
+
 	existingMap, exists := state.taskToID[task.Arn]
 	if !exists {
 		existingMap = make(map[string]*api.DockerContainer, len(task.Containers))
 		state.taskToID[task.Arn] = existingMap
 	}
 	existingMap[container.Container.Name] = container
-
-	if container.DockerID != "" {
-		state.idToContainer[container.DockerID] = container
-	}
 }
 
 // AddImageState adds an image.ImageState to be stored