From d9e9d6be4fcb1c9616cfbff3b371dae09c916b47 Mon Sep 17 00:00:00 2001 From: Qiaozp Date: Tue, 16 Aug 2022 14:55:09 +0800 Subject: [PATCH 1/2] Fix: rollback delete resource logic Signed-off-by: Qiaozp --- controllers/configuration_controller.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index 75f7460e..ba5eabaa 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -359,9 +359,12 @@ func (r *ConfigurationReconciler) terraformDestroy(ctx context.Context, configur return err } - deleteConfigurationDirectly := deletable || meta.DeleteResource + // Sub-resources can be deleted directly without waiting destroy job is done means: + // - Configuration is deletable (no cloud resources are provisioned or force delete is set) + // - OR user want to keep the resource when delete the configuration CR + notWaitingDestroyJob := deletable || !meta.DeleteResource - if !deleteConfigurationDirectly { + if !notWaitingDestroyJob { if err := k8sClient.Get(ctx, client.ObjectKey{Name: meta.DestroyJobName, Namespace: meta.ControllerNamespace}, &destroyJob); err != nil { if kerrors.IsNotFound(err) { if err := r.Client.Get(ctx, client.ObjectKey{Name: configuration.Name, Namespace: configuration.Namespace}, &v1beta2.Configuration{}); err == nil { @@ -390,15 +393,14 @@ func (r *ConfigurationReconciler) terraformDestroy(ctx context.Context, configur return nil } // When the deletion Job process succeeded, clean up work is starting. - if !deleteConfigurationDirectly { + if !notWaitingDestroyJob { if err := k8sClient.Get(ctx, client.ObjectKey{Name: meta.DestroyJobName, Namespace: meta.ControllerNamespace}, &destroyJob); err != nil { return err } if destroyJob.Status.Succeeded == int32(1) { return r.cleanUpSubResources(ctx, configuration, meta) } - } - if deleteConfigurationDirectly { + } else { return r.cleanUpSubResources(ctx, configuration, meta) } From 1f49401f92f603d5284a9ba0b9fb3ebdbdf017e5 Mon Sep 17 00:00:00 2001 From: Qiaozp Date: Tue, 16 Aug 2022 15:06:16 +0800 Subject: [PATCH 2/2] fix test Signed-off-by: Qiaozp --- controllers/configuration_controller_test.go | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index 12f2ebdf..1fae88aa 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -1384,9 +1384,9 @@ func TestTerraformDestroy(t *testing.T) { }, VariableSecretName: fmt.Sprintf(TFVariableSecret, secretSuffix), ConfigurationCMName: configurationCMName, + // True is default value if user ignores configuration.Spec.DeleteResource + DeleteResource: true, } - metaWithDeleteResource := baseMeta - metaWithDeleteResource.DeleteResource = true metaWithLegacyResource := baseMeta metaWithLegacyResource.LegacySubResources = LegacySubResources{ Namespace: legacyNamespace, @@ -1443,16 +1443,6 @@ func TestTerraformDestroy(t *testing.T) { objects: []client.Object{readyProvider, baseConfiguration, baseConfigurationCM}, keptResources: []client.Object{baseConfigurationCM}, }, - { - name: "set DeleteResource, directly clean resources", - args: args{ - configuration: baseConfiguration, - meta: &metaWithDeleteResource, - }, - want: want{}, - objects: []client.Object{readyProvider, baseConfiguration, baseConfigurationCM}, - deletedResources: []client.Object{baseConfigurationCM}, - }, { name: "provider is not ready, cloud resource couldn't be created, delete directly", args: args{