chore: deprecation notice for non-unique loggingRef (#1391)

add notice to logging status when there are multiple logging resources with the same ref

Signed-off-by: Peter Wilcsinszky <[email protected]>

controllers/logging/logging_controller.go

@@ -160,7 +160,12 @@ func (r *LoggingReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	}()
 
 	reconcilers := []resources.ContextAwareComponentReconciler{
-		model.NewValidationReconciler(r.Client, loggingResources, &secretLoaderFactory{Client: r.Client, Path: fluentd.OutputSecretPath}),
+		model.NewValidationReconciler(
+			r.Client,
+			loggingResources,
+			&secretLoaderFactory{Client: r.Client, Path: fluentd.OutputSecretPath},
+			log.WithName("validation"),
+		),
 	}
 
 	if logging.Spec.FluentdSpec != nil && logging.Spec.SyslogNGSpec != nil {
@@ -262,6 +267,7 @@ func (r *LoggingReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 			return *result, err
 		}
 	}
+
 	return ctrl.Result{}, nil
 }
 

controllers/logging/logging_controller_test.go

@@ -266,6 +266,64 @@ func TestSingleFlowWithClusterOutput(t *testing.T) {
 	g.Expect(string(secret.Data[fluentd.AppConfigKey])).Should(gomega.ContainSubstring("a:b"))
 }
 
+func TestLogginResourcesWithNonUniqueLoggingRefs(t *testing.T) {
+	g := gomega.NewGomegaWithT(t)
+	defer beforeEach(t)()
+
+	logging1 := &v1beta1.Logging{
+		ObjectMeta: v1.ObjectMeta{
+			Name: "test-1",
+		},
+		Spec: v1beta1.LoggingSpec{
+			ControlNamespace: controlNamespace,
+		},
+	}
+	logging2 := &v1beta1.Logging{
+		ObjectMeta: v1.ObjectMeta{
+			Name: "test-2",
+		},
+		Spec: v1beta1.LoggingSpec{
+			ControlNamespace: controlNamespace,
+		},
+	}
+	logging3 := &v1beta1.Logging{
+		ObjectMeta: v1.ObjectMeta{
+			Name: "test-3",
+		},
+		Spec: v1beta1.LoggingSpec{
+			LoggingRef:       "test",
+			ControlNamespace: controlNamespace,
+		},
+	}
+
+	defer ensureCreated(t, logging1)()
+	defer ensureCreated(t, logging2)()
+	defer ensureCreated(t, logging3)()
+
+	// The first logging resource won't be populated with a warning initially since at the time of creation it is unique
+	g.Eventually(getLoggingProblems(logging1)).WithPolling(time.Second).WithTimeout(5 * time.Second).Should(gomega.BeEmpty())
+	g.Eventually(getLoggingProblems(logging2)).WithPolling(time.Second).WithTimeout(5 * time.Second).Should(gomega.ContainElement(gomega.ContainSubstring("Deprecated")))
+	g.Eventually(getLoggingProblems(logging3)).WithPolling(time.Second).WithTimeout(5 * time.Second).Should(gomega.BeEmpty())
+
+	g.Eventually(func() error {
+		l := &v1beta1.Logging{}
+		if err := mgr.GetClient().Get(context.TODO(), client.ObjectKeyFromObject(logging1), l); err != nil {
+			return err
+		}
+		l.Spec.ErrorOutputRef = "trigger reconcile"
+		return mgr.GetClient().Update(context.TODO(), l)
+	}).WithPolling(time.Second).WithTimeout(5 * time.Second).Should(gomega.Succeed())
+	g.Eventually(getLoggingProblems(logging1)).WithPolling(time.Second).WithTimeout(5 * time.Second).Should(gomega.ContainElement(gomega.ContainSubstring("Deprecated")))
+}
+
+func getLoggingProblems(logging *v1beta1.Logging) func() ([]string, error) {
+	return func() ([]string, error) {
+		l := &v1beta1.Logging{}
+		err := mgr.GetClient().Get(context.TODO(), client.ObjectKeyFromObject(logging), l)
+		return l.Status.Problems, err
+	}
+}
+
 func TestSingleClusterFlowWithClusterOutputFromExternalNamespace(t *testing.T) {
 	g := gomega.NewGomegaWithT(t)
 	defer beforeEach(t)()

pkg/resources/model/reconciler.go

@@ -23,6 +23,7 @@ import (
 	"emperror.dev/errors"
 	"github.com/cisco-open/operator-tools/pkg/secret"
 	"github.com/cisco-open/operator-tools/pkg/utils"
+	"github.com/go-logr/logr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
@@ -35,6 +36,7 @@ func NewValidationReconciler(
 	repo client.StatusClient,
 	resources LoggingResources,
 	secrets SecretLoaderFactory,
+	logger logr.Logger,
 ) func(ctx context.Context) (*reconcile.Result, error) {
 	return func(ctx context.Context) (*reconcile.Result, error) {
 		var patchRequests []patchRequest
@@ -203,6 +205,23 @@ func NewValidationReconciler(
 
 		resources.Logging.Status.Problems = nil
 
+		loggingsForTheSameRef := make([]string, 0)
+		for _, l := range resources.AllLoggings {
+			if l.Name == resources.Logging.Name {
+				continue
+			}
+			if l.Spec.LoggingRef == resources.Logging.Spec.LoggingRef {
+				loggingsForTheSameRef = append(loggingsForTheSameRef, l.Name)
+			}
+		}
+
+		if len(loggingsForTheSameRef) > 0 {
+			problem := fmt.Sprintf("Deprecated behaviour! Other logging resources exist with the same loggingRef: %s. This is going to be an error with the next major release.",
+				strings.Join(loggingsForTheSameRef, ","))
+			logger.Info(fmt.Sprintf("WARNING %s", problem))
+			resources.Logging.Status.Problems = append(resources.Logging.Status.Problems, problem)
+		}
+
 		if len(resources.Logging.Spec.NodeAgents) > 0 || len(resources.NodeAgents) > 0 {
 			// load agents from standalone NodeAgent resources and additionally with inline nodeAgents from the logging resource
 			// for compatibility reasons

pkg/resources/model/repository.go

@@ -45,6 +45,12 @@ func (r LoggingResourceRepository) LoggingResourcesFor(ctx context.Context, logg
 
 	var err error
 
+	allLoggings := &v1beta1.LoggingList{}
+	if err := r.Client.List(ctx, allLoggings); err != nil {
+		errs = errors.Append(errs, err)
+	}
+	res.AllLoggings = allLoggings.Items
+
 	res.Fluentd.ClusterFlows, err = r.ClusterFlowsFor(ctx, logging)
 	errs = errors.Append(errs, err)
 

pkg/resources/model/resources.go

@@ -19,11 +19,12 @@ import (
 )
 
 type LoggingResources struct {
-	Logging    v1beta1.Logging
-	Fluentd    FluentdLoggingResources
-	SyslogNG   SyslogNGLoggingResources
-	NodeAgents []v1beta1.NodeAgent
-	Fluentbits []v1beta1.FluentbitAgent
+	AllLoggings []v1beta1.Logging
+	Logging     v1beta1.Logging
+	Fluentd     FluentdLoggingResources
+	SyslogNG    SyslogNGLoggingResources
+	NodeAgents  []v1beta1.NodeAgent
+	Fluentbits  []v1beta1.FluentbitAgent
 }
 
 type FluentdLoggingResources struct {