From 8d2072d4f846b255c52c0084796b4e85e4950f73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=90=B4=E5=B0=B1=E4=B8=9A?= Date: Fri, 18 Aug 2023 18:05:44 +0800 Subject: [PATCH] Before deleting, check if there are other configurations using the same backend. If there are, do not proceed with the deletion (#374) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 吴就业 --- controllers/configuration/configuration.go | 70 ++++ .../configuration/configuration_test.go | 334 ++++++++++++++++++ controllers/configuration_controller.go | 7 +- 3 files changed, 409 insertions(+), 2 deletions(-) diff --git a/controllers/configuration/configuration.go b/controllers/configuration/configuration.go index 4d2c1529..753d71cb 100644 --- a/controllers/configuration/configuration.go +++ b/controllers/configuration/configuration.go @@ -3,6 +3,7 @@ package configuration import ( "context" "fmt" + "reflect" "strconv" "strings" @@ -159,3 +160,72 @@ func GetProviderNamespacedName(configuration v1beta2.Configuration) *crossplane. Namespace: provider.DefaultNamespace, } } + +// GetConfigurationsWithSameBackendReference will get configurations referencing the same backend +func GetConfigurationsWithSameBackendReference(ctx context.Context, k8sClient client.Client, configuration *v1beta2.Configuration) ([]*crossplane.Reference, error) { + var ( + configurationRefs = make([]*crossplane.Reference, 0) + selector func(referenceBackend, backend *v1beta2.Backend) bool + ) + + backend := configuration.Spec.Backend + if backend == nil { + return configurationRefs, nil + } + + switch { + case backend.Inline != "": + selector = func(referenceBackend, backend *v1beta2.Backend) bool { + if backend == nil { + return false + } + return referenceBackend.Inline == backend.Inline + } + case backend.BackendType == "s3" && backend.S3 != nil: + selector = func(referenceBackend, backend *v1beta2.Backend) bool { + if backend == nil { + return false + } + return referenceBackend.BackendType == backend.BackendType && reflect.DeepEqual(referenceBackend.S3, backend.S3) + } + case backend.BackendType == "kubernetes" && backend.Kubernetes != nil: + selector = func(referenceBackend, backend *v1beta2.Backend) bool { + if backend == nil { + return false + } + return referenceBackend.BackendType == backend.BackendType && reflect.DeepEqual(referenceBackend.Kubernetes, backend.Kubernetes) + } + case backend.SecretSuffix != "": + selector = func(referenceBackend, backend *v1beta2.Backend) bool { + if backend == nil { + return false + } + return referenceBackend.SecretSuffix == backend.SecretSuffix + } + } + + if selector == nil { + return configurationRefs, nil + } + + configurations := &v1beta2.ConfigurationList{} + if err := k8sClient.List(ctx, configurations); err != nil { + return configurationRefs, client.IgnoreNotFound(err) + } + + for _, item := range configurations.Items { + if item.Name == configuration.Name && item.Namespace == configuration.Namespace { + continue + } + // reflect.DeepEqual(backend, item.Spec.Backend) This approach may not yield accurate results. + if !selector(backend, item.Spec.Backend) { + continue + } + configurationRefs = append(configurationRefs, &crossplane.Reference{ + Name: item.Name, + Namespace: item.Namespace, + }) + } + + return configurationRefs, nil +} diff --git a/controllers/configuration/configuration_test.go b/controllers/configuration/configuration_test.go index 0f30a933..e2e6f2ec 100644 --- a/controllers/configuration/configuration_test.go +++ b/controllers/configuration/configuration_test.go @@ -2,6 +2,8 @@ package configuration import ( "context" + "fmt" + "reflect" "strings" "testing" @@ -395,3 +397,335 @@ func TestSetRegion(t *testing.T) { }) } } + +func TestGetConfigurationsWithSameBackendReference(t *testing.T) { + ctx := context.Background() + s := runtime.NewScheme() + _ = v1beta2.AddToScheme(s) + k8sClient := fake.NewClientBuilder().WithScheme(s).Build() + + var ( + region = "cn" + namespace = "default" + ) + + configuration1 := v1beta2.Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c1", + Namespace: "default", + }, + Spec: v1beta2.ConfigurationSpec{}, + } + assert.Nil(t, k8sClient.Create(ctx, &configuration1)) + + configuration2 := v1beta2.Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c2", + Namespace: "default", + }, + Spec: v1beta2.ConfigurationSpec{ + Backend: &v1beta2.Backend{ + SecretSuffix: "s1", + }, + }, + } + assert.Nil(t, k8sClient.Create(ctx, &configuration2)) + + configuration3 := v1beta2.Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c3", + Namespace: "default", + }, + Spec: v1beta2.ConfigurationSpec{ + Backend: &v1beta2.Backend{ + SecretSuffix: "s2", + }, + }, + } + assert.Nil(t, k8sClient.Create(ctx, &configuration3)) + + configuration4 := v1beta2.Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c4", + Namespace: "default", + }, + Spec: v1beta2.ConfigurationSpec{ + Backend: &v1beta2.Backend{ + SecretSuffix: "s2", + }, + }, + } + assert.Nil(t, k8sClient.Create(ctx, &configuration4)) + + configuration5 := v1beta2.Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c5", + Namespace: "default", + }, + Spec: v1beta2.ConfigurationSpec{ + Backend: &v1beta2.Backend{ + Inline: "inline", + }, + }, + } + assert.Nil(t, k8sClient.Create(ctx, &configuration5)) + + configuration6 := v1beta2.Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c6", + Namespace: "default", + }, + Spec: v1beta2.ConfigurationSpec{ + Backend: &v1beta2.Backend{ + Inline: "inline", + }, + }, + } + assert.Nil(t, k8sClient.Create(ctx, &configuration6)) + + configuration7 := v1beta2.Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c7", + Namespace: "default", + }, + Spec: v1beta2.ConfigurationSpec{ + Backend: &v1beta2.Backend{ + BackendType: "s3", + S3: &v1beta2.S3BackendConf{ + Region: ®ion, + Bucket: "test", + Key: "test", + }, + }, + }, + } + assert.Nil(t, k8sClient.Create(ctx, &configuration7)) + + configuration8 := v1beta2.Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c8", + Namespace: "default", + }, + Spec: v1beta2.ConfigurationSpec{ + Backend: &v1beta2.Backend{ + BackendType: "s3", + S3: &v1beta2.S3BackendConf{ + Region: ®ion, + Bucket: "test", + Key: "test", + }, + }, + }, + } + assert.Nil(t, k8sClient.Create(ctx, &configuration8)) + + configuration9 := v1beta2.Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c9", + Namespace: "default", + }, + Spec: v1beta2.ConfigurationSpec{ + Backend: &v1beta2.Backend{ + BackendType: "s3", + S3: &v1beta2.S3BackendConf{ + Region: ®ion, + Bucket: "test2", + Key: "test2", + }, + }, + }, + } + assert.Nil(t, k8sClient.Create(ctx, &configuration9)) + + configuration10 := v1beta2.Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c10", + Namespace: "default", + }, + Spec: v1beta2.ConfigurationSpec{ + Backend: &v1beta2.Backend{ + BackendType: "kubernetes", + Kubernetes: &v1beta2.KubernetesBackendConf{ + SecretSuffix: "k8s", + Namespace: &namespace, + }, + }, + }, + } + assert.Nil(t, k8sClient.Create(ctx, &configuration10)) + + configuration11 := v1beta2.Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c11", + Namespace: "default", + }, + Spec: v1beta2.ConfigurationSpec{ + Backend: &v1beta2.Backend{ + BackendType: "kubernetes", + Kubernetes: &v1beta2.KubernetesBackendConf{ + SecretSuffix: "k8s", + Namespace: &namespace, + }, + }, + }, + } + assert.Nil(t, k8sClient.Create(ctx, &configuration11)) + + configuration12 := v1beta2.Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c12", + Namespace: "default", + }, + Spec: v1beta2.ConfigurationSpec{ + Backend: &v1beta2.Backend{ + BackendType: "kubernetes", + Kubernetes: &v1beta2.KubernetesBackendConf{ + SecretSuffix: "k8s2", + Namespace: &namespace, + }, + }, + }, + } + assert.Nil(t, k8sClient.Create(ctx, &configuration12)) + + type args struct { + backend *v1beta2.Backend + } + + type want struct { + references []*crossplane.Reference + errMsg string + } + + emptyReferences := make([]*crossplane.Reference, 0) + + testcases := map[string]struct { + args args + want want + }{ + "Using the default backend": { + args: args{ + backend: nil, + }, + want: want{ + references: emptyReferences, + errMsg: "", + }, + }, + "Configured with secret suffix": { + args: args{ + backend: &v1beta2.Backend{SecretSuffix: "s2"}, + }, + want: want{ + references: []*crossplane.Reference{ + { + Name: "c3", + Namespace: "default", + }, + { + Name: "c4", + Namespace: "default", + }, + }, + errMsg: "", + }, + }, + "Configured with inline": { + args: args{ + backend: &v1beta2.Backend{Inline: "inline"}, + }, + want: want{ + references: []*crossplane.Reference{ + { + Name: "c5", + Namespace: "default", + }, + { + Name: "c6", + Namespace: "default", + }, + }, + errMsg: "", + }, + }, + "Using S3 as the backend": { + args: args{ + backend: &v1beta2.Backend{ + BackendType: "s3", + S3: &v1beta2.S3BackendConf{ + Region: ®ion, + Bucket: "test", + Key: "test", + }, + }, + }, + want: want{ + references: []*crossplane.Reference{ + { + Name: "c7", + Namespace: "default", + }, + { + Name: "c8", + Namespace: "default", + }, + }, + errMsg: "", + }, + }, + "Using Kubernetes as the backend": { + args: args{ + backend: &v1beta2.Backend{ + BackendType: "kubernetes", + Kubernetes: &v1beta2.KubernetesBackendConf{ + SecretSuffix: "k8s", + Namespace: &namespace, + }, + }, + }, + want: want{ + references: []*crossplane.Reference{ + { + Name: "c10", + Namespace: "default", + }, + { + Name: "c11", + Namespace: "default", + }, + }, + errMsg: "", + }, + }, + } + for name, tc := range testcases { + t.Run(name, func(t *testing.T) { + refs, err := GetConfigurationsWithSameBackendReference(ctx, k8sClient, &v1beta2.Configuration{ + Spec: v1beta2.ConfigurationSpec{ + Backend: tc.args.backend, + }, + }) + if tc.want.errMsg != "" && !strings.Contains(err.Error(), tc.want.errMsg) { + t.Errorf("GetConfigurationsWithSameBackendReference() error = %v, wantErr %v", err, tc.want.errMsg) + } + if !reflect.DeepEqual(refs, tc.want.references) { + wantReferencesStr := "" + for _, r := range tc.want.references { + if wantReferencesStr != "" { + wantReferencesStr += "," + } + wantReferencesStr += fmt.Sprintf("%v", r) + } + referencesStr := "" + for _, r := range refs { + if referencesStr != "" { + referencesStr += "," + } + referencesStr += fmt.Sprintf("%v", r) + } + t.Errorf("GetConfigurationsWithSameBackendReference() want = [%s], got [%s]", wantReferencesStr, referencesStr) + } + }) + } + +} diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index e3029ba4..832b54a3 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -183,8 +183,11 @@ func (r *ConfigurationReconciler) Reconcile(ctx context.Context, req ctrl.Reques } } + backendOtherReferences, _ := tfcfg.GetConfigurationsWithSameBackendReference(ctx, r.Client, &configuration) + // If no tfState has been generated, then perform a quick cleanup without dispatching destroying job. - if meta.isTFStateGenerated(ctx) { + // OR. Has other configurations referencing the same backend, then perform a quick cleanup without dispatching destroying job. + if meta.isTFStateGenerated(ctx) && len(backendOtherReferences) == 0 { if err := r.terraformDestroy(ctx, configuration, meta); err != nil { if err.Error() == types.MessageDestroyJobNotCompleted { return ctrl.Result{RequeueAfter: 3 * time.Second}, nil @@ -192,7 +195,7 @@ func (r *ConfigurationReconciler) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{RequeueAfter: 3 * time.Second}, errors.Wrap(err, "continue reconciling to destroy cloud resource") } } else { - klog.Infof("No need to execute terraform destroy command, because tfstate file not found: %s/%s", configuration.Namespace, configuration.Name) + klog.Infof("No need to execute terraform destroy command, because tfstate file not generated or has other configurations referencing the same backend, configuration: %s/%s", configuration.Namespace, configuration.Name) if err := r.cleanUpSubResources(ctx, configuration, meta); err != nil { klog.Warningf("Ignoring error when clean up sub-resources, for no resource is actually created: %s", err) }