From 4e33efc0a2764717fcff6fc62bbbbec6d71added Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B0=A2=E9=87=91=E8=99=8E?= <1050780355@qq.com> Date: Thu, 14 May 2026 11:55:15 +0800 Subject: [PATCH 1/3] fix: cleanup orphaned components when parent cluster is missing When a Cluster is deleted but its owned Components still exist without a deletionTimestamp (Running orphans), the componentLoadResourcesTransformer enters an infinite 1s requeue loop because it cannot find the Cluster. PR #18 fixed the Terminating orphan case (Components with deletionTimestamp) by handling NotFound in componentDeletionTransformer. However, it did not cover the Running orphan case where the Component has no deletionTimestamp set - this happens when Kubernetes GC fails to complete the cascade deletion. This fix adds orphan detection in componentLoadResourcesTransformer: 1. When Cluster is NotFound, check if the Component has an ownerReference pointing to that Cluster (confirming it's an orphan, not a misconfigured Component). 2. If orphaned, trigger deletion via client.Delete() to set deletionTimestamp. 3. On the next reconcile, componentDeletionTransformer (with PR #18 fix) handles the actual cleanup. This eliminates the infinite requeue loop that wastes controller resources and causes log spam across all affected namespaces. --- controllers/apps/component_controller.go | 2 +- .../transformer_component_load_resources.go | 42 ++++++++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/controllers/apps/component_controller.go b/controllers/apps/component_controller.go index cae17b2e7aa..d88749a1339 100644 --- a/controllers/apps/component_controller.go +++ b/controllers/apps/component_controller.go @@ -148,7 +148,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( // handle finalizers and referenced definition labels &componentMetaTransformer{}, // validate referenced componentDefinition objects, and build synthesized component - &componentLoadResourcesTransformer{}, + &componentLoadResourcesTransformer{Client: r.Client}, // do validation for the spec & definition consistency &componentValidationTransformer{}, // handle sidecar container diff --git a/controllers/apps/transformer_component_load_resources.go b/controllers/apps/transformer_component_load_resources.go index b1552905699..e68f7a931bf 100644 --- a/controllers/apps/transformer_component_load_resources.go +++ b/controllers/apps/transformer_component_load_resources.go @@ -22,8 +22,11 @@ package apps import ( "fmt" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1" "github.com/apecloud/kubeblocks/pkg/controller/component" "github.com/apecloud/kubeblocks/pkg/controller/graph" @@ -31,7 +34,9 @@ import ( ) // componentLoadResourcesTransformer handles referenced resources validation and load them into context -type componentLoadResourcesTransformer struct{} +type componentLoadResourcesTransformer struct { + Client client.Client +} var _ graph.Transformer = &componentLoadResourcesTransformer{} @@ -52,6 +57,9 @@ func (t *componentLoadResourcesTransformer) Transform(ctx graph.TransformContext cluster := &appsv1alpha1.Cluster{} err = transCtx.Client.Get(transCtx.Context, types.NamespacedName{Name: clusterName, Namespace: comp.Namespace}, cluster) if err != nil { + if apierrors.IsNotFound(err) && isOrphanedComponent(comp, clusterName) { + return t.handleOrphanedComponent(transCtx, comp) + } return newRequeueError(requeueDuration, err.Error()) } transCtx.Cluster = cluster @@ -64,6 +72,38 @@ func (t *componentLoadResourcesTransformer) Transform(ctx graph.TransformContext return err } +// isOrphanedComponent checks if the Component is orphaned by verifying its ownerReferences +// point to the given Cluster name. If the Component has an ownerReference of kind Cluster +// with the expected name, and that Cluster no longer exists, the Component is orphaned. +func isOrphanedComponent(comp *appsv1alpha1.Component, clusterName string) bool { + for _, ref := range comp.GetOwnerReferences() { + if ref.Kind == "Cluster" && ref.Name == clusterName { + return true + } + } + return false +} + +// handleOrphanedComponent handles the case where a Component's parent Cluster has been deleted +// but the Component itself was not properly garbage collected (no deletionTimestamp set). +// This triggers deletion of the Component so the componentDeletionTransformer can handle +// the actual cleanup in the next reconcile cycle. +func (t *componentLoadResourcesTransformer) handleOrphanedComponent(transCtx *componentTransformContext, comp *appsv1alpha1.Component) error { + transCtx.Logger.Info("detected orphaned component without deletionTimestamp, triggering deletion", + "component", comp.Name, "namespace", comp.Namespace) + + // Use the real client (not the GraphClient) to delete the Component object. + // This sets the deletionTimestamp on the Component, which will be picked up by the + // componentDeletionTransformer on the next reconcile cycle. The deletion transformer + // already handles the case where the Cluster is missing (PR #18). + if err := t.Client.Delete(transCtx.Context, comp); err != nil { + if !apierrors.IsNotFound(err) { + return newRequeueError(requeueDuration, fmt.Sprintf("failed to delete orphaned component %s: %s", comp.Name, err.Error())) + } + } + return newRequeueError(requeueDuration, fmt.Sprintf("orphaned component %s: deletion triggered, waiting for componentDeletionTransformer cleanup", comp.Name)) +} + func (t *componentLoadResourcesTransformer) transformForGeneratedComponent(transCtx *componentTransformContext) error { reqCtx := ictrlutil.RequestCtx{ Ctx: transCtx.Context, From 17586a645aa0ef5416686af53fb59fe9a84bc57e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B0=A2=E9=87=91=E8=99=8E?= <1050780355@qq.com> Date: Thu, 14 May 2026 16:35:39 +0800 Subject: [PATCH 2/3] fix: broaden orphaned component cleanup --- .../transformer_component_load_resources.go | 19 ++- ...ansformer_component_load_resources_test.go | 159 ++++++++++++++++++ 2 files changed, 173 insertions(+), 5 deletions(-) create mode 100644 controllers/apps/transformer_component_load_resources_test.go diff --git a/controllers/apps/transformer_component_load_resources.go b/controllers/apps/transformer_component_load_resources.go index e68f7a931bf..2d048b288a7 100644 --- a/controllers/apps/transformer_component_load_resources.go +++ b/controllers/apps/transformer_component_load_resources.go @@ -21,6 +21,7 @@ package apps import ( "fmt" + "strings" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" @@ -28,6 +29,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1" + "github.com/apecloud/kubeblocks/pkg/constant" "github.com/apecloud/kubeblocks/pkg/controller/component" "github.com/apecloud/kubeblocks/pkg/controller/graph" ictrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil" @@ -72,16 +74,23 @@ func (t *componentLoadResourcesTransformer) Transform(ctx graph.TransformContext return err } -// isOrphanedComponent checks if the Component is orphaned by verifying its ownerReferences -// point to the given Cluster name. If the Component has an ownerReference of kind Cluster -// with the expected name, and that Cluster no longer exists, the Component is orphaned. +// isOrphanedComponent checks if a Component belongs to a missing Cluster. +// OwnerReferences are preferred, but legacy/partially-upgraded Components can lose +// them while still carrying KubeBlocks labels and generated names. func isOrphanedComponent(comp *appsv1alpha1.Component, clusterName string) bool { for _, ref := range comp.GetOwnerReferences() { - if ref.Kind == "Cluster" && ref.Name == clusterName { + if ref.Kind == appsv1alpha1.ClusterKind && ref.Name == clusterName { return true } } - return false + if comp.Labels[constant.KBAppClusterUIDLabelKey] != "" { + return true + } + if comp.Labels[constant.KBAppComponentLabelKey] != "" && strings.HasPrefix(comp.Name, clusterName+"-") { + return true + } + return comp.Labels[constant.AppManagedByLabelKey] == constant.AppName && + strings.HasPrefix(comp.Name, clusterName+"-") } // handleOrphanedComponent handles the case where a Component's parent Cluster has been deleted diff --git a/controllers/apps/transformer_component_load_resources_test.go b/controllers/apps/transformer_component_load_resources_test.go new file mode 100644 index 00000000000..29f9ac5a784 --- /dev/null +++ b/controllers/apps/transformer_component_load_resources_test.go @@ -0,0 +1,159 @@ +/* +Copyright (C) 2022-2024 ApeCloud Co., Ltd + +This file is part of KubeBlocks project + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . +*/ + +package apps + +import ( + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/rand" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1" + "github.com/apecloud/kubeblocks/pkg/constant" + "github.com/apecloud/kubeblocks/pkg/controller/graph" + "github.com/apecloud/kubeblocks/pkg/controller/model" +) + +var _ = Describe("component load resources transformer", func() { + const ( + clusterName = "missing-cluster" + compName = "mysql" + ) + + var ( + comp *appsv1alpha1.Component + transCtx *componentTransformContext + transformer *componentLoadResourcesTransformer + nameSuffix string + ) + + newComponent := func(labels map[string]string) *appsv1alpha1.Component { + if labels == nil { + labels = map[string]string{} + } + labels[constant.AppInstanceLabelKey] = clusterName + return &appsv1alpha1.Component{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testCtx.DefaultNamespace, + Name: fmt.Sprintf("%s-%s", constant.GenerateClusterComponentName(clusterName, compName), nameSuffix), + Labels: labels, + Finalizers: []string{constant.DBComponentFinalizerName}, + }, + Spec: appsv1alpha1.ComponentSpec{ + CompDef: "test-compdef", + Replicas: 1, + }, + } + } + + runTransform := func() error { + graphCli := model.NewGraphClient(&mockReader{}) + dag := graph.NewDAG() + graphCli.Root(dag, comp, comp, model.ActionStatusPtr()) + transCtx = &componentTransformContext{ + Context: ctx, + Client: graphCli, + EventRecorder: nil, + Logger: logger, + Component: comp, + ComponentOrig: comp.DeepCopy(), + } + transformer = &componentLoadResourcesTransformer{Client: k8sClient} + return transformer.Transform(transCtx, dag) + } + + BeforeEach(func() { + comp = nil + nameSuffix = rand.String(6) + }) + + AfterEach(func() { + if comp != nil { + fetched := &appsv1alpha1.Component{} + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(comp), fetched); err == nil { + controllerutil.RemoveFinalizer(fetched, constant.DBComponentFinalizerName) + _ = k8sClient.Update(ctx, fetched) + _ = client.IgnoreNotFound(k8sClient.Delete(ctx, fetched)) + } + } + }) + + expectDeletionTriggered := func() { + Expect(k8sClient.Create(ctx, comp)).Should(Succeed()) + Expect(runTransform()).Should(HaveOccurred()) + + fetched := &appsv1alpha1.Component{} + Expect(k8sClient.Get(ctx, types.NamespacedName{ + Namespace: comp.Namespace, + Name: comp.Name, + }, fetched)).Should(Succeed()) + Expect(fetched.DeletionTimestamp.IsZero()).Should(BeFalse()) + } + + It("deletes an orphaned component identified by ownerReference", func() { + comp = newComponent(nil) + comp.OwnerReferences = []metav1.OwnerReference{ + { + APIVersion: appsv1alpha1.APIVersion, + Kind: appsv1alpha1.ClusterKind, + Name: clusterName, + }, + } + + expectDeletionTriggered() + }) + + It("deletes an orphaned component identified by cluster UID label", func() { + comp = newComponent(map[string]string{ + constant.KBAppClusterUIDLabelKey: "missing-cluster-uid", + }) + + expectDeletionTriggered() + }) + + It("deletes an orphaned component identified by generated component labels", func() { + comp = newComponent(map[string]string{ + constant.AppManagedByLabelKey: constant.AppName, + constant.KBAppComponentLabelKey: compName, + }) + + expectDeletionTriggered() + }) + + It("does not delete a component without KubeBlocks ownership metadata", func() { + comp = newComponent(nil) + + Expect(k8sClient.Create(ctx, comp)).Should(Succeed()) + Expect(runTransform()).Should(HaveOccurred()) + + fetched := &appsv1alpha1.Component{} + Expect(k8sClient.Get(ctx, types.NamespacedName{ + Namespace: comp.Namespace, + Name: comp.Name, + }, fetched)).Should(Succeed()) + Expect(fetched.DeletionTimestamp.IsZero()).Should(BeTrue()) + }) +}) From d85d0b3d4ed4767edae0e9fb504e71b5456dc96a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B0=A2=E9=87=91=E8=99=8E?= <1050780355@qq.com> Date: Fri, 15 May 2026 11:56:14 +0800 Subject: [PATCH 3/3] fix: cleanup deleting orphaned component dependents --- config/rbac/role.yaml | 17 ++ controllers/apps/cluster_plan_builder.go | 4 + controllers/apps/component_controller.go | 3 + controllers/apps/transform_types.go | 4 + .../apps/transformer_component_deletion.go | 85 ++++++-- .../transformer_component_deletion_test.go | 181 ++++++++++++++++++ deploy/helm/config/rbac/role.yaml | 17 ++ 7 files changed, 300 insertions(+), 11 deletions(-) create mode 100644 controllers/apps/transformer_component_deletion_test.go diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 89c9abc07fd..a428216cd02 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -573,6 +573,23 @@ rules: - pods/status verbs: - get +- apiGroups: + - policy + resources: + - poddisruptionbudgets + verbs: + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - policy + resources: + - poddisruptionbudgets/finalizers + verbs: + - update - apiGroups: - "" resources: diff --git a/controllers/apps/cluster_plan_builder.go b/controllers/apps/cluster_plan_builder.go index 64c8af73b10..df7ca35d790 100644 --- a/controllers/apps/cluster_plan_builder.go +++ b/controllers/apps/cluster_plan_builder.go @@ -29,6 +29,7 @@ import ( snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -41,6 +42,7 @@ import ( appsv1beta1 "github.com/apecloud/kubeblocks/apis/apps/v1beta1" dpv1alpha1 "github.com/apecloud/kubeblocks/apis/dataprotection/v1alpha1" extensionsv1alpha1 "github.com/apecloud/kubeblocks/apis/extensions/v1alpha1" + "github.com/apecloud/kubeblocks/apis/workloads/legacy" workloadsv1alpha1 "github.com/apecloud/kubeblocks/apis/workloads/v1alpha1" "github.com/apecloud/kubeblocks/pkg/constant" "github.com/apecloud/kubeblocks/pkg/controller/graph" @@ -121,6 +123,8 @@ func init() { model.AddScheme(extensionsv1alpha1.AddToScheme) model.AddScheme(workloadsv1alpha1.AddToScheme) model.AddScheme(appsv1beta1.AddToScheme) + model.AddScheme(apiextv1.AddToScheme) + model.AddScheme(legacy.AddToScheme) } // PlanBuilder implementation diff --git a/controllers/apps/component_controller.go b/controllers/apps/component_controller.go index d88749a1339..da75850a143 100644 --- a/controllers/apps/component_controller.go +++ b/controllers/apps/component_controller.go @@ -93,6 +93,9 @@ type ComponentReconciler struct { // +kubebuilder:rbac:groups=core,resources=pods/finalizers,verbs=update // +kubebuilder:rbac:groups=core,resources=pods/exec,verbs=create +// +kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets,verbs=get;list;watch;update;patch;delete +// +kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets/finalizers,verbs=update + // read only + watch access // +kubebuilder:rbac:groups=storage.k8s.io,resources=storageclasses,verbs=get;list;watch diff --git a/controllers/apps/transform_types.go b/controllers/apps/transform_types.go index 3ca02f57d68..2188d472014 100644 --- a/controllers/apps/transform_types.go +++ b/controllers/apps/transform_types.go @@ -22,6 +22,7 @@ package apps import ( snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" batchv1 "k8s.io/api/batch/v1" + apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -32,6 +33,7 @@ import ( appsv1beta1 "github.com/apecloud/kubeblocks/apis/apps/v1beta1" dpv1alpha1 "github.com/apecloud/kubeblocks/apis/dataprotection/v1alpha1" extensionsv1alpha1 "github.com/apecloud/kubeblocks/apis/extensions/v1alpha1" + "github.com/apecloud/kubeblocks/apis/workloads/legacy" workloads "github.com/apecloud/kubeblocks/apis/workloads/v1alpha1" ) @@ -48,7 +50,9 @@ func init() { utilruntime.Must(snapshotv1.AddToScheme(rscheme)) utilruntime.Must(extensionsv1alpha1.AddToScheme(rscheme)) utilruntime.Must(batchv1.AddToScheme(rscheme)) + utilruntime.Must(apiextv1.AddToScheme(rscheme)) utilruntime.Must(workloads.AddToScheme(rscheme)) + utilruntime.Must(legacy.AddToScheme(rscheme)) } type gvkNObjKey struct { diff --git a/controllers/apps/transformer_component_deletion.go b/controllers/apps/transformer_component_deletion.go index aad861a60d4..7790f4406f6 100644 --- a/controllers/apps/transformer_component_deletion.go +++ b/controllers/apps/transformer_component_deletion.go @@ -21,18 +21,21 @@ package apps import ( "fmt" + "reflect" "time" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" + policyv1 "k8s.io/api/policy/v1" rbacv1 "k8s.io/api/rbac/v1" - "k8s.io/apimachinery/pkg/types" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1" dpv1alpha1 "github.com/apecloud/kubeblocks/apis/dataprotection/v1alpha1" + "github.com/apecloud/kubeblocks/apis/workloads/legacy" workloads "github.com/apecloud/kubeblocks/apis/workloads/v1alpha1" "github.com/apecloud/kubeblocks/pkg/constant" "github.com/apecloud/kubeblocks/pkg/controller/component" @@ -59,11 +62,13 @@ func (t *componentDeletionTransformer) Transform(ctx graph.TransformContext, dag } graphCli, _ := transCtx.Client.(model.GraphClient) comp := transCtx.Component + clusterMissing := false cluster, err := t.getCluster(transCtx, comp) if err != nil { if !apierrors.IsNotFound(err) { return newRequeueError(requeueDuration, err.Error()) } + clusterMissing = true // Cluster has been deleted, use a default cluster with Delete termination policy // to allow the component deletion to proceed cluster = t.newDefaultCluster(comp) @@ -79,16 +84,18 @@ func (t *componentDeletionTransformer) Transform(ctx graph.TransformContext, dag } // step2: do the pre-terminate action if needed - if err := component.ReconcileCompPreTerminate(reqCtx, transCtx.Client, graphCli, cluster, comp, dag); err != nil { - reqCtx.Log.Info("failed to reconcile component pre-terminate action", "component", comp.Name, "error", err) - if intctrlutil.IsTargetError(err, intctrlutil.ErrorTypeExpectedInProcess) { - // waiting for the preTerminate action to be done, and watch the action finish event to trigger the next reconcile - return nil - } - if intctrlutil.IsTargetError(err, intctrlutil.ErrorTypeRequeue) { - return newRequeueError(time.Second*1, "request to requeue the component pre-terminate action") + if !clusterMissing { + if err := component.ReconcileCompPreTerminate(reqCtx, transCtx.Client, graphCli, cluster, comp, dag); err != nil { + reqCtx.Log.Info("failed to reconcile component pre-terminate action", "component", comp.Name, "error", err) + if intctrlutil.IsTargetError(err, intctrlutil.ErrorTypeExpectedInProcess) { + // waiting for the preTerminate action to be done, and watch the action finish event to trigger the next reconcile + return nil + } + if intctrlutil.IsTargetError(err, intctrlutil.ErrorTypeRequeue) { + return newRequeueError(time.Second*1, "request to requeue the component pre-terminate action") + } + return err } - return err } // step3: delete the sub-resources @@ -108,7 +115,11 @@ func (t *componentDeletionTransformer) Transform(ctx graph.TransformContext, dag // handleCompDeleteWhenScaleIn handles the component deletion when scale-in, this scenario will delete all the sub-resources owned by the component by default. func (t *componentDeletionTransformer) handleCompDeleteWhenScaleIn(transCtx *componentTransformContext, graphCli model.GraphClient, dag *graph.DAG, comp *appsv1alpha1.Component, matchLabels map[string]string) error { - return t.deleteCompResources(transCtx, graphCli, dag, comp, matchLabels, kindsForCompWipeOut()) + toDeleteKinds, err := compDeleteKindsWithLegacyRSM(transCtx, graphCli, kindsForCompWipeOut()) + if err != nil { + return err + } + return t.deleteCompResources(transCtx, graphCli, dag, comp, matchLabels, toDeleteKinds) } // handleCompDeleteWhenClusterDelete handles the component deletion when the cluster is being deleted, the sub-resources owned by the component depends on the cluster's TerminationPolicy. @@ -121,6 +132,11 @@ func (t *componentDeletionTransformer) handleCompDeleteWhenClusterDelete(transCt case appsv1alpha1.WipeOut: toDeleteKinds = kindsForCompWipeOut() } + var err error + toDeleteKinds, err = compDeleteKindsWithLegacyRSM(transCtx, graphCli, toDeleteKinds) + if err != nil { + return err + } return t.deleteCompResources(transCtx, graphCli, dag, comp, matchLabels, toDeleteKinds) } @@ -132,6 +148,13 @@ func (t *componentDeletionTransformer) deleteCompResources(transCtx *componentTr if err != nil { return newRequeueError(requeueDuration, err.Error()) } + dependents, err := readCompOwnedObjects(transCtx, comp, toDeleteKinds...) + if err != nil { + return newRequeueError(requeueDuration, err.Error()) + } + for key, object := range dependents { + snapshot[key] = object + } if len(snapshot) > 0 { // delete the sub-resources owned by the component before deleting the component for _, object := range snapshot { @@ -181,6 +204,45 @@ func (t *componentDeletionTransformer) newDefaultCluster(comp *appsv1alpha1.Comp } } +func readCompOwnedObjects(transCtx *componentTransformContext, comp *appsv1alpha1.Component, kinds ...client.ObjectList) (model.ObjectSnapshot, error) { + snapshot := make(model.ObjectSnapshot) + for _, list := range kinds { + if err := transCtx.GetClient().List(transCtx.GetContext(), list, client.InNamespace(comp.Namespace)); err != nil { + if model.IsPolicyV1DiscoveryNotFoundError(err) { + continue + } + return nil, err + } + items := reflect.ValueOf(list).Elem().FieldByName("Items") + if !items.IsValid() { + return nil, fmt.Errorf("ObjectList has no Items field: %s", list.GetObjectKind().GroupVersionKind().String()) + } + for i := 0; i < items.Len(); i++ { + object := items.Index(i).Addr().Interface().(client.Object) + if !model.IsOwnerOf(comp, object) { + continue + } + key, err := model.GetGVKName(object) + if err != nil { + return nil, err + } + snapshot[*key] = object + } + } + return snapshot, nil +} + +func compDeleteKindsWithLegacyRSM(transCtx *componentTransformContext, graphCli model.GraphClient, kinds []client.ObjectList) ([]client.ObjectList, error) { + exists, err := legacyCRDExists(transCtx.Context, graphCli) + if err != nil { + return nil, err + } + if exists { + kinds = append(kinds, &legacy.ReplicatedStateMachineList{}) + } + return kinds, nil +} + func compOwnedKinds() []client.ObjectList { return []client.ObjectList{ &workloads.InstanceSetList{}, @@ -191,6 +253,7 @@ func compOwnedKinds() []client.ObjectList { &dpv1alpha1.RestoreList{}, &dpv1alpha1.BackupList{}, &appsv1alpha1.ConfigurationList{}, + &policyv1.PodDisruptionBudgetList{}, } } diff --git a/controllers/apps/transformer_component_deletion_test.go b/controllers/apps/transformer_component_deletion_test.go new file mode 100644 index 00000000000..bb623547e2d --- /dev/null +++ b/controllers/apps/transformer_component_deletion_test.go @@ -0,0 +1,181 @@ +/* +Copyright (C) 2022-2024 ApeCloud Co., Ltd + +This file is part of KubeBlocks project + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . +*/ + +package apps + +import ( + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + policyv1 "k8s.io/api/policy/v1" + apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1" + "github.com/apecloud/kubeblocks/apis/workloads/legacy" + "github.com/apecloud/kubeblocks/pkg/constant" + "github.com/apecloud/kubeblocks/pkg/controller/graph" + "github.com/apecloud/kubeblocks/pkg/controller/model" +) + +var _ = Describe("component deletion transformer", func() { + const ( + clusterName = "missing-cluster" + compName = "mysql" + ) + + var ( + comp *appsv1alpha1.Component + transCtx *componentTransformContext + graphCli model.GraphClient + dag *graph.DAG + ) + + ownerRef := func() []metav1.OwnerReference { + controller := true + return []metav1.OwnerReference{ + { + APIVersion: appsv1alpha1.APIVersion, + Kind: appsv1alpha1.ComponentKind, + Name: comp.Name, + UID: comp.UID, + Controller: &controller, + }, + } + } + + BeforeEach(func() { + now := metav1.NewTime(time.Now()) + comp = &appsv1alpha1.Component{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testCtx.DefaultNamespace, + Name: constant.GenerateClusterComponentName(clusterName, compName), + UID: types.UID("component-uid"), + DeletionTimestamp: &now, + Labels: map[string]string{ + constant.AppInstanceLabelKey: clusterName, + constant.KBAppComponentLabelKey: compName, + }, + }, + Spec: appsv1alpha1.ComponentSpec{ + CompDef: "missing-compdef", + }, + Status: appsv1alpha1.ComponentStatus{ + Phase: appsv1alpha1.DeletingClusterCompPhase, + }, + } + }) + + It("deletes ownerReference dependents when parent cluster is missing", func() { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: comp.Namespace, + Name: comp.Name + "-env", + OwnerReferences: ownerRef(), + Finalizers: []string{constant.DBComponentFinalizerName}, + }, + } + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: comp.Namespace, + Name: comp.Name + "-account-root", + OwnerReferences: ownerRef(), + Finalizers: []string{constant.DBComponentFinalizerName}, + }, + } + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: comp.Namespace, + Name: comp.Name, + OwnerReferences: ownerRef(), + Finalizers: []string{constant.DBComponentFinalizerName}, + }, + } + pdb := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: comp.Namespace, + Name: comp.Name, + OwnerReferences: ownerRef(), + Finalizers: []string{constant.DBClusterFinalizerName}, + }, + } + rsm := &legacy.ReplicatedStateMachine{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: comp.Namespace, + Name: comp.Name, + OwnerReferences: ownerRef(), + Finalizers: []string{constant.DBClusterFinalizerName, "rsm.workloads.kubeblocks.io/finalizer"}, + }, + } + rsmCRD := &apiextv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "replicatedstatemachines.workloads.kubeblocks.io", + }, + Spec: apiextv1.CustomResourceDefinitionSpec{ + Group: "workloads.kubeblocks.io", + Names: apiextv1.CustomResourceDefinitionNames{ + Plural: "replicatedstatemachines", + Kind: "ReplicatedStateMachine", + }, + Scope: apiextv1.NamespaceScoped, + Versions: []apiextv1.CustomResourceDefinitionVersion{ + {Name: "v1alpha1", Served: true, Storage: true}, + }, + }, + } + unowned := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: comp.Namespace, + Name: "unowned", + }, + } + + cli := fake.NewClientBuilder(). + WithScheme(rscheme). + WithObjects(comp, cm, secret, svc, pdb, rsm, rsmCRD, unowned). + Build() + graphCli = model.NewGraphClient(cli) + dag = graph.NewDAG() + graphCli.Root(dag, comp, comp, model.ActionStatusPtr()) + transCtx = &componentTransformContext{ + Context: ctx, + Client: graphCli, + EventRecorder: nil, + Logger: logger, + Component: comp, + ComponentOrig: comp.DeepCopy(), + } + + err := (&componentDeletionTransformer{}).Transform(transCtx, dag) + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring("not all component sub-resources deleted")) + + Expect(graphCli.IsAction(dag, cm, model.ActionDeletePtr())).Should(BeTrue()) + Expect(graphCli.IsAction(dag, secret, model.ActionDeletePtr())).Should(BeTrue()) + Expect(graphCli.IsAction(dag, svc, model.ActionDeletePtr())).Should(BeTrue()) + Expect(graphCli.IsAction(dag, pdb, model.ActionDeletePtr())).Should(BeTrue()) + Expect(graphCli.IsAction(dag, rsm, model.ActionDeletePtr())).Should(BeTrue()) + Expect(graphCli.IsAction(dag, unowned, model.ActionDeletePtr())).Should(BeFalse()) + }) +}) diff --git a/deploy/helm/config/rbac/role.yaml b/deploy/helm/config/rbac/role.yaml index 89c9abc07fd..a428216cd02 100644 --- a/deploy/helm/config/rbac/role.yaml +++ b/deploy/helm/config/rbac/role.yaml @@ -573,6 +573,23 @@ rules: - pods/status verbs: - get +- apiGroups: + - policy + resources: + - poddisruptionbudgets + verbs: + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - policy + resources: + - poddisruptionbudgets/finalizers + verbs: + - update - apiGroups: - "" resources: