Change KRO labels

- Based on proposal: kubernetes-sigs#639
- Move to new labels
- Add deprecate TODO for old labels
- Add new testcase for nested RGD and update existing testcase labels
- Add docs
- Have separate instance labellers for different reconcile paths.
  This is needed when we have RGD2 instance in RGD1 resources.
  Today both paths use the same label resulting in conflict.
  path 1: RGD2.instance created as part of an RGD1 instance reconciliation
  path 2: RGD2.instance reconciled by RGD2 reconciler

Author: barney-s

Makefile

@@ -275,6 +275,7 @@ deploy-kind: ko
 	# This generates deployment with ko://... used in image.
 	# ko then intercepts it builds image, pushes to kind node, replaces the image in deployment and applies it
 	helm template kro ./helm --namespace kro-system --set image.pullPolicy=Never --set image.ko=true --set config.allowCRDDeletion=true | $(KO) apply -f -
+	sleep 5
 	kubectl wait --for=condition=ready --timeout=1m pod -n kro-system -l app.kubernetes.io/component=controller
 	$(KUBECTL) --context kind-${KIND_CLUSTER_NAME} get pods -A
 

pkg/controller/instance/controller.go

@@ -81,9 +81,12 @@ type Controller struct {
 	// managing instances for.
 	// TODO: use a read-only interface for the ResourceGraphDefinition
 	rgd *graph.Graph
-	// instanceLabeler is responsible for applying consistent labels
+	// definedByRGDLabeler is responsible for applying consistent labels
 	// to resources managed by this controller.
-	instanceLabeler metadata.Labeler
+	definedByRGDLabeler metadata.Labeler
+	// reconciledByRGDLabeler is responsible for applying source labels
+	// to instance managed by this controller.
+	reconciledByRGDLabeler metadata.Labeler
 	// reconcileConfig holds the configuration parameters for the reconciliation
 	// process.
 	reconcileConfig ReconcileConfig
@@ -99,14 +102,16 @@ func NewController(
 	rgd *graph.Graph,
 	clientSet kroclient.SetInterface,
 	defaultServiceAccounts map[string]string,
-	instanceLabeler metadata.Labeler,
+	definedByRGDLabeler metadata.Labeler,
+	reconciledByRGDLabeler metadata.Labeler,
 ) *Controller {
 	return &Controller{
 		log:                    log,
 		gvr:                    gvr,
 		clientSet:              clientSet,
 		rgd:                    rgd,
-		instanceLabeler:        instanceLabeler,
+		definedByRGDLabeler:    definedByRGDLabeler,
+		reconciledByRGDLabeler: reconciledByRGDLabeler,
 		reconcileConfig:        reconcileConfig,
 		defaultServiceAccounts: defaultServiceAccounts,
 	}
@@ -138,7 +143,8 @@ func (c *Controller) Reconcile(ctx context.Context, req ctrl.Request) error {
 		return fmt.Errorf("failed to create runtime resource graph definition: %w", err)
 	}
 
-	instanceSubResourcesLabeler, err := metadata.NewInstanceLabeler(instance).Merge(c.instanceLabeler)
+	managedByInstanceLabeler := metadata.NewManagedByInstanceLabeler(instance, instance.GroupVersionKind())
+	instanceSubResourcesLabeler, err := managedByInstanceLabeler.Merge(c.definedByRGDLabeler)
 	if err != nil {
 		return fmt.Errorf("failed to create instance sub-resources labeler: %w", err)
 	}
@@ -155,7 +161,7 @@ func (c *Controller) Reconcile(ctx context.Context, req ctrl.Request) error {
 		gvr:                         c.gvr,
 		client:                      executionClient,
 		runtime:                     rgRuntime,
-		instanceLabeler:             c.instanceLabeler,
+		instanceLabeler:             c.reconciledByRGDLabeler,
 		instanceSubResourcesLabeler: instanceSubResourcesLabeler,
 		reconcileConfig:             c.reconcileConfig,
 		// Fresh instance state at each reconciliation loop.

pkg/controller/resourcegraphdefinition/controller.go

@@ -137,7 +137,7 @@ func (r *ResourceGraphDefinitionReconciler) findRGDsForCRD(ctx context.Context,
 		return nil
 	}
 
-	rgdName, ok := crd.Labels[metadata.ResourceGraphDefinitionNameLabel]
+	rgdName, ok := crd.Labels[metadata.DefinedByRGDLabel]
 	if !ok {
 		return nil
 	}

pkg/controller/resourcegraphdefinition/controller_reconcile.go

@@ -48,14 +48,15 @@ func (r *ResourceGraphDefinitionReconciler) reconcileResourceGraphDefinition(ctx
 	mark.ResourceGraphValid()
 
 	// Setup metadata labeling
-	graphExecLabeler, err := r.setupLabeler(rgd)
+	definedByRGDLabeler, reconciledByRGDLabeler, err := r.setupLabeler(rgd)
 	if err != nil {
 		mark.FailedLabelerSetup(err.Error())
 		return nil, nil, fmt.Errorf("failed to setup labeler: %w", err)
 	}
 
 	crd := processedRGD.Instance.GetCRD()
-	graphExecLabeler.ApplyLabels(&crd.ObjectMeta)
+	// TODO(barney-s): should we apply the source labeler to the crd here instead ?
+	definedByRGDLabeler.ApplyLabels(&crd.ObjectMeta)
 
 	// Ensure CRD exists and is up to date
 	log.V(1).Info("reconciling resource graph definition CRD")
@@ -71,7 +72,7 @@ func (r *ResourceGraphDefinitionReconciler) reconcileResourceGraphDefinition(ctx
 
 	// Setup and start microcontroller
 	gvr := processedRGD.Instance.GetGroupVersionResource()
-	controller := r.setupMicroController(gvr, processedRGD, rgd.Spec.DefaultServiceAccounts, graphExecLabeler)
+	controller := r.setupMicroController(gvr, processedRGD, rgd.Spec.DefaultServiceAccounts, definedByRGDLabeler, reconciledByRGDLabeler)
 
 	log.V(1).Info("reconciling resource graph definition micro controller")
 	// TODO: the context that is passed here is tied to the reconciliation of the rgd, we might need to make
@@ -87,17 +88,32 @@ func (r *ResourceGraphDefinitionReconciler) reconcileResourceGraphDefinition(ctx
 }
 
 // setupLabeler creates and merges the required labelers for the resource graph definition
-func (r *ResourceGraphDefinitionReconciler) setupLabeler(rgd *v1alpha1.ResourceGraphDefinition) (metadata.Labeler, error) {
-	rgLabeler := metadata.NewResourceGraphDefinitionLabeler(rgd)
-	return r.metadataLabeler.Merge(rgLabeler)
+func (r *ResourceGraphDefinitionReconciler) setupLabeler(rgd *v1alpha1.ResourceGraphDefinition) (metadata.Labeler, metadata.Labeler, error) {
+	var err error
+	// Setup metadata labeling
+	rgLabeler := metadata.NewDefinedByRGDLabeler(rgd)
+	rgSourceLabeler := metadata.NewReconciledByRGDLabeler(rgd)
+
+	mergedLabeler, err := r.metadataLabeler.Merge(rgLabeler)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	mergedSourceLabeler, err := r.metadataLabeler.Merge(rgSourceLabeler)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	return mergedLabeler, mergedSourceLabeler, nil
 }
 
 // setupMicroController creates a new controller instance with the required configuration
 func (r *ResourceGraphDefinitionReconciler) setupMicroController(
 	gvr schema.GroupVersionResource,
 	processedRGD *graph.Graph,
 	defaultSVCs map[string]string,
-	labeler metadata.Labeler,
+	definedByRGDLabeler metadata.Labeler,
+	reconciledByRGDLabeler metadata.Labeler,
 ) *instancectrl.Controller {
 	instanceLogger := r.instanceLogger.WithName(fmt.Sprintf("%s-controller", gvr.Resource)).WithValues(
 		"controller", gvr.Resource,
@@ -116,7 +132,8 @@ func (r *ResourceGraphDefinitionReconciler) setupMicroController(
 		processedRGD,
 		r.clientSet,
 		defaultSVCs,
-		labeler,
+		definedByRGDLabeler,
+		reconciledByRGDLabeler,
 	)
 }
 

pkg/metadata/labels.go

@@ -20,6 +20,7 @@ import (
 	"strings"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"sigs.k8s.io/release-utils/version"
 
@@ -37,6 +38,24 @@ const (
 	OwnedLabel      = LabelKROPrefix + "owned"
 	KROVersionLabel = LabelKROPrefix + "kro-version"
 
+	// ReconciledByRGDLabel is added to an instance to indicate which RGD is reconciling it.
+	ReconciledByRGDLabel = LabelKROPrefix + "reconciled-by-rgd"
+
+	// DefinedByRGDLabel is added to resources created by an instance reconciliation to indicate which RGD they come from.
+	DefinedByRGDLabel = LabelKROPrefix + "defined-by-rgd"
+
+	// ManagedByInstanceGroupLabel is added to resources to link back to the instance's group that created them.
+	ManagedByInstanceGroupLabel = LabelKROPrefix + "managed-by-instance-group"
+	// ManagedByInstanceVersionLabel is added to resources to link back to the instance's group that created them.
+	ManagedByInstanceVersionLabel = LabelKROPrefix + "managed-by-instance-version"
+	// ManagedByInstanceKindLabel is added to resources to link back to the instance's kind that created them.
+	ManagedByInstanceKindLabel = LabelKROPrefix + "managed-by-instance-kind"
+	// ManagedByInstanceNamespaceLabel is added to resources to link back to the instance's namespace that created them.
+	ManagedByInstanceNamespaceLabel = LabelKROPrefix + "managed-by-instance-namespace"
+	// ManagedByInstanceNameLabel is added to resources to link back to the instance's name that created them.
+	ManagedByInstanceNameLabel = LabelKROPrefix + "managed-by-instance-name"
+
+	// TODO (barney-s) BEGIN: deprecate by 0.7 --------V
 	InstanceIDLabel        = LabelKROPrefix + "instance-id"
 	InstanceLabel          = LabelKROPrefix + "instance-name"
 	InstanceNamespaceLabel = LabelKROPrefix + "instance-namespace"
@@ -45,6 +64,7 @@ const (
 	ResourceGraphDefinitionNameLabel      = LabelKROPrefix + "resource-graph-definition-name"
 	ResourceGraphDefinitionNamespaceLabel = LabelKROPrefix + "resource-graph-definition-namespace"
 	ResourceGraphDefinitionVersionLabel   = LabelKROPrefix + "resource-graph-definition-version"
+	// TODO (barney-s) END: deprecate by 0.7  ----------^
 )
 
 // IsKROOwned returns true if the resource is owned by KRO.
@@ -115,23 +135,37 @@ func (gl GenericLabeler) Copy() map[string]string {
 	return newGenericLabeler
 }
 
-// NewResourceGraphDefinitionLabeler returns a new labeler that sets the
-// ResourceGraphDefinitionLabel and ResourceGraphDefinitionIDLabel labels on a resource.
-func NewResourceGraphDefinitionLabeler(rgMeta metav1.Object) GenericLabeler {
+// NewDefinedByRGDLabeler returns a new labeler that sets the DefinedByRGDLabel on a resource.
+func NewDefinedByRGDLabeler(rgd metav1.Object) GenericLabeler {
+	return map[string]string{
+		DefinedByRGDLabel: rgd.GetName(),
+		// TODO (barney-s) BEGIN: deprecate by 0.7 --------V
+		ResourceGraphDefinitionIDLabel:   string(rgd.GetUID()),
+		ResourceGraphDefinitionNameLabel: rgd.GetName(),
+		// TODO (barney-s) END: deprecate by 0.7  ----------^
+	}
+}
+
+// NewReconciledByRGDLabeler returns a new labeler that sets the ReconciledByRGDLabel on a resource.
+func NewReconciledByRGDLabeler(rgd metav1.Object) GenericLabeler {
 	return map[string]string{
-		ResourceGraphDefinitionIDLabel:   string(rgMeta.GetUID()),
-		ResourceGraphDefinitionNameLabel: rgMeta.GetName(),
+		ReconciledByRGDLabel: rgd.GetName(),
 	}
 }
 
-// NewInstanceLabeler returns a new labeler that sets the InstanceLabel and
-// InstanceIDLabel labels on a resource. The InstanceLabel is the namespace
-// and name of the instance that was reconciled to create the resource.
-func NewInstanceLabeler(instanceMeta metav1.Object) GenericLabeler {
+// NewManagedByInstanceLabeler returns a new labeler that sets the labels to link to the creating instance on a resource.
+func NewManagedByInstanceLabeler(instance metav1.Object, gvk schema.GroupVersionKind) GenericLabeler {
 	return map[string]string{
-		InstanceIDLabel:        string(instanceMeta.GetUID()),
-		InstanceLabel:          instanceMeta.GetName(),
-		InstanceNamespaceLabel: instanceMeta.GetNamespace(),
+		ManagedByInstanceGroupLabel:     gvk.Group,
+		ManagedByInstanceKindLabel:      gvk.Kind,
+		ManagedByInstanceVersionLabel:   gvk.Version,
+		ManagedByInstanceNamespaceLabel: instance.GetNamespace(),
+		ManagedByInstanceNameLabel:      instance.GetName(),
+		// TODO (barney-s) BEGIN: deprecate by 0.7 --------V
+		InstanceIDLabel:        string(instance.GetUID()),
+		InstanceLabel:          instance.GetName(),
+		InstanceNamespaceLabel: instance.GetNamespace(),
+		// TODO (barney-s) END: deprecate by 0.7  ----------^
 	}
 }
 

pkg/metadata/labels_test.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/release-utils/version"
 )
@@ -193,10 +194,13 @@ func TestNewResourceGraphDefinitionLabeler(t *testing.T) {
 		name := "rgd-name"
 		uid := types.UID("rgd-uid")
 		obj := &mockObject{ObjectMeta: metav1.ObjectMeta{Name: name, UID: uid}}
-		labeler := NewResourceGraphDefinitionLabeler(obj)
+		labeler := NewDefinedByRGDLabeler(obj)
 		assert.Equal(t, GenericLabeler{
-			ResourceGraphDefinitionNameLabel: name,
+			DefinedByRGDLabel: name,
+			// TODO (barney-s) BEGIN: deprecate by 0.7 --------V
 			ResourceGraphDefinitionIDLabel:   string(uid),
+			ResourceGraphDefinitionNameLabel: name,
+			// TODO (barney-s) END: deprecate by 0.7 --------V
 		}, labeler)
 	})
 }
@@ -205,13 +209,21 @@ func TestNewInstanceLabeler(t *testing.T) {
 	t.Run("NewInstanceLabeler", func(t *testing.T) {
 		name := "instance-name"
 		namespace := "instance-namespace"
-		uid := types.UID("instance-uid")
-		obj := &mockObject{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace, UID: uid}}
-		labeler := NewInstanceLabeler(obj)
+		uid := "someuid"
+		obj := &mockObject{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace, UID: types.UID(uid)}}
+		gvk := schema.GroupVersionKind{Group: "kro.run", Version: "v1alpha1", Kind: "SomeKind"}
+		labeler := NewManagedByInstanceLabeler(obj, gvk)
 		assert.Equal(t, GenericLabeler{
+			ManagedByInstanceGroupLabel:     gvk.Group,
+			ManagedByInstanceVersionLabel:   gvk.Version,
+			ManagedByInstanceKindLabel:      gvk.Kind,
+			ManagedByInstanceNamespaceLabel: namespace,
+			ManagedByInstanceNameLabel:      name,
+			// TODO (barney-s) BEGIN: deprecate by 0.7 --------V
+			InstanceIDLabel:        uid,
 			InstanceLabel:          name,
 			InstanceNamespaceLabel: namespace,
-			InstanceIDLabel:        string(uid),
+			// TODO (barney-s) END: deprecate by 0.7 --------V
 		}, labeler)
 	})
 }