fix IT with minor changes

apis/placement/v1alpha1/eviction_types.go

@@ -6,6 +6,7 @@ Licensed under the MIT license.
 package v1alpha1
 
 import (
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -118,6 +119,18 @@ type ClusterResourcePlacementEvictionList struct {
 	Items []ClusterResourcePlacementEviction `json:"items"`
 }
 
+// SetConditions set the given conditions on the ClusterResourceBinding.
+func (e *ClusterResourcePlacementEviction) SetConditions(conditions ...metav1.Condition) {
+	for _, c := range conditions {
+		meta.SetStatusCondition(&e.Status.Conditions, c)
+	}
+}
+
+// GetCondition returns the condition of the given ClusterResourceBinding.
+func (e *ClusterResourcePlacementEviction) GetCondition(conditionType string) *metav1.Condition {
+	return meta.FindStatusCondition(e.Status.Conditions, conditionType)
+}
+
 func init() {
 	SchemeBuilder.Register(
 		&ClusterResourcePlacementEviction{},

pkg/controllers/clusterresourceplacementeviction/controller.go

@@ -10,7 +10,6 @@ import (
 	"time"
 
 	"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"
 	"k8s.io/apimachinery/pkg/util/intstr"
@@ -31,11 +30,11 @@ const (
 	reasonClusterResourcePlacementEvictionExecuted    = "ClusterResourcePlacementEvictionExecuted"
 	reasonClusterResourcePlacementEvictionNotExecuted = "ClusterResourcePlacementEvictionNotExecuted"
 
-	invalidEvictionMissingCRP       = "Failed to find cluster resource placement targeted by eviction"
-	invalidEvictionMissingCRB       = "Failed to find cluster resource binding for cluster targeted by eviction"
-	executedEvictionNoPDB           = "Executed eviction, no ClusterResourcePlacementDisruptionBudget specified"
-	executedEvictionPDBSpecified    = "Eviction is allowed by specified ClusterResourcePlacementDisruptionBudget"
-	notExecutedEvictionPDBSpecified = "Eviction is blocked by specified ClusterResourcePlacementDisruptionBudget"
+	evictionInvalidMissingCRP   = "Failed to find cluster resource placement targeted by eviction"
+	evictionInvalidMissingCRB   = "Failed to find cluster resource binding for cluster targeted by eviction"
+	evictionAllowedNoPDB        = "Eviction Allowed, no ClusterResourcePlacementDisruptionBudget specified"
+	evictionAllowedPDBSpecified = "Eviction is allowed by specified ClusterResourcePlacementDisruptionBudget"
+	evictionBlockedPDBSpecified = "Eviction is blocked by specified ClusterResourcePlacementDisruptionBudget"
 )
 
 // Reconciler reconciles a ClusterResourcePlacementEviction object.
@@ -45,19 +44,30 @@ type Reconciler struct {
 
 func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtime.Result, error) {
 	startTime := time.Now()
-	klog.V(2).InfoS("ClusterResourcePlacementEviction reconciliation starts", "clusterResourcePlacementEviction", req.NamespacedName)
+	evictionName := req.NamespacedName.Name
+	klog.V(2).InfoS("ClusterResourcePlacementEviction reconciliation starts", "clusterResourcePlacementEviction", evictionName)
 	defer func() {
 		latency := time.Since(startTime).Milliseconds()
-		klog.V(2).InfoS("ClusterResourcePlacementEviction reconciliation ends", "clusterResourcePlacementEviction", req.NamespacedName, "latency", latency)
+		klog.V(2).InfoS("ClusterResourcePlacementEviction reconciliation ends", "clusterResourcePlacementEviction", evictionName, "latency", latency)
 	}()
 
 	var eviction placementv1alpha1.ClusterResourcePlacementEviction
 	if err := r.Client.Get(ctx, req.NamespacedName, &eviction); err != nil {
-		klog.ErrorS(err, "Failed to get cluster resource placement eviction", "clusterResourcePlacementEviction", req.NamespacedName)
+		klog.ErrorS(err, "Failed to get cluster resource placement eviction", "clusterResourcePlacementEviction", evictionName)
 		return runtime.Result{}, client.IgnoreNotFound(err)
 	}
 
-	// Need to check if eviction is invalid/executed and exit.
+	validCondition := eviction.GetCondition(string(placementv1alpha1.PlacementEvictionConditionTypeValid))
+	if condition.IsConditionStatusFalse(validCondition, eviction.GetGeneration()) {
+		klog.V(2).InfoS("Invalid eviction, no need to reconcile", "clusterResourcePlacementEviction", evictionName)
+		return runtime.Result{}, nil
+	}
+
+	executedCondition := eviction.GetCondition(string(placementv1alpha1.PlacementEvictionConditionTypeExecuted))
+	if condition.IsConditionStatusTrue(executedCondition, eviction.GetGeneration()) {
+		klog.V(2).InfoS("Executed eviction, no need to reconcile", "clusterResourcePlacementEviction", evictionName)
+		return runtime.Result{}, nil
+	}
 
 	isCRPPresent := true
 	var crp placementv1beta1.ClusterResourcePlacement
@@ -68,9 +78,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtim
 		isCRPPresent = false
 	}
 	if !isCRPPresent {
-		klog.V(2).InfoS(invalidEvictionMissingCRP, "clusterResourcePlacementEviction", req.NamespacedName, "clusterResourcePlacement", eviction.Spec.PlacementName)
+		klog.V(2).InfoS(evictionInvalidMissingCRP, "clusterResourcePlacementEviction", evictionName, "clusterResourcePlacement", eviction.Spec.PlacementName)
 		// mark eviction as invalid.
-		markEvictionInvalid(&eviction, invalidEvictionMissingCRP)
+		markEvictionInvalid(&eviction, evictionInvalidMissingCRP)
 		return runtime.Result{}, r.updateEvictionStatus(ctx, &eviction)
 	}
 
@@ -86,13 +96,12 @@ func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtim
 		}
 	}
 	if evictionTargetBinding == nil {
-		klog.V(2).InfoS(invalidEvictionMissingCRB, "clusterResourcePlacementEviction", req.NamespacedName, "clusterName", eviction.Spec.ClusterName)
+		klog.V(2).InfoS(evictionInvalidMissingCRB, "clusterResourcePlacementEviction", evictionName, "clusterName", eviction.Spec.ClusterName)
 		// mark eviction as invalid.
-		markEvictionInvalid(&eviction, invalidEvictionMissingCRB)
+		markEvictionInvalid(&eviction, evictionInvalidMissingCRB)
 		return runtime.Result{}, r.updateEvictionStatus(ctx, &eviction)
 	}
 
-	// mark eviction as valid.
 	markEvictionValid(&eviction)
 	isDBPresent := true
 	var db placementv1alpha1.ClusterResourcePlacementDisruptionBudget
@@ -107,15 +116,15 @@ func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtim
 		if err := r.deleteClusterResourceBinding(ctx, evictionTargetBinding); err != nil {
 			return runtime.Result{}, err
 		}
-		markEvictionExecuted(&eviction, executedEvictionNoPDB)
+		markEvictionExecuted(&eviction, evictionAllowedNoPDB)
 		return runtime.Result{}, r.updateEvictionStatus(ctx, &eviction)
 	}
 
 	if isEvictionAllowed(crp, crbList.Items, db) {
 		if err := r.deleteClusterResourceBinding(ctx, evictionTargetBinding); err != nil {
 			return runtime.Result{}, err
 		}
-		markEvictionExecuted(&eviction, executedEvictionPDBSpecified)
+		markEvictionExecuted(&eviction, evictionAllowedPDBSpecified)
 	} else {
 		markEvictionNotExecuted(&eviction)
 	}
@@ -124,20 +133,22 @@ func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtim
 }
 
 func (r *Reconciler) updateEvictionStatus(ctx context.Context, eviction *placementv1alpha1.ClusterResourcePlacementEviction) error {
+	evictionRef := klog.KObj(eviction)
 	if err := r.Client.Status().Update(ctx, eviction); err != nil {
-		klog.ErrorS(err, "Failed to update eviction status", "clusterResourcePlacementEviction", klog.KObj(eviction))
+		klog.ErrorS(err, "Failed to update eviction status", "clusterResourcePlacementEviction", evictionRef)
 		return controller.NewUpdateIgnoreConflictError(err)
 	}
-	klog.V(2).InfoS("Updated the status of a eviction", "clusterResourcePlacementEviction", klog.KObj(eviction))
+	klog.V(2).InfoS("Updated the status of a eviction", "clusterResourcePlacementEviction", evictionRef)
 	return nil
 }
 
 func (r *Reconciler) deleteClusterResourceBinding(ctx context.Context, binding *placementv1beta1.ClusterResourceBinding) error {
+	bindingRef := klog.KObj(binding)
 	if err := r.Client.Delete(ctx, binding); err != nil {
-		klog.ErrorS(err, "Failed to delete cluster resource binding", "clusterResourceBinding", klog.KObj(binding))
+		klog.ErrorS(err, "Failed to delete cluster resource binding", "clusterResourceBinding", bindingRef)
 		return controller.NewDeleteIgnoreNotFoundError(err)
 	}
-	klog.V(2).InfoS("Issued delete on cluster resource binding, eviction succeeded", "clusterResourceBinding", klog.KObj(binding))
+	klog.V(2).InfoS("Issued delete on cluster resource binding, eviction succeeded", "clusterResourceBinding", bindingRef)
 	return nil
 }
 
@@ -183,7 +194,7 @@ func markEvictionValid(eviction *placementv1alpha1.ClusterResourcePlacementEvict
 		ObservedGeneration: eviction.Generation,
 		Reason:             reasonClusterResourcePlacementEvictionValid,
 	}
-	meta.SetStatusCondition(&eviction.Status.Conditions, cond)
+	eviction.SetConditions(cond)
 }
 
 func markEvictionInvalid(eviction *placementv1alpha1.ClusterResourcePlacementEviction, message string) {
@@ -194,7 +205,7 @@ func markEvictionInvalid(eviction *placementv1alpha1.ClusterResourcePlacementEvi
 		Reason:             reasonClusterResourcePlacementEvictionInvalid,
 		Message:            message,
 	}
-	meta.SetStatusCondition(&eviction.Status.Conditions, cond)
+	eviction.SetConditions(cond)
 }
 
 func markEvictionExecuted(eviction *placementv1alpha1.ClusterResourcePlacementEviction, message string) {
@@ -205,7 +216,7 @@ func markEvictionExecuted(eviction *placementv1alpha1.ClusterResourcePlacementEv
 		Reason:             reasonClusterResourcePlacementEvictionExecuted,
 		Message:            message,
 	}
-	meta.SetStatusCondition(&eviction.Status.Conditions, cond)
+	eviction.SetConditions(cond)
 }
 
 func markEvictionNotExecuted(eviction *placementv1alpha1.ClusterResourcePlacementEviction) {
@@ -214,9 +225,9 @@ func markEvictionNotExecuted(eviction *placementv1alpha1.ClusterResourcePlacemen
 		Status:             metav1.ConditionTrue,
 		ObservedGeneration: eviction.Generation,
 		Reason:             reasonClusterResourcePlacementEvictionExecuted,
-		Message:            notExecutedEvictionPDBSpecified,
+		Message:            evictionBlockedPDBSpecified,
 	}
-	meta.SetStatusCondition(&eviction.Status.Conditions, cond)
+	eviction.SetConditions(cond)
 }
 
 // SetupWithManager sets up the controller with the Manager.

pkg/controllers/clusterresourceplacementeviction/controller_intergration_test.go

@@ -42,11 +42,10 @@ var (
 )
 
 const (
-	eventuallyDuration     = time.Minute * 2
-	longEventuallyDuration = time.Minute * 5
-	eventuallyInterval     = time.Millisecond * 250
-	consistentlyDuration   = time.Second * 15
-	consistentlyInterval   = time.Millisecond * 500
+	eventuallyDuration   = time.Minute * 2
+	eventuallyInterval   = time.Millisecond * 250
+	consistentlyDuration = time.Second * 15
+	consistentlyInterval = time.Millisecond * 500
 )
 
 var _ = Describe("Test ClusterResourcePlacementEviction Controller", Ordered, func() {
@@ -122,7 +121,7 @@ var _ = Describe("Test ClusterResourcePlacementEviction Controller", Ordered, fu
 	})
 
 	// This is missing case, where no CRB is present there is another case where CRB is present but it's missing CRP tracking label.
-	FContext("Valid Eviction - ClusterResourcePlacementDisruptionBudget is not present", func() {
+	Context("Valid Eviction - ClusterResourcePlacementDisruptionBudget is not present", func() {
 		evictionName := fmt.Sprintf(evictionNameTemplate, GinkgoParallelProcess())
 		crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess())
 		crbName := fmt.Sprintf(crbNameTemplate, GinkgoParallelProcess())
@@ -191,6 +190,10 @@ var _ = Describe("Test ClusterResourcePlacementEviction Controller", Ordered, fu
 			Consistently(func() bool {
 				return k8serrors.IsNotFound(k8sClient.Get(ctx, types.NamespacedName{Name: crbName}, &crb))
 			}, consistentlyDuration, consistentlyInterval).Should(BeTrue())
+
+			// Clean up resources.
+			Expect(k8sClient.Delete(ctx, &eviction)).Should(Succeed())
+			Expect(k8sClient.Delete(ctx, &crp)).Should(Succeed())
 		})
 	})
 })