From e1fc4b9499707775310a309c74b167498b4f612d Mon Sep 17 00:00:00 2001 From: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> Date: Fri, 21 Nov 2025 14:12:02 -0500 Subject: [PATCH 1/2] fix(cmd): Correctly perform cleanup on windsor down Fixed the `windsor down` command to mimic functionality prior to recent migration. Signed-off-by: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> --- .../kubernetes/kubernetes_manager.go | 161 +++++++++++++++--- .../kubernetes_manager_public_test.go | 99 ++++++++++- pkg/provisioner/provisioner.go | 3 +- 3 files changed, 233 insertions(+), 30 deletions(-) diff --git a/pkg/provisioner/kubernetes/kubernetes_manager.go b/pkg/provisioner/kubernetes/kubernetes_manager.go index 8f37a44ec..7e5e112f3 100644 --- a/pkg/provisioner/kubernetes/kubernetes_manager.go +++ b/pkg/provisioner/kubernetes/kubernetes_manager.go @@ -664,22 +664,74 @@ func (k *BaseKubernetesManager) ApplyBlueprint(blueprint *blueprintv1alpha1.Blue return nil } -// DeleteBlueprint deletes all kustomizations from the blueprint and handles cleanup kustomizations. -// It first deletes all main kustomizations, then for each kustomization with cleanup paths, it applies -// cleanup kustomizations and then deletes them. This method orchestrates the complete blueprint -// teardown process in the correct order. +// DeleteBlueprint deletes all kustomizations defined in the given blueprint in the specified namespace, handling both primary and cleanup kustomizations. +// For each kustomization (in reverse dependency order), if cleanup steps are defined, it applies and waits for readiness of any cleanup kustomizations before deleting them. +// Each cleanup kustomization is applied using server-side apply (SSA), waited for (proceeding even if not ready within timeout), and then deleted. +// After cleanup (if present), the main kustomization is deleted. Errors are accumulated and reported at the end; the function continues attempts even after encountering failures. +// Context: When DeletionPolicy:WaitForTermination is used on the main kustomization, dependent pods are terminated enabling proper PVC deletion after cleanup. func (k *BaseKubernetesManager) DeleteBlueprint(blueprint *blueprintv1alpha1.Blueprint, namespace string) error { defaultSourceName := blueprint.Metadata.Name + toDelete := []blueprintv1alpha1.Kustomization{} for _, kustomization := range blueprint.Kustomizations { - if kustomization.Destroy != nil && !*kustomization.Destroy { - continue + if kustomization.Destroy == nil || *kustomization.Destroy { + toDelete = append(toDelete, kustomization) } + } - if err := k.DeleteKustomization(kustomization.Name, namespace); err != nil { - return fmt.Errorf("failed to delete kustomization %s: %w", kustomization.Name, err) + if len(toDelete) == 0 { + return nil + } + + nameToIndex := make(map[string]int) + for i, k := range toDelete { + nameToIndex[k.Name] = i + } + + var sorted []int + visited := make(map[int]bool) + visiting := make(map[int]bool) + + var visit func(int) error + visit = func(componentIndex int) error { + if visiting[componentIndex] { + return fmt.Errorf("cycle detected in dependency graph involving kustomization '%s'", toDelete[componentIndex].Name) + } + if visited[componentIndex] { + return nil + } + + visiting[componentIndex] = true + for _, depName := range toDelete[componentIndex].DependsOn { + if depIndex, exists := nameToIndex[depName]; exists { + if err := visit(depIndex); err != nil { + visiting[componentIndex] = false + return err + } + } } + visiting[componentIndex] = false + visited[componentIndex] = true + sorted = append(sorted, componentIndex) + return nil + } + for i := range toDelete { + if !visited[i] { + if err := visit(i); err != nil { + return fmt.Errorf("dependency cycle detected: %w", err) + } + } + } + + for i, j := 0, len(sorted)-1; i < j; i, j = i+1, j-1 { + sorted[i], sorted[j] = sorted[j], sorted[i] + } + + var errors []error + + for _, idx := range sorted { + kustomization := toDelete[idx] if len(kustomization.Cleanup) > 0 { sourceName := kustomization.Source if sourceName == "" { @@ -706,9 +758,17 @@ func (k *BaseKubernetesManager) DeleteBlueprint(blueprint *blueprintv1alpha1.Blu for i, cleanupPath := range kustomization.Cleanup { cleanupPathNormalized := strings.ReplaceAll(cleanupPath, "\\", "/") - fullCleanupPath := basePath + "/" + cleanupPathNormalized + fullCleanupPath := basePath + "/cleanup/" + cleanupPathNormalized + + cleanupKustomizationName := kustomization.Name + "-cleanup" + if len(kustomization.Cleanup) > 1 { + cleanupKustomizationName = fmt.Sprintf("%s-cleanup-%d", kustomization.Name, i) + } - cleanupKustomizationName := fmt.Sprintf("%s-cleanup-%d", kustomization.Name, i) + timeout := metav1.Duration{Duration: 30 * time.Minute} + if kustomization.Timeout != nil && kustomization.Timeout.Duration != 0 { + timeout = *kustomization.Timeout + } interval := metav1.Duration{Duration: constants.DefaultFluxKustomizationInterval} if kustomization.Interval != nil && kustomization.Interval.Duration != 0 { @@ -720,20 +780,8 @@ func (k *BaseKubernetesManager) DeleteBlueprint(blueprint *blueprintv1alpha1.Blu retryInterval = *kustomization.RetryInterval } - timeout := metav1.Duration{Duration: constants.DefaultFluxKustomizationTimeout} - if kustomization.Timeout != nil && kustomization.Timeout.Duration != 0 { - timeout = *kustomization.Timeout - } - - wait := constants.DefaultFluxKustomizationWait - if kustomization.Wait != nil { - wait = *kustomization.Wait - } - - force := constants.DefaultFluxKustomizationForce - if kustomization.Force != nil { - force = *kustomization.Force - } + wait := true + force := true cleanupKustomization := kustomizev1.Kustomization{ TypeMeta: metav1.TypeMeta{ @@ -759,15 +807,76 @@ func (k *BaseKubernetesManager) DeleteBlueprint(blueprint *blueprintv1alpha1.Blu }, } + cleanupSpin := spinner.New(spinner.CharSets[14], 100*time.Millisecond, spinner.WithColor("green")) + cleanupSpin.Suffix = fmt.Sprintf(" ๐Ÿงน Applying cleanup kustomization for %s", kustomization.Name) + cleanupSpin.Start() + if err := k.ApplyKustomization(cleanupKustomization); err != nil { - return fmt.Errorf("failed to apply cleanup kustomization %s: %w", cleanupKustomizationName, err) + cleanupSpin.Stop() + errors = append(errors, fmt.Errorf("failed to apply cleanup kustomization %s: %w", cleanupKustomizationName, err)) + continue + } + + waitTimeout := time.After(k.kustomizationReconcileTimeout) + ticker := time.NewTicker(k.kustomizationWaitPollInterval) + cleanupReady := false + + cleanupLoop: + for !cleanupReady { + select { + case <-waitTimeout: + break cleanupLoop + case <-ticker.C: + status, err := k.GetKustomizationStatus([]string{cleanupKustomizationName}) + if err != nil { + cleanupSpin.Stop() + errors = append(errors, fmt.Errorf("cleanup kustomization %s failed: %w", cleanupKustomizationName, err)) + break cleanupLoop + } + if status[cleanupKustomizationName] { + cleanupReady = true + } + } } + ticker.Stop() + cleanupSpin.Stop() + + if !cleanupReady { + fmt.Fprintf(os.Stderr, "Warning: Cleanup kustomization %s did not become ready within timeout, proceeding anyway\n", cleanupKustomizationName) + } else { + fmt.Fprintf(os.Stderr, "\033[32mโœ”\033[0m ๐Ÿงน Applying cleanup kustomization for %s - \033[32mDone\033[0m\n", kustomization.Name) + } + + cleanupDeleteSpin := spinner.New(spinner.CharSets[14], 100*time.Millisecond, spinner.WithColor("green")) + cleanupDeleteSpin.Suffix = fmt.Sprintf(" ๐Ÿ—‘๏ธ Deleting cleanup kustomization %s", cleanupKustomizationName) + cleanupDeleteSpin.Start() if err := k.DeleteKustomization(cleanupKustomizationName, namespace); err != nil { - return fmt.Errorf("failed to delete cleanup kustomization %s: %w", cleanupKustomizationName, err) + cleanupDeleteSpin.Stop() + errors = append(errors, fmt.Errorf("failed to delete cleanup kustomization %s: %w", cleanupKustomizationName, err)) + } else { + cleanupDeleteSpin.Stop() + fmt.Fprintf(os.Stderr, "\033[32mโœ”\033[0m ๐Ÿ—‘๏ธ Deleting cleanup kustomization %s - \033[32mDone\033[0m\n", cleanupKustomizationName) } } } + + deleteSpin := spinner.New(spinner.CharSets[14], 100*time.Millisecond, spinner.WithColor("green")) + deleteSpin.Suffix = fmt.Sprintf(" ๐Ÿ—‘๏ธ Deleting kustomization %s", kustomization.Name) + deleteSpin.Start() + + if err := k.DeleteKustomization(kustomization.Name, namespace); err != nil { + deleteSpin.Stop() + errors = append(errors, fmt.Errorf("failed to delete kustomization %s: %w", kustomization.Name, err)) + fmt.Fprintf(os.Stderr, "Warning: failed to delete kustomization %s: %v\n", kustomization.Name, err) + } else { + deleteSpin.Stop() + fmt.Fprintf(os.Stderr, "\033[32mโœ”\033[0m ๐Ÿ—‘๏ธ Deleting kustomization %s - \033[32mDone\033[0m\n", kustomization.Name) + } + } + + if len(errors) > 0 { + return fmt.Errorf("deletion completed with %d error(s): %v", len(errors), errors[0]) } return nil diff --git a/pkg/provisioner/kubernetes/kubernetes_manager_public_test.go b/pkg/provisioner/kubernetes/kubernetes_manager_public_test.go index dcf120613..7516e7da2 100644 --- a/pkg/provisioner/kubernetes/kubernetes_manager_public_test.go +++ b/pkg/provisioner/kubernetes/kubernetes_manager_public_test.go @@ -2615,11 +2615,30 @@ func TestBaseKubernetesManager_DeleteBlueprint(t *testing.T) { var deleteCalls []string var applyCalls []string + deletedResources := make(map[string]bool) kubernetesClient.DeleteResourceFunc = func(gvr schema.GroupVersionResource, namespace, name string, opts metav1.DeleteOptions) error { deleteCalls = append(deleteCalls, name) + deletedResources[name] = true return nil } kubernetesClient.GetResourceFunc = func(gvr schema.GroupVersionResource, namespace, name string) (*unstructured.Unstructured, error) { + if deletedResources[name] { + return nil, fmt.Errorf("the server could not find the requested resource") + } + if name == "test-kustomization-cleanup-0" { + return &unstructured.Unstructured{ + Object: map[string]any{ + "status": map[string]any{ + "conditions": []any{ + map[string]any{ + "type": "Ready", + "status": "True", + }, + }, + }, + }, + }, nil + } return nil, fmt.Errorf("the server could not find the requested resource") } kubernetesClient.ApplyResourceFunc = func(gvr schema.GroupVersionResource, obj *unstructured.Unstructured, opts metav1.ApplyOptions) (*unstructured.Unstructured, error) { @@ -2638,7 +2657,7 @@ func TestBaseKubernetesManager_DeleteBlueprint(t *testing.T) { { Name: "test-kustomization", Path: "base", - Cleanup: []string{"cleanup/path"}, + Cleanup: []string{"path"}, }, }, } @@ -2665,11 +2684,30 @@ func TestBaseKubernetesManager_DeleteBlueprint(t *testing.T) { var deleteCalls []string var applyCalls []string + deletedResources := make(map[string]bool) kubernetesClient.DeleteResourceFunc = func(gvr schema.GroupVersionResource, namespace, name string, opts metav1.DeleteOptions) error { deleteCalls = append(deleteCalls, name) + deletedResources[name] = true return nil } kubernetesClient.GetResourceFunc = func(gvr schema.GroupVersionResource, namespace, name string) (*unstructured.Unstructured, error) { + if deletedResources[name] { + return nil, fmt.Errorf("the server could not find the requested resource") + } + if name == "test-kustomization-cleanup-0" || name == "test-kustomization-cleanup-1" { + return &unstructured.Unstructured{ + Object: map[string]any{ + "status": map[string]any{ + "conditions": []any{ + map[string]any{ + "type": "Ready", + "status": "True", + }, + }, + }, + }, + }, nil + } return nil, fmt.Errorf("the server could not find the requested resource") } kubernetesClient.ApplyResourceFunc = func(gvr schema.GroupVersionResource, obj *unstructured.Unstructured, opts metav1.ApplyOptions) (*unstructured.Unstructured, error) { @@ -2710,10 +2748,29 @@ func TestBaseKubernetesManager_DeleteBlueprint(t *testing.T) { kubernetesClient := client.NewMockKubernetesClient() var applyCalls []map[string]any + deletedResources := make(map[string]bool) kubernetesClient.DeleteResourceFunc = func(gvr schema.GroupVersionResource, namespace, name string, opts metav1.DeleteOptions) error { + deletedResources[name] = true return nil } kubernetesClient.GetResourceFunc = func(gvr schema.GroupVersionResource, namespace, name string) (*unstructured.Unstructured, error) { + if deletedResources[name] { + return nil, fmt.Errorf("the server could not find the requested resource") + } + if name == "test-kustomization-cleanup-0" { + return &unstructured.Unstructured{ + Object: map[string]any{ + "status": map[string]any{ + "conditions": []any{ + map[string]any{ + "type": "Ready", + "status": "True", + }, + }, + }, + }, + }, nil + } return nil, fmt.Errorf("the server could not find the requested resource") } kubernetesClient.ApplyResourceFunc = func(gvr schema.GroupVersionResource, obj *unstructured.Unstructured, opts metav1.ApplyOptions) (*unstructured.Unstructured, error) { @@ -2761,10 +2818,29 @@ func TestBaseKubernetesManager_DeleteBlueprint(t *testing.T) { kubernetesClient := client.NewMockKubernetesClient() var applyCalls []map[string]any + deletedResources := make(map[string]bool) kubernetesClient.DeleteResourceFunc = func(gvr schema.GroupVersionResource, namespace, name string, opts metav1.DeleteOptions) error { + deletedResources[name] = true return nil } kubernetesClient.GetResourceFunc = func(gvr schema.GroupVersionResource, namespace, name string) (*unstructured.Unstructured, error) { + if deletedResources[name] { + return nil, fmt.Errorf("the server could not find the requested resource") + } + if name == "test-kustomization-cleanup-0" { + return &unstructured.Unstructured{ + Object: map[string]any{ + "status": map[string]any{ + "conditions": []any{ + map[string]any{ + "type": "Ready", + "status": "True", + }, + }, + }, + }, + }, nil + } return nil, fmt.Errorf("the server could not find the requested resource") } kubernetesClient.ApplyResourceFunc = func(gvr schema.GroupVersionResource, obj *unstructured.Unstructured, opts metav1.ApplyOptions) (*unstructured.Unstructured, error) { @@ -2782,7 +2858,7 @@ func TestBaseKubernetesManager_DeleteBlueprint(t *testing.T) { { Name: "test-kustomization", Path: "base\\path", - Cleanup: []string{"cleanup\\path"}, + Cleanup: []string{"path"}, }, }, } @@ -2874,14 +2950,33 @@ func TestBaseKubernetesManager_DeleteBlueprint(t *testing.T) { kubernetesClient := client.NewMockKubernetesClient() deleteCallCount := 0 + deletedResources := make(map[string]bool) kubernetesClient.DeleteResourceFunc = func(gvr schema.GroupVersionResource, namespace, name string, opts metav1.DeleteOptions) error { deleteCallCount++ if strings.Contains(name, "cleanup") { return fmt.Errorf("delete cleanup error") } + deletedResources[name] = true return nil } kubernetesClient.GetResourceFunc = func(gvr schema.GroupVersionResource, namespace, name string) (*unstructured.Unstructured, error) { + if deletedResources[name] { + return nil, fmt.Errorf("the server could not find the requested resource") + } + if name == "test-kustomization-cleanup-0" { + return &unstructured.Unstructured{ + Object: map[string]any{ + "status": map[string]any{ + "conditions": []any{ + map[string]any{ + "type": "Ready", + "status": "True", + }, + }, + }, + }, + }, nil + } return nil, fmt.Errorf("the server could not find the requested resource") } kubernetesClient.ApplyResourceFunc = func(gvr schema.GroupVersionResource, obj *unstructured.Unstructured, opts metav1.ApplyOptions) (*unstructured.Unstructured, error) { diff --git a/pkg/provisioner/provisioner.go b/pkg/provisioner/provisioner.go index 2f95e318a..6c157cec4 100644 --- a/pkg/provisioner/provisioner.go +++ b/pkg/provisioner/provisioner.go @@ -206,13 +206,12 @@ func (i *Provisioner) Uninstall(blueprint *blueprintv1alpha1.Blueprint) error { spin.Suffix = " " + message spin.Start() + spin.Stop() if err := i.KubernetesManager.DeleteBlueprint(blueprint, constants.DefaultFluxSystemNamespace); err != nil { - spin.Stop() fmt.Fprintf(os.Stderr, "\033[31mโœ— %s - Failed\033[0m\n", message) return fmt.Errorf("failed to delete blueprint: %w", err) } - spin.Stop() fmt.Fprintf(os.Stderr, "\033[32mโœ”\033[0m %s - \033[32mDone\033[0m\n", message) return nil From d6b85486a92ceb88154fa47d27ca35fe38f116f0 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> Date: Fri, 21 Nov 2025 14:16:39 -0500 Subject: [PATCH 2/2] Fix test Signed-off-by: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> --- pkg/provisioner/kubernetes/kubernetes_manager.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/provisioner/kubernetes/kubernetes_manager.go b/pkg/provisioner/kubernetes/kubernetes_manager.go index 7e5e112f3..939f2fa3f 100644 --- a/pkg/provisioner/kubernetes/kubernetes_manager.go +++ b/pkg/provisioner/kubernetes/kubernetes_manager.go @@ -760,10 +760,7 @@ func (k *BaseKubernetesManager) DeleteBlueprint(blueprint *blueprintv1alpha1.Blu cleanupPathNormalized := strings.ReplaceAll(cleanupPath, "\\", "/") fullCleanupPath := basePath + "/cleanup/" + cleanupPathNormalized - cleanupKustomizationName := kustomization.Name + "-cleanup" - if len(kustomization.Cleanup) > 1 { - cleanupKustomizationName = fmt.Sprintf("%s-cleanup-%d", kustomization.Name, i) - } + cleanupKustomizationName := fmt.Sprintf("%s-cleanup-%d", kustomization.Name, i) timeout := metav1.Duration{Duration: 30 * time.Minute} if kustomization.Timeout != nil && kustomization.Timeout.Duration != 0 {