image manager: cleanup 'dead' and 'created' containers

also cleanup of 'dangling' images that have no tags or names associated
with them (ie, they show as <none> in 'docker images')

closes #1684

unit tests

Author: sparrc

agent/config/config.go

@@ -306,7 +306,7 @@ func (cfg *Config) validateAndOverrideBounds() error {
 	// If a value has been set for taskCleanupWaitDuration and the value is less than the minimum allowed cleanup duration,
 	// print a warning and override it
 	if cfg.TaskCleanupWaitDuration < minimumTaskCleanupWaitDuration {
-		seelog.Warnf("Invalid value for image cleanup duration, will be overridden with the default value: %s. Parsed value: %v, minimum value: %v.", DefaultTaskCleanupWaitDuration.String(), cfg.TaskCleanupWaitDuration, minimumTaskCleanupWaitDuration)
+		seelog.Warnf("Invalid value for ECS_ENGINE_TASK_CLEANUP_WAIT_DURATION, will be overridden with the default value: %s. Parsed value: %v, minimum value: %v.", DefaultTaskCleanupWaitDuration.String(), cfg.TaskCleanupWaitDuration, minimumTaskCleanupWaitDuration)
 		cfg.TaskCleanupWaitDuration = DefaultTaskCleanupWaitDuration
 	}
 
@@ -316,7 +316,7 @@ func (cfg *Config) validateAndOverrideBounds() error {
 	}
 
 	if cfg.ImageCleanupInterval < minimumImageCleanupInterval {
-		seelog.Warnf("Invalid value for image cleanup duration, will be overridden with the default value: %s. Parsed value: %v, minimum value: %v.", DefaultImageCleanupTimeInterval.String(), cfg.ImageCleanupInterval, minimumImageCleanupInterval)
+		seelog.Warnf("Invalid value for ECS_IMAGE_CLEANUP_INTERVAL, will be overridden with the default value: %s. Parsed value: %v, minimum value: %v.", DefaultImageCleanupTimeInterval.String(), cfg.ImageCleanupInterval, minimumImageCleanupInterval)
 		cfg.ImageCleanupInterval = DefaultImageCleanupTimeInterval
 	}
 

agent/engine/docker_image_manager.go

@@ -312,6 +312,7 @@ func (imageManager *dockerImageManager) removeUnusedImages(ctx context.Context)
 
 	var numECSImagesDeleted int
 	imageManager.imageStatesConsideredForDeletion = imageManager.imagesConsiderForDeletion(imageManager.getAllImageStates())
+
 	for i := 0; i < imageManager.numImagesToDelete; i++ {
 		err := imageManager.removeLeastRecentlyUsedImage(ctx)
 		numECSImagesDeleted = i
@@ -342,9 +343,13 @@ func (imageManager *dockerImageManager) removeNonECSContainers(ctx context.Conte
 			continue
 		}
 
-		finishedTime, _ := time.Parse(time.Now().String(), response.State.FinishedAt)
+		seelog.Infof("Inspecting Non-ECS Container ID [%s] for removal, Finished [%s] Status [%s]", id, response.State.FinishedAt, response.State.Status)
+		finishedTime, err := time.Parse(time.RFC3339Nano, response.State.FinishedAt)
+		if err != nil {
+			seelog.Errorf("Error parsing time string for container. id: %s, time: %s err: %s", id, response.State.FinishedAt, err)
+		}
 
-		if response.State.Status == "exited" && time.Now().Sub(finishedTime) > imageManager.nonECSContainerCleanupWaitDuration {
+		if (response.State.Status == "exited" || response.State.Status == "dead" || response.State.Status == "created") && time.Now().Sub(finishedTime) > imageManager.nonECSContainerCleanupWaitDuration {
 			nonECSContainerRemoveAvailableIDs = append(nonECSContainerRemoveAvailableIDs, id)
 		}
 	}
@@ -353,13 +358,13 @@ func (imageManager *dockerImageManager) removeNonECSContainers(ctx context.Conte
 		if numNonECSContainerDeleted == imageManager.numNonECSContainersToDelete {
 			break
 		}
-		seelog.Infof("Removing non-ECS container id: %s", id)
+		seelog.Debugf("Removing non-ECS Container ID %s", id)
 		err := imageManager.client.RemoveContainer(ctx, id, dockerclient.RemoveContainerTimeout)
 		if err == nil {
-			seelog.Infof("Image removed: %s", id)
+			seelog.Infof("Container ID Removed: %s", id)
 			numNonECSContainerDeleted++
 		} else {
-			seelog.Errorf("Error removing Image %s - %v", id, err)
+			seelog.Errorf("Error Removing Container ID %s - %s", id, err)
 			continue
 		}
 	}
@@ -379,70 +384,75 @@ func (imageManager *dockerImageManager) getNonECSContainerIDs(ctx context.Contex
 	return nonECSContainersIDs, nil
 }
 
-type ImageWithSize struct {
+type ImageWithSizeID struct {
 	ImageName string
+	ImageID   string
 	Size      int64
 }
 
 func (imageManager *dockerImageManager) removeNonECSImages(ctx context.Context, nonECSImagesNumToDelete int) {
 	if nonECSImagesNumToDelete == 0 {
 		return
 	}
-	var nonECSImageNames = imageManager.getNonECSImageNames(ctx)
-	var nonECSImageNamesRemoveEligible []string
-	for _, nonECSImage := range nonECSImageNames {
-		if !isInExclusionList(nonECSImage, imageManager.imageCleanupExclusionList) {
-			nonECSImageNamesRemoveEligible = append(nonECSImageNamesRemoveEligible, nonECSImage)
+	nonECSImages := imageManager.getNonECSImages(ctx)
+	var nonECSImagesRmEligible []ImageWithSizeID
+	for _, nonECSImage := range nonECSImages {
+		if !isInExclusionList(nonECSImage.ImageName, imageManager.imageCleanupExclusionList) {
+			nonECSImagesRmEligible = append(nonECSImagesRmEligible, nonECSImage)
 		}
 	}
 
-	var imageWithSizeList []ImageWithSize
-	for _, imageName := range nonECSImageNamesRemoveEligible {
-		resp, iiErr := imageManager.client.InspectImage(imageName)
-		if iiErr != nil {
-			seelog.Errorf("Error inspecting non-ECS image name: %s - %v", imageName, iiErr)
+	// Get the all image sizes
+	for _, image := range nonECSImagesRmEligible {
+		resp, err := imageManager.client.InspectImage(image.ImageID)
+		if err != nil {
+			seelog.Errorf("Error inspecting non-ECS image: %s (%s), %s", image.ImageName, image.ImageID, err)
 			continue
 		}
-		imageWithSizeList = append(imageWithSizeList, ImageWithSize{imageName, resp.Size})
+		image.Size = resp.Size
 	}
 	// we want to sort images with size ascending
-	sort.Slice(imageWithSizeList, func(i, j int) bool {
-		return imageWithSizeList[i].Size < imageWithSizeList[j].Size
+	sort.Slice(nonECSImagesRmEligible, func(i, j int) bool {
+		return nonECSImagesRmEligible[i].Size < nonECSImagesRmEligible[j].Size
 	})
 
 	// we will remove the remaining nonECSImages in each performPeriodicImageCleanup call()
 	var numImagesAlreadyDeleted = 0
-	for _, kv := range imageWithSizeList {
+	for _, kv := range nonECSImagesRmEligible {
 		if numImagesAlreadyDeleted == nonECSImagesNumToDelete {
 			break
 		}
-		seelog.Infof("Removing non-ECS Image: %s", kv.ImageName)
-		err := imageManager.client.RemoveImage(ctx, kv.ImageName, dockerclient.RemoveImageTimeout)
+		seelog.Infof("Removing non-ECS Image: %s (%s)", kv.ImageName, kv.ImageID)
+		err := imageManager.client.RemoveImage(ctx, kv.ImageID, dockerclient.RemoveImageTimeout)
 		if err != nil {
-			seelog.Errorf("Error removing Image %s - %v", kv.ImageName, err)
+			seelog.Errorf("Error removing Image %s (%s) - %v", kv.ImageName, kv.ImageID, err)
 			continue
 		} else {
-			seelog.Infof("Image removed: %s", kv.ImageName)
+			seelog.Infof("Image removed: %s (%s)", kv.ImageName, kv.ImageID)
 			numImagesAlreadyDeleted++
 		}
 	}
 }
 
-func (imageManager *dockerImageManager) getNonECSImageNames(ctx context.Context) []string {
-	response := imageManager.client.ListImages(ctx, dockerclient.ListImagesTimeout)
-	var allImagesNames []string
-	for _, name := range response.RepoTags {
-		allImagesNames = append(allImagesNames, name)
+// getNonECSImages returns type ImageWithSizeID but does NOT populate the Size field.
+func (imageManager *dockerImageManager) getNonECSImages(ctx context.Context) []ImageWithSizeID {
+	r := imageManager.client.ListImages(ctx, dockerclient.ListImagesTimeout)
+	var allImages []ImageWithSizeID
+	for i := 0; i < len(r.RepoTags); i++ {
+		allImages = append(allImages, ImageWithSizeID{ImageName: r.RepoTags[i], ImageID: r.ImageIDs[i]})
 	}
-	var ecsImageNames []string
+	var ecsImageIDs []string
 	for _, imageState := range imageManager.getAllImageStates() {
-		for _, imageName := range imageState.Image.Names {
-			ecsImageNames = append(ecsImageNames, imageName)
-		}
+		ecsImageIDs = append(ecsImageIDs, imageState.Image.ImageID)
 	}
 
-	var nonECSImageNames = exclude(allImagesNames, ecsImageNames)
-	return nonECSImageNames
+	var nonECSImages []ImageWithSizeID
+	for _, image := range allImages {
+		if !isInExclusionList(image.ImageID, ecsImageIDs) {
+			nonECSImages = append(nonECSImages, image)
+		}
+	}
+	return nonECSImages
 }
 
 func isInExclusionList(imageName string, imageExclusionList []string) bool {
@@ -477,8 +487,9 @@ func (imageManager *dockerImageManager) imagesConsiderForDeletion(allImageStates
 	for _, imageState := range allImageStates {
 		if imageManager.isExcludedFromCleanup(imageState) {
 			//imageState that we want to keep
-			seelog.Infof("Image excluded from deletion: [%s]", imageState.String())
+			seelog.Debugf("Image excluded from deletion: [%s]", imageState.String())
 		} else {
+			seelog.Debugf("Image going to be considered for deletion: [%s]", imageState.String())
 			imagesConsiderForDeletionMap[imageState.Image.ImageID] = imageState
 		}
 	}