Fix issue where the Operator did not delete secondary resources when a Coherence resource was updated (#792)

Author: thegridman

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 (

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() {

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"

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))
+}