From e411ea5e7bf265583a73c81bbbb9cc0b99facd28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E6=96=87=E8=B1=AA?= Date: Thu, 25 Apr 2024 17:26:33 +0800 Subject: [PATCH] fix(add test unit): addon controller adds support for checking if an addon is used by a cluster before deleting or disabling it.(#6434) --- .../extensions/addon_controller_stages.go | 45 +++++++ .../extensions/addon_controller_test.go | 111 +++++++++++++++++- controllers/extensions/suite_test.go | 4 + 3 files changed, 159 insertions(+), 1 deletion(-) diff --git a/controllers/extensions/addon_controller_stages.go b/controllers/extensions/addon_controller_stages.go index 9a496dc8ac6..09081c14d09 100644 --- a/controllers/extensions/addon_controller_stages.go +++ b/controllers/extensions/addon_controller_stages.go @@ -37,6 +37,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1" extensionsv1alpha1 "github.com/apecloud/kubeblocks/apis/extensions/v1alpha1" "github.com/apecloud/kubeblocks/pkg/constant" intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil" @@ -170,6 +171,23 @@ func (r *fetchNDeletionCheckStage) Handle(ctx context.Context) { } r.reqCtx.Log.V(1).Info("get addon", "generation", addon.Generation, "observedGeneration", addon.Status.ObservedGeneration) r.reqCtx.UpdateCtxValue(operandValueKey, addon) + + // CheckIfAddonUsedByCluster, if err, skip the deletion stage + if !addon.GetDeletionTimestamp().IsZero() || (addon.Spec.InstallSpec != nil && !addon.Spec.InstallSpec.GetEnabled()) { + used, err := CheckIfAddonUsedByCluster(ctx, r.reconciler.Client, addon.Name, r.reqCtx.Req.Namespace) + if err != nil { + if used { + r.reconciler.Event(addon, corev1.EventTypeNormal, "Addon is used by some clusters", + fmt.Sprintf("Addon is used by %s", err.Error())) + r.setRequeueAfter(time.Second, "Waiting for the cluster to end") + return + } else { + r.setRequeueWithErr(err, "") + return + } + } + } + res, err := intctrlutil.HandleCRDeletion(*r.reqCtx, r.reconciler, addon, addonFinalizerName, func() (*ctrl.Result, error) { r.deletionStage.Handle(ctx) return r.deletionStage.doReturn() @@ -1082,3 +1100,30 @@ func findDataKey[V string | []byte](data map[string]V, refObj extensionsv1alpha1 } return false } + +func CheckIfAddonUsedByCluster(ctx context.Context, c client.Client, addonName string, namespace string) (bool, error) { + labelSelector := metav1.LabelSelector{ + MatchLabels: map[string]string{"clusterdefinition.kubeblocks.io/name": addonName}, + } + + selector, err := metav1.LabelSelectorAsSelector(&labelSelector) + if err != nil { + return false, err + } + + var clusters appsv1alpha1.ClusterList + if err := c.List(ctx, &clusters, client.InNamespace(namespace), client.MatchingLabelsSelector{Selector: selector}); err != nil { + return false, err + } + + if len(clusters.Items) > 0 { + clusterNames := make([]string, len(clusters.Items)) + for i, cluster := range clusters.Items { + clusterNames[i] = cluster.Name + } + errMsg := strings.Join(clusterNames, ", ") + return true, fmt.Errorf(errMsg) + } + + return false, nil +} diff --git a/controllers/extensions/addon_controller_test.go b/controllers/extensions/addon_controller_test.go index f3ae5ad598b..f6b7ec1c81d 100644 --- a/controllers/extensions/addon_controller_test.go +++ b/controllers/extensions/addon_controller_test.go @@ -39,6 +39,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/reconcile" + appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1" extensionsv1alpha1 "github.com/apecloud/kubeblocks/apis/extensions/v1alpha1" "github.com/apecloud/kubeblocks/pkg/constant" "github.com/apecloud/kubeblocks/pkg/generics" @@ -48,6 +49,14 @@ import ( ) var _ = Describe("Addon controller", func() { + const ( + clusterDefName = "test-clusterdef" + clusterVersionName = "test-clusterversion" + compDefName = "test-compdef" + compVersionName = "test-compversion" + clusterName = "test-cluster" // this become cluster prefix name if used with testapps.NewClusterFactory().WithRandomName() + ) + cleanEnv := func() { // must wait till resources deleted and no longer existed before the testcases start, // otherwise if later it needs to create some new resource objects with the same name, @@ -93,6 +102,8 @@ var _ = Describe("Addon controller", func() { Context("Addon controller test", func() { var addon *extensionsv1alpha1.Addon var key types.NamespacedName + var clusterKey types.NamespacedName + BeforeEach(func() { cleanEnv() const distro = "kubeblocks" @@ -290,6 +301,12 @@ var _ = Describe("Addon controller", func() { g.Expect(apierrors.IsNotFound(err)).Should(BeTrue()) } + checkAddonNotDeleted := func(g Gomega) { + addon = &extensionsv1alpha1.Addon{} + err := testCtx.Cli.Get(ctx, key, addon) + g.Expect(err).To(Not(HaveOccurred())) + } + disableAddon := func(genID int) { addon = &extensionsv1alpha1.Addon{} Expect(testCtx.Cli.Get(ctx, key, addon)).To(Not(HaveOccurred())) @@ -298,6 +315,24 @@ var _ = Describe("Addon controller", func() { disablingPhaseCheck(genID) } + disableAddonFailedCheck := func(genID, obGenID int, expectPhase extensionsv1alpha1.AddonPhase) { + addon = &extensionsv1alpha1.Addon{} + Expect(testCtx.Cli.Get(ctx, key, addon)).To(Not(HaveOccurred())) + addon.Spec.InstallSpec.Enabled = false + Expect(testCtx.Cli.Update(ctx, addon)).Should(Succeed()) + + Eventually(func(g Gomega) { + _, err := doReconcile() + Expect(err).To(Not(HaveOccurred())) + addon = &extensionsv1alpha1.Addon{} + g.Expect(testCtx.Cli.Get(ctx, key, addon)).To(Not(HaveOccurred())) + g.Expect(addon.Generation).Should(BeEquivalentTo(genID)) + g.Expect(addon.Spec.InstallSpec).ShouldNot(BeNil()) + g.Expect(addon.Status.ObservedGeneration).Should(BeEquivalentTo(obGenID)) + g.Expect(addon.Status.Phase).Should(Equal(expectPhase)) + }).Should(Succeed()) + } + fakeHelmRelease := func() { // create fake helm release helmRelease := &corev1.Secret{ @@ -425,7 +460,6 @@ var _ = Describe("Addon controller", func() { By("By disabling enabled addon") fakeHelmRelease() disableAddon(3) - By("By failed an uninstallation job") fakeUninstallationFailedJob(3) @@ -630,6 +664,81 @@ var _ = Describe("Addon controller", func() { }) addonStatusPhaseCheck(2, extensionsv1alpha1.AddonFailed, nil) }) + + It("should successfully reconcile a custom resource for Addon deleting with used by clusters", func() { + createAutoInstallAddon() + + By("By enable addon with fake completed install job status") + fakeInstallationCompletedJob(2) + + By("By creating cluster with addon") + clusterObj := testapps.NewClusterFactory(testCtx.DefaultNamespace, clusterName, + "test-cd", "test-cv"). + AddComponent(addon.Name, addon.Name).SetReplicas(1). + WithRandomName(). + AddLabels(constant.ClusterDefLabelKey, addon.Name). + Create(&testCtx). + GetObject() + clusterKey = client.ObjectKeyFromObject(clusterObj) + + cluster := &appsv1alpha1.Cluster{} + Eventually(func(g Gomega) { + err := testCtx.Cli.Get(ctx, clusterKey, cluster) + g.Expect(err).To(Not(HaveOccurred())) + }).Should(Succeed()) + + By("By delete addon with disabling status") + Expect(testCtx.Cli.Delete(ctx, addon)).To(Not(HaveOccurred())) + Eventually(func(g Gomega) { + _, err := doReconcile() + g.Expect(err).To(Not(HaveOccurred())) + checkAddonNotDeleted(g) + }).Should(Succeed()) + + By("By deleting cluster") + Expect(testCtx.Cli.Delete(ctx, cluster)).To(Not(HaveOccurred())) + + By("By checking addon with no used cluster") + Eventually(func(g Gomega) { + _, err := doReconcile() + g.Expect(err).To(Not(HaveOccurred())) + checkAddonDeleted(g) + }).Should(Succeed()) + }) + + It("should successfully reconcile a custom resource for Addon disabling with used by clusters", func() { + createAutoInstallAddon() + + By("By enable addon with fake completed install job status") + fakeInstallationCompletedJob(2) + + By("By creating cluster with addon") + clusterObj := testapps.NewClusterFactory(testCtx.DefaultNamespace, clusterName, + "test-cd", "test-cv"). + AddComponent(addon.Name, addon.Name).SetReplicas(1). + WithRandomName(). + AddLabels(constant.ClusterDefLabelKey, addon.Name). + Create(&testCtx). + GetObject() + clusterKey = client.ObjectKeyFromObject(clusterObj) + + cluster := &appsv1alpha1.Cluster{} + Eventually(func(g Gomega) { + err := testCtx.Cli.Get(ctx, clusterKey, cluster) + g.Expect(err).To(Not(HaveOccurred())) + }).Should(Succeed()) + + By("By disabling enabled addon") + fakeHelmRelease() + disableAddonFailedCheck(3, 2, extensionsv1alpha1.AddonEnabled) + + By("By deleting cluster") + Expect(testCtx.Cli.Delete(ctx, cluster)).To(Not(HaveOccurred())) + + By("By checking addon with no used cluster") + disablingPhaseCheck(3) + }) + }) Context("Addon controller SetupWithManager", func() { diff --git a/controllers/extensions/suite_test.go b/controllers/extensions/suite_test.go index 25e80eaae57..0bb9676e426 100644 --- a/controllers/extensions/suite_test.go +++ b/controllers/extensions/suite_test.go @@ -36,6 +36,7 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" + appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1" extensionsv1alpha1 "github.com/apecloud/kubeblocks/apis/extensions/v1alpha1" "github.com/apecloud/kubeblocks/pkg/constant" @@ -96,6 +97,9 @@ var _ = BeforeSuite(func() { err = extensionsv1alpha1.AddToScheme(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) + err = appsv1alpha1.AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) + // +kubebuilder:scaffold:scheme k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})