address review comments

pkg/utils/kubernetes/reduce/filters_test.go

@@ -8,8 +8,10 @@ import (
 	"github.com/stretchr/testify/require"
 	admregv1 "k8s.io/api/admissionregistration/v1"
 	appsv1 "k8s.io/api/apps/v1"
+	autoscalingv2 "k8s.io/api/autoscaling/v2"
 	corev1 "k8s.io/api/core/v1"
 	discoveryv1 "k8s.io/api/discovery/v1"
+	policyv1 "k8s.io/api/policy/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
@@ -990,3 +992,93 @@ func TestFilterClusterRoles(t *testing.T) {
 		})
 	}
 }
+
+func TestFilterHPA(t *testing.T) {
+	now := time.Now()
+	testCases := []struct {
+		name          string
+		hpas          []autoscalingv2.HorizontalPodAutoscaler
+		expectedNames []string
+	}{
+		{
+			name: "the newer ones must be returned to be deleted",
+			hpas: []autoscalingv2.HorizontalPodAutoscaler{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:              "older",
+						CreationTimestamp: metav1.NewTime(now.Add(-time.Second)),
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:              "newer",
+						CreationTimestamp: metav1.NewTime(now),
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:              "newer-2",
+						CreationTimestamp: metav1.NewTime(now.Add(time.Second)),
+					},
+				},
+			},
+			expectedNames: []string{"newer", "newer-2"},
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			filtered := FilterHPAs(tc.hpas)
+			filteredNames := lo.Map(filtered, func(hpa autoscalingv2.HorizontalPodAutoscaler, _ int) string {
+				return hpa.Name
+			})
+			require.ElementsMatch(t, filteredNames, tc.expectedNames)
+		})
+	}
+}
+
+func TestFilterPodDisruptionBudgets(t *testing.T) {
+	now := time.Now()
+	testCases := []struct {
+		name          string
+		pdbs          []policyv1.PodDisruptionBudget
+		expectedNames []string
+	}{
+		{
+			name: "the newer ones must be returned to be deleted",
+			pdbs: []policyv1.PodDisruptionBudget{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:              "older",
+						CreationTimestamp: metav1.NewTime(now.Add(-time.Second)),
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:              "newer",
+						CreationTimestamp: metav1.NewTime(now),
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:              "newer-2",
+						CreationTimestamp: metav1.NewTime(now.Add(time.Second)),
+					},
+				},
+			},
+			expectedNames: []string{"newer", "newer-2"},
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			filtered := FilterPodDisruptionBudgets(tc.pdbs)
+			filteredNames := lo.Map(filtered, func(pdb policyv1.PodDisruptionBudget, _ int) string {
+				return pdb.Name
+			})
+			require.ElementsMatch(t, filteredNames, tc.expectedNames)
+		})
+	}
+}

pkg/utils/kubernetes/reduce/reduce.go

@@ -152,8 +152,8 @@ func ReduceNetworkPolicies(ctx context.Context, k8sClient client.Client, network
 
 // +kubebuilder:rbac:groups=autoscaling,resources=horizontalpodautoscalers,verbs=delete
 
-// HPAFilterFunc filters a list of HorizontalPodAutoscalers.
-type HPAFilterFunc func(hpas []autoscalingv2.HorizontalPodAutoscaler) []autoscalingv2.HorizontalPodAutoscaler
+// HPAFilterFunc filters a list of HorizontalPodAutoscalers and returns the ones that should be deleted.
+type HPAFilterFunc func([]autoscalingv2.HorizontalPodAutoscaler) []autoscalingv2.HorizontalPodAutoscaler
 
 // ReduceHPAs detects the best HorizontalPodAutoscaler in the set and deletes all the others.
 func ReduceHPAs(ctx context.Context, k8sClient client.Client, hpas []autoscalingv2.HorizontalPodAutoscaler, filter HPAFilterFunc) error {
@@ -168,8 +168,8 @@ func ReduceHPAs(ctx context.Context, k8sClient client.Client, hpas []autoscaling
 
 // +kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets,verbs=delete
 
-// PDBFilterFunc filters a list of PodDisruptionBudgets.
-type PDBFilterFunc func(hpas []policyv1.PodDisruptionBudget) []policyv1.PodDisruptionBudget
+// PDBFilterFunc filters a list of PodDisruptionBudgets and returns the ones that should be deleted.
+type PDBFilterFunc func([]policyv1.PodDisruptionBudget) []policyv1.PodDisruptionBudget
 
 // ReducePodDisruptionBudgets detects the best PodDisruptionBudget in the set and deletes all the others.
 func ReducePodDisruptionBudgets(ctx context.Context, k8sClient client.Client, pdbs []policyv1.PodDisruptionBudget, filter PDBFilterFunc) error {

pkg/utils/test/predicates.go

@@ -604,7 +604,13 @@ func AnyPodDisruptionBudget() PodDisruptionBudgetRequirement {
 }
 
 // DataPlaneHasPodDisruptionBudget is a helper function for tests that returns a function
-// that can be used to check if a DataPlane has a PodDisruptionBudget.
+// that can be used to check if a DataPlane has a PodDisruptionBudget. It expects there is
+// only a single PodDisruptionBudget for the DataPlane with the following requirements:
+// - it is owned by the DataPlane,
+// - its `app` label matches the DP name,
+// - its `gateway-operator.konghq.com/managed-by` label is set to `dataplane`.
+// Additionally, the caller can provide a requirement function that will be used to verify
+// the PodDisruptionBudget (e.g. to check if it has an expected status).
 // Should be used in conjunction with require.Eventually or assert.Eventually.
 func DataPlaneHasPodDisruptionBudget(
 	t *testing.T,
@@ -618,14 +624,6 @@ func DataPlaneHasPodDisruptionBudget(
 	const dataplaneDeploymentAppLabel = "app"
 
 	return DataPlanePredicate(t, ctx, dataplaneName, func(dataplane *operatorv1beta1.DataPlane) bool {
-		deployments := MustListDataPlaneDeployments(t, ctx, dataplane, clients, client.MatchingLabels{
-			dataplaneDeploymentAppLabel:          dataplane.Name,
-			consts.GatewayOperatorManagedByLabel: consts.DataPlaneManagedLabelValue,
-		})
-		if len(deployments) != 1 {
-			return false
-		}
-
 		pdbs := MustListDataPlanePodDisruptionBudgets(t, ctx, dataplane, clients, client.MatchingLabels{
 			dataplaneDeploymentAppLabel:          dataplane.Name,
 			consts.GatewayOperatorManagedByLabel: consts.DataPlaneManagedLabelValue,