Skip to content

Commit

Permalink
do not wait for updateRun completion upon deleting
Browse files Browse the repository at this point in the history
  • Loading branch information
jwtty committed Nov 15, 2024
1 parent 0ce12dd commit bfa4ef8
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 134 deletions.
17 changes: 1 addition & 16 deletions pkg/controllers/updaterun/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
119 changes: 1 addition & 118 deletions pkg/controllers/updaterun/controller_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -67,102 +65,21 @@ 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())

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())

By("Checking the clusterStagedUpdateRun is deleted")
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)
Expand Down Expand Up @@ -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{}
Expand Down

0 comments on commit bfa4ef8

Please sign in to comment.