From cd4b3f7f9df036ff26b253c27008113fcc9f214b Mon Sep 17 00:00:00 2001 From: Kenny Lee Sin Cheong Date: Tue, 17 Sep 2019 13:07:26 -0400 Subject: [PATCH 1/4] ManagedDatabase currentVersion change predicate --- Makefile | 2 +- api/v1alpha1/manageddatabase_types.go | 1 + controllers/event_filters.go | 64 +++++++++++++++++++++++ controllers/manageddatabase_controller.go | 1 + go.mod | 7 +-- go.sum | 13 +++++ 6 files changed, 84 insertions(+), 4 deletions(-) create mode 100644 controllers/event_filters.go diff --git a/Makefile b/Makefile index ca1e01e..cc43bc7 100644 --- a/Makefile +++ b/Makefile @@ -36,7 +36,7 @@ deploy: manifests # Generate manifests e.g. CRD, RBAC etc. manifests: controller-gen - $(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases + $(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./api/...;./controllers/..." output:crd:artifacts:config=config/crd/bases # Run go fmt against code fmt: diff --git a/api/v1alpha1/manageddatabase_types.go b/api/v1alpha1/manageddatabase_types.go index eb0b111..899c110 100644 --- a/api/v1alpha1/manageddatabase_types.go +++ b/api/v1alpha1/manageddatabase_types.go @@ -53,6 +53,7 @@ type ManagedDatabaseStatus struct { // +kubebuilder:object:root=true // ManagedDatabase is the Schema for the manageddatabases API +// +kubebuilder:subresource:status type ManagedDatabase struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` diff --git a/controllers/event_filters.go b/controllers/event_filters.go new file mode 100644 index 0000000..b062a7a --- /dev/null +++ b/controllers/event_filters.go @@ -0,0 +1,64 @@ +package controllers + +import ( + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" + + dba "github.com/app-sre/dba-operator/api/v1alpha1" +) + +var log = ctrl.Log.WithName("predicate").WithName("eventFilters") + +var _ predicate.Predicate = ManagedDatabaseVersionChangedPredicate{} + +// ManagedDatabaseVersionChangedPredicate implements an update predicate function on Generation change. +// +// The metadata.generation field of an object is incremented by the API server when writes are made to +// the spec field of an object. This allows a controller to ignore update events where the spec is unchanged, +// and only the metadata and/or status fields are changed. +// +// This predicate will skip update events that have no change in the object's metadata.generation field, +// unless it is a ManagedDatabase, and its Status.CurrentVersion subresource changes. +type ManagedDatabaseVersionChangedPredicate struct { + predicate.Funcs +} + +// Update implements filter for validating generation change +func (ManagedDatabaseVersionChangedPredicate) Update(e event.UpdateEvent) bool { + if e.MetaOld == nil { + log.Error(nil, "Update event has no old metadata", "event", e) + return false + } + if e.ObjectOld == nil { + log.Error(nil, "Update event has no old runtime object to update", "event", e) + return false + } + if e.ObjectNew == nil { + log.Error(nil, "Update event has no new runtime object for update", "event", e) + return false + } + if e.MetaNew == nil { + log.Error(nil, "Update event has no new metadata", "event", e) + return false + } + // Check that the status subresource is enabled, and for currentVersion changes + // in the ManagedDatabase. + if e.MetaNew.GetGeneration() > 0 && e.MetaNew.GetGeneration() == e.MetaOld.GetGeneration() { + oldMdb, ok := e.ObjectOld.(*dba.ManagedDatabase) + if !ok { + return false + } + + newMdb, ok := e.ObjectNew.(*dba.ManagedDatabase) + if !ok { + return false + } + + if oldMdb.Status.CurrentVersion != newMdb.Status.CurrentVersion { + return true + } + return false + } + return true +} diff --git a/controllers/manageddatabase_controller.go b/controllers/manageddatabase_controller.go index 22fb945..1bbcdb3 100644 --- a/controllers/manageddatabase_controller.go +++ b/controllers/manageddatabase_controller.go @@ -358,6 +358,7 @@ func (c *ManagedDatabaseController) SetupWithManager(mgr ctrl.Manager) error { For(&dba.ManagedDatabase{}). Owns(&batchv1.Job{}). Owns(&corev1.Secret{}). + WithEventFilter(&ManagedDatabaseVersionChangedPredicate{}). Complete(reconcile.Func(c.ReconcileManagedDatabase)) if err != nil { return fmt.Errorf("Unable to finish operator setup: %w", err) diff --git a/go.mod b/go.mod index d6bb9c3..7f6d7e4 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/app-sre/dba-operator -go 1.12 +go 1.13 require ( github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751 // indirect @@ -15,9 +15,10 @@ require ( github.com/prometheus/common v0.0.0-20180801064454-c7de2306084e github.com/sirupsen/logrus v1.4.2 // indirect gopkg.in/alecthomas/kingpin.v2 v2.2.6 // indirect + gopkg.in/yaml.v2 v2.2.2 k8s.io/api v0.0.0-20190409021203-6e4e0e4f393b k8s.io/apimachinery v0.0.0-20190404173353-6a84e37a896d k8s.io/client-go v11.0.1-0.20190409021438-1a26190bd76a+incompatible - sigs.k8s.io/controller-runtime v0.2.0-beta.4 - sigs.k8s.io/controller-tools v0.2.0-beta.4 // indirect + sigs.k8s.io/controller-runtime v0.2.1 + sigs.k8s.io/controller-tools v0.2.1 // indirect ) diff --git a/go.sum b/go.sum index d78cbe0..15b1a2a 100644 --- a/go.sum +++ b/go.sum @@ -9,6 +9,9 @@ github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/deckarep/golang-set v1.7.1 h1:SCQV0S6gTtp6itiFrTqI+pfmJ4LN85S1YzhDf9rTHJQ= +github.com/deckarep/golang-set v1.7.1/go.mod h1:93vsz/8Wt4joVM7c2AVqh+YRMiUSc14yDtF28KmMOgQ= +github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3ebgob9U8Nd0kOddGdZWjyMGR8Wziv+TBNwSE= github.com/evanphx/json-patch v4.5.0+incompatible h1:ouOWdg56aJriqS0huScTkVXPC5IcNrDCXZ6OoTAWu7M= github.com/evanphx/json-patch v4.5.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= @@ -27,11 +30,15 @@ github.com/golang/groupcache v0.0.0-20180513044358-24b0969c4cb7 h1:u4bArs140e9+A github.com/golang/groupcache v0.0.0-20180513044358-24b0969c4cb7/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/protobuf v1.2.0 h1:P3YflyNX/ehuJFLhxviNdFxQPkGK5cDcApsge1SqnvM= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= +github.com/golang/protobuf v1.3.2 h1:6nsPYzhq5kReh6QImI3k5qWzO4PEbvbIW2cwSfR/6xs= +github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/gofuzz v0.0.0-20170612174753-24818f796faf h1:+RRA9JqSOZFfKrOeqr2z77+8R2RKyh8PG66dcu1V0ck= github.com/google/gofuzz v0.0.0-20170612174753-24818f796faf/go.mod h1:HP5RmnzzSNb993RKQDq4+1A4ia9nllfqcQFTQJedwGI= github.com/googleapis/gnostic v0.2.0 h1:l6N3VoaVzTncYYW+9yOz2LJJammFZGBO13sqgEhpy9g= github.com/googleapis/gnostic v0.2.0/go.mod h1:sJBsCZ4ayReDTBIg8b9dl28c5xFWyhBTVRp3pOg5EKY= +github.com/googleapis/gnostic v0.3.1 h1:WeAefnSUHlBb0iJKwxFDZdbfGwkd7xRNuV+IpXMJhYk= +github.com/googleapis/gnostic v0.3.1/go.mod h1:on+2t9HRStVgn95RSsFWFz+6Q0Snyqv1awfrALZdbtU= github.com/hashicorp/golang-lru v0.0.0-20180201235237-0fb14efe8c47 h1:UnszMmmmm5vLwWzDjTFVIkfhvWF1NdrmChl8L2NUDCw= github.com/hashicorp/golang-lru v0.0.0-20180201235237-0fb14efe8c47/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI= @@ -120,6 +127,8 @@ golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGm golang.org/x/tools v0.0.0-20190501045030-23463209683d/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= gomodules.xyz/jsonpatch/v2 v2.0.0 h1:OyHbl+7IOECpPKfVK42oFr6N7+Y2dR+Jsb/IiDV3hOo= gomodules.xyz/jsonpatch/v2 v2.0.0/go.mod h1:IhYNNY4jnS53ZnfE4PAmpKtDpTCj1JFXc+3mwe7XcUU= +gomodules.xyz/jsonpatch/v2 v2.0.1 h1:xyiBuvkD2g5n7cYzx6u2sxQvsAy4QJsZFCzGVdzOXZ0= +gomodules.xyz/jsonpatch/v2 v2.0.1/go.mod h1:IhYNNY4jnS53ZnfE4PAmpKtDpTCj1JFXc+3mwe7XcUU= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= gopkg.in/alecthomas/kingpin.v2 v2.2.6 h1:jMFz6MfLP0/4fUyZle81rXUoxOBFi19VUFKVDOQfozc= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= @@ -134,6 +143,7 @@ gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWD gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v3 v3.0.0-20190709130402-674ba3eaed22/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= k8s.io/api v0.0.0-20190409021203-6e4e0e4f393b h1:aBGgKJUM9Hk/3AE8WaZIApnTxG35kbuQba2w+SXqezo= k8s.io/api v0.0.0-20190409021203-6e4e0e4f393b/go.mod h1:iuAfoD4hCxJ8Onx9kaTIt30j7jUFS00AXQi6QMi99vA= k8s.io/apiextensions-apiserver v0.0.0-20190409022649-727a075fdec8 h1:q1Qvjzs/iEdXF6A1a8H3AKVFDzJNcJn3nXMs6R6qFtA= @@ -151,7 +161,10 @@ k8s.io/utils v0.0.0-20190506122338-8fab8cb257d5 h1:VBM/0P5TWxwk+Nw6Z+lAw3DKgO76g k8s.io/utils v0.0.0-20190506122338-8fab8cb257d5/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew= sigs.k8s.io/controller-runtime v0.2.0-beta.4 h1:S1XVfRWR1MuIXZdkYx3jN8JDw+bbQxmWZroy0i87z/A= sigs.k8s.io/controller-runtime v0.2.0-beta.4/go.mod h1:HweyYKQ8fBuzdu2bdaeBJvsFgAi/OqBBnrVGXcqKhME= +sigs.k8s.io/controller-runtime v0.2.1 h1:XwUV7gwU/2Uerl9Vb5TpoA3wMQgOxI/LdLq8UhkSSRA= +sigs.k8s.io/controller-runtime v0.2.1/go.mod h1:9dyohw3ZtoXQuV1e766PHUn+cmrRCIcBh6XIMFNMZ+I= sigs.k8s.io/controller-tools v0.2.0-beta.4/go.mod h1:8t/X+FVWvk6TaBcsa+UKUBbn7GMtvyBKX30SGl4em6Y= +sigs.k8s.io/controller-tools v0.2.1/go.mod h1:cenyhL7t2e7izk/Zy7ZxDqQ9YEj0niU5VDL1PWMgZ5s= sigs.k8s.io/testing_frameworks v0.1.1 h1:cP2l8fkA3O9vekpy5Ks8mmA0NW/F7yBdXf8brkWhVrs= sigs.k8s.io/testing_frameworks v0.1.1/go.mod h1:VVBKrHmJ6Ekkfz284YKhQePcdycOzNH9qL6ht1zEr/U= sigs.k8s.io/yaml v1.1.0 h1:4A07+ZFc2wgJwo8YNlQpr1rVlgUDlxXHhPJciaPY5gs= From 47e573de111494a845d2e3464e29641c0c1f788e Mon Sep 17 00:00:00 2001 From: Kenny Lee Sin Cheong Date: Tue, 17 Sep 2019 15:06:51 -0400 Subject: [PATCH 2/4] Predicate tests --- controllers/event_filters_test.go | 56 +++++++++++++++++++++++++++++++ go.mod | 1 + go.sum | 2 ++ 3 files changed, 59 insertions(+) create mode 100644 controllers/event_filters_test.go diff --git a/controllers/event_filters_test.go b/controllers/event_filters_test.go new file mode 100644 index 0000000..4828f28 --- /dev/null +++ b/controllers/event_filters_test.go @@ -0,0 +1,56 @@ +package controllers + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + dba "github.com/app-sre/dba-operator/api/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/event" +) + +var managedDatabaseUpdatesForPredicateTests = []struct { + oldGeneration int + oldCurrentVersion string + newGeneration int + newCurrentVersion string + expected bool +}{ + {0, "v1", 0, "v2", true}, // Status subresource not enabled, currentVersion change + {0, "v1", 0, "v1", true}, // Status subresource not enabled, some update other than currentVersion + {1, "v1", 1, "v1", false}, // Same generation, same currentVersion (change to some other field) + {1, "v1", 1, "v2", true}, // currentVersion change, same generation + {1, "v1", 2, "v1", true}, // Generation change, same currentVersion + {1, "v1", 2, "v2", true}, // Generation change & version change +} + +func managedDatabase(generation int64, currentVersion string) *dba.ManagedDatabase { + managedDatabase := &dba.ManagedDatabase{ + ObjectMeta: metav1.ObjectMeta{ + Generation: generation, + }, + Status: dba.ManagedDatabaseStatus{ + CurrentVersion: currentVersion, + }, + } + return managedDatabase +} + +func TestManagedDatabaseVersionChangedPredicate(t *testing.T) { + mdbPredicate := ManagedDatabaseVersionChangedPredicate{} + + for _, update := range managedDatabaseUpdatesForPredicateTests { + oldManagedDatabase := managedDatabase(int64(update.oldGeneration), update.oldCurrentVersion) + newManagedDatabase := managedDatabase(int64(update.newGeneration), update.newCurrentVersion) + + updateEvent := event.UpdateEvent{ + MetaOld: &oldManagedDatabase.ObjectMeta, + ObjectOld: oldManagedDatabase, + MetaNew: &newManagedDatabase.ObjectMeta, + ObjectNew: newManagedDatabase, + } + + assert.Equal(t, mdbPredicate.Update(updateEvent), update.expected) + } +} diff --git a/go.mod b/go.mod index 7f6d7e4..6b5877b 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,7 @@ require ( github.com/prometheus/client_golang v0.9.0 github.com/prometheus/common v0.0.0-20180801064454-c7de2306084e github.com/sirupsen/logrus v1.4.2 // indirect + github.com/stretchr/testify v1.3.0 gopkg.in/alecthomas/kingpin.v2 v2.2.6 // indirect gopkg.in/yaml.v2 v2.2.2 k8s.io/api v0.0.0-20190409021203-6e4e0e4f393b diff --git a/go.sum b/go.sum index 15b1a2a..288a778 100644 --- a/go.sum +++ b/go.sum @@ -74,6 +74,7 @@ github.com/pborman/uuid v0.0.0-20170612153648-e790cca94e6c h1:MUyE44mTvnI5A0xrxI github.com/pborman/uuid v0.0.0-20170612153648-e790cca94e6c/go.mod h1:VyrYX9gd7irzKovcSS6BIIEwPRkP2Wm2m9ufcdFSJ34= github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/prometheus/client_golang v0.9.0 h1:tXuTFVHC03mW0D+Ua1Q2d1EAVqLTuggX50V0VLICCzY= github.com/prometheus/client_golang v0.9.0/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw= @@ -93,6 +94,7 @@ github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnIn github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= +github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= go.uber.org/atomic v1.3.2 h1:2Oa65PReHzfn29GpvgsYwloV9AVFHPDk8tYxt2c2tr4= go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= From 19702654d7275e3eeee45fb92da9400300059701 Mon Sep 17 00:00:00 2001 From: Kenny Lee Sin Cheong Date: Mon, 30 Sep 2019 13:34:10 -0400 Subject: [PATCH 3/4] Move event filters table in test function --- controllers/event_filters_test.go | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/controllers/event_filters_test.go b/controllers/event_filters_test.go index 4828f28..5353a63 100644 --- a/controllers/event_filters_test.go +++ b/controllers/event_filters_test.go @@ -10,21 +10,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/event" ) -var managedDatabaseUpdatesForPredicateTests = []struct { - oldGeneration int - oldCurrentVersion string - newGeneration int - newCurrentVersion string - expected bool -}{ - {0, "v1", 0, "v2", true}, // Status subresource not enabled, currentVersion change - {0, "v1", 0, "v1", true}, // Status subresource not enabled, some update other than currentVersion - {1, "v1", 1, "v1", false}, // Same generation, same currentVersion (change to some other field) - {1, "v1", 1, "v2", true}, // currentVersion change, same generation - {1, "v1", 2, "v1", true}, // Generation change, same currentVersion - {1, "v1", 2, "v2", true}, // Generation change & version change -} - func managedDatabase(generation int64, currentVersion string) *dba.ManagedDatabase { managedDatabase := &dba.ManagedDatabase{ ObjectMeta: metav1.ObjectMeta{ @@ -38,6 +23,21 @@ func managedDatabase(generation int64, currentVersion string) *dba.ManagedDataba } func TestManagedDatabaseVersionChangedPredicate(t *testing.T) { + var managedDatabaseUpdatesForPredicateTests = []struct { + oldGeneration int + oldCurrentVersion string + newGeneration int + newCurrentVersion string + expected bool + }{ + {0, "v1", 0, "v2", true}, // Status subresource not enabled, currentVersion change + {0, "v1", 0, "v1", true}, // Status subresource not enabled, some update other than currentVersion + {1, "v1", 1, "v1", false}, // Same generation, same currentVersion (change to some other field) + {1, "v1", 1, "v2", true}, // currentVersion change, same generation + {1, "v1", 2, "v1", true}, // Generation change, same currentVersion + {1, "v1", 2, "v2", true}, // Generation change & version change + } + mdbPredicate := ManagedDatabaseVersionChangedPredicate{} for _, update := range managedDatabaseUpdatesForPredicateTests { From 7804ad6284649fcd4cb6c8ab5c142f056c398d9f Mon Sep 17 00:00:00 2001 From: Kenny Lee Sin Cheong Date: Mon, 30 Sep 2019 16:11:17 -0400 Subject: [PATCH 4/4] Add check for resourceVersions on non-ManagedDataabse types --- controllers/event_filters.go | 32 ++++++++---- controllers/event_filters_test.go | 86 +++++++++++++++++++++++++------ 2 files changed, 90 insertions(+), 28 deletions(-) diff --git a/controllers/event_filters.go b/controllers/event_filters.go index b062a7a..66d0025 100644 --- a/controllers/event_filters.go +++ b/controllers/event_filters.go @@ -18,8 +18,9 @@ var _ predicate.Predicate = ManagedDatabaseVersionChangedPredicate{} // the spec field of an object. This allows a controller to ignore update events where the spec is unchanged, // and only the metadata and/or status fields are changed. // -// This predicate will skip update events that have no change in the object's metadata.generation field, -// unless it is a ManagedDatabase, and its Status.CurrentVersion subresource changes. +// This predicate will skip update events by a ManagedDatabase where its metadata.generation field is not changed, +// unless its Status.CurrentVersion subresource changes. For other object types, update are sent on +// ResourceVersion changes (default). type ManagedDatabaseVersionChangedPredicate struct { predicate.Funcs } @@ -42,23 +43,32 @@ func (ManagedDatabaseVersionChangedPredicate) Update(e event.UpdateEvent) bool { log.Error(nil, "Update event has no new metadata", "event", e) return false } - // Check that the status subresource is enabled, and for currentVersion changes - // in the ManagedDatabase. - if e.MetaNew.GetGeneration() > 0 && e.MetaNew.GetGeneration() == e.MetaOld.GetGeneration() { - oldMdb, ok := e.ObjectOld.(*dba.ManagedDatabase) - if !ok { - return false - } - newMdb, ok := e.ObjectNew.(*dba.ManagedDatabase) - if !ok { + oldMdb, ok := e.ObjectOld.(*dba.ManagedDatabase) + if !ok { + if e.MetaNew.GetResourceVersion() == e.MetaOld.GetResourceVersion() { return false } + return true + } + + newMdb, ok := e.ObjectNew.(*dba.ManagedDatabase) + if !ok { + return false + } + // Check that the status subresource is enabled, and for currentVersion changes + // in the ManagedDatabase. + if e.MetaNew.GetGeneration() > 0 && e.MetaNew.GetGeneration() == e.MetaOld.GetGeneration() { if oldMdb.Status.CurrentVersion != newMdb.Status.CurrentVersion { return true } return false } + + if e.MetaNew.GetResourceVersion() == e.MetaOld.GetResourceVersion() { + return false + } + return true } diff --git a/controllers/event_filters_test.go b/controllers/event_filters_test.go index 5353a63..82344c8 100644 --- a/controllers/event_filters_test.go +++ b/controllers/event_filters_test.go @@ -6,14 +6,16 @@ import ( "github.com/stretchr/testify/assert" dba "github.com/app-sre/dba-operator/api/v1alpha1" + batchv1 "k8s.io/api/batch/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/event" ) -func managedDatabase(generation int64, currentVersion string) *dba.ManagedDatabase { +func managedDatabase(generation int64, resourceVersion, currentVersion string) *dba.ManagedDatabase { managedDatabase := &dba.ManagedDatabase{ ObjectMeta: metav1.ObjectMeta{ - Generation: generation, + ResourceVersion: resourceVersion, + Generation: generation, }, Status: dba.ManagedDatabaseStatus{ CurrentVersion: currentVersion, @@ -22,27 +24,44 @@ func managedDatabase(generation int64, currentVersion string) *dba.ManagedDataba return managedDatabase } +func job(generation int64, resourceVersion string, active int32) *batchv1.Job { + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + ResourceVersion: resourceVersion, + Generation: generation, + }, + Status: batchv1.JobStatus{ + Active: active, + }, + } + return job +} + func TestManagedDatabaseVersionChangedPredicate(t *testing.T) { + mdbPredicate := ManagedDatabaseVersionChangedPredicate{} + var managedDatabaseUpdatesForPredicateTests = []struct { - oldGeneration int - oldCurrentVersion string - newGeneration int - newCurrentVersion string - expected bool + oldGeneration int + oldResourceVersion string + oldCurrentVersion string + newGeneration int + newResourceVersion string + newCurrentVersion string + expected bool }{ - {0, "v1", 0, "v2", true}, // Status subresource not enabled, currentVersion change - {0, "v1", 0, "v1", true}, // Status subresource not enabled, some update other than currentVersion - {1, "v1", 1, "v1", false}, // Same generation, same currentVersion (change to some other field) - {1, "v1", 1, "v2", true}, // currentVersion change, same generation - {1, "v1", 2, "v1", true}, // Generation change, same currentVersion - {1, "v1", 2, "v2", true}, // Generation change & version change + {0, "1", "v1", 0, "1", "v1", false}, // No-change update + {1, "1", "v1", 1, "1", "v1", false}, // No-change update w/ generation field set + {0, "1", "v1", 0, "2", "v2", true}, // Generation not set, currentVersion change (resourceVersion change) + {0, "1", "v1", 0, "2", "v1", true}, // Generation not set, some update other than currentVersion + {1, "1", "v1", 1, "2", "v1", false}, // Same generation, same currentVersion (change to some other field) + {1, "1", "v1", 1, "2", "v2", true}, // currentVersion change, same generation + {1, "1", "v1", 2, "2", "v1", true}, // Generation change, same currentVersion + {1, "1", "v1", 2, "2", "v2", true}, // Generation change & currentVersion change } - mdbPredicate := ManagedDatabaseVersionChangedPredicate{} - for _, update := range managedDatabaseUpdatesForPredicateTests { - oldManagedDatabase := managedDatabase(int64(update.oldGeneration), update.oldCurrentVersion) - newManagedDatabase := managedDatabase(int64(update.newGeneration), update.newCurrentVersion) + oldManagedDatabase := managedDatabase(int64(update.oldGeneration), update.oldResourceVersion, update.oldCurrentVersion) + newManagedDatabase := managedDatabase(int64(update.newGeneration), update.newResourceVersion, update.newCurrentVersion) updateEvent := event.UpdateEvent{ MetaOld: &oldManagedDatabase.ObjectMeta, @@ -53,4 +72,37 @@ func TestManagedDatabaseVersionChangedPredicate(t *testing.T) { assert.Equal(t, mdbPredicate.Update(updateEvent), update.expected) } + + var jobUpdatesForPredicateTests = []struct { + oldGeneration int + oldResourceVersion string + oldActive int + newGeneration int + newResourceVersion string + newActive int + expected bool + }{ + {0, "1", 0, 0, "1", 0, false}, // No-change update + {1, "1", 0, 1, "1", 0, false}, // No-change update w/ generation field set + {0, "1", 0, 0, "2", 1, true}, // Status change, no generation, version update + {1, "1", 0, 1, "2", 1, true}, // Status change, generation, version update + } + + // Any other objects other than ManagedDatabase should reconcile unless there is no changes. + // + // NOTE: It doesn't look like generation in incremented (or used) for all types by the API server. + // For example, as of 1.15, changing the Job types does not make use of the generation field. + for _, update := range jobUpdatesForPredicateTests { + oldJob := job(int64(update.oldGeneration), update.oldResourceVersion, int32(update.oldActive)) + newJob := job(int64(update.newGeneration), update.newResourceVersion, int32(update.newActive)) + + updateEvent := event.UpdateEvent{ + MetaOld: &oldJob.ObjectMeta, + ObjectOld: oldJob, + MetaNew: &newJob.ObjectMeta, + ObjectNew: newJob, + } + + assert.Equal(t, mdbPredicate.Update(updateEvent), update.expected) + } }