Skip to content

Commit

Permalink
Merge pull request #341 from chivalryq/delete-resource
Browse files Browse the repository at this point in the history
Fix: rollback delete resource logic
  • Loading branch information
Somefive authored Aug 16, 2022
2 parents 989407e + 1f49401 commit b224570
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 17 deletions.
12 changes: 7 additions & 5 deletions controllers/configuration_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}

Expand Down
14 changes: 2 additions & 12 deletions controllers/configuration_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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{
Expand Down

0 comments on commit b224570

Please sign in to comment.