Merge pull request #341 from chivalryq/delete-resource

Fix: rollback delete resource logic
kubevela · Aug 16, 2022 · b224570 · b224570
2 parents 989407e + 1f49401
commit b224570
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 17 deletions.
diff --git 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)
 	}
 

diff --git 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{