Fix: allow Configuration to delete when Provider is not ready (#222)

* Fix: allow Configuration to delete when Provider is not ready When the provider doesn't exist or is not ready, allow the deletion of the Configuration Signed-off-by: Zheng Xi Zhou <[email protected]>
kubevela · Jan 14, 2022 · dea5e9b · dea5e9b
1 parent 9a6ba96
commit dea5e9b
Show file tree

Hide file tree

Showing 8 changed files with 561 additions and 118 deletions.
diff --git a/api/types/state.go b/api/types/state.go
@@ -21,6 +21,8 @@ type ConfigurationState string
 
 // Reasons a resource is or is not ready.
 const (
+	Authorizing                          ConfigurationState = "Authorizing"
+	ProviderNotFound                     ConfigurationState = "ProviderNotFound"
 	ProviderNotReady                     ConfigurationState = "ProviderNotReady"
 	ConfigurationStaticCheckFailed       ConfigurationState = "ConfigurationSpecNotValid"
 	Available                            ConfigurationState = "Available"
@@ -46,6 +48,8 @@ const (
 	MessageCloudResourceDeployed = "Cloud resources are deployed and ready to use"
 	// MessageCloudResourceDestroying is the message when cloud resource is being destroyed
 	MessageCloudResourceDestroying = "Cloud resources is being destroyed..."
+	// ErrProviderNotFound means provider not found
+	ErrProviderNotFound = "provider not found"
 	// ErrProviderNotReady means provider object is not ready
 	ErrProviderNotReady = "Provider is not ready"
 	// ConfigurationReloadingAsHCLChanged means Configuration changed and needs reloading

diff --git a/controllers/configuration/configuration.go b/controllers/configuration/configuration.go
@@ -2,6 +2,7 @@ package configuration
 
 import (
 	"context"
+	"fmt"
 	"strconv"
 	"strings"
 
@@ -15,7 +16,9 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/oam-dev/terraform-controller/api/types"
+	crossplane "github.com/oam-dev/terraform-controller/api/types/crossplane-runtime"
 	"github.com/oam-dev/terraform-controller/api/v1beta1"
+	"github.com/oam-dev/terraform-controller/controllers/provider"
 )
 
 const (
@@ -122,6 +125,29 @@ func Get(ctx context.Context, k8sClient client.Client, namespacedName apitypes.N
 	return *configuration, nil
 }
 
+// IsDeletable will check whether the Configuration can be deleted immediately
+// If deletable, it means no external cloud resources are provisioned
+func IsDeletable(ctx context.Context, k8sClient client.Client, configuration *v1beta1.Configuration) (bool, error) {
+	providerRef := GetProviderNamespacedName(*configuration)
+	providerObj, err := provider.GetProviderFromConfiguration(ctx, k8sClient, providerRef.Namespace, providerRef.Name)
+	if err != nil {
+		return false, err
+	}
+	// allow Configuration to delete when the Provider doesn't exist or is not ready, which means external cloud resources are
+	// not provisioned at all
+	if providerObj == nil || providerObj.Status.State == types.ProviderIsNotReady {
+		return true, nil
+	}
+
+	if configuration.Status.Apply.State == types.ConfigurationProvisioningAndChecking {
+		warning := fmt.Sprintf("Destroy could not complete and needs to wait for Provision to complete first: %s", types.MessageCloudResourceProvisioningAndChecking)
+		klog.Warning(warning)
+		return false, errors.New(warning)
+	}
+
+	return false, nil
+}
+
 // ReplaceTerraformSource will replace the Terraform source from GitHub to Gitee
 func ReplaceTerraformSource(remote string, githubBlockedStr string) string {
 	klog.InfoS("Whether GitHub is blocked", "githubBlocked", githubBlockedStr)
@@ -154,3 +180,14 @@ func ReplaceTerraformSource(remote string, githubBlockedStr string) string {
 	}
 	return remote
 }
+
+// GetProviderNamespacedName will get the provider namespaced name
+func GetProviderNamespacedName(configuration v1beta1.Configuration) *crossplane.Reference {
+	if configuration.Spec.ProviderReference != nil {
+		return configuration.Spec.ProviderReference
+	}
+	return &crossplane.Reference{
+		Name:      provider.DefaultName,
+		Namespace: provider.DefaultNamespace,
+	}
+}
diff --git a/controllers/configuration/configuration_test.go b/controllers/configuration/configuration_test.go
@@ -1,7 +1,18 @@
 package configuration
 
 import (
+	"context"
+	"strings"
 	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	"github.com/oam-dev/terraform-controller/api/types"
+	crossplane "github.com/oam-dev/terraform-controller/api/types/crossplane-runtime"
+	"github.com/oam-dev/terraform-controller/api/v1beta1"
 )
 
 func TestReplaceTerraformSource(t *testing.T) {
@@ -51,3 +62,140 @@ func TestReplaceTerraformSource(t *testing.T) {
 		})
 	}
 }
+
+func TestIsDeletable(t *testing.T) {
+	ctx := context.Background()
+	s := runtime.NewScheme()
+	v1beta1.AddToScheme(s)
+	provider2 := &v1beta1.Provider{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "default",
+			Namespace: "default",
+		},
+		Status: v1beta1.ProviderStatus{
+			State: types.ProviderIsNotReady,
+		},
+	}
+	provider3 := &v1beta1.Provider{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "default",
+			Namespace: "default",
+		},
+		Status: v1beta1.ProviderStatus{
+			State: types.ProviderIsReady,
+		},
+	}
+	k8sClient1 := fake.NewClientBuilder().WithScheme(s).Build()
+	k8sClient2 := fake.NewClientBuilder().WithScheme(s).WithObjects(provider2).Build()
+	k8sClient3 := fake.NewClientBuilder().WithScheme(s).WithObjects(provider3).Build()
+	k8sClient4 := fake.NewClientBuilder().Build()
+
+	configuration := &v1beta1.Configuration{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "abc",
+		},
+	}
+	configuration.Spec.ProviderReference = &crossplane.Reference{
+		Name:      "default",
+		Namespace: "default",
+	}
+
+	type args struct {
+		configuration *v1beta1.Configuration
+		k8sClient     client.Client
+	}
+	type want struct {
+		deletable bool
+		errMsg    string
+	}
+	testcases := []struct {
+		name string
+		args args
+		want want
+	}{
+		{
+			name: "provider is not found",
+			args: args{
+				k8sClient:     k8sClient1,
+				configuration: &v1beta1.Configuration{},
+			},
+			want: want{
+				deletable: true,
+			},
+		},
+		{
+			name: "provider is not ready, use default providerRef",
+			args: args{
+				k8sClient:     k8sClient2,
+				configuration: &v1beta1.Configuration{},
+			},
+			want: want{
+				deletable: true,
+			},
+		},
+		{
+			name: "provider is not ready, providerRef is set in configuration spec",
+			args: args{
+				k8sClient:     k8sClient2,
+				configuration: configuration,
+			},
+			want: want{
+				deletable: true,
+			},
+		},
+		{
+			name: "configuration is provisioning",
+			args: args{
+				k8sClient: k8sClient3,
+				configuration: &v1beta1.Configuration{
+					Status: v1beta1.ConfigurationStatus{
+						Apply: v1beta1.ConfigurationApplyStatus{
+							State: types.ConfigurationProvisioningAndChecking,
+						},
+					},
+				},
+			},
+			want: want{
+				errMsg: "Destroy could not complete and needs to wait for Provision to complete first",
+			},
+		},
+		{
+			name: "configuration is ready",
+			args: args{
+				k8sClient: k8sClient3,
+				configuration: &v1beta1.Configuration{
+					Status: v1beta1.ConfigurationStatus{
+						Apply: v1beta1.ConfigurationApplyStatus{
+							State: types.Available,
+						},
+					},
+				},
+			},
+			want: want{},
+		},
+		{
+			name: "failed to get provider",
+			args: args{
+				k8sClient:     k8sClient4,
+				configuration: &v1beta1.Configuration{},
+			},
+			want: want{
+				errMsg: "failed to get Provider object",
+			},
+		},
+	}
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			got, err := IsDeletable(ctx, tc.args.k8sClient, tc.args.configuration)
+			if err != nil {
+				if !strings.Contains(err.Error(), tc.want.errMsg) {
+					t.Errorf("IsDeletable() error = %v, wantErr %v", err, tc.want.errMsg)
+					return
+				}
+			}
+			if got != tc.want.deletable {
+				t.Errorf("IsDeletable() = %v, want %v", got, tc.want.deletable)
+			}
+		})
+	}
+}