From 5dc2f204324accb6f84746b9e657e941167b630a Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sat, 1 Jul 2023 03:22:01 -0400 Subject: [PATCH] fix: remove all containers and images by label on app destroy Previously, we would filter containers and images by name, which could be fail if the app was renamed or no longer was tagged "correctly" due to a rebuild. Now, we filter by label, ensuring an app is completely cleaned up on deletion. Also: - Move docker image cleanup from apps plugin to builder plugin: This isn't where it belongs - images should be cleaned up in a builder plugin (or the builders plugin) and not in the apps plugin, which has nothing to do with docker images anyways. - Remove the code from the scheduler plugin, as that will only do anything if the scheduler is enabled for the app. - Bypass image removal if there are no images to remove - Always delete app containers: The herokuish builder creates intermediate build containers - something that should be refactored - and ideally this gets run in that builder plugin, but keeping the logic here ensures we don't have code duplication. This is otherwise safe as it will be a no-op if there are no containers to clean up. --- plugins/apps/functions.go | 48 -------------------- plugins/apps/triggers.go | 14 ------ plugins/builder/functions.go | 53 ++++++++++++++++++++++ plugins/builder/triggers.go | 20 +++++++- plugins/common/docker.go | 4 ++ plugins/scheduler-docker-local/post-delete | 18 +------- 6 files changed, 78 insertions(+), 79 deletions(-) diff --git a/plugins/apps/functions.go b/plugins/apps/functions.go index 13cb9105e..60cfb6e4c 100644 --- a/plugins/apps/functions.go +++ b/plugins/apps/functions.go @@ -1,7 +1,6 @@ package apps import ( - "bytes" "errors" "fmt" "os" @@ -95,53 +94,6 @@ func destroyApp(appName string) error { return nil } -func listImagesByAppLabel(appName string) ([]string, error) { - command := []string{ - common.DockerBin(), - "image", - "list", - "--quiet", - "--filter", - fmt.Sprintf("label=com.dokku.app-name=%v", appName), - } - - var stderr bytes.Buffer - listCmd := common.NewShellCmd(strings.Join(command, " ")) - listCmd.ShowOutput = false - listCmd.Command.Stderr = &stderr - b, err := listCmd.Output() - - if err != nil { - return []string{}, errors.New(strings.TrimSpace(stderr.String())) - } - - output := strings.Split(strings.TrimSpace(string(b[:])), "\n") - return output, nil -} - -func listImagesByImageRepo(imageRepo string) ([]string, error) { - command := []string{ - common.DockerBin(), - "image", - "list", - "--quiet", - imageRepo, - } - - var stderr bytes.Buffer - listCmd := common.NewShellCmd(strings.Join(command, " ")) - listCmd.ShowOutput = false - listCmd.Command.Stderr = &stderr - b, err := listCmd.Output() - - if err != nil { - return []string{}, errors.New(strings.TrimSpace(stderr.String())) - } - - output := strings.Split(strings.TrimSpace(string(b[:])), "\n") - return output, nil -} - // creates an app if allowed func maybeCreateApp(appName string) error { if err := appExists(appName); err == nil { diff --git a/plugins/apps/triggers.go b/plugins/apps/triggers.go index c980b3203..9a82d9363 100644 --- a/plugins/apps/triggers.go +++ b/plugins/apps/triggers.go @@ -101,19 +101,5 @@ func TriggerPostDelete(appName string) error { common.LogWarn(err.Error()) } - imagesByAppLabel, err := listImagesByAppLabel(appName) - if err != nil { - common.LogWarn(err.Error()) - } - - imageRepo := common.GetAppImageRepo(appName) - imagesByRepo, err := listImagesByImageRepo(imageRepo) - if err != nil { - common.LogWarn(err.Error()) - } - - images := append(imagesByAppLabel, imagesByRepo...) - common.RemoveImages(images) - return nil } diff --git a/plugins/builder/functions.go b/plugins/builder/functions.go index eb90e10ab..e7ac81337 100644 --- a/plugins/builder/functions.go +++ b/plugins/builder/functions.go @@ -1,11 +1,64 @@ package builder import ( + "bytes" + "errors" + "fmt" "io/ioutil" "os" "path" + "strings" + + "github.com/dokku/dokku/plugins/common" ) +func listImagesByAppLabel(appName string) ([]string, error) { + command := []string{ + common.DockerBin(), + "image", + "list", + "--quiet", + "--filter", + fmt.Sprintf("label=com.dokku.app-name=%v", appName), + } + + var stderr bytes.Buffer + listCmd := common.NewShellCmd(strings.Join(command, " ")) + listCmd.ShowOutput = false + listCmd.Command.Stderr = &stderr + b, err := listCmd.Output() + + if err != nil { + return []string{}, errors.New(strings.TrimSpace(stderr.String())) + } + + output := strings.Split(strings.TrimSpace(string(b[:])), "\n") + return output, nil +} + +func listImagesByImageRepo(imageRepo string) ([]string, error) { + command := []string{ + common.DockerBin(), + "image", + "list", + "--quiet", + imageRepo, + } + + var stderr bytes.Buffer + listCmd := common.NewShellCmd(strings.Join(command, " ")) + listCmd.ShowOutput = false + listCmd.Command.Stderr = &stderr + b, err := listCmd.Output() + + if err != nil { + return []string{}, errors.New(strings.TrimSpace(stderr.String())) + } + + output := strings.Split(strings.TrimSpace(string(b[:])), "\n") + return output, nil +} + func removeAllContents(basePath string) error { dir, err := ioutil.ReadDir(basePath) if err != nil { diff --git a/plugins/builder/triggers.go b/plugins/builder/triggers.go index be1bfe560..2b4cc240b 100644 --- a/plugins/builder/triggers.go +++ b/plugins/builder/triggers.go @@ -129,5 +129,23 @@ func TriggerPostAppRenameSetup(oldAppName string, newAppName string) error { // TriggerPostDelete destroys the builder property for a given app container func TriggerPostDelete(appName string) error { - return common.PropertyDestroy("builder", appName) + if err := common.PropertyDestroy("builder", appName); err != nil { + return err + } + + imagesByAppLabel, err := listImagesByAppLabel(appName) + if err != nil { + common.LogWarn(err.Error()) + } + + imageRepo := common.GetAppImageRepo(appName) + imagesByRepo, err := listImagesByImageRepo(imageRepo) + if err != nil { + common.LogWarn(err.Error()) + } + + images := append(imagesByAppLabel, imagesByRepo...) + common.RemoveImages(images) + + return nil } diff --git a/plugins/common/docker.go b/plugins/common/docker.go index 70bae5703..9eea856d7 100644 --- a/plugins/common/docker.go +++ b/plugins/common/docker.go @@ -329,6 +329,10 @@ func ListDanglingImages(appName string) ([]string, error) { // RemoveImages removes images by ID func RemoveImages(imageIDs []string) { + if len(imageIDs) == 0 { + return + } + command := []string{ DockerBin(), "image", diff --git a/plugins/scheduler-docker-local/post-delete b/plugins/scheduler-docker-local/post-delete index dd1bce3c9..26c631ec1 100755 --- a/plugins/scheduler-docker-local/post-delete +++ b/plugins/scheduler-docker-local/post-delete @@ -12,23 +12,9 @@ trigger-scheduler-docker-local-post-delete() { fn-plugin-property-destroy "scheduler-docker-local" "$APP" rm -rf "${DOKKU_LIB_ROOT}/data/scheduler-docker-local/$APP" - local DOKKU_SCHEDULER=$(get_app_scheduler "$APP") - if [[ "$DOKKU_SCHEDULER" != "docker-local" ]]; then - return - fi - - local IMAGE_REPO=$(get_app_image_repo "$APP") - - # remove all application containers & images + # remove all application containers # shellcheck disable=SC2046 - local DOKKU_APP_CIDS=$("$DOCKER_BIN" container list --all --no-trunc | grep "dokku/${APP}:" | awk '{ print $1 }' | xargs) - if [[ -n "$DOKKU_APP_CIDS" ]]; then - # shellcheck disable=SC2086 - "$DOCKER_BIN" container rm --force $DOKKU_APP_CIDS >/dev/null 2>&1 || true - fi - - # shellcheck disable=SC2046 - "$DOCKER_BIN" image remove $("$DOCKER_BIN" image list --quiet "$IMAGE_REPO" | xargs) &>/dev/null || true + "$DOCKER_BIN" container ls --filter "label=com.dokku.app-name=${APP}" -q | xargs -n1 -I {} "$DOCKER_BIN" container rm --force {} &>/dev/null || true } trigger-scheduler-docker-local-post-delete "$@"