Skip to content

Commit

Permalink
remove pdb
Browse files Browse the repository at this point in the history
  • Loading branch information
pepov committed Jun 6, 2024
1 parent eed564f commit a3fff6b
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 174 deletions.
6 changes: 0 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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.
Expand Down
21 changes: 10 additions & 11 deletions docs/legacy.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ The legacy version was last available with the 0.1.1 version.
- PersistentVolumeClaim
- Service
- ServiceAccount
- PodDisruptionBudget
- Unstructured
- Node

Expand All @@ -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.

Expand All @@ -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).
84 changes: 0 additions & 84 deletions patch/ignorepdb.go

This file was deleted.

53 changes: 0 additions & 53 deletions tests/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(),
Expand Down
22 changes: 2 additions & 20 deletions tests/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 (
Expand Down Expand Up @@ -196,7 +195,6 @@ func testMatchOnObject(testItem *TestItem, ignoreField string) error {
opts := []patch.CalculateOption{
patch.IgnoreStatusFields(),
patch.IgnoreVolumeClaimTemplateTypeMetaAndStatus(),
patch.IgnorePDBSelector(),
patch.IgnoreField(ignoreField),
}

Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit a3fff6b

Please sign in to comment.