Feat: allow delete the provisioning resources (#354)

* Feat: allow delete configuration halfway the apply * Modify chart * Add test Fix test script * smaller quota Signed-off-by: Qiaozp <[email protected]> --------- Signed-off-by: Qiaozp <[email protected]>
kubevela · Feb 9, 2023 · e2f31a4 · e2f31a4
1 parent 58fad4c
commit e2f31a4
Show file tree

Hide file tree

Showing 17 changed files with 240 additions and 74 deletions.
diff --git a/Makefile b/Makefile
@@ -82,7 +82,7 @@ ifeq (, $(shell which controller-gen))
 	CONTROLLER_GEN_TMP_DIR=$$(mktemp -d) ;\
 	cd $$CONTROLLER_GEN_TMP_DIR ;\
 	go mod init tmp ;\
-	go install sigs.k8s.io/controller-tools/cmd/[email protected].0 ;\
+	go install sigs.k8s.io/controller-tools/cmd/[email protected].2 ;\
 	rm -rf $$CONTROLLER_GEN_TMP_DIR ;\
 	}
 CONTROLLER_GEN=$(GOBIN)/controller-gen
@@ -267,7 +267,7 @@ custom: custom-credentials custom-provider
 
 
 configuration:
-	go test -coverprofile=e2e-coverage1.xml -v $(go list ./e2e/...|grep -v controllernamespace) -count=1
+	go test -coverprofile=e2e-coverage1.xml -v $(shell go list ./e2e/...|grep -v controllernamespace) -count=1
 	go test -v ./e2e/controllernamespace/...
 
 e2e-setup: install-chart alibaba

diff --git a/api/v1beta2/configuration_types.go b/api/v1beta2/configuration_types.go
@@ -165,7 +165,8 @@ type S3BackendConf struct {
 // Configuration is the Schema for the configurations API
 // +kubebuilder:storageversion
 // +kubebuilder:subresource:status
-// +kubebuilder:printcolumn:name="STATE",type="string",JSONPath=".status.apply.state"
+// +kubebuilder:printcolumn:name="APPLY",type="string",JSONPath=".status.apply.state"
+// +kubebuilder:printcolumn:name="DESTROY",type="string",JSONPath=".status.destroy.state"
 // +kubebuilder:printcolumn:name="AGE",type="date",JSONPath=".metadata.creationTimestamp"
 type Configuration struct {
 	metav1.TypeMeta   `json:",inline"`

diff --git a/chart/crds/terraform.core.oam.dev_configurations.yaml b/chart/crds/terraform.core.oam.dev_configurations.yaml
@@ -4,7 +4,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.6.0
+    controller-gen.kubebuilder.io/version: v0.6.2
   creationTimestamp: null
   name: configurations.terraform.core.oam.dev
 spec:
@@ -218,7 +218,10 @@ spec:
       status: {}
   - additionalPrinterColumns:
     - jsonPath: .status.apply.state
-      name: STATE
+      name: APPLY
+      type: string
+    - jsonPath: .status.destroy.state
+      name: DESTROY
       type: string
     - jsonPath: .metadata.creationTimestamp
       name: AGE

diff --git a/chart/crds/terraform.core.oam.dev_providers.yaml b/chart/crds/terraform.core.oam.dev_providers.yaml
@@ -4,7 +4,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.6.0
+    controller-gen.kubebuilder.io/version: v0.6.2
   creationTimestamp: null
   name: providers.terraform.core.oam.dev
 spec:

diff --git a/chart/templates/terraform_controller.yaml b/chart/templates/terraform_controller.yaml
@@ -24,6 +24,7 @@ spec:
             {{- if .Values.controllerNamespace }}
             - --controller-namespace={{ .Values.controllerNamespace }}
             {{- end }}
+            - --feature-gates=AllowDeleteProvisioningResource={{ .Values.featureGates.AllowDeleteProvisioningResource }}
           env:
             - name: CONTROLLER_NAMESPACE
               valueFrom:

diff --git a/chart/values.yaml b/chart/values.yaml
@@ -15,13 +15,19 @@ jobNodeSelector: ""
 
 resources:
   limits:
-    cpu: "1000m"
-    memory: "2Gi"
+    cpu: "500m"
+    memory: "500Mi"
   requests:
-    cpu: "1000m"
-    memory: "2Gi"
+    cpu: "250m"
+    memory: "250Mi"
 
 backend:
   namespace: vela-system
 
 githubBlocked: "'false'"
+
+featureGates:
+  # Enable the feature of allowing to delete a configuration whose cloud resources is not fully provisioned, or error happens
+  # This guarantees that the partial cloud resources will be deleted when the configuration is deleted
+  # Default value is true
+  AllowDeleteProvisioningResource: true
diff --git a/controllers/configuration/configuration.go b/controllers/configuration/configuration.go
@@ -9,13 +9,15 @@ import (
 	"github.com/pkg/errors"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	apitypes "k8s.io/apimachinery/pkg/types"
+	"k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/klog/v2"
 	"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/api/v1beta2"
+	"github.com/oam-dev/terraform-controller/controllers/features"
 	"github.com/oam-dev/terraform-controller/controllers/provider"
 )
 
@@ -82,9 +84,13 @@ func Get(ctx context.Context, k8sClient client.Client, namespacedName apitypes.N
 
 // IsDeletable will check whether the Configuration can be deleted immediately
 // If deletable, it means
+// - feature gate AllowDeleteProvisioningResource is enabled
 // - no external cloud resources are provisioned
 // - it's in force-delete state
 func IsDeletable(ctx context.Context, k8sClient client.Client, configuration *v1beta2.Configuration) (bool, error) {
+	if feature.DefaultFeatureGate.Enabled(features.AllowDeleteProvisioningResource) {
+		return true, nil
+	}
 	if configuration.Spec.ForceDelete != nil && *configuration.Spec.ForceDelete {
 		return true, nil
 	}

diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go
@@ -28,14 +28,14 @@ import (
 	"time"
 
 	"github.com/go-logr/logr"
-	"github.com/oam-dev/terraform-controller/controllers/configuration/backend"
 	"github.com/pkg/errors"
 	batchv1 "k8s.io/api/batch/v1"
 	v1 "k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -46,6 +46,8 @@ import (
 	"github.com/oam-dev/terraform-controller/api/v1beta1"
 	"github.com/oam-dev/terraform-controller/api/v1beta2"
 	tfcfg "github.com/oam-dev/terraform-controller/controllers/configuration"
+	"github.com/oam-dev/terraform-controller/controllers/configuration/backend"
+	"github.com/oam-dev/terraform-controller/controllers/features"
 	"github.com/oam-dev/terraform-controller/controllers/provider"
 	"github.com/oam-dev/terraform-controller/controllers/terraform"
 )
@@ -170,6 +172,7 @@ func (r *ConfigurationReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	if isDeleting {
 		// terraform destroy
 		klog.InfoS("performing Configuration Destroy", "Namespace", req.Namespace, "Name", req.Name, "JobName", meta.DestroyJobName)
+		// if allow to delete halfway, we will not check the status of the apply job.
 
 		_, err := terraform.GetTerraformStatus(ctx, meta.ControllerNamespace, meta.DestroyJobName, terraformContainerName, terraformInitContainerName)
 		if err != nil {
@@ -404,11 +407,20 @@ func (r *ConfigurationReconciler) terraformDestroy(ctx context.Context, configur
 	}
 
 	// 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)
+	// - Configuration is deletable (no cloud resources are provisioned or force delete is set) WHEN not allow to delete halfway.
 	// - OR user want to keep the resource when delete the configuration CR
-	notWaitingDestroyJob := deletable || !meta.DeleteResource
+	// If allowed to delete halfway, there could be parts of cloud resources are provisioned, so we need to wait destroy job is done.
+	notWaitingDestroyJob := deletable && !feature.DefaultFeatureGate.Enabled(features.AllowDeleteProvisioningResource) || !meta.DeleteResource
+	// If the configuration is deletable, and it is caused by AllowDeleteProvisioningResource feature, the apply job may be still running, we should clean it first to avoid data race.
+	needCleanApplyJob := deletable && feature.DefaultFeatureGate.Enabled(features.AllowDeleteProvisioningResource)
 
 	if !notWaitingDestroyJob {
+		if needCleanApplyJob {
+			err := deleteApplyJob(ctx, meta, k8sClient)
+			if err != nil {
+				return err
+			}
+		}
 		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 {
@@ -430,7 +442,7 @@ func (r *ConfigurationReconciler) terraformDestroy(ctx context.Context, configur
 	}
 
 	if configuration.Spec.ForceDelete != nil && *configuration.Spec.ForceDelete {
-		// Try to clean up more sub-resources as possible. Ignore the issues if it hit any.
+		// Try to clean up as more sub-resources as possible. Ignore the issues if it hit any.
 		if err := r.cleanUpSubResources(ctx, configuration, meta); err != nil {
 			klog.Warningf("Failed to clean up sub-resources, but it's ignored as the resources are being forced to delete: %s", err)
 		}

diff --git a/controllers/features/feature_gate.go b/controllers/features/feature_gate.go
@@ -0,0 +1,35 @@
+/*
+Copyright 2023 The KubeVela Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package features
+
+import (
+	"k8s.io/apimachinery/pkg/util/runtime"
+	"k8s.io/apiserver/pkg/util/feature"
+	"k8s.io/component-base/featuregate"
+)
+
+const (
+	AllowDeleteProvisioningResource featuregate.Feature = "AllowDeleteProvisioningResource"
+)
+
+var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
+	AllowDeleteProvisioningResource: {Default: false, PreRelease: featuregate.Alpha},
+}
+
+func init() {
+	runtime.Must(feature.DefaultMutableFeatureGate.Add(defaultFeatureGates))
+}
diff --git a/controllers/terraform/status.go b/controllers/terraform/status.go
@@ -35,6 +35,7 @@ func GetTerraformStatus(ctx context.Context, jobNamespace, jobName, containerNam
 	return state, errors.New(errMsg)
 }
 
+// analyzeTerraformLog will analyze the logs of Terraform apply pod, returns true if check is ok.
 func analyzeTerraformLog(logs string, stage types.Stage) (bool, types.ConfigurationState, string) {
 	lines := strings.Split(logs, "\n")
 	for i, line := range lines {