From 290729b2e331d8f186fa875a168bf8db6903b182 Mon Sep 17 00:00:00 2001 From: Salah Aldeen Al Saleh Date: Thu, 23 Sep 2021 11:05:07 -0700 Subject: [PATCH] fix image garbage collection and add the ability to ignore enforcing rollbacks (#2201) * fix image garbage collection and add the ability to ignore enforcing rollbacks --- cmd/kots/cli/garbage-collect-images.go | 12 ++++++++- pkg/handlers/garbage_collect_images.go | 15 +++++++++++- pkg/kotsutil/kots.go | 4 +-- pkg/operator/client/client.go | 2 +- pkg/registry/images.go | 34 ++++++++++++++++++++------ 5 files changed, 54 insertions(+), 13 deletions(-) diff --git a/cmd/kots/cli/garbage-collect-images.go b/cmd/kots/cli/garbage-collect-images.go index 69ede350de..b40a694514 100644 --- a/cmd/kots/cli/garbage-collect-images.go +++ b/cmd/kots/cli/garbage-collect-images.go @@ -1,6 +1,7 @@ package cli import ( + "bytes" "encoding/json" "fmt" "io/ioutil" @@ -82,7 +83,14 @@ func GarbageCollectImagesCmd() *cobra.Command { os.Exit(2) // not returning error here as we don't want to show the entire stack trace to normal users } - newReq, err := http.NewRequest("POST", url, nil) + requestPayload := map[string]interface{}{ + "ignoreRollback": v.GetBool("ignore-rollback"), + } + requestBody, err := json.Marshal(requestPayload) + if err != nil { + return errors.Wrap(err, "failed to marshal request json") + } + newReq, err := http.NewRequest("POST", url, bytes.NewBuffer(requestBody)) if err != nil { return errors.Wrap(err, "failed to create request") } @@ -122,5 +130,7 @@ func GarbageCollectImagesCmd() *cobra.Command { }, } + cmd.Flags().Bool("ignore-rollback", false, "force images garbage collection even if rollback is enabled for the application") + return cmd } diff --git a/pkg/handlers/garbage_collect_images.go b/pkg/handlers/garbage_collect_images.go index 97b7d8ce66..7f1c789a31 100644 --- a/pkg/handlers/garbage_collect_images.go +++ b/pkg/handlers/garbage_collect_images.go @@ -1,6 +1,7 @@ package handlers import ( + "encoding/json" "net/http" "github.com/pkg/errors" @@ -12,6 +13,10 @@ import ( "github.com/replicatedhq/kots/pkg/store" ) +type GarbageCollectImagesRequest struct { + IgnoreRollback bool `json:"ignoreRollback,omitempty"` +} + type GarbageCollectImagesResponse struct { Error string `json:"error,omitempty"` } @@ -19,6 +24,14 @@ type GarbageCollectImagesResponse struct { func (h *Handler) GarbageCollectImages(w http.ResponseWriter, r *http.Request) { response := GarbageCollectImagesResponse{} + garbageCollectImagesRequest := GarbageCollectImagesRequest{} + if err := json.NewDecoder(r.Body).Decode(&garbageCollectImagesRequest); err != nil { + response.Error = "failed to decode request" + logger.Error(errors.Wrap(err, response.Error)) + JSON(w, http.StatusBadRequest, response) + return + } + installParams, err := kotsutil.GetInstallationParams(kotsadmtypes.KotsadmConfigMap) if err != nil { response.Error = "failed to get app registry info" @@ -66,7 +79,7 @@ func (h *Handler) GarbageCollectImages(w http.ResponseWriter, r *http.Request) { go func() { for _, app := range apps { logger.Infof("Deleting images for app %s", app.Slug) - err := registry.DeleteUnusedImages(app.ID) + err := registry.DeleteUnusedImages(app.ID, garbageCollectImagesRequest.IgnoreRollback) if err != nil { if _, ok := err.(registry.AppRollbackError); ok { logger.Infof("not garbage collecting images because version allows rollbacks: %v", err) diff --git a/pkg/kotsutil/kots.go b/pkg/kotsutil/kots.go index 0b739c70d1..f982067843 100644 --- a/pkg/kotsutil/kots.go +++ b/pkg/kotsutil/kots.go @@ -671,8 +671,9 @@ func GetInstallationParams(configMapName string) (InstallationParams, error) { return autoConfig, errors.Wrap(err, "failed to get k8s clientset") } - kotsadmConfigMap, err := clientset.CoreV1().ConfigMaps(util.PodNamespace).Get(context.TODO(), configMapName, metav1.GetOptions{}) + autoConfig.EnableImageDeletion = IsKurl(clientset) + kotsadmConfigMap, err := clientset.CoreV1().ConfigMaps(util.PodNamespace).Get(context.TODO(), configMapName, metav1.GetOptions{}) if err != nil { if kuberneteserrors.IsNotFound(err) { return autoConfig, nil @@ -684,7 +685,6 @@ func GetInstallationParams(configMapName string) (InstallationParams, error) { autoConfig.SkipImagePush, _ = strconv.ParseBool(kotsadmConfigMap.Data["initial-app-images-pushed"]) autoConfig.SkipPreflights, _ = strconv.ParseBool(kotsadmConfigMap.Data["skip-preflights"]) autoConfig.RegistryIsReadOnly, _ = strconv.ParseBool(kotsadmConfigMap.Data["registry-is-read-only"]) - autoConfig.EnableImageDeletion = IsKurl(clientset) return autoConfig, nil } diff --git a/pkg/operator/client/client.go b/pkg/operator/client/client.go index 9bbd0eb1b2..e7ccb949f7 100644 --- a/pkg/operator/client/client.go +++ b/pkg/operator/client/client.go @@ -362,7 +362,7 @@ func (c *Client) setDeployResults(args operatortypes.DeployAppArgs, results Depl if !results.IsError { go func() { - err := registry.DeleteUnusedImages(args.AppID) + err := registry.DeleteUnusedImages(args.AppID, false) if err != nil { if _, ok := err.(registry.AppRollbackError); ok { logger.Infof("not garbage collecting images because version allows rollbacks: %v", err) diff --git a/pkg/registry/images.go b/pkg/registry/images.go index 2b6701f807..e3aeb9645e 100644 --- a/pkg/registry/images.go +++ b/pkg/registry/images.go @@ -41,7 +41,7 @@ func (e AppRollbackError) Error() string { return fmt.Sprintf("app:%s, version:%d", e.AppID, e.Sequence) } -func DeleteUnusedImages(appID string) error { +func DeleteUnusedImages(appID string, ignoreRollback bool) error { installParams, err := kotsutil.GetInstallationParams(kotsadmtypes.KotsadmConfigMap) if err != nil { return errors.Wrap(err, "failed to get app registry info") @@ -68,6 +68,9 @@ func DeleteUnusedImages(appID string) error { return nil } + // we check all apps here because different apps could share the same images, + // and the images could be active in one but not the other. + // so, we also do not delete the images if rollback is enabled for any app. appIDs, err := store.GetStore().GetAppIDsFromRegistry(registrySettings.Hostname) if err != nil { return errors.Wrap(err, "failed to get apps with registry") @@ -75,25 +78,43 @@ func DeleteUnusedImages(appID string) error { activeVersions := []*versiontypes.AppVersion{} for _, appID := range appIDs { - downstreams, err := store.GetStore().ListDownstreamsForApp(appID) + a, err := store.GetStore().GetApp(appID) + if err != nil { + errors.Wrap(err, "failed to get app") + } + + if !ignoreRollback { + // rollback support is detected from the latest available version, not the currently deployed one + allowRollback, err := store.GetStore().IsRollbackSupportedForVersion(a.ID, a.CurrentSequence) + if err != nil { + return errors.Wrap(err, "failed to check if rollback is supported") + } + if allowRollback { + return AppRollbackError{AppID: a.ID, Sequence: a.CurrentSequence} + } + } else { + logger.Info("ignoring the fact that rollback is enabled and will continue with the images removal process") + } + + downstreams, err := store.GetStore().ListDownstreamsForApp(a.ID) if err != nil { return errors.Wrap(err, "failed to list downstreams for app") } for _, d := range downstreams { - curSequence, err := store.GetStore().GetCurrentParentSequence(appID, d.ClusterID) + curSequence, err := store.GetStore().GetCurrentParentSequence(a.ID, d.ClusterID) if err != nil { return errors.Wrap(err, "failed to get current parent sequence") } - curVersion, err := store.GetStore().GetAppVersion(appID, curSequence) + curVersion, err := store.GetStore().GetAppVersion(a.ID, curSequence) if err != nil { return errors.Wrap(err, "failed to get app version") } activeVersions = append(activeVersions, curVersion) - laterVersions, err := store.GetStore().GetAppVersionsAfter(appID, curSequence) + laterVersions, err := store.GetStore().GetAppVersionsAfter(a.ID, curSequence) if err != nil { return errors.Wrapf(err, "failed to get versions after %d", curVersion.Sequence) } @@ -109,9 +130,6 @@ func DeleteUnusedImages(appID string) error { if version.KOTSKinds == nil { continue } - if version.KOTSKinds.KotsApplication.Spec.AllowRollback { - return AppRollbackError{AppID: version.AppID, Sequence: version.Sequence} - } for _, i := range version.KOTSKinds.Installation.Spec.KnownImages { imagesDedup[i.Image] = struct{}{} }