From bfa4ef8cb57366352f249aa6245a1458a25c2409 Mon Sep 17 00:00:00 2001 From: Wantong Jiang Date: Fri, 15 Nov 2024 18:38:13 +0000 Subject: [PATCH] do not wait for updateRun completion upon deleting --- pkg/controllers/updaterun/controller.go | 17 +-- .../updaterun/controller_integration_test.go | 119 +----------------- 2 files changed, 2 insertions(+), 134 deletions(-) diff --git a/pkg/controllers/updaterun/controller.go b/pkg/controllers/updaterun/controller.go index 4cd0914d0..d570bf399 100644 --- a/pkg/controllers/updaterun/controller.go +++ b/pkg/controllers/updaterun/controller.go @@ -13,7 +13,6 @@ import ( "go.goms.io/fleet/pkg/utils" "go.goms.io/fleet/pkg/utils/controller" - "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" "k8s.io/client-go/util/workqueue" @@ -30,11 +29,6 @@ import ( placementv1alpha1 "go.goms.io/fleet/apis/placement/v1alpha1" ) -const ( - // stageUpdateRunDeletionWaitTime is the time to wait before checking whether the stage update run is completed before deletion. - stageUpdateRunDeletionWaitTime = 60 * time.Second -) - // Reconciler reconciles a ClusterStagedUpdateRun object. type Reconciler struct { client.Client @@ -80,18 +74,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtim } // handleDelete handles the deletion of the clusterStagedUpdateRun object. -// We need to wait for the update run to stop before deleting the clusterStagedUpdateRun object. -// We will delete all the dependent resources, including approvalRequest objects, of the clusterStagedUpdateRun object. +// We delete all the dependent resources, including approvalRequest objects, of the clusterStagedUpdateRun object. func (r *Reconciler) handleDelete(ctx context.Context, updateRun *placementv1alpha1.ClusterStagedUpdateRun) (bool, time.Duration, error) { runObjRef := klog.KObj(updateRun) - // check if the update run is still running. - // the update run is considered as running if started condition is true while finished condition is not true or false. - if meta.IsStatusConditionTrue(updateRun.Status.Conditions, string(placementv1alpha1.StagedUpdateRunConditionProgressing)) && - !meta.IsStatusConditionTrue(updateRun.Status.Conditions, string(placementv1alpha1.StagedUpdateRunConditionSucceeded)) && - !meta.IsStatusConditionFalse(updateRun.Status.Conditions, string(placementv1alpha1.StagedUpdateRunConditionSucceeded)) { - klog.V(2).InfoS("The clusterStagedUpdateRun is still running, waiting for it to stop", "clusterStagedUpdateRun", runObjRef) - return false, stageUpdateRunDeletionWaitTime, nil - } // delete all the associated approvalRequests. approvalRequest := &placementv1alpha1.ClusterApprovalRequest{} if err := r.Client.DeleteAllOf(ctx, approvalRequest, client.MatchingLabels{placementv1alpha1.TargetUpdateRunLabel: updateRun.GetName()}); err != nil { diff --git a/pkg/controllers/updaterun/controller_integration_test.go b/pkg/controllers/updaterun/controller_integration_test.go index d83011d29..fd3e2707a 100644 --- a/pkg/controllers/updaterun/controller_integration_test.go +++ b/pkg/controllers/updaterun/controller_integration_test.go @@ -16,13 +16,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" placementv1alpha1 "go.goms.io/fleet/apis/placement/v1alpha1" "go.goms.io/fleet/pkg/utils" - "go.goms.io/fleet/pkg/utils/condition" ) const ( @@ -67,7 +65,7 @@ var _ = Describe("Test the clusterStagedUpdateRun controller", func() { }) Context("Test deleting a clusterStagedUpdateRun", func() { - It("Should delete the clusterStagedUpdateRun if it's not started yet", func() { + It("Should delete the clusterStagedUpdateRun without any clusterApprovalRequests", func() { By("Creating a new clusterStagedUpdateRun") updateRun := getTestClusterStagedUpdateRun(testUpdateRunName) Expect(k8sClient.Create(ctx, updateRun)).Should(Succeed()) @@ -75,11 +73,6 @@ var _ = Describe("Test the clusterStagedUpdateRun controller", func() { By("Checking the finalizer is added") validateUpdateRunHasFinalizer(ctx, k8sClient, updateRun) - By("Updating the clusterStagedUpdateRun to initialized") - initcond := getTrueCondition(updateRun, string(placementv1alpha1.StagedUpdateRunConditionInitialized)) - meta.SetStatusCondition(&updateRun.Status.Conditions, initcond) - Expect(k8sClient.Status().Update(ctx, updateRun)).Should(Succeed(), "failed to update the clusterStagedUpdateRun") - By("Deleting the clusterStagedUpdateRun") Expect(k8sClient.Delete(ctx, updateRun)).Should(Succeed()) @@ -87,82 +80,6 @@ var _ = Describe("Test the clusterStagedUpdateRun controller", func() { validateUpdateRunIsDeleted(ctx, k8sClient, updateRunNamespacedName) }) - It("Should delete the clusterStagedUpdateRun if it finished and succeeded", func() { - By("Creating a new clusterStagedUpdateRun") - updateRun := getTestClusterStagedUpdateRun(testUpdateRunName) - Expect(k8sClient.Create(ctx, updateRun)).Should(Succeed()) - - By("Checking the finalizer is added") - validateUpdateRunHasFinalizer(ctx, k8sClient, updateRun) - - By("Updating the clusterStagedUpdateRun to succeeded") - startedcond := getTrueCondition(updateRun, string(placementv1alpha1.StagedUpdateRunConditionProgressing)) - finishedcond := getTrueCondition(updateRun, string(placementv1alpha1.StagedUpdateRunConditionSucceeded)) - meta.SetStatusCondition(&updateRun.Status.Conditions, startedcond) - meta.SetStatusCondition(&updateRun.Status.Conditions, finishedcond) - Expect(k8sClient.Status().Update(ctx, updateRun)).Should(Succeed(), "failed to update the clusterStagedUpdateRun") - - By("Deleting the clusterStagedUpdateRun") - Expect(k8sClient.Delete(ctx, updateRun)).Should(Succeed()) - - By("Checking the clusterStagedUpdateRun is deleted") - validateUpdateRunIsDeleted(ctx, k8sClient, updateRunNamespacedName) - }) - - It("Should delete the clusterStagedUpdateRun if it finished but failed", func() { - By("Creating a new clusterStagedUpdateRun") - updateRun := getTestClusterStagedUpdateRun(testUpdateRunName) - Expect(k8sClient.Create(ctx, updateRun)).Should(Succeed()) - - By("Checking the finalizer is added") - validateUpdateRunHasFinalizer(ctx, k8sClient, updateRun) - - By("Updating the clusterStagedUpdateRun to failed") - startedcond := getTrueCondition(updateRun, string(placementv1alpha1.StagedUpdateRunConditionProgressing)) - finishedcond := getFalseCondition(updateRun, string(placementv1alpha1.StagedUpdateRunConditionSucceeded)) - meta.SetStatusCondition(&updateRun.Status.Conditions, startedcond) - meta.SetStatusCondition(&updateRun.Status.Conditions, finishedcond) - Expect(k8sClient.Status().Update(ctx, updateRun)).Should(Succeed(), "failed to update the clusterStagedUpdateRun") - - By("Deleting the clusterStagedUpdateRun") - Expect(k8sClient.Delete(ctx, updateRun)).Should(Succeed()) - - By("Checking the clusterStagedUpdateRun is deleted") - validateUpdateRunIsDeleted(ctx, k8sClient, updateRunNamespacedName) - }) - - It("Should not delete the clusterStagedUpdateRun if it's still progressing'", func() { - By("Creating a new clusterStagedUpdateRun") - updateRun := getTestClusterStagedUpdateRun(testUpdateRunName) - Expect(k8sClient.Create(ctx, updateRun)).Should(Succeed()) - - By("Checking the finalizer is added") - validateUpdateRunHasFinalizer(ctx, k8sClient, updateRun) - - By("Updating the clusterStagedUpdateRun to progressing") - startedcond := getTrueCondition(updateRun, string(placementv1alpha1.StagedUpdateRunConditionProgressing)) - meta.SetStatusCondition(&updateRun.Status.Conditions, startedcond) - Expect(k8sClient.Status().Update(ctx, updateRun)).Should(Succeed(), "failed to add condition to the clusterStagedUpdateRun") - - By("Deleting the clusterStagedUpdateRun") - Expect(k8sClient.Delete(ctx, updateRun)).Should(Succeed()) - - By("Checking the clusterStagedUpdateRun is not deleted") - Consistently(func() error { - if err := k8sClient.Get(ctx, updateRunNamespacedName, updateRun); errors.IsNotFound(err) { - return fmt.Errorf("clusterStagedUpdateRun %s does not exist: %w", testUpdateRunName, err) - } - return nil - }, duration, interval).Should(Succeed(), "Failed to find clusterStagedUpdateRun %s", testUpdateRunName) - - By("Removing the finalizer") - controllerutil.RemoveFinalizer(updateRun, placementv1alpha1.ClusterStagedUpdateRunFinalizer) - Expect(k8sClient.Update(ctx, updateRun)).Should(Succeed(), "failed to remove finalizer from the clusterStagedUpdateRun") - - By("Checking the clusterStagedUpdateRun is deleted") - validateUpdateRunIsDeleted(ctx, k8sClient, updateRunNamespacedName) - }) - It("Should delete all ClusterApprovalRequest objects associated with the clusterStagedUpdateRun", func() { By("Creating a new clusterStagedUpdateRun") updateRun := getTestClusterStagedUpdateRun(testUpdateRunName) @@ -247,40 +164,6 @@ func validateUpdateRunHasFinalizer(ctx context.Context, k8sClient client.Client, }, timeout, interval).Should(Succeed(), "Failed to add finalizer to clusterStagedUpdateRun %s", namepsacedName) } -func getTrueCondition(updateRun *placementv1alpha1.ClusterStagedUpdateRun, condType string) metav1.Condition { - reason := "" - switch condType { - case string(placementv1alpha1.StagedUpdateRunConditionInitialized): - reason = condition.UpdateRunInitializeSucceededReason - case string(placementv1alpha1.StagedUpdateRunConditionProgressing): - reason = condition.UpdateRunStartedReason - case string(placementv1alpha1.StagedUpdateRunConditionSucceeded): - reason = condition.UpdateRunSucceededReason - } - return metav1.Condition{ - Status: metav1.ConditionTrue, - Type: condType, - ObservedGeneration: updateRun.Generation, - Reason: reason, - } -} - -func getFalseCondition(updateRun *placementv1alpha1.ClusterStagedUpdateRun, condType string) metav1.Condition { - reason := "" - switch condType { - case string(placementv1alpha1.StagedUpdateRunConditionInitialized): - reason = condition.UpdateRunInitializeFailedReason - case string(placementv1alpha1.StagedUpdateRunConditionSucceeded): - reason = condition.UpdateRunFailedReason - } - return metav1.Condition{ - Status: metav1.ConditionFalse, - Type: condType, - ObservedGeneration: updateRun.Generation, - Reason: reason, - } -} - func validateUpdateRunIsDeleted(ctx context.Context, k8sClient client.Client, name types.NamespacedName) { Eventually(func() error { updateRun := &placementv1alpha1.ClusterStagedUpdateRun{}