From b874e266dca994433b184d15b0ba04a7a6da7a3f Mon Sep 17 00:00:00 2001 From: Jonathan Knight Date: Thu, 21 Aug 2025 13:24:16 +0300 Subject: [PATCH] Fix issue where the Operator did not delete secondary resources when a Coherence resource was updated --- controllers/coherence_controller.go | 5 +- controllers/reconciler/base_controller.go | 27 ++++++- pkg/utils/storage.go | 23 ++++++ pkg/utils/storage_test.go | 97 +++++++++++++++++++++++ 4 files changed, 146 insertions(+), 6 deletions(-) create mode 100644 pkg/utils/storage_test.go diff --git a/controllers/coherence_controller.go b/controllers/coherence_controller.go index e81e910e6..22042c897 100644 --- a/controllers/coherence_controller.go +++ b/controllers/coherence_controller.go @@ -9,6 +9,9 @@ package controllers import ( "context" "fmt" + "strconv" + "time" + "github.com/go-logr/logr" coh "github.com/oracle/coherence-operator/api/v1" "github.com/oracle/coherence-operator/controllers/errorhandling" @@ -37,8 +40,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" - "strconv" - "time" ) const ( diff --git a/controllers/reconciler/base_controller.go b/controllers/reconciler/base_controller.go index 16ade0a21..49483cd9e 100644 --- a/controllers/reconciler/base_controller.go +++ b/controllers/reconciler/base_controller.go @@ -9,6 +9,10 @@ package reconciler import ( "context" "fmt" + "sort" + "strings" + "sync" + "github.com/go-logr/logr" coh "github.com/oracle/coherence-operator/api/v1" "github.com/oracle/coherence-operator/controllers/errorhandling" @@ -30,9 +34,6 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/reconcile" - "sort" - "strings" - "sync" ) //goland:noinspection GoUnusedConst @@ -722,9 +723,27 @@ func (in *ReconcileSecondaryResource) CanWatch() bool { return !in.S // ReconcileAllResourceOfKind reconciles the state of all the desired resources of the specified Kind for the reconciler func (in *ReconcileSecondaryResource) ReconcileAllResourceOfKind(ctx context.Context, request reconcile.Request, deployment coh.CoherenceResource, storage utils.Storage) (reconcile.Result, error) { - logger := in.GetLog().WithValues("Namespace", request.Namespace, "Name", request.Name, "Kind", in.Kind.Name()) + logger := in.GetLog().WithValues("Namespace", request.Namespace, "CoherenceName", request.Name, "Kind", in.Kind.Name()) var err error + + for _, del := range storage.GetDeletions() { + if del.Kind == in.Kind { + logger.Info("Deleting resource", "Name", del.Name) + resource, exists, _ := in.FindResource(ctx, request.Namespace, del.Name) + if exists { + err = in.Delete(ctx, request.Namespace, del.Name, logger) + if err != nil && !errorhandling.IsNotFound(err) { + logger.Info("Failed to delete resource", "Name", del.Name, "error", err.Error()) + return reconcile.Result{}, errors.Wrapf(err, "Failed to delete resource %v/%s", in.Kind, del.Name) + } + if err == nil { + in.GetEventRecorder().Event(resource, corev1.EventTypeNormal, EventReasonDeleted, fmt.Sprintf("Deleted resource after update to Coherence resource %s", deployment.GetName())) + } + } + } + } + resources := storage.GetLatest().GetResourcesOfKind(in.Kind) for _, res := range resources { if res.IsDelete() { diff --git a/pkg/utils/storage.go b/pkg/utils/storage.go index 08d945833..3bb2bcb7e 100644 --- a/pkg/utils/storage.go +++ b/pkg/utils/storage.go @@ -10,6 +10,7 @@ import ( "context" "encoding/json" "fmt" + coh "github.com/oracle/coherence-operator/api/v1" "github.com/oracle/coherence-operator/pkg/patching" "github.com/pkg/errors" @@ -47,6 +48,9 @@ type Storage interface { ResetHash(context.Context, coh.CoherenceResource) error // IsJob returns true if the Coherence deployment is a Job IsJob(reconcile.Request) bool + // GetDeletions returns an array of resources that existed in the previous version + // but do not exist in the latest version + GetDeletions() []coh.Resource } // NewStorage creates a new storage for the given key. @@ -78,6 +82,25 @@ func (in *secretStore) IsJob(request reconcile.Request) bool { return found } +func (in *secretStore) GetDeletions() []coh.Resource { + var deletions []coh.Resource + if in != nil { + for _, prev := range in.previous.Items { + found := false + for _, res := range in.latest.Items { + if prev.Name == res.Name && prev.Kind == res.Kind { + found = true + break + } + } + if !found { + deletions = append(deletions, prev) + } + } + } + return deletions +} + func (in *secretStore) createSecretStruct() *corev1.Secret { labels := make(map[string]string) labels[coh.LabelCoherenceStore] = "true" diff --git a/pkg/utils/storage_test.go b/pkg/utils/storage_test.go new file mode 100644 index 000000000..746f2c650 --- /dev/null +++ b/pkg/utils/storage_test.go @@ -0,0 +1,97 @@ +/* + * Copyright (c) 2020, 2025, Oracle and/or its affiliates. + * Licensed under the Universal Permissive License v 1.0 as shown at + * http://oss.oracle.com/licenses/upl. + */ + +package utils + +import ( + "testing" + + . "github.com/onsi/gomega" + coh "github.com/oracle/coherence-operator/api/v1" +) + +func TestGetDeletionsWhenLatestAndPrevAreNil(t *testing.T) { + g := NewGomegaWithT(t) + s := &secretStore{} + g.Expect(s.GetDeletions()).To(BeNil()) +} + +func TestGetDeletionsWhenLatestAndPrevItemsAreNil(t *testing.T) { + g := NewGomegaWithT(t) + s := &secretStore{ + latest: coh.Resources{}, + previous: coh.Resources{}, + } + g.Expect(s.GetDeletions()).To(BeNil()) +} + +func TestGetDeletionsWhenLatestAndPrevItemsAreEmpty(t *testing.T) { + g := NewGomegaWithT(t) + s := &secretStore{ + latest: coh.Resources{ + Items: make([]coh.Resource, 0), + }, + previous: coh.Resources{ + Items: make([]coh.Resource, 0), + }, + } + g.Expect(s.GetDeletions()).To(BeNil()) +} + +func TestGetDeletionsWhenPrevItemsAreEmpty(t *testing.T) { + g := NewGomegaWithT(t) + s := &secretStore{ + latest: coh.Resources{ + Items: []coh.Resource{ + {Name: "foo", Kind: coh.ResourceTypeService}, + {Name: "svc1", Kind: coh.ResourceTypeService}, + {Name: "foo", Kind: coh.ResourceTypeSecret}, + {Name: "bar", Kind: coh.ResourceTypeSecret}, + {Name: "sec2", Kind: coh.ResourceTypeSecret}, + }, + }, + previous: coh.Resources{ + Items: make([]coh.Resource, 0), + }, + } + g.Expect(s.GetDeletions()).To(BeNil()) +} + +func TestGetDeletions(t *testing.T) { + g := NewGomegaWithT(t) + + s := &secretStore{ + latest: coh.Resources{ + Items: []coh.Resource{ + {Name: "svc1", Kind: coh.ResourceTypeService}, + {Name: "foo", Kind: coh.ResourceTypeSecret}, + {Name: "bar", Kind: coh.ResourceTypeSecret}, + {Name: "sec2", Kind: coh.ResourceTypeSecret}, + }, + }, + previous: coh.Resources{ + Items: []coh.Resource{ + {Name: "foo", Kind: coh.ResourceTypeService}, + {Name: "bar", Kind: coh.ResourceTypeService}, + {Name: "svc1", Kind: coh.ResourceTypeService}, + {Name: "svc2", Kind: coh.ResourceTypeService}, + {Name: "foo", Kind: coh.ResourceTypeSecret}, + {Name: "sec1", Kind: coh.ResourceTypeSecret}, + {Name: "sec2", Kind: coh.ResourceTypeSecret}, + }, + }, + } + g.Expect(s.GetDeletions()).NotTo(BeNil()) + + expected := []coh.Resource{ + {Name: "foo", Kind: coh.ResourceTypeService}, + {Name: "bar", Kind: coh.ResourceTypeService}, + {Name: "svc2", Kind: coh.ResourceTypeService}, + {Name: "sec1", Kind: coh.ResourceTypeSecret}, + } + + g.Expect(s.GetDeletions()).To(Equal(expected)) +}