From 3f52fd28c5985168818e9ddb2abaef7a048d555a Mon Sep 17 00:00:00 2001 From: Arvind Thirumurugan Date: Thu, 7 Nov 2024 15:41:22 -0800 Subject: [PATCH] add more UTs --- .../controller.go | 2 +- .../controller_test.go | 297 +++++++++++++++++- 2 files changed, 290 insertions(+), 9 deletions(-) diff --git a/pkg/controllers/clusterresourceplacementeviction/controller.go b/pkg/controllers/clusterresourceplacementeviction/controller.go index 679c42555..4d92f05a4 100644 --- a/pkg/controllers/clusterresourceplacementeviction/controller.go +++ b/pkg/controllers/clusterresourceplacementeviction/controller.go @@ -182,7 +182,7 @@ func isEvictionAllowed(policy *placementv1beta1.PlacementPolicy, bindings []plac } if db.Spec.MinAvailable != nil { minAvailable, _ := intstr.GetScaledValueFromIntOrPercent(db.Spec.MinAvailable, targetNumber, true) - if currentAvailableCount > minAvailable { + if currentAvailableCount > minAvailable || minAvailable == 0 { return true } } diff --git a/pkg/controllers/clusterresourceplacementeviction/controller_test.go b/pkg/controllers/clusterresourceplacementeviction/controller_test.go index 94cd99bc4..6c71c868d 100644 --- a/pkg/controllers/clusterresourceplacementeviction/controller_test.go +++ b/pkg/controllers/clusterresourceplacementeviction/controller_test.go @@ -22,7 +22,7 @@ func TestGetOrCreateClusterSchedulingPolicySnapshot(t *testing.T) { Reason: "available", ObservedGeneration: 0, } - scheduledBinding := placementv1beta1.ClusterResourceBinding{ + scheduledUnavailableBinding := placementv1beta1.ClusterResourceBinding{ ObjectMeta: metav1.ObjectMeta{ Name: "scheduled-binding", Labels: map[string]string{placementv1beta1.CRPTrackingLabel: "test-crp"}, @@ -49,6 +49,21 @@ func TestGetOrCreateClusterSchedulingPolicySnapshot(t *testing.T) { Conditions: []metav1.Condition{availableCondition}, }, } + anotherBoundAvailableBinding := placementv1beta1.ClusterResourceBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "another-bound-available-binding", + Labels: map[string]string{placementv1beta1.CRPTrackingLabel: "test-crp"}, + }, + Spec: placementv1beta1.ResourceBindingSpec{ + State: placementv1beta1.BindingStateBound, + ResourceSnapshotName: "test-resource-snapshot", + SchedulingPolicySnapshotName: "test-scheduling-policy-snapshot", + TargetCluster: "test-cluster-3", + }, + Status: placementv1beta1.ResourceBindingStatus{ + Conditions: []metav1.Condition{availableCondition}, + }, + } boundUnavailableBinding := placementv1beta1.ClusterResourceBinding{ ObjectMeta: metav1.ObjectMeta{ Name: "bound-unavailable-binding", @@ -58,7 +73,7 @@ func TestGetOrCreateClusterSchedulingPolicySnapshot(t *testing.T) { State: placementv1beta1.BindingStateBound, ResourceSnapshotName: "test-resource-snapshot", SchedulingPolicySnapshotName: "test-scheduling-policy-snapshot", - TargetCluster: "test-cluster-3", + TargetCluster: "test-cluster-4", }, } unScheduledAvailableBinding := placementv1beta1.ClusterResourceBinding{ @@ -70,7 +85,7 @@ func TestGetOrCreateClusterSchedulingPolicySnapshot(t *testing.T) { State: placementv1beta1.BindingStateBound, ResourceSnapshotName: "test-resource-snapshot", SchedulingPolicySnapshotName: "test-scheduling-policy-snapshot", - TargetCluster: "test-cluster-2", + TargetCluster: "test-cluster-5", }, Status: placementv1beta1.ResourceBindingStatus{ Conditions: []metav1.Condition{availableCondition}, @@ -103,11 +118,30 @@ func TestGetOrCreateClusterSchedulingPolicySnapshot(t *testing.T) { want: false, }, { - name: "PlacementType PickAll, maxUnavailable specified as Integer greater than zero - block eviction", + name: "PlacementType PickAll, maxUnavailable specified as Integer one - block eviction", + policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickAllPlacementType, + }, + bindings: []placementv1beta1.ClusterResourceBinding{scheduledUnavailableBinding}, + disruptionBudget: placementv1alpha1.ClusterResourcePlacementDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-disruption-budget", + }, + Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{ + MaxUnavailable: &intstr.IntOrString{ + Type: intstr.Int, + IntVal: 1, + }, + }, + }, + want: false, + }, + { + name: "PlacementType PickAll, maxUnavailable specified as Integer one - allow eviction", policy: &placementv1beta1.PlacementPolicy{ PlacementType: placementv1beta1.PickAllPlacementType, }, - bindings: []placementv1beta1.ClusterResourceBinding{scheduledBinding, boundAvailableBinding}, + bindings: []placementv1beta1.ClusterResourceBinding{boundAvailableBinding}, disruptionBudget: placementv1alpha1.ClusterResourcePlacementDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{ Name: "test-disruption-budget", @@ -119,14 +153,52 @@ func TestGetOrCreateClusterSchedulingPolicySnapshot(t *testing.T) { }, }, }, + want: true, + }, + { + name: "PlacementType PickAll, maxUnavailable specified as Integer greater than one - block eviction", + policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickAllPlacementType, + }, + bindings: []placementv1beta1.ClusterResourceBinding{scheduledUnavailableBinding, boundAvailableBinding, boundUnavailableBinding, unScheduledAvailableBinding}, + disruptionBudget: placementv1alpha1.ClusterResourcePlacementDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-disruption-budget", + }, + Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{ + MaxUnavailable: &intstr.IntOrString{ + Type: intstr.Int, + IntVal: 2, + }, + }, + }, want: false, }, { - name: "PlacementType PickAll, maxUnavailable specified as Integer large number - allows eviction", + name: "PlacementType PickAll, maxUnavailable specified as Integer greater than one - allow eviction", + policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickAllPlacementType, + }, + bindings: []placementv1beta1.ClusterResourceBinding{scheduledUnavailableBinding, boundAvailableBinding, unScheduledAvailableBinding}, + disruptionBudget: placementv1alpha1.ClusterResourcePlacementDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-disruption-budget", + }, + Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{ + MaxUnavailable: &intstr.IntOrString{ + Type: intstr.Int, + IntVal: 2, + }, + }, + }, + want: true, + }, + { + name: "PlacementType PickAll, maxUnavailable specified as Integer large number greater than target number - allows eviction", policy: &placementv1beta1.PlacementPolicy{ PlacementType: placementv1beta1.PickAllPlacementType, }, - bindings: []placementv1beta1.ClusterResourceBinding{scheduledBinding, boundAvailableBinding, boundUnavailableBinding, unScheduledAvailableBinding}, + bindings: []placementv1beta1.ClusterResourceBinding{scheduledUnavailableBinding, boundAvailableBinding, boundUnavailableBinding, unScheduledAvailableBinding}, disruptionBudget: placementv1alpha1.ClusterResourcePlacementDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{ Name: "test-disruption-budget", @@ -141,7 +213,7 @@ func TestGetOrCreateClusterSchedulingPolicySnapshot(t *testing.T) { want: true, }, { - name: "PlacementType PickAll, maxUnavailable specified as percentage 0% - block eviction", + name: "PlacementType PickAll, maxUnavailable specified as percentage zero - block eviction", policy: &placementv1beta1.PlacementPolicy{ PlacementType: placementv1beta1.PickAllPlacementType, }, @@ -159,6 +231,215 @@ func TestGetOrCreateClusterSchedulingPolicySnapshot(t *testing.T) { }, want: false, }, + { + name: "PlacementType PickAll, maxUnavailable specified as percentage greater than zero, rounds up to 1 - block eviction", + policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickAllPlacementType, + }, + bindings: []placementv1beta1.ClusterResourceBinding{scheduledUnavailableBinding}, + disruptionBudget: placementv1alpha1.ClusterResourcePlacementDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-disruption-budget", + }, + Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{ + MaxUnavailable: &intstr.IntOrString{ + Type: intstr.String, + StrVal: "10%", + }, + }, + }, + want: false, + }, + { + name: "PlacementType PickAll, maxUnavailable specified as percentage greater than zero, rounds up to 1 - allow eviction", + policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickAllPlacementType, + }, + bindings: []placementv1beta1.ClusterResourceBinding{boundAvailableBinding}, + disruptionBudget: placementv1alpha1.ClusterResourcePlacementDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-disruption-budget", + }, + Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{ + MaxUnavailable: &intstr.IntOrString{ + Type: intstr.String, + StrVal: "10%", + }, + }, + }, + want: true, + }, + { + name: "PlacementType PickAll, maxUnavailable specified as percentage greater than zero, rounds up to greater than 1 - block eviction", + policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickAllPlacementType, + }, + bindings: []placementv1beta1.ClusterResourceBinding{scheduledUnavailableBinding, boundAvailableBinding, boundUnavailableBinding, unScheduledAvailableBinding}, + disruptionBudget: placementv1alpha1.ClusterResourcePlacementDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-disruption-budget", + }, + Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{ + MaxUnavailable: &intstr.IntOrString{ + Type: intstr.String, + StrVal: "40%", + }, + }, + }, + want: false, + }, + { + name: "PlacementType PickAll, maxUnavailable specified as percentage greater than zero, rounds up to greater than 1 - allow eviction", + policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickAllPlacementType, + }, + bindings: []placementv1beta1.ClusterResourceBinding{scheduledUnavailableBinding, boundAvailableBinding, unScheduledAvailableBinding}, + disruptionBudget: placementv1alpha1.ClusterResourcePlacementDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-disruption-budget", + }, + Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{ + MaxUnavailable: &intstr.IntOrString{ + Type: intstr.String, + StrVal: "50%", + }, + }, + }, + want: true, + }, + { + name: "PlacementType PickAll, maxUnavailable specified as percentage hundred - allow eviction", + policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickAllPlacementType, + }, + bindings: []placementv1beta1.ClusterResourceBinding{scheduledUnavailableBinding, boundAvailableBinding, boundUnavailableBinding, unScheduledAvailableBinding}, + disruptionBudget: placementv1alpha1.ClusterResourcePlacementDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-disruption-budget", + }, + Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{ + MaxUnavailable: &intstr.IntOrString{ + Type: intstr.String, + StrVal: "100%", + }, + }, + }, + want: true, + }, + { + name: "PlacementType PickAll, minAvailable specified as Integer zero - allow eviction", + policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickAllPlacementType, + }, + bindings: []placementv1beta1.ClusterResourceBinding{scheduledUnavailableBinding, boundUnavailableBinding}, + disruptionBudget: placementv1alpha1.ClusterResourcePlacementDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-disruption-budget", + }, + Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{ + MinAvailable: &intstr.IntOrString{ + Type: intstr.Int, + IntVal: 0, + }, + }, + }, + want: true, + }, + { + name: "PlacementType PickAll, minAvailable specified as Integer one - block eviction, binding unavailable", + policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickAllPlacementType, + }, + bindings: []placementv1beta1.ClusterResourceBinding{scheduledUnavailableBinding}, + disruptionBudget: placementv1alpha1.ClusterResourcePlacementDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-disruption-budget", + }, + Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{ + MinAvailable: &intstr.IntOrString{ + Type: intstr.Int, + IntVal: 1, + }, + }, + }, + want: false, + }, + { + name: "PlacementType PickAll, minAvailable specified as Integer one - block eviction, binding available", + policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickAllPlacementType, + }, + bindings: []placementv1beta1.ClusterResourceBinding{boundAvailableBinding}, + disruptionBudget: placementv1alpha1.ClusterResourcePlacementDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-disruption-budget", + }, + Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{ + MinAvailable: &intstr.IntOrString{ + Type: intstr.Int, + IntVal: 1, + }, + }, + }, + want: false, + }, + { + name: "PlacementType PickAll, minAvailable specified as Integer one - allow eviction", + policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickAllPlacementType, + }, + bindings: []placementv1beta1.ClusterResourceBinding{boundAvailableBinding, unScheduledAvailableBinding}, + disruptionBudget: placementv1alpha1.ClusterResourcePlacementDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-disruption-budget", + }, + Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{ + MinAvailable: &intstr.IntOrString{ + Type: intstr.Int, + IntVal: 1, + }, + }, + }, + want: true, + }, + { + name: "PlacementType PickAll, minUnavailable specified as Integer greater than one - block eviction", + policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickAllPlacementType, + }, + bindings: []placementv1beta1.ClusterResourceBinding{boundAvailableBinding, unScheduledAvailableBinding}, + disruptionBudget: placementv1alpha1.ClusterResourcePlacementDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-disruption-budget", + }, + Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{ + MinAvailable: &intstr.IntOrString{ + Type: intstr.Int, + IntVal: 2, + }, + }, + }, + want: false, + }, + { + name: "PlacementType PickAll, minUnavailable specified as Integer greater than one - allow eviction", + policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickAllPlacementType, + }, + bindings: []placementv1beta1.ClusterResourceBinding{scheduledUnavailableBinding, boundAvailableBinding, anotherBoundAvailableBinding, unScheduledAvailableBinding}, + disruptionBudget: placementv1alpha1.ClusterResourcePlacementDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-disruption-budget", + }, + Spec: placementv1alpha1.PlacementDisruptionBudgetSpec{ + MinAvailable: &intstr.IntOrString{ + Type: intstr.Int, + IntVal: 2, + }, + }, + }, + want: true, + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) {