From cac006bf3aed25e6a6956f77758305d2a07cce15 Mon Sep 17 00:00:00 2001 From: Pete Savage Date: Mon, 13 Jan 2025 18:46:42 +0000 Subject: [PATCH 1/2] Update Clowder to reconcile on changes to non-app secrets/configmaps * Add hashCache to the Environment reconciler for usage later * Remove all Secrets/ConfigMaps from Environment at start of reconciliation cycle * Embue HashObject with the always flag * Change updateHashCacheForConfigMapAndSecret to try to read in the sec/config and return true if always set * Slight concern of erroring if not in the cache, but * Make app_interface add the secret to the cache at the start of reconciliation so it should be there, and also link the objects. --- .../clowdenvironment_controller.go | 1 + .../clowdenvironment_reconciliation.go | 5 +++++ controllers/cloud.redhat.com/handlers.go | 10 +++++++++- .../cloud.redhat.com/hashcache/hashcache.go | 17 ++++++++++------- .../hashcache/hashcache_test.go | 16 ++++++++-------- .../providers/kafka/appinterface.go | 3 +++ 6 files changed, 36 insertions(+), 16 deletions(-) diff --git a/controllers/cloud.redhat.com/clowdenvironment_controller.go b/controllers/cloud.redhat.com/clowdenvironment_controller.go index c3396e44c..c5d0aa1e5 100644 --- a/controllers/cloud.redhat.com/clowdenvironment_controller.go +++ b/controllers/cloud.redhat.com/clowdenvironment_controller.go @@ -165,6 +165,7 @@ func (r *ClowdEnvironmentReconciler) Reconcile(ctx context.Context, req ctrl.Req env: &env, log: &log, oldStatus: env.Status.DeepCopy(), + hashCache: r.HashCache, } result, resErr := reconciliation.Reconcile() diff --git a/controllers/cloud.redhat.com/clowdenvironment_reconciliation.go b/controllers/cloud.redhat.com/clowdenvironment_reconciliation.go index 3b215aabb..a98e49a15 100644 --- a/controllers/cloud.redhat.com/clowdenvironment_reconciliation.go +++ b/controllers/cloud.redhat.com/clowdenvironment_reconciliation.go @@ -7,6 +7,7 @@ import ( crd "github.com/RedHatInsights/clowder/apis/cloud.redhat.com/v1alpha1" "github.com/RedHatInsights/clowder/controllers/cloud.redhat.com/clowderconfig" + "github.com/RedHatInsights/clowder/controllers/cloud.redhat.com/hashcache" "github.com/RedHatInsights/clowder/controllers/cloud.redhat.com/providers" rc "github.com/RedHatInsights/rhc-osdk-utils/resourceCache" "github.com/go-logr/logr" @@ -57,6 +58,7 @@ type ClowdEnvironmentReconciliation struct { env *crd.ClowdEnvironment log *logr.Logger oldStatus *crd.ClowdEnvironmentStatus + hashCache *hashcache.HashCache } // Returns a list of step methods that should be run during reconciliation @@ -285,6 +287,9 @@ func (r *ClowdEnvironmentReconciliation) isTargetNamespaceMarkedForDeletion() (c } func (r *ClowdEnvironmentReconciliation) runProviders() (ctrl.Result, error) { + + r.hashCache.RemoveClowdObjectFromObjects(r.env) + provider := providers.Provider{ Ctx: r.ctx, Client: r.client, diff --git a/controllers/cloud.redhat.com/handlers.go b/controllers/cloud.redhat.com/handlers.go index 0e9ba89c8..cc73d6182 100644 --- a/controllers/cloud.redhat.com/handlers.go +++ b/controllers/cloud.redhat.com/handlers.go @@ -106,7 +106,15 @@ func (e *enqueueRequestForObjectCustom) updateHashCacheForConfigMapAndSecret(obj switch obj.(type) { case *core.ConfigMap, *core.Secret: if obj.GetAnnotations()[clowderconfig.LoadedConfig.Settings.RestarterAnnotationName] == "true" { - return e.hashCache.CreateOrUpdateObject(obj) + return e.hashCache.CreateOrUpdateObject(obj, false) + } else { + hcOjb, err := e.hashCache.Read(obj) + if err != nil { + return false, err + } + if hcOjb.Always { + return e.hashCache.CreateOrUpdateObject(obj, false) + } } } return false, nil diff --git a/controllers/cloud.redhat.com/hashcache/hashcache.go b/controllers/cloud.redhat.com/hashcache/hashcache.go index 3d152042a..67b82b848 100644 --- a/controllers/cloud.redhat.com/hashcache/hashcache.go +++ b/controllers/cloud.redhat.com/hashcache/hashcache.go @@ -32,6 +32,7 @@ type HashObject struct { Hash string ClowdApps map[types.NamespacedName]bool ClowdEnvs map[types.NamespacedName]bool + Always bool } type HashCache struct { @@ -46,11 +47,12 @@ func NewHashCache() HashCache { } } -func NewHashObject(hash string) HashObject { +func NewHashObject(hash string, always bool) HashObject { return HashObject{ Hash: hash, ClowdApps: map[types.NamespacedName]bool{}, ClowdEnvs: map[types.NamespacedName]bool{}, + Always: always, } } @@ -101,7 +103,7 @@ func (hc *HashCache) RemoveClowdObjectFromObjects(obj client.Object) { } } -func (hc *HashCache) CreateOrUpdateObject(obj client.Object) (bool, error) { +func (hc *HashCache) CreateOrUpdateObject(obj client.Object, always bool) (bool, error) { hc.lock.Lock() defer hc.lock.Unlock() @@ -129,7 +131,7 @@ func (hc *HashCache) CreateOrUpdateObject(obj client.Object) (bool, error) { hashObject, ok := hc.data[id] if !ok { - hashObj := NewHashObject(hash) + hashObj := NewHashObject(hash, always) hc.data[id] = &hashObj return true, nil } @@ -178,10 +180,6 @@ func (hc *HashCache) GetSuperHashForClowdObject(clowdObj object.ClowdObject) str func (hc *HashCache) AddClowdObjectToObject(clowdObj object.ClowdObject, obj client.Object) error { - if obj.GetAnnotations()[clowderconfig.LoadedConfig.Settings.RestarterAnnotationName] != "true" { - return nil - } - var oType string switch obj.(type) { @@ -198,6 +196,11 @@ func (hc *HashCache) AddClowdObjectToObject(clowdObj object.ClowdObject, obj cli if !ok { return ItemNotFoundError{item: fmt.Sprintf("%s/%s", id.NN.Name, id.NN.Namespace)} } + + if obj.GetAnnotations()[clowderconfig.LoadedConfig.Settings.RestarterAnnotationName] != "true" && !hc.data[id].Always { + return nil + } + hc.lock.Lock() defer hc.lock.Unlock() diff --git a/controllers/cloud.redhat.com/hashcache/hashcache_test.go b/controllers/cloud.redhat.com/hashcache/hashcache_test.go index 118e122df..d63d464ad 100644 --- a/controllers/cloud.redhat.com/hashcache/hashcache_test.go +++ b/controllers/cloud.redhat.com/hashcache/hashcache_test.go @@ -21,7 +21,7 @@ func TestHashCacheAddItemAndRetrieve(t *testing.T) { } hc := NewHashCache() - update, err := hc.CreateOrUpdateObject(sec) + update, err := hc.CreateOrUpdateObject(sec, false) assert.NoError(t, err) assert.True(t, update) obj, err := hc.Read(sec) @@ -39,7 +39,7 @@ func TestHashCacheDeleteItem(t *testing.T) { } hc := NewHashCache() - shouldUpdate, err := hc.CreateOrUpdateObject(sec) + shouldUpdate, err := hc.CreateOrUpdateObject(sec, false) assert.True(t, shouldUpdate) assert.NoError(t, err) obj, err := hc.Read(sec) @@ -63,7 +63,7 @@ func TestHashCacheUpdateItem(t *testing.T) { } hc := NewHashCache() - _, err := hc.CreateOrUpdateObject(sec) + _, err := hc.CreateOrUpdateObject(sec, false) assert.NoError(t, err) obj, err := hc.Read(sec) @@ -75,7 +75,7 @@ func TestHashCacheUpdateItem(t *testing.T) { "test2": []byte("test2"), } - update, err := hc.CreateOrUpdateObject(sec) + update, err := hc.CreateOrUpdateObject(sec, false) assert.NoError(t, err) assert.True(t, update) obj, err = hc.Read(sec) @@ -120,7 +120,7 @@ func TestHashCacheAddClowdObj(t *testing.T) { } hc := NewHashCache() - _, err := hc.CreateOrUpdateObject(sec) + _, err := hc.CreateOrUpdateObject(sec, false) assert.NoError(t, err) err = hc.AddClowdObjectToObject(capp, sec) @@ -152,7 +152,7 @@ func TestHashCacheDeleteClowdObj(t *testing.T) { } hc := NewHashCache() - _, err := hc.CreateOrUpdateObject(sec) + _, err := hc.CreateOrUpdateObject(sec, false) assert.NoError(t, err) err = hc.AddClowdObjectToObject(capp, sec) @@ -196,7 +196,7 @@ func TestHashCacheSuperCache(t *testing.T) { } hc := NewHashCache() - _, err := hc.CreateOrUpdateObject(sec) + _, err := hc.CreateOrUpdateObject(sec, false) assert.NoError(t, err) err = hc.AddClowdObjectToObject(capp, sec) assert.NoError(t, err) @@ -204,7 +204,7 @@ func TestHashCacheSuperCache(t *testing.T) { assert.NoError(t, err) assert.Contains(t, obj.ClowdApps, clowdObjNamespaceName) - _, err = hc.CreateOrUpdateObject(sec2) + _, err = hc.CreateOrUpdateObject(sec2, false) assert.NoError(t, err) err = hc.AddClowdObjectToObject(capp, sec2) assert.NoError(t, err) diff --git a/controllers/cloud.redhat.com/providers/kafka/appinterface.go b/controllers/cloud.redhat.com/providers/kafka/appinterface.go index eedae1f3b..4adc1a8f4 100644 --- a/controllers/cloud.redhat.com/providers/kafka/appinterface.go +++ b/controllers/cloud.redhat.com/providers/kafka/appinterface.go @@ -51,6 +51,9 @@ func (a *appInterface) setKafkaCA(broker *config.BrokerConfig) error { return err } + a.HashCache.CreateOrUpdateObject(&kafkaCASecret, true) + a.HashCache.AddClowdObjectToObject(a.Env, &kafkaCASecret) + broker.Cacert = utils.StringPtr(string(kafkaCASecret.Data["ca.crt"])) broker.Port = utils.IntPtr(9093) broker.SecurityProtocol = utils.StringPtr("SSL") From 17a5866f158d1215426f04051543d8fab0ba72c2 Mon Sep 17 00:00:00 2001 From: Pete Savage Date: Tue, 14 Jan 2025 10:02:01 +0000 Subject: [PATCH 2/2] Fixes errors not being handled --- .../cloud.redhat.com/providers/kafka/appinterface.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/controllers/cloud.redhat.com/providers/kafka/appinterface.go b/controllers/cloud.redhat.com/providers/kafka/appinterface.go index 4adc1a8f4..a5d3ea289 100644 --- a/controllers/cloud.redhat.com/providers/kafka/appinterface.go +++ b/controllers/cloud.redhat.com/providers/kafka/appinterface.go @@ -51,8 +51,15 @@ func (a *appInterface) setKafkaCA(broker *config.BrokerConfig) error { return err } - a.HashCache.CreateOrUpdateObject(&kafkaCASecret, true) - a.HashCache.AddClowdObjectToObject(a.Env, &kafkaCASecret) + _, err := a.HashCache.CreateOrUpdateObject(&kafkaCASecret, true) + if err != nil { + return err + } + + err = a.HashCache.AddClowdObjectToObject(a.Env, &kafkaCASecret) + if err != nil { + return err + } broker.Cacert = utils.StringPtr(string(kafkaCASecret.Data["ca.crt"])) broker.Port = utils.IntPtr(9093)