Skip to content

Commit b22ef25

Browse files
barney-sbarney-s
committed
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 - 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
1 parent ee1f8f9 commit b22ef25

28 files changed

+300
-182
lines changed

‎Makefile‎

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

‎pkg/controller/instance/controller.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,12 @@ type Controller struct {
8181
// managing instances for.
8282
// TODO: use a read-only interface for the ResourceGraphDefinition
8383
rgd *graph.Graph
84-
// instanceLabeler is responsible for applying consistent labels
84+
// definedByRGDLabeler is responsible for applying consistent labels
8585
// to resources managed by this controller.
86-
instanceLabeler metadata.Labeler
86+
definedByRGDLabeler metadata.Labeler
87+
// reconciledByRGDLabeler is responsible for applying source labels
88+
// to instance managed by this controller.
89+
reconciledByRGDLabeler metadata.Labeler
8790
// reconcileConfig holds the configuration parameters for the reconciliation
8891
// process.
8992
reconcileConfig ReconcileConfig
@@ -99,14 +102,16 @@ func NewController(
99102
rgd *graph.Graph,
100103
clientSet kroclient.SetInterface,
101104
defaultServiceAccounts map[string]string,
102-
instanceLabeler metadata.Labeler,
105+
definedByRGDLabeler metadata.Labeler,
106+
reconciledByRGDLabeler metadata.Labeler,
103107
) *Controller {
104108
return &Controller{
105109
log: log,
106110
gvr: gvr,
107111
clientSet: clientSet,
108112
rgd: rgd,
109-
instanceLabeler: instanceLabeler,
113+
definedByRGDLabeler: definedByRGDLabeler,
114+
reconciledByRGDLabeler: reconciledByRGDLabeler,
110115
reconcileConfig: reconcileConfig,
111116
defaultServiceAccounts: defaultServiceAccounts,
112117
}
@@ -138,7 +143,8 @@ func (c *Controller) Reconcile(ctx context.Context, req ctrl.Request) error {
138143
return fmt.Errorf("failed to create runtime resource graph definition: %w", err)
139144
}
140145

141-
instanceSubResourcesLabeler, err := metadata.NewInstanceLabeler(instance).Merge(c.instanceLabeler)
146+
managedByInstanceLabeler := metadata.NewManagedByInstanceLabeler(instance, instance.GroupVersionKind())
147+
instanceSubResourcesLabeler, err := managedByInstanceLabeler.Merge(c.definedByRGDLabeler)
142148
if err != nil {
143149
return fmt.Errorf("failed to create instance sub-resources labeler: %w", err)
144150
}
@@ -155,7 +161,7 @@ func (c *Controller) Reconcile(ctx context.Context, req ctrl.Request) error {
155161
gvr: c.gvr,
156162
client: executionClient,
157163
runtime: rgRuntime,
158-
instanceLabeler: c.instanceLabeler,
164+
instanceLabeler: c.reconciledByRGDLabeler,
159165
instanceSubResourcesLabeler: instanceSubResourcesLabeler,
160166
reconcileConfig: c.reconcileConfig,
161167
// Fresh instance state at each reconciliation loop.

‎pkg/controller/resourcegraphdefinition/controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ func (r *ResourceGraphDefinitionReconciler) findRGDsForCRD(ctx context.Context,
137137
return nil
138138
}
139139

140-
rgdName, ok := crd.Labels[metadata.ResourceGraphDefinitionNameLabel]
140+
rgdName, ok := crd.Labels[metadata.DefinedByRGDLabel]
141141
if !ok {
142142
return nil
143143
}

‎pkg/controller/resourcegraphdefinition/controller_reconcile.go‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,15 @@ func (r *ResourceGraphDefinitionReconciler) reconcileResourceGraphDefinition(ctx
4848
mark.ResourceGraphValid()
4949

5050
// Setup metadata labeling
51-
graphExecLabeler, err := r.setupLabeler(rgd)
51+
definedByRGDLabeler, reconciledByRGDLabeler, err := r.setupLabeler(rgd)
5252
if err != nil {
5353
mark.FailedLabelerSetup(err.Error())
5454
return nil, nil, fmt.Errorf("failed to setup labeler: %w", err)
5555
}
5656

5757
crd := processedRGD.Instance.GetCRD()
58-
graphExecLabeler.ApplyLabels(&crd.ObjectMeta)
58+
// TODO(barney-s): should we apply the source labeler to the crd here instead ?
59+
definedByRGDLabeler.ApplyLabels(&crd.ObjectMeta)
5960

6061
// Ensure CRD exists and is up to date
6162
log.V(1).Info("reconciling resource graph definition CRD")
@@ -71,7 +72,7 @@ func (r *ResourceGraphDefinitionReconciler) reconcileResourceGraphDefinition(ctx
7172

7273
// Setup and start microcontroller
7374
gvr := processedRGD.Instance.GetGroupVersionResource()
74-
controller := r.setupMicroController(gvr, processedRGD, rgd.Spec.DefaultServiceAccounts, graphExecLabeler)
75+
controller := r.setupMicroController(gvr, processedRGD, rgd.Spec.DefaultServiceAccounts, definedByRGDLabeler, reconciledByRGDLabeler)
7576

7677
log.V(1).Info("reconciling resource graph definition micro controller")
7778
// 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
8788
}
8889

8990
// setupLabeler creates and merges the required labelers for the resource graph definition
90-
func (r *ResourceGraphDefinitionReconciler) setupLabeler(rgd *v1alpha1.ResourceGraphDefinition) (metadata.Labeler, error) {
91-
rgLabeler := metadata.NewResourceGraphDefinitionLabeler(rgd)
92-
return r.metadataLabeler.Merge(rgLabeler)
91+
func (r *ResourceGraphDefinitionReconciler) setupLabeler(rgd *v1alpha1.ResourceGraphDefinition) (metadata.Labeler, metadata.Labeler, error) {
92+
var err error
93+
// Setup metadata labeling
94+
rgLabeler := metadata.NewDefinedByRGDLabeler(rgd)
95+
rgSourceLabeler := metadata.NewReconciledByRGDLabeler(rgd)
96+
97+
mergedLabeler, err := r.metadataLabeler.Merge(rgLabeler)
98+
if err != nil {
99+
return nil, nil, err
100+
}
101+
102+
mergedSourceLabeler, err := r.metadataLabeler.Merge(rgSourceLabeler)
103+
if err != nil {
104+
return nil, nil, err
105+
}
106+
107+
return mergedLabeler, mergedSourceLabeler, nil
93108
}
94109

95110
// setupMicroController creates a new controller instance with the required configuration
96111
func (r *ResourceGraphDefinitionReconciler) setupMicroController(
97112
gvr schema.GroupVersionResource,
98113
processedRGD *graph.Graph,
99114
defaultSVCs map[string]string,
100-
labeler metadata.Labeler,
115+
definedByRGDLabeler metadata.Labeler,
116+
reconciledByRGDLabeler metadata.Labeler,
101117
) *instancectrl.Controller {
102118
instanceLogger := r.instanceLogger.WithName(fmt.Sprintf("%s-controller", gvr.Resource)).WithValues(
103119
"controller", gvr.Resource,
@@ -116,7 +132,8 @@ func (r *ResourceGraphDefinitionReconciler) setupMicroController(
116132
processedRGD,
117133
r.clientSet,
118134
defaultSVCs,
119-
labeler,
135+
definedByRGDLabeler,
136+
reconciledByRGDLabeler,
120137
)
121138
}
122139

‎pkg/metadata/labels.go‎

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"strings"
2121

2222
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
23+
"k8s.io/apimachinery/pkg/runtime/schema"
2324
"k8s.io/apimachinery/pkg/util/validation"
2425
"sigs.k8s.io/release-utils/version"
2526

@@ -37,6 +38,24 @@ const (
3738
OwnedLabel = LabelKROPrefix + "owned"
3839
KROVersionLabel = LabelKROPrefix + "kro-version"
3940

41+
// ReconciledByRGDLabel is added to an instance to indicate which RGD is reconciling it.
42+
ReconciledByRGDLabel = LabelKROPrefix + "reconciled-by-rgd"
43+
44+
// DefinedByRGDLabel is added to resources created by an instance reconciliation to indicate which RGD they come from.
45+
DefinedByRGDLabel = LabelKROPrefix + "defined-by-rgd"
46+
47+
// ManagedByInstanceGroupLabel is added to resources to link back to the instance's group that created them.
48+
ManagedByInstanceGroupLabel = LabelKROPrefix + "managed-by-instance-group"
49+
// ManagedByInstanceVersionLabel is added to resources to link back to the instance's group that created them.
50+
ManagedByInstanceVersionLabel = LabelKROPrefix + "managed-by-instance-version"
51+
// ManagedByInstanceKindLabel is added to resources to link back to the instance's kind that created them.
52+
ManagedByInstanceKindLabel = LabelKROPrefix + "managed-by-instance-kind"
53+
// ManagedByInstanceNamespaceLabel is added to resources to link back to the instance's namespace that created them.
54+
ManagedByInstanceNamespaceLabel = LabelKROPrefix + "managed-by-instance-namespace"
55+
// ManagedByInstanceNameLabel is added to resources to link back to the instance's name that created them.
56+
ManagedByInstanceNameLabel = LabelKROPrefix + "managed-by-instance-name"
57+
58+
// TODO (barney-s) BEGIN: deprecate by 0.7 --------V
4059
InstanceIDLabel = LabelKROPrefix + "instance-id"
4160
InstanceLabel = LabelKROPrefix + "instance-name"
4261
InstanceNamespaceLabel = LabelKROPrefix + "instance-namespace"
@@ -45,6 +64,7 @@ const (
4564
ResourceGraphDefinitionNameLabel = LabelKROPrefix + "resource-graph-definition-name"
4665
ResourceGraphDefinitionNamespaceLabel = LabelKROPrefix + "resource-graph-definition-namespace"
4766
ResourceGraphDefinitionVersionLabel = LabelKROPrefix + "resource-graph-definition-version"
67+
// TODO (barney-s) END: deprecate by 0.7 ----------^
4868
)
4969

5070
// IsKROOwned returns true if the resource is owned by KRO.
@@ -115,23 +135,37 @@ func (gl GenericLabeler) Copy() map[string]string {
115135
return newGenericLabeler
116136
}
117137

118-
// NewResourceGraphDefinitionLabeler returns a new labeler that sets the
119-
// ResourceGraphDefinitionLabel and ResourceGraphDefinitionIDLabel labels on a resource.
120-
func NewResourceGraphDefinitionLabeler(rgMeta metav1.Object) GenericLabeler {
138+
// NewDefinedByRGDLabeler returns a new labeler that sets the DefinedByRGDLabel on a resource.
139+
func NewDefinedByRGDLabeler(rgd metav1.Object) GenericLabeler {
140+
return map[string]string{
141+
DefinedByRGDLabel: rgd.GetName(),
142+
// TODO (barney-s) BEGIN: deprecate by 0.7 --------V
143+
ResourceGraphDefinitionIDLabel: string(rgd.GetUID()),
144+
ResourceGraphDefinitionNameLabel: rgd.GetName(),
145+
// TODO (barney-s) END: deprecate by 0.7 ----------^
146+
}
147+
}
148+
149+
// NewReconciledByRGDLabeler returns a new labeler that sets the ReconciledByRGDLabel on a resource.
150+
func NewReconciledByRGDLabeler(rgd metav1.Object) GenericLabeler {
121151
return map[string]string{
122-
ResourceGraphDefinitionIDLabel: string(rgMeta.GetUID()),
123-
ResourceGraphDefinitionNameLabel: rgMeta.GetName(),
152+
ReconciledByRGDLabel: rgd.GetName(),
124153
}
125154
}
126155

127-
// NewInstanceLabeler returns a new labeler that sets the InstanceLabel and
128-
// InstanceIDLabel labels on a resource. The InstanceLabel is the namespace
129-
// and name of the instance that was reconciled to create the resource.
130-
func NewInstanceLabeler(instanceMeta metav1.Object) GenericLabeler {
156+
// NewManagedByInstanceLabeler returns a new labeler that sets the labels to link to the creating instance on a resource.
157+
func NewManagedByInstanceLabeler(instance metav1.Object, gvk schema.GroupVersionKind) GenericLabeler {
131158
return map[string]string{
132-
InstanceIDLabel: string(instanceMeta.GetUID()),
133-
InstanceLabel: instanceMeta.GetName(),
134-
InstanceNamespaceLabel: instanceMeta.GetNamespace(),
159+
ManagedByInstanceGroupLabel: gvk.Group,
160+
ManagedByInstanceKindLabel: gvk.Kind,
161+
ManagedByInstanceVersionLabel: gvk.Version,
162+
ManagedByInstanceNamespaceLabel: instance.GetNamespace(),
163+
ManagedByInstanceNameLabel: instance.GetName(),
164+
// TODO (barney-s) BEGIN: deprecate by 0.7 --------V
165+
InstanceIDLabel: string(instance.GetUID()),
166+
InstanceLabel: instance.GetName(),
167+
InstanceNamespaceLabel: instance.GetNamespace(),
168+
// TODO (barney-s) END: deprecate by 0.7 ----------^
135169
}
136170
}
137171

‎pkg/metadata/labels_test.go‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919

2020
"github.com/stretchr/testify/assert"
2121
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
22+
"k8s.io/apimachinery/pkg/runtime/schema"
2223
"k8s.io/apimachinery/pkg/types"
2324
"sigs.k8s.io/release-utils/version"
2425
)
@@ -193,10 +194,13 @@ func TestNewResourceGraphDefinitionLabeler(t *testing.T) {
193194
name := "rgd-name"
194195
uid := types.UID("rgd-uid")
195196
obj := &mockObject{ObjectMeta: metav1.ObjectMeta{Name: name, UID: uid}}
196-
labeler := NewResourceGraphDefinitionLabeler(obj)
197+
labeler := NewDefinedByRGDLabeler(obj)
197198
assert.Equal(t, GenericLabeler{
198-
ResourceGraphDefinitionNameLabel: name,
199+
DefinedByRGDLabel: name,
200+
// TODO (barney-s) BEGIN: deprecate by 0.7 --------V
199201
ResourceGraphDefinitionIDLabel: string(uid),
202+
ResourceGraphDefinitionNameLabel: name,
203+
// TODO (barney-s) END: deprecate by 0.7 --------V
200204
}, labeler)
201205
})
202206
}
@@ -205,13 +209,21 @@ func TestNewInstanceLabeler(t *testing.T) {
205209
t.Run("NewInstanceLabeler", func(t *testing.T) {
206210
name := "instance-name"
207211
namespace := "instance-namespace"
208-
uid := types.UID("instance-uid")
209-
obj := &mockObject{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace, UID: uid}}
210-
labeler := NewInstanceLabeler(obj)
212+
uid := "someuid"
213+
obj := &mockObject{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace, UID: types.UID(uid)}}
214+
gvk := schema.GroupVersionKind{Group: "kro.run", Version: "v1alpha1", Kind: "SomeKind"}
215+
labeler := NewManagedByInstanceLabeler(obj, gvk)
211216
assert.Equal(t, GenericLabeler{
217+
ManagedByInstanceGroupLabel: gvk.Group,
218+
ManagedByInstanceVersionLabel: gvk.Version,
219+
ManagedByInstanceKindLabel: gvk.Kind,
220+
ManagedByInstanceNamespaceLabel: namespace,
221+
ManagedByInstanceNameLabel: name,
222+
// TODO (barney-s) BEGIN: deprecate by 0.7 --------V
223+
InstanceIDLabel: string(uid),
212224
InstanceLabel: name,
213225
InstanceNamespaceLabel: namespace,
214-
InstanceIDLabel: string(uid),
226+
// TODO (barney-s) END: deprecate by 0.7 --------V
215227
}, labeler)
216228
})
217229
}

‎pkg/metadata/selectors.go‎

Lines changed: 0 additions & 60 deletions
This file was deleted.

0 commit comments

Comments
 (0)