Align pdb resourceapply logic with library-go, add respective test

pkg/cloud/cloud.go

@@ -39,7 +39,7 @@ func GetResources(operatorConfig config.OperatorConfig) ([]client.Object, error)
 	substitutedObjects := common.SubstituteCommonPartsFromConfig(operatorConfig, renderedObjects)
 	commonResources, err := common.GetCommonResources(operatorConfig)
 	if err != nil {
-		klog.Errorf("can not create common resources, such as pdb: %v", err)
+		klog.Errorf("can not create common resources %v", err)
 		return nil, err
 	}
 	substitutedObjects = append(substitutedObjects, commonResources...)

pkg/controllers/resourceapply/resourceapply.go

@@ -8,6 +8,8 @@ import (
 	"fmt"
 	"reflect"
 
+	"k8s.io/apimachinery/pkg/api/equality"
+
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	policyv1 "k8s.io/api/policy/v1"
@@ -287,15 +289,10 @@ func applyDaemonSet(ctx context.Context, client coreclientv1.Client, recorder re
 
 func applyPodDisruptionBudget(ctx context.Context, client coreclientv1.Client, recorder record.EventRecorder, requiredOriginal *policyv1.PodDisruptionBudget) (bool, error) {
 	required := requiredOriginal.DeepCopy()
-	err := SetSpecHashAnnotation(&required.ObjectMeta, required.Spec)
-	if err != nil {
-		return false, err
-	}
 
 	existing := &policyv1.PodDisruptionBudget{}
-	err = client.Get(ctx, coreclientv1.ObjectKeyFromObject(required), existing)
+	err := client.Get(ctx, coreclientv1.ObjectKeyFromObject(required), existing)
 	if apierrors.IsNotFound(err) {
-		required.Annotations[generationAnnotation] = "1"
 		err = client.Create(ctx, required)
 		if err != nil {
 			recorder.Event(required, corev1.EventTypeWarning, "Create failed", err.Error())
@@ -312,22 +309,17 @@ func applyPodDisruptionBudget(ctx context.Context, client coreclientv1.Client, r
 	modified := resourcemerge.BoolPtr(false)
 	existingCopy := existing.DeepCopy()
 
-	expectedGeneration := ""
-	if _, ok := existingCopy.Annotations[generationAnnotation]; ok {
-		expectedGeneration = existingCopy.Annotations[generationAnnotation]
-	}
-
 	resourcemerge.EnsureObjectMeta(modified, &existingCopy.ObjectMeta, required.ObjectMeta)
-	if !*modified && expectedGeneration == fmt.Sprintf("%x", existingCopy.GetGeneration()) {
+	contentSame := equality.Semantic.DeepEqual(existingCopy.Spec, required.Spec)
+
+	if !*modified && contentSame {
 		return false, nil
 	}
 
 	// at this point we know that we're going to perform a write.  We're just trying to get the object correct
 	toWrite := existingCopy // shallow copy so the code reads easier
 	toWrite.Spec = *required.Spec.DeepCopy()
 
-	toWrite.Annotations[generationAnnotation] = fmt.Sprintf("%x", existingCopy.GetGeneration()+1)
-
 	err = client.Update(ctx, toWrite)
 	if err != nil {
 		recorder.Event(required, corev1.EventTypeWarning, "Update failed", err.Error())

pkg/controllers/resourceapply/resourceapply_test.go

@@ -8,9 +8,11 @@ import (
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	policyv1 "k8s.io/api/policy/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/utils/pointer"
@@ -61,6 +63,10 @@ func cleanupResources(t *testing.T, g *WithT, ctx context.Context, cl client.Cli
 		for _, obj := range typedList.Items {
 			deleteResouce(g, &obj)
 		}
+	case *policyv1.PodDisruptionBudgetList:
+		for _, obj := range typedList.Items {
+			deleteResouce(g, &obj)
+		}
 	default:
 		t.Fatal("can not cast list type for cleanup")
 	}
@@ -656,3 +662,95 @@ func workloadDaemonSetWithDefaultSpecHash() *appsv1.DaemonSet {
 	w.Annotations[specHashAnnotation] = "eaeff6ac704fb141d5085803b5b3cc12067ef98c9f2ba8c1052df81faa53299c"
 	return w
 }
+
+func TestApplyPDB(t *testing.T) {
+	cl, tearDownFn := setupEnvtest(t)
+	defer tearDownFn(t)
+
+	tests := []struct {
+		name     string
+		existing *policyv1.PodDisruptionBudget
+		input    *policyv1.PodDisruptionBudget
+
+		expectedModified bool
+	}{
+		{
+			name:             "create",
+			input:            podDisruptionBudget(),
+			expectedModified: true,
+		},
+		{
+			name: "skip on extra label",
+			existing: func() *policyv1.PodDisruptionBudget {
+				pdb := podDisruptionBudget()
+				pdb.Labels = map[string]string{"bar": "baz"}
+				return pdb
+			}(),
+			input: podDisruptionBudget(),
+
+			expectedModified: false,
+		},
+		{
+			name: "update on missing label",
+			existing: func() *policyv1.PodDisruptionBudget {
+				pdb := podDisruptionBudget()
+				pdb.Labels = map[string]string{"bar": "baz"}
+				return pdb
+			}(),
+			input: func() *policyv1.PodDisruptionBudget {
+				pdb := podDisruptionBudget()
+				pdb.Labels = map[string]string{"new": "merge"}
+				return pdb
+			}(),
+
+			expectedModified: true,
+		},
+		{
+			name:     "update on mismatch data",
+			existing: podDisruptionBudget(),
+			input: func() *policyv1.PodDisruptionBudget {
+				pdb := podDisruptionBudget()
+				minAvailable := intstr.FromInt(3)
+				pdb.Spec.MinAvailable = &minAvailable
+				return pdb
+			}(),
+
+			expectedModified: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+			recorder := record.NewFakeRecorder(1000)
+			ctx := context.TODO()
+			defer cleanupResources(t, g, ctx, cl, &policyv1.PodDisruptionBudgetList{})
+
+			if tt.existing != nil {
+				g.Expect(cl.Create(context.TODO(), tt.existing)).To(Succeed())
+			}
+			actualModified, err := applyPodDisruptionBudget(context.TODO(), cl, recorder, tt.input)
+			g.Expect(err).NotTo(HaveOccurred())
+			g.Expect(tt.expectedModified).To(BeEquivalentTo(actualModified), "Resource was modified")
+		})
+	}
+}
+
+func podDisruptionBudget() *policyv1.PodDisruptionBudget {
+	minAvailable := intstr.FromInt(1)
+	return &policyv1.PodDisruptionBudget{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "PodDisruptionBudget",
+			APIVersion: "policy/v1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "pdbName",
+			Namespace: "default",
+		},
+		Spec: policyv1.PodDisruptionBudgetSpec{
+			MinAvailable: &minAvailable,
+			Selector: &metav1.LabelSelector{
+				MatchLabels: map[string]string{"foo": "bar"},
+			},
+		},
+	}
+}