Chore(Internal): Add new shared reconciliation functions and migrate GrafanaFolder

api/v1beta1/common.go

@@ -39,3 +39,12 @@ type GrafanaCommonSpec struct {
 	// +optional
 	AllowCrossNamespaceImport *bool `json:"allowCrossNamespaceImport,omitempty"`
 }
+
+// Common Functions that all CRs should implement, excluding Grafana
+// +kubebuilder:object:generate=false
+type CommonResource interface {
+	MatchLabels() *metav1.LabelSelector
+	MatchNamespace() string
+	AllowCrossNamespace() bool
+	ResyncPeriodHasElapsed() bool
+}

api/v1beta1/grafanafolder_types.go

@@ -151,13 +151,6 @@ func (in *GrafanaFolder) Unchanged() bool {
 	return in.Hash() == in.Status.Hash
 }
 
-func (in *GrafanaFolder) IsAllowCrossNamespaceImport() bool {
-	if in.Spec.AllowCrossNamespaceImport != nil {
-		return *in.Spec.AllowCrossNamespaceImport
-	}
-	return false
-}
-
 func (in *GrafanaFolder) GetTitle() string {
 	if in.Spec.Title != "" {
 		return in.Spec.Title
@@ -170,3 +163,18 @@ func (in *GrafanaFolder) ResyncPeriodHasElapsed() bool {
 	deadline := in.Status.LastResync.Add(in.Spec.ResyncPeriod.Duration)
 	return time.Now().After(deadline)
 }
+
+func (in *GrafanaFolder) MatchLabels() *metav1.LabelSelector {
+	return in.Spec.InstanceSelector
+}
+
+func (in *GrafanaFolder) MatchNamespace() string {
+	return in.ObjectMeta.Namespace
+}
+
+func (in *GrafanaFolder) AllowCrossNamespace() bool {
+	if in.Spec.AllowCrossNamespaceImport != nil {
+		return *in.Spec.AllowCrossNamespaceImport
+	}
+	return false
+}

controllers/controller_shared.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/go-logr/logr"
 	operatorapi "github.com/grafana/grafana-operator/v5/api"
 	"github.com/grafana/grafana-operator/v5/api/v1beta1"
 	grafanav1beta1 "github.com/grafana/grafana-operator/v5/api/v1beta1"
@@ -31,11 +32,11 @@ const (
 	conditionInvalidSpec        = "InvalidSpec"
 )
 
-const annotationAppliedNotificationPolicy = "operator.grafana.com/applied-notificationpolicy"
-
 //+kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=get;list;watch;create;update;patch;delete
 
+// Gets all instances matching labelSelector
 func GetMatchingInstances(ctx context.Context, k8sClient client.Client, labelSelector *metav1.LabelSelector) (v1beta1.GrafanaList, error) {
+	// Should never happen, sanity check
 	if labelSelector == nil {
 		return v1beta1.GrafanaList{}, nil
 	}
@@ -58,6 +59,91 @@ func GetMatchingInstances(ctx context.Context, k8sClient client.Client, labelSel
 	return selectedList, err
 }
 
+// Only matching instances in the scope of the resource are returned
+// Resources with allowCrossNamespaceImport expands the scope to the entire cluster
+// Intended to be used in reconciler functions
+func GetScopedMatchingInstances(log logr.Logger, ctx context.Context, k8sClient client.Client, cr v1beta1.CommonResource) ([]v1beta1.Grafana, error) {
+	instanceSelector := cr.MatchLabels()
+
+	// Should never happen, sanity check
+	if instanceSelector == nil {
+		return []v1beta1.Grafana{}, nil
+	}
+
+	opts := []client.ListOption{
+		// Matches all instances when MatchLabels is undefined
+		client.MatchingLabels(instanceSelector.MatchLabels),
+	}
+
+	if !cr.AllowCrossNamespace() {
+		// Only query resource namespace
+		opts = append(opts, client.InNamespace(cr.MatchNamespace()))
+	}
+
+	var list v1beta1.GrafanaList
+	err := k8sClient.List(ctx, &list, opts...)
+	if err != nil || len(list.Items) == 0 {
+		return []v1beta1.Grafana{}, err
+	}
+
+	selectedList := []v1beta1.Grafana{}
+	var unready_instances []string
+	for _, instance := range list.Items {
+		// Matches all instances when MatchExpressions is undefined
+		selected := labelsSatisfyMatchExpressions(instance.Labels, instanceSelector.MatchExpressions)
+		if !selected {
+			continue
+		}
+		// admin url is required to interact with Grafana
+		// the instance or route might not yet be ready
+		if instance.Status.Stage != v1beta1.OperatorStageComplete || instance.Status.StageStatus != v1beta1.OperatorStageResultSuccess {
+			unready_instances = append(unready_instances, instance.Name)
+			continue
+		}
+		selectedList = append(selectedList, instance)
+	}
+	if len(unready_instances) > 1 {
+		log.Info("Grafana instances not ready", "instances", unready_instances)
+	}
+
+	return selectedList, nil
+}
+
+// Same as GetScopedMatchingInstances, except the scope is always global
+// Intended to be used in finalizer and onDelete functions due to allowCrossNamespaceImport being a mutable field
+// Not using this may leave behind resources in instances no longer in scope.
+func GetAllMatchingInstances(ctx context.Context, k8sClient client.Client, cr v1beta1.CommonResource) ([]v1beta1.Grafana, error) {
+	instanceSelector := cr.MatchLabels()
+
+	// Should never happen, sanity check
+	if instanceSelector == nil {
+		return []v1beta1.Grafana{}, nil
+	}
+
+	var list v1beta1.GrafanaList
+	err := k8sClient.List(ctx, &list, client.MatchingLabels(instanceSelector.MatchLabels))
+	if err != nil || len(list.Items) == 0 {
+		return []v1beta1.Grafana{}, err
+	}
+
+	selectedList := []v1beta1.Grafana{}
+	for _, instance := range list.Items {
+		// Matches all instances when MatchExpressions is undefined
+		selected := labelsSatisfyMatchExpressions(instance.Labels, instanceSelector.MatchExpressions)
+		if !selected {
+			continue
+		}
+		// admin url is required to interact with Grafana
+		// the instance or route might not yet be ready
+		if instance.Status.Stage != v1beta1.OperatorStageComplete || instance.Status.StageStatus != v1beta1.OperatorStageResultSuccess {
+			continue
+		}
+		selectedList = append(selectedList, instance)
+	}
+
+	return selectedList, nil
+}
+
 // getFolderUID fetches the folderUID from an existing GrafanaFolder CR declared in the specified namespace
 func getFolderUID(ctx context.Context, k8sClient client.Client, ref operatorapi.FolderReferencer) (string, error) {
 	if ref.FolderUID() != "" {
@@ -145,6 +231,28 @@ func ReconcilePlugins(ctx context.Context, k8sClient client.Client, scheme *runt
 	return nil
 }
 
+// Correctly determine cause of no matching instance from error
+func setNoMatchingInstancesCondition(conditions *[]metav1.Condition, generation int64, err error) {
+	var reason, message string
+	if err != nil {
+		reason = "ErrFetchingInstances"
+		message = fmt.Sprintf("error occurred during fetching of instances: %s", err.Error())
+	} else {
+		reason = "EmptyAPIReply"
+		message = "Instances could not be fetched, reconciliation will be retried"
+	}
+	meta.SetStatusCondition(conditions, metav1.Condition{
+		Type:               conditionNoMatchingInstance,
+		Status:             "True",
+		ObservedGeneration: generation,
+		Reason:             reason,
+		Message:            message,
+		LastTransitionTime: metav1.Time{
+			Time: time.Now(),
+		},
+	})
+}
+
 func setNoMatchingInstance(conditions *[]metav1.Condition, generation int64, reason, message string) {
 	meta.SetStatusCondition(conditions, metav1.Condition{
 		Type:               conditionNoMatchingInstance,

controllers/controller_shared_test.go

@@ -17,10 +17,17 @@ limitations under the License.
 package controllers
 
 import (
+	"context"
 	"testing"
 
+	"github.com/grafana/grafana-operator/v5/api/v1beta1"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
 	"github.com/stretchr/testify/assert"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
 func TestLabelsSatisfyMatchExpressions(t *testing.T) {
@@ -226,3 +233,122 @@ func TestLabelsSatisfyMatchExpressions(t *testing.T) {
 		})
 	}
 }
+
+var _ = Describe("GetMatchingInstances functions", Ordered, func() {
+	refTrue := true
+	refFalse := false
+	namespace := corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "get-matching-test",
+		},
+	}
+	allowFolder := v1beta1.GrafanaFolder{
+		TypeMeta: v1.TypeMeta{
+			APIVersion: "grafana.integreatly.org/v1beta1",
+			Kind:       "GrafanaFolder",
+		},
+		ObjectMeta: v1.ObjectMeta{
+			Name:      "allow-cross-namespace",
+			Namespace: namespace.Name,
+		},
+		Spec: v1beta1.GrafanaFolderSpec{
+			GrafanaCommonSpec: v1beta1.GrafanaCommonSpec{
+				AllowCrossNamespaceImport: &refTrue,
+				InstanceSelector: &v1.LabelSelector{
+					MatchLabels: map[string]string{
+						"test": "folder",
+					},
+				},
+			},
+		},
+	}
+	// Create duplicate resources, changing key fields
+	denyFolder := allowFolder.DeepCopy()
+	denyFolder.Name = "deny-cross-namespace"
+	denyFolder.Spec.AllowCrossNamespaceImport = &refFalse
+
+	matchAllFolder := allowFolder.DeepCopy()
+	matchAllFolder.Name = "invalid-match-labels"
+	matchAllFolder.Spec.InstanceSelector = &metav1.LabelSelector{} // InstanceSelector is never nil
+
+	DefaultGrafana := v1beta1.Grafana{
+		TypeMeta: v1.TypeMeta{
+			APIVersion: "grafana.integreatly.org/v1beta1",
+			Kind:       "Grafana",
+		},
+		ObjectMeta: v1.ObjectMeta{
+			Name:      "instance",
+			Namespace: "default",
+			Labels: map[string]string{
+				"test": "folder",
+			},
+		},
+		Spec: v1beta1.GrafanaSpec{},
+	}
+	matchesNothingGrafana := DefaultGrafana.DeepCopy()
+	matchesNothingGrafana.Name = "match-nothing-instance"
+	matchesNothingGrafana.Labels = nil
+
+	secondNamespaceGrafana := DefaultGrafana.DeepCopy()
+	secondNamespaceGrafana.Name = "second-namespace-instance"
+	secondNamespaceGrafana.Namespace = namespace.Name
+
+	// Status update is skipped for this
+	unreadyGrafana := DefaultGrafana.DeepCopy()
+	unreadyGrafana.Name = "unready-instance"
+
+	ctx := context.Background()
+	// Pre-create all resources
+	BeforeAll(func() { // Necessary to use assertions
+		Expect(k8sClient.Create(ctx, &namespace)).NotTo(HaveOccurred())
+		Expect(k8sClient.Create(ctx, &allowFolder)).NotTo(HaveOccurred())
+		Expect(k8sClient.Create(ctx, denyFolder)).NotTo(HaveOccurred())
+		Expect(k8sClient.Create(ctx, matchAllFolder)).NotTo(HaveOccurred())
+		Expect(k8sClient.Create(ctx, unreadyGrafana)).NotTo(HaveOccurred())
+
+		grafanas := []v1beta1.Grafana{DefaultGrafana, *matchesNothingGrafana, *secondNamespaceGrafana}
+		for _, instance := range grafanas {
+			Expect(k8sClient.Create(ctx, &instance)).NotTo(HaveOccurred())
+
+			// Apply status to pass instance ready check
+			instance.Status.Stage = v1beta1.OperatorStageComplete
+			instance.Status.StageStatus = v1beta1.OperatorStageResultSuccess
+			Expect(k8sClient.Status().Update(ctx, &instance)).ToNot(HaveOccurred())
+		}
+	})
+
+	Context("Ensure AllowCrossNamespaceImport is upheld by GetScopedMatchingInstances", func() {
+		testLog := log.FromContext(ctx).WithSink(log.NullLogSink{})
+		It("Finds all ready instances when instanceSelector is empty", func() {
+			instances, err := GetScopedMatchingInstances(testLog, ctx, k8sClient, matchAllFolder)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(instances).To(HaveLen(3))
+		})
+		It("Finds all ready and Matching instances", func() {
+			instances, err := GetScopedMatchingInstances(testLog, ctx, k8sClient, &allowFolder)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(instances).ToNot(BeEmpty())
+			Expect(instances).To(HaveLen(2))
+		})
+		It("Finds matching and ready and matching instance in namespace", func() {
+			instances, err := GetScopedMatchingInstances(testLog, ctx, k8sClient, denyFolder)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(instances).ToNot(BeEmpty())
+			Expect(instances).To(HaveLen(1))
+		})
+	})
+
+	Context("Ensure AllowCrossNamespaceImport is ignored by GetAllMatchingInstances", func() {
+		It("Finds all ready instances when instanceSelector is empty", func() {
+			instances, err := GetAllMatchingInstances(ctx, k8sClient, matchAllFolder)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(instances).To(HaveLen(3))
+		})
+		It("Finds matching and ready instances ignoring AllowCrossNamespaceImport", func() {
+			instances, err := GetAllMatchingInstances(ctx, k8sClient, denyFolder)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(instances).ToNot(BeEmpty())
+			Expect(instances).To(HaveLen(2))
+		})
+	})
+})