Enhance fallback to noop logic for apps to account for multiple names…

…paces
carvel-dev · Nov 8, 2023 · cf93a64 · cf93a64
1 parent 606e658
commit cf93a64
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 17 deletions.
diff --git a/pkg/app/app.go b/pkg/app/app.go
@@ -276,6 +276,37 @@ func (a *App) ConfigMapRefs() map[reftracker.RefKey]struct{} {
 	return configMaps
 }
 
+func (a *App) isSafeToFallbackToNoopDelete() bool {
+	if a.app.Status.Deploy == nil || a.app.Status.Deploy.KappDeployStatus == nil || a.app.Spec.Cluster != nil {
+		return false
+	}
+
+	// Ensure that app config map will be garbage collected due to namespace termination
+	status, err := a.compInfo.NamespaceStatus(a.app.Namespace)
+	if err != nil {
+		a.log.Error(err, "Error getting app namespace status", "app", a.app.Name, "namespace", a.app.Namespace)
+	}
+	if status.Phase != v1.NamespaceTerminating {
+		return false
+	}
+
+	// Ensure that no cluster scoped resources are created by the app
+	// and all affected namespaces are terminating
+	for _, ns := range a.app.Status.Deploy.KappDeployStatus.AssociatedResources.Namespaces {
+		if ns == "(cluster)" {
+			return false
+		}
+		status, err := a.compInfo.NamespaceStatus(ns)
+		if err != nil {
+			a.log.Error(err, "Error getting app namespace status", "app", a.app.Name, "namespace", a.app.Namespace)
+		}
+		if status.Phase != v1.NamespaceTerminating {
+			return false
+		}
+	}
+	return true
+}
+
 // HasImageOrImgpkgBundle is used to determine if the
 // App's spec contains a fetch stage for an image or
 // imgpkgbundle. It is mainly used to determine whether
@@ -292,11 +323,3 @@ func (a App) HasImageOrImgpkgBundle() bool {
 	}
 	return false
 }
-
-func (a App) isNamespaceTerminating() bool {
-	status, err := a.compInfo.NamespaceStatus(a.app.Namespace)
-	if err != nil {
-		a.log.Error(err, "Error getting app namespace status", "app", a.app.Name, "namespace", a.app.Namespace)
-	}
-	return status.Phase == v1.NamespaceTerminating
-}
diff --git a/pkg/app/app_deploy.go b/pkg/app/app_deploy.go
@@ -57,15 +57,7 @@ func (a *App) delete(changedFunc func(exec.CmdRunResult)) exec.CmdRunResult {
 
 	var result exec.CmdRunResult
 
-	appResourcesInSameNs := a.app.Status.Deploy != nil && a.app.Status.Deploy.KappDeployStatus != nil &&
-		len(a.app.Status.Deploy.KappDeployStatus.AssociatedResources.Namespaces) == 1 &&
-		a.app.Status.Deploy.KappDeployStatus.AssociatedResources.Namespaces[0] == a.app.Namespace
-
-	// Use noopDelete if the namespace is terminating and app resources are in same namespace because
-	// the app resources will be automatically deleted including the kapp ServiceAccount
-	noopDelete := a.isNamespaceTerminating() && appResourcesInSameNs && a.app.Spec.Cluster == nil
-
-	if !a.app.Spec.NoopDelete && !noopDelete {
+	if !a.app.Spec.NoopDelete && !a.isSafeToFallbackToNoopDelete() {
 		for _, dep := range a.app.Spec.Deploy {
 			switch {
 			case dep.Kapp != nil: