From 8bca590baf5f635947d866b211db78ed2bbef3f4 Mon Sep 17 00:00:00 2001 From: free6om Date: Fri, 30 Aug 2024 18:06:31 +0800 Subject: [PATCH 1/3] feat: support PVC ownerReference --- apis/apps/v1alpha1/cluster_types.go | 1 + .../apps/transformer_cluster_deletion.go | 4 ++- .../apps/transformer_component_deletion.go | 20 +-------------- pkg/controller/instanceset/instance_util.go | 5 ++++ .../instanceset/reconciler_deletion.go | 25 +++---------------- 5 files changed, 13 insertions(+), 42 deletions(-) diff --git a/apis/apps/v1alpha1/cluster_types.go b/apis/apps/v1alpha1/cluster_types.go index 549ac293327..ed505b5c7e3 100644 --- a/apis/apps/v1alpha1/cluster_types.go +++ b/apis/apps/v1alpha1/cluster_types.go @@ -92,6 +92,7 @@ type ClusterSpec struct { // - `DoNotTerminate`: Prevents deletion of the Cluster. This policy ensures that all resources remain intact. // - `Halt`: Deletes Cluster resources like Pods and Services but retains Persistent Volume Claims (PVCs), // allowing for data preservation while stopping other operations. + // Warning: Halt policy is deprecated in 0.9.1 and will have same meaning as DoNotTerminate. // - `Delete`: Extends the `Halt` policy by also removing PVCs, leading to a thorough cleanup while // removing all persistent data. // - `WipeOut`: An aggressive policy that deletes all Cluster resources, including volume snapshots and diff --git a/controllers/apps/transformer_cluster_deletion.go b/controllers/apps/transformer_cluster_deletion.go index 27e445af50a..b76b05ceee6 100644 --- a/controllers/apps/transformer_cluster_deletion.go +++ b/controllers/apps/transformer_cluster_deletion.go @@ -66,7 +66,9 @@ func (t *clusterDeletionTransformer) Transform(ctx graph.TransformContext, dag * "spec.terminationPolicy %s is preventing deletion.", cluster.Spec.TerminationPolicy) return graph.ErrPrematureStop case appsv1alpha1.Halt: - toDeleteNamespacedKinds, toDeleteNonNamespacedKinds = kindsForHalt() + transCtx.EventRecorder.Eventf(cluster, corev1.EventTypeWarning, "Halt", + "spec.terminationPolicy %s is preventing deletion. Halt policy is deprecated is 0.9.1 and will have same meaning as DoNotTerminate.", cluster.Spec.TerminationPolicy) + return graph.ErrPrematureStop case appsv1alpha1.Delete: toDeleteNamespacedKinds, toDeleteNonNamespacedKinds = kindsForDelete() case appsv1alpha1.WipeOut: diff --git a/controllers/apps/transformer_component_deletion.go b/controllers/apps/transformer_component_deletion.go index 2b8cf275569..f7af5daa1a8 100644 --- a/controllers/apps/transformer_component_deletion.go +++ b/controllers/apps/transformer_component_deletion.go @@ -20,7 +20,6 @@ along with this program. If not, see . package apps import ( - "context" "fmt" "time" @@ -90,25 +89,14 @@ func (t *componentDeletionTransformer) handleCompDeleteWhenScaleIn(transCtx *com // handleCompDeleteWhenClusterDelete handles the component deletion when the cluster is being deleted, the sub-resources owned by the component depends on the cluster's TerminationPolicy. func (t *componentDeletionTransformer) handleCompDeleteWhenClusterDelete(transCtx *componentTransformContext, graphCli model.GraphClient, dag *graph.DAG, cluster *appsv1alpha1.Cluster, comp *appsv1alpha1.Component, matchLabels map[string]string) error { - var ( - toPreserveKinds, toDeleteKinds []client.ObjectList - ) + var toDeleteKinds []client.ObjectList switch cluster.Spec.TerminationPolicy { - case appsv1alpha1.Halt: - toPreserveKinds = compOwnedPreserveKinds() - toDeleteKinds = kindsForCompHalt() case appsv1alpha1.Delete: toDeleteKinds = kindsForCompDelete() case appsv1alpha1.WipeOut: toDeleteKinds = kindsForCompWipeOut() } - if len(toPreserveKinds) > 0 { - // preserve the objects owned by the component when the component is being deleted - if err := preserveCompObjects(transCtx.Context, transCtx.Client, graphCli, dag, comp, matchLabels, toPreserveKinds); err != nil { - return newRequeueError(requeueDuration, err.Error()) - } - } return t.deleteCompResources(transCtx, graphCli, dag, comp, matchLabels, toDeleteKinds) } @@ -218,9 +206,3 @@ func kindsForCompDelete() []client.ObjectList { func kindsForCompWipeOut() []client.ObjectList { return kindsForCompDelete() } - -// preserveCompObjects preserves the objects owned by the component when the component is being deleted -func preserveCompObjects(ctx context.Context, cli client.Reader, graphCli model.GraphClient, dag *graph.DAG, - comp *appsv1alpha1.Component, ml client.MatchingLabels, toPreserveKinds []client.ObjectList) error { - return preserveObjects(ctx, cli, graphCli, dag, comp, ml, toPreserveKinds, constant.DBComponentFinalizerName, constant.LastAppliedClusterAnnotationKey) -} diff --git a/pkg/controller/instanceset/instance_util.go b/pkg/controller/instanceset/instance_util.go index 919f28daefb..4e07d9af9c3 100644 --- a/pkg/controller/instanceset/instance_util.go +++ b/pkg/controller/instanceset/instance_util.go @@ -420,6 +420,11 @@ func buildInstanceByTemplate(name string, template *instanceTemplateExt, parent if err := controllerutil.SetControllerReference(parent, pod, model.GetScheme()); err != nil { return nil, err } + for _, pvc := range pvcs { + if err = controllerutil.SetControllerReference(parent, pvc, model.GetScheme()); err != nil { + return nil, err + } + } inst := &instance{ pod: pod, pvcs: pvcs, diff --git a/pkg/controller/instanceset/reconciler_deletion.go b/pkg/controller/instanceset/reconciler_deletion.go index e2362a8fc95..a6373720484 100644 --- a/pkg/controller/instanceset/reconciler_deletion.go +++ b/pkg/controller/instanceset/reconciler_deletion.go @@ -20,9 +20,6 @@ along with this program. If not, see . package instanceset import ( - corev1 "k8s.io/api/core/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/apecloud/kubeblocks/pkg/controller/kubebuilderx" "github.com/apecloud/kubeblocks/pkg/controller/model" ) @@ -42,12 +39,9 @@ func (r *deletionReconciler) PreCondition(tree *kubebuilderx.ObjectTree) *kubebu func (r *deletionReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuilderx.Result, error) { // delete secondary objects first - // retain all pvcs - // TODO(free6om): respect PVCManagementPolicy - allObjects := tree.GetSecondaryObjects() - objects := filterByType[*corev1.PersistentVolumeClaim](allObjects) - if len(objects) > 0 { - return kubebuilderx.Continue, tree.Delete(objects...) + if len(tree.GetSecondaryObjects()) > 0 { + tree.DeleteSecondaryObjects() + return kubebuilderx.Continue, nil } // delete root object @@ -55,19 +49,6 @@ func (r *deletionReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuild return kubebuilderx.Continue, nil } -func filterByType[T client.Object](snapshot model.ObjectSnapshot) []client.Object { - var objects []client.Object - for _, object := range snapshot { - switch object.(type) { - case T: - continue - default: - objects = append(objects, object) - } - } - return objects -} - func NewDeletionReconciler() kubebuilderx.Reconciler { return &deletionReconciler{} } From 9a220699cabfd17bb8d2131d6156e9b95c670b42 Mon Sep 17 00:00:00 2001 From: free6om Date: Fri, 30 Aug 2024 18:19:28 +0800 Subject: [PATCH 2/3] fix broken ut --- controllers/apps/cluster_controller_test.go | 34 ++++++--------------- 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/controllers/apps/cluster_controller_test.go b/controllers/apps/cluster_controller_test.go index 6c093ac7c37..4a74db6ecaa 100644 --- a/controllers/apps/cluster_controller_test.go +++ b/controllers/apps/cluster_controller_test.go @@ -912,37 +912,23 @@ var _ = Describe("Cluster Controller", func() { Context: testCtx.Ctx, Client: testCtx.Cli, } - preserveKinds := haltPreserveKinds() - preserveObjs, err := getOwningNamespacedObjects(transCtx.Context, transCtx.Client, clusterObj.Namespace, getAppInstanceML(*clusterObj), preserveKinds) + namespacedKinds, clusteredKinds := kindsForWipeOut() + allKinds := append(namespacedKinds, clusteredKinds...) + createdObjs, err := getOwningNamespacedObjects(transCtx.Context, transCtx.Client, clusterObj.Namespace, getAppInstanceML(*clusterObj), allKinds) Expect(err).Should(Succeed()) - for _, obj := range preserveObjs { - // Expect(obj.GetFinalizers()).Should(ContainElements(constant.DBClusterFinalizerName)) - Expect(obj.GetAnnotations()).ShouldNot(HaveKey(constant.LastAppliedClusterAnnotationKey)) - } By("delete the cluster") testapps.DeleteObject(&testCtx, clusterKey, &appsv1alpha1.Cluster{}) + Consistently(testapps.CheckObjExists(&testCtx, clusterKey, &appsv1alpha1.Cluster{}, true)).Should(Succeed()) - By("wait for the cluster to terminate") - Eventually(testapps.CheckObjExists(&testCtx, clusterKey, &appsv1alpha1.Cluster{}, false)).Should(Succeed()) - - By("check expected preserved objects") - keptObjs, err := getOwningNamespacedObjects(transCtx.Context, transCtx.Client, clusterObj.Namespace, getAppInstanceML(*clusterObj), preserveKinds) + By("check all cluster resources again") + objs, err := getOwningNamespacedObjects(transCtx.Context, transCtx.Client, clusterObj.Namespace, getAppInstanceML(*clusterObj), allKinds) Expect(err).Should(Succeed()) - for key, obj := range preserveObjs { - Expect(keptObjs).Should(HaveKey(key)) - keptObj := keptObjs[key] - Expect(obj.GetUID()).Should(BeEquivalentTo(keptObj.GetUID())) - Expect(keptObj.GetFinalizers()).ShouldNot(ContainElements(constant.DBClusterFinalizerName)) - Expect(keptObj.GetAnnotations()).Should(HaveKey(constant.LastAppliedClusterAnnotationKey)) + // check all objects existed before cluster deletion still be there + for key, obj := range createdObjs { + Expect(objs).Should(HaveKey(key)) + Expect(obj.GetUID()).Should(BeEquivalentTo(objs[key].GetUID())) } - - By("check all other resources deleted") - namespacedKinds, clusteredKinds := kindsForHalt() - kindsToDelete := append(namespacedKinds, clusteredKinds...) - otherObjs, err := getOwningNamespacedObjects(transCtx.Context, transCtx.Client, clusterObj.Namespace, getAppInstanceML(*clusterObj), kindsToDelete) - Expect(err).Should(Succeed()) - Expect(otherObjs).Should(HaveLen(0)) } testClusterHaltNRecovery := func(createObj func(appsv1alpha1.TerminationPolicyType)) { From e5af2b342bb123508411b93750d1b7b22271f7d2 Mon Sep 17 00:00:00 2001 From: free6om Date: Fri, 30 Aug 2024 18:24:30 +0800 Subject: [PATCH 3/3] fix cicd error --- config/crd/bases/apps.kubeblocks.io_clusters.yaml | 1 + deploy/helm/crds/apps.kubeblocks.io_clusters.yaml | 1 + docs/developer_docs/api-reference/cluster.md | 6 ++++-- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/config/crd/bases/apps.kubeblocks.io_clusters.yaml b/config/crd/bases/apps.kubeblocks.io_clusters.yaml index 696a0ca9ca2..e0766e1fe10 100644 --- a/config/crd/bases/apps.kubeblocks.io_clusters.yaml +++ b/config/crd/bases/apps.kubeblocks.io_clusters.yaml @@ -16963,6 +16963,7 @@ spec: - `DoNotTerminate`: Prevents deletion of the Cluster. This policy ensures that all resources remain intact. - `Halt`: Deletes Cluster resources like Pods and Services but retains Persistent Volume Claims (PVCs), allowing for data preservation while stopping other operations. + Warning: Halt policy is deprecated in 0.9.1 and will have same meaning as DoNotTerminate. - `Delete`: Extends the `Halt` policy by also removing PVCs, leading to a thorough cleanup while removing all persistent data. - `WipeOut`: An aggressive policy that deletes all Cluster resources, including volume snapshots and diff --git a/deploy/helm/crds/apps.kubeblocks.io_clusters.yaml b/deploy/helm/crds/apps.kubeblocks.io_clusters.yaml index 696a0ca9ca2..e0766e1fe10 100644 --- a/deploy/helm/crds/apps.kubeblocks.io_clusters.yaml +++ b/deploy/helm/crds/apps.kubeblocks.io_clusters.yaml @@ -16963,6 +16963,7 @@ spec: - `DoNotTerminate`: Prevents deletion of the Cluster. This policy ensures that all resources remain intact. - `Halt`: Deletes Cluster resources like Pods and Services but retains Persistent Volume Claims (PVCs), allowing for data preservation while stopping other operations. + Warning: Halt policy is deprecated in 0.9.1 and will have same meaning as DoNotTerminate. - `Delete`: Extends the `Halt` policy by also removing PVCs, leading to a thorough cleanup while removing all persistent data. - `WipeOut`: An aggressive policy that deletes all Cluster resources, including volume snapshots and diff --git a/docs/developer_docs/api-reference/cluster.md b/docs/developer_docs/api-reference/cluster.md index a023352644a..f22e8d21d47 100644 --- a/docs/developer_docs/api-reference/cluster.md +++ b/docs/developer_docs/api-reference/cluster.md @@ -193,7 +193,8 @@ Choose a policy based on the desired level of resource cleanup and data preserva
  • DoNotTerminate: Prevents deletion of the Cluster. This policy ensures that all resources remain intact.
  • Halt: Deletes Cluster resources like Pods and Services but retains Persistent Volume Claims (PVCs), -allowing for data preservation while stopping other operations.
    • +allowing for data preservation while stopping other operations. +Warning: Halt policy is deprecated in 0.9.1 and will have same meaning as DoNotTerminate.
  • Delete: Extends the Halt policy by also removing PVCs, leading to a thorough cleanup while removing all persistent data.
  • WipeOut: An aggressive policy that deletes all Cluster resources, including volume snapshots and @@ -5222,7 +5223,8 @@ Choose a policy based on the desired level of resource cleanup and data preserva
    • DoNotTerminate: Prevents deletion of the Cluster. This policy ensures that all resources remain intact.
    • Halt: Deletes Cluster resources like Pods and Services but retains Persistent Volume Claims (PVCs), -allowing for data preservation while stopping other operations.
      • +allowing for data preservation while stopping other operations. +Warning: Halt policy is deprecated in 0.9.1 and will have same meaning as DoNotTerminate.
    • Delete: Extends the Halt policy by also removing PVCs, leading to a thorough cleanup while removing all persistent data.
    • WipeOut: An aggressive policy that deletes all Cluster resources, including volume snapshots and