remove pdb

pepov · pepov · commit a3fff6b70b19 · 2024-06-06T16:08:15.000+02:00
diff --git a/README.md b/README.md
@@ -65,7 +65,6 @@ In certain cases there is a need to filter out certain fields when the patch gen
 To help in these scenarios there are the following options to be used when calculating diffs:
 - `IgnoreStatusFields`
 - `IgnoreVolumeClaimTemplateTypeMetaAndStatus`
-- `IgnorePDBSelector`
 - `IgnoreField("field-name-to-ignore")`
 
 Example:
@@ -88,11 +87,6 @@ This CalculateOptions removes status fields from both objects before comparing.
 
 This CalculateOption clears volumeClaimTemplate fields from both objects before comparing (applies to statefulsets).
 
-#### IgnorePdbSelector
-
-Checks `selector` fields of PDB objects before comparing and removes them if they match. `reflect.DeepEquals` is used for the equality check.
-This is required because map fields using `patchStrategy:"replace"` will always diff regardless if they are otherwise equal.
-
 #### IgnoreField("field-name-to-ignore")
 
 This CalculateOption removes the field provided (as a string) in the call before comparing them. A common usage might be to remove the metadata fields by using the `IgnoreField("metadata")` option.
diff --git a/docs/legacy.md b/docs/legacy.md
@@ -20,7 +20,6 @@ The legacy version was last available with the 0.1.1 version.
 - PersistentVolumeClaim
 - Service
 - ServiceAccount
-- PodDisruptionBudget
 - Unstructured
 - Node
 
@@ -33,11 +32,11 @@ objectMatcher.Match(e.ObjectOld, e.ObjectNew)
 
 ### The idea
 
-There are existing libraries in the wild that can calculate a patch by giving them two different objects. If the patch is empty the two objects match and we are ready, right? 
+There are existing libraries in the wild that can calculate a patch by giving them two different objects. If the patch is empty the two objects match and we are ready, right?
 Well not quite. JSON Merge Patch, defined by [rfc7396](https://tools.ietf.org/html/rfc7396) replaces lists completely which is not always what we need. Kubernetes defines
 and uses a modified version called [strategic merge patch](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-api-machinery/strategic-merge-patch.md).
 
-Strategic Merge Patch extends the JSON Merge Patch format by adding explicit directives for deleting, replacing, ordering and merging lists. 
+Strategic Merge Patch extends the JSON Merge Patch format by adding explicit directives for deleting, replacing, ordering and merging lists.
 It uses the go struct tag of the API objects to determine what lists should be merged and which ones should not. Worth to note it's not tied to
 Kubernetes objects only, so we can use this for matching custom go structs as well.
 
@@ -46,21 +45,21 @@ Kubernetes objects only, so we can use this for matching custom go structs as we
 #### Defaults and version compatibility
 
 As outlined previously Kubernetes objects are amended with different default values when submitted. For example PodSpec.RestartPolicy will be set to "Always" when
-ommitted from the object, so we will have a mismatch when we try to compare it later. This library uses the same functions to set the default values on the local 
-objects before matching so that they won't differ. 
+ommitted from the object, so we will have a mismatch when we try to compare it later. This library uses the same functions to set the default values on the local
+objects before matching so that they won't differ.
 
-Since the defaults functions are defined in the main kubernetes repo, there is a higher chance that objects decorated using these functions will be incompatible 
+Since the defaults functions are defined in the main kubernetes repo, there is a higher chance that objects decorated using these functions will be incompatible
 when comparing them with objects coming from different server versions. Also since the library depends on the kubernetes repo it is more tightly coupled to it's
 version.
 
-To preserve compatibility between the client and server versions  [.circleci/config.yml](.circleci/config.yml) contains jobs that run the integration test suite against Kubernetes 
+To preserve compatibility between the client and server versions  [.circleci/config.yml](.circleci/config.yml) contains jobs that run the integration test suite against Kubernetes
 versions from 1.10 to 1.14. The library itself is known and tested to be working with operators depending on Kubernetes client version 1.12 and 1.13.
 
 #### Generated values
 
-There are values that are generated by the API Server dynamically. To workaround this the library removes null fields from the patch as long as it's not inside a list. 
-(In case of lists, even if we remove null fields we would still left with `setElementOrder` directives) 
-This works as long as we don't set/unset complete fields on the objects conditionally, because in that case we would miss to detect a change to unset something. 
+There are values that are generated by the API Server dynamically. To workaround this the library removes null fields from the patch as long as it's not inside a list.
+(In case of lists, even if we remove null fields we would still left with `setElementOrder` directives)
+This works as long as we don't set/unset complete fields on the objects conditionally, because in that case we would miss to detect a change to unset something.
 
-In case a field gets removed from somewhere inside a list we have to explicitly tell to ignore it. One example is NodePort in Service objects, see [service.go](service.go). 
+In case a field gets removed from somewhere inside a list we have to explicitly tell to ignore it. One example is NodePort in Service objects, see [service.go](service.go).
 Another example is Volume and VolumeMount generated automatically for the service account token, see [pod.go](pod.go).
diff --git a/patch/ignorepdb.go b/patch/ignorepdb.go
diff --git a/tests/integration_test.go b/tests/integration_test.go
@@ -24,7 +24,6 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	"k8s.io/api/autoscaling/v2beta1"
 	v1 "k8s.io/api/core/v1"
-	v1beta12 "k8s.io/api/policy/v1beta1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	crdv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -704,58 +703,6 @@ func TestIntegration(t *testing.T) {
 					},
 				},
 			}),
-		NewTestMatch("pdb match",
-			&v1beta12.PodDisruptionBudget{
-				TypeMeta: metav1.TypeMeta{
-					APIVersion: v1beta12.SchemeGroupVersion.String(),
-					Kind:       "PodDisruptionBudget",
-				},
-				ObjectMeta: standardObjectMeta(),
-				Spec: v1beta12.PodDisruptionBudgetSpec{
-					MinAvailable: intstrRef(intstr.FromInt(1)),
-					Selector: &metav1.LabelSelector{
-						MatchLabels: map[string]string{
-							"app": "example",
-						},
-					},
-				},
-			}),
-		NewTestDiff("pdb diff on matchlabels",
-			&v1beta12.PodDisruptionBudget{
-				TypeMeta: metav1.TypeMeta{
-					APIVersion: v1beta12.SchemeGroupVersion.String(),
-					Kind:       "PodDisruptionBudget",
-				},
-				ObjectMeta: standardObjectMeta(),
-				Spec: v1beta12.PodDisruptionBudgetSpec{
-					MinAvailable: intstrRef(intstr.FromInt(1)),
-					Selector: &metav1.LabelSelector{
-						MatchLabels: map[string]string{
-							"app": "example",
-						},
-					},
-				},
-			}).
-			withRemoteChange(func(i interface{}) {
-				pdb := i.(*v1beta12.PodDisruptionBudget)
-				pdb.Spec.Selector.MatchLabels = map[string]string{
-					"app": "example2",
-				}
-			}),
-		NewTestMatch("pdb match even though status changes",
-			&v1beta12.PodDisruptionBudget{
-				ObjectMeta: standardObjectMeta(),
-				Spec: v1beta12.PodDisruptionBudgetSpec{
-					MinAvailable: intstrRef(intstr.FromInt(1)),
-				},
-			}).
-			withRemoteChange(func(i interface{}) {
-				pdb := i.(*v1beta12.PodDisruptionBudget)
-				pdb.Status.CurrentHealthy = 1
-				pdb.Status.DesiredHealthy = 1
-				pdb.Status.ExpectedPods = 1
-				pdb.Status.ObservedGeneration = 1
-			}),
 		NewTestMatch("pvc match",
 			&v1.PersistentVolumeClaim{
 				ObjectMeta: standardObjectMeta(),
diff --git a/tests/main_test.go b/tests/main_test.go
@@ -23,16 +23,13 @@ import (
 	"testing"
 
 	"emperror.dev/errors"
-	"github.com/cisco-open/k8s-objectmatcher/patch"
 	admregv1 "k8s.io/api/admissionregistration/v1"
 	appsv1 "k8s.io/api/apps/v1"
 	"k8s.io/api/autoscaling/v2beta1"
 	v1 "k8s.io/api/core/v1"
-	policyv1beta1 "k8s.io/api/policy/v1beta1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	crdv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apiextension "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
-	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -41,6 +38,8 @@ import (
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/klog/v2"
+
+	"github.com/cisco-open/k8s-objectmatcher/patch"
 )
 
 var (
@@ -196,7 +195,6 @@ func testMatchOnObject(testItem *TestItem, ignoreField string) error {
 	opts := []patch.CalculateOption{
 		patch.IgnoreStatusFields(),
 		patch.IgnoreVolumeClaimTemplateTypeMetaAndStatus(),
-		patch.IgnorePDBSelector(),
 		patch.IgnoreField(ignoreField),
 	}
 
@@ -354,22 +352,6 @@ func testMatchOnObject(testItem *TestItem, ignoreField string) error {
 				log.Printf("Failed to remove object %s %+v", existing.GetName(), err)
 			}
 		}()
-	case *policyv1beta1.PodDisruptionBudget:
-		existing, err = testContext.Client.PolicyV1beta1().PodDisruptionBudgets(newObject.GetNamespace()).Create(context.Background(), newObject.(*policyv1beta1.PodDisruptionBudget), metav1.CreateOptions{})
-		if err != nil {
-			return errors.WrapWithDetails(err, "failed to create object", "object", newObject)
-		}
-		defer func() {
-			err = testContext.Client.PolicyV1beta1().PodDisruptionBudgets(newObject.GetNamespace()).Delete(context.Background(), existing.GetName(), deleteOptions)
-			if err != nil {
-				log.Printf("Failed to remove object %s %+v", existing.GetName(), err)
-			}
-		}()
-		// In case of PodDisruptionBudget resources `patchStrategy:"replace"` results in a constant diff from kubernetes version 1.21
-		// The IgnorePDBSelector CalculateOption can only help in this situation if either the current or modified object contains gvk information, this is why it's set here explicitly
-		te, _ := meta.TypeAccessor(existing)
-		te.SetKind("PodDisruptionBudget")
-		te.SetAPIVersion("policy/v1beta1")
 	case *v1.PersistentVolumeClaim:
 		existing, err = testContext.Client.CoreV1().PersistentVolumeClaims(newObject.GetNamespace()).Create(context.Background(), newObject.(*v1.PersistentVolumeClaim), metav1.CreateOptions{})
 		if err != nil {
