Skip to content

✨ ✨ feature feat: add labels to managedcluster resource if present #1123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

✨ ✨ feature feat: add labels to managedcluster resource if present #1123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
38f00fd
feat: add labels to managedcluster resource if present and have the a…
ramekris3163 Aug 12, 2025
e10651e
update ocm api to latest commit
ramekris3163 Aug 19, 2025
797c297
update crds
ramekris3163 Aug 19, 2025
fee6150
fix verification errors.
ramekris3163 Aug 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added deploy/.DS_Store
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems not necessary?

Binary file not shown.
2 changes: 1 addition & 1 deletion ...y/cluster-manager/olm-catalog/latest/manifests/cluster-manager.clusterserviceversion.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ metadata:
categories: Integration & Delivery,OpenShift Optional
certified: "false"
containerImage: quay.io/open-cluster-management/registration-operator:latest
createdAt: "2025-08-05T10:41:01Z"
createdAt: "2025-08-19T20:16:48Z"
description: Manages the installation and upgrade of the ClusterManager.
operators.operatorframework.io/builder: operator-sdk-v1.32.0
operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
Expand Down
6 changes: 6 additions & 0 deletions ...et/chart/klusterlet/crds/0000_00_operator.open-cluster-management.io_klusterlets.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,12 @@ spec:
required:
- maxCustomClusterClaims
type: object
clusterLabels:
additionalProperties:
type: string
description: ClusterLabels is labels set on ManagedCluster when
creating only, other actors can update it afterwards.
type: object
featureGates:
Comment on lines +275 to 281
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix grammar in user-facing description (pluralization and phrasing).

Current text is awkward: "ClusterLabels is labels set on ManagedCluster when creating only, other actors can update it afterwards."

Tighten it to be grammatically correct and clearer.

-                    description: ClusterLabels is labels set on ManagedCluster when
-                      creating only, other actors can update it afterwards.
+                    description: ClusterLabels are labels set on the ManagedCluster at creation time; other actors can update them afterwards.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
clusterLabels:
additionalProperties:
type: string
description: ClusterLabels is labels set on ManagedCluster when
creating only, other actors can update it afterwards.
type: object
featureGates:
clusterLabels:
additionalProperties:
type: string
description: ClusterLabels are labels set on the ManagedCluster at creation time; other actors can update them afterwards.
type: object
featureGates:
🤖 Prompt for AI Agents 
In
deploy/klusterlet/chart/klusterlet/crds/0000_00_operator.open-cluster-management.io_klusterlets.crd.yaml
around lines 275 to 281, the user-facing description for clusterLabels is
grammatically awkward and unclear; replace it with a concise, correct sentence
such as: "ClusterLabels are labels set on the ManagedCluster when it is created;
other actors may update them afterwards." Update the description field to use
pluralization and clearer phrasing while preserving meaning.

description: "FeatureGates represents the list of feature gates
for registration\nIf it is set empty, default feature gates
Expand Down
3 changes: 3 additions & 0 deletions deploy/klusterlet/chart/klusterlet/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ klusterlet:
# - feature: ""
# mode: ""
# clientCertExpirationSeconds: 600
# clusterLabels:
# environment: "production"
# region: "us-west-2"
workConfiguration: {}
# featureGates:
# - feature: ""
Expand Down
6 changes: 6 additions & 0 deletions ...y/klusterlet/config/crds/0000_00_operator.open-cluster-management.io_klusterlets.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,12 @@ spec:
required:
- maxCustomClusterClaims
type: object
clusterLabels:
additionalProperties:
type: string
description: ClusterLabels is labels set on ManagedCluster when
creating only, other actors can update it afterwards.
type: object
featureGates:
Comment on lines +275 to 281
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix grammar in user-facing description (pluralization and phrasing).

Mirror the same correction here to keep copies consistent.

-                    description: ClusterLabels is labels set on ManagedCluster when
-                      creating only, other actors can update it afterwards.
+                    description: ClusterLabels are labels set on the ManagedCluster at creation time; other actors can update them afterwards.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
clusterLabels:
additionalProperties:
type: string
description: ClusterLabels is labels set on ManagedCluster when
creating only, other actors can update it afterwards.
type: object
featureGates:
clusterLabels:
additionalProperties:
type: string
description: ClusterLabels are labels set on the ManagedCluster at creation time; other actors can update them afterwards.
type: object
featureGates:
🤖 Prompt for AI Agents 
In
deploy/klusterlet/config/crds/0000_00_operator.open-cluster-management.io_klusterlets.crd.yaml
around lines 275 to 281, the user-facing description for clusterLabels has
grammar issues; update the description to use correct pluralization and clearer
phrasing (e.g., "ClusterLabels are labels set on ManagedClusters when creating
them; other actors can update them afterwards."), and mirror the same corrected
sentence in any other copies of this description to keep consistency.

description: "FeatureGates represents the list of feature gates
for registration\nIf it is set empty, default feature gates
Expand Down
2 changes: 1 addition & 1 deletion deploy/klusterlet/olm-catalog/latest/manifests/klusterlet.clusterserviceversion.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ metadata:
categories: Integration & Delivery,OpenShift Optional
certified: "false"
containerImage: quay.io/open-cluster-management/registration-operator:latest
createdAt: "2025-08-05T10:41:01Z"
createdAt: "2025-08-19T20:16:48Z"
description: Manages the installation and upgrade of the Klusterlet.
operators.operatorframework.io/builder: operator-sdk-v1.32.0
operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
Expand Down
6 changes: 6 additions & 0 deletions ...sterlet/olm-catalog/latest/manifests/operator.open-cluster-management.io_klusterlets.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,12 @@ spec:
required:
- maxCustomClusterClaims
type: object
clusterLabels:
additionalProperties:
type: string
description: ClusterLabels is labels set on ManagedCluster when
creating only, other actors can update it afterwards.
type: object
featureGates:
Comment on lines +275 to 281
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix grammar in user-facing description (pluralization and phrasing).

Align wording with the other CRD copies.

-                    description: ClusterLabels is labels set on ManagedCluster when
-                      creating only, other actors can update it afterwards.
+                    description: ClusterLabels are labels set on the ManagedCluster at creation time; other actors can update them afterwards.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
clusterLabels:
additionalProperties:
type: string
description: ClusterLabels is labels set on ManagedCluster when
creating only, other actors can update it afterwards.
type: object
featureGates:
clusterLabels:
additionalProperties:
type: string
description: ClusterLabels are labels set on the ManagedCluster at creation time; other actors can update them afterwards.
type: object
featureGates:
🤖 Prompt for AI Agents 
In
deploy/klusterlet/olm-catalog/latest/manifests/operator.open-cluster-management.io_klusterlets.yaml
around lines 275 to 281, the user-facing description for clusterLabels is
grammatically incorrect (singular/plural mismatch and awkward phrasing); update
the description to match the other CRD copies and use plural/pluralized nouns
and clearer phrasing — for example replace with: "ClusterLabels are labels set
on ManagedClusters at creation time; other actors may update them afterwards."
Ensure this exact revised sentence is used in the description field.

description: "FeatureGates represents the list of feature gates
for registration\nIf it is set empty, default feature gates
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ require (
k8s.io/kubectl v0.33.3
k8s.io/utils v0.0.0-20241210054802-24370beab758
open-cluster-management.io/addon-framework v1.0.1-0.20250722093201-ee47752c02f3
open-cluster-management.io/api v1.0.1-0.20250730122947-5e3423e7794a
open-cluster-management.io/api v1.0.1-0.20250818020747-c0d6364fa4a6
open-cluster-management.io/sdk-go v1.0.1-0.20250805021042-68bb7fc51d4e
sigs.k8s.io/about-api v0.0.0-20250131010323-518069c31c03
sigs.k8s.io/cluster-inventory-api v0.0.0-20240730014211-ef0154379848
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,8 +496,8 @@ k8s.io/utils v0.0.0-20241210054802-24370beab758 h1:sdbE21q2nlQtFh65saZY+rRM6x6aJ
k8s.io/utils v0.0.0-20241210054802-24370beab758/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
open-cluster-management.io/addon-framework v1.0.1-0.20250722093201-ee47752c02f3 h1:r7f57/YPg4caE2N1JQX5sVdBMW5f4VftIUZoW2AWzMs=
open-cluster-management.io/addon-framework v1.0.1-0.20250722093201-ee47752c02f3/go.mod h1:U/AQsLpMi4jay9SC3x+uSh2vsB7ZZPLm63MiRnA9mX4=
open-cluster-management.io/api v1.0.1-0.20250730122947-5e3423e7794a h1:RtfQsfeU+uedsi7btdGdYeSVJw6v2Zp1pMB5XkgHRW8=
open-cluster-management.io/api v1.0.1-0.20250730122947-5e3423e7794a/go.mod h1:KEj/4wbUjdbWktrKLL8+mWzAIzE6Ii3bcRr4CvnBNEg=
open-cluster-management.io/api v1.0.1-0.20250818020747-c0d6364fa4a6 h1:h+ROW7XVTrzWvRH3v3HEuq8lg0kklanK8kFwDIwO3DY=
open-cluster-management.io/api v1.0.1-0.20250818020747-c0d6364fa4a6/go.mod h1:KEj/4wbUjdbWktrKLL8+mWzAIzE6Ii3bcRr4CvnBNEg=
open-cluster-management.io/sdk-go v1.0.1-0.20250805021042-68bb7fc51d4e h1:jI/RdWsyShHxLxAt5OilP0YOiH2qsSYcDtHPS1bT8Is=
open-cluster-management.io/sdk-go v1.0.1-0.20250805021042-68bb7fc51d4e/go.mod h1:sHOVhUgA286ceEq3IjFWqxobt9Lu+VBCAUZByFgN0oM=
sigs.k8s.io/about-api v0.0.0-20250131010323-518069c31c03 h1:1ShFiMjGQOR/8jTBkmZrk1gORxnvMwm1nOy2/DbHg4U=
Expand Down
3 changes: 3 additions & 0 deletions manifests/klusterlet/management/klusterlet-agent-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ spec:
{{if .ClusterAnnotationsString}}
- "--cluster-annotations={{ .ClusterAnnotationsString }}"
{{end}}
{{if .ClusterLabelsString}}
- "--cluster-labels={{ .ClusterLabelsString }}"
{{end}}
{{if eq .InstallMode "SingletonHosted"}}
- "--spoke-kubeconfig=/spoke/config/kubeconfig"
- "--terminate-on-files=/spoke/config/kubeconfig"
Expand Down
3 changes: 3 additions & 0 deletions manifests/klusterlet/management/klusterlet-registration-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ spec:
{{if .ClusterAnnotationsString}}
- "--cluster-annotations={{ .ClusterAnnotationsString }}"
{{end}}
{{if .ClusterLabelsString}}
- "--cluster-labels={{ .ClusterLabelsString }}"
{{end}}
Comment on lines +91 to +93
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Confirm registration agent supports --cluster-labels and values flow from Helm.

Validate that:

  • Registration agent’s options include the --cluster-labels flag.
  • The klusterlet controller populates .ClusterLabelsString from Helm values and passes it to both registration and agent deployments.

Run this script:

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Registration deployment renders the new flag:"
rg -n --no-heading 'registration-agent' -A20 manifests/klusterlet/management/klusterlet-registration-deployment.yaml | sed 's/^/YAML: /'

echo
echo "Search for registration option flag definition and usage:"
rg -n --no-heading 'cluster-labels' -A3 -B3 pkg/registration | sed 's/^/GO: /'

echo
echo "Check Helm values/docs for cluster labels sample:"
fd -a 'values.yaml' deploy | while read -r f; do
  echo "== $f =="
  rg -n --no-heading 'clusterLabels' -A3 -B3 "$f" || true
done

Length of output: 5146

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "1) Search for --cluster-labels in registration deployment manifest:"
rg -n --no-heading -- '--cluster-labels' -A2 -B2 manifests/klusterlet/management/klusterlet-registration-deployment.yaml || echo "Not found"

echo
echo "2) Search for .ClusterLabelsString in registration deployment manifest:"
rg -n --no-heading 'ClusterLabelsString' -A2 -B2 manifests/klusterlet/management/klusterlet-registration-deployment.yaml || echo "Not found"

echo
echo "3) Search for --cluster-labels in agent deployment manifest:"
rg -n --no-heading -- '--cluster-labels' -A2 -B2 manifests/klusterlet/management/klusterlet-agent-deployment.yaml || echo "Not found"

echo
echo "4) Verify Helm chart defines Values.clusterLabels and helper to build ClusterLabelsString:"
echo "4a) Look for clusterLabels in klusterlet chart values:"
rg -n --no-heading 'clusterLabels' -A3 -B3 deploy/klusterlet/chart/klusterlet/values.yaml || echo "No clusterLabels in values.yaml"
echo
echo "4b) Search for clusterLabelsString helper in klusterlet chart templates:"
rg -n --no-heading 'clusterLabelsString' -A3 -B3 deploy/klusterlet/chart/klusterlet/templates || echo "No helper found"

Length of output: 2493

Ensure Helm chart exposes and injects clusterLabels

We’ve confirmed that:

  • The registration agent’s Go code defines and handles the --cluster‐labels flag.
  • Both the registration and agent manifests conditionally include - "--cluster-labels={{ .ClusterLabelsString }}" when .ClusterLabelsString is set.
  • However, the Klusterlet Helm chart currently comments out the clusterLabels section in values.yaml and provides no helper to build .ClusterLabelsString.

To complete the feature, please:

  • In deploy/klusterlet/chart/klusterlet/values.yaml, uncomment or add a top‐level clusterLabels: {} map.

  • In deploy/klusterlet/chart/klusterlet/templates/_helpers.tpl, define a helper, e.g.:

    {{- define "klusterlet.clusterLabelsString" -}}
{{- $labels := .Values.clusterLabels | default dict }}
{{- $pairs := slice }}
{{- range $k, $v := $labels }}
  {{- $pairs = append $pairs (printf "%s=%s" $k $v) }}
{{- end }}
{{- join "," $pairs }}
{{- end }}

  • In both klusterlet-registration-deployment.yaml and klusterlet-agent-deployment.yaml, set:

    {{- $cl := include "klusterlet.clusterLabelsString" . | quote }}
{{- if ne $cl "\"\"" }}
- "--cluster-labels={{ trim $cl "\"" }}"
{{- end }}

This will ensure Values.clusterLabels flows through to .ClusterLabelsString and into both deployments.

🤖 Prompt for AI Agents 
In manifests/klusterlet/management/klusterlet-registration-deployment.yaml
around lines 91-93 the template expects .ClusterLabelsString but the Helm chart
doesn't expose or build Values.clusterLabels; add a top-level clusterLabels: {}
map to deploy/klusterlet/chart/klusterlet/values.yaml, create a helper in
deploy/klusterlet/chart/klusterlet/templates/_helpers.tpl that builds a
comma-separated "key=value" string from .Values.clusterLabels (defaulting to
empty dict), and update both klusterlet-registration-deployment.yaml and
klusterlet-agent-deployment.yaml to call that helper, quote the result, check
for non-empty string, and inject the --cluster-labels flag with the trimmed
value so the flag is only added when labels are provided.

{{if gt .RegistrationKubeAPIQPS 0.0}}
- "--kube-api-qps={{ .RegistrationKubeAPIQPS }}"
{{end}}
Expand Down
8 changes: 8 additions & 0 deletions pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ type klusterletConfig struct {
Replica int32
ClientCertExpirationSeconds int32
ClusterAnnotationsString string
ClusterLabelsString string
RegistrationKubeAPIQPS float32
RegistrationKubeAPIBurst int32
WorkKubeAPIQPS float32
Expand Down Expand Up @@ -389,6 +390,13 @@ func (n *klusterletController) sync(ctx context.Context, controllerContext facto
annotationsArray = append(annotationsArray, fmt.Sprintf("%s=%s", k, v))
}
config.ClusterAnnotationsString = strings.Join(annotationsArray, ",")

// construct cluster labels string, the final format is "key1=value1,key2=value2"
var labelsArray []string
for k, v := range klusterlet.Spec.RegistrationConfiguration.ClusterLabels {
labelsArray = append(labelsArray, fmt.Sprintf("%s=%s", k, v))
}
config.ClusterLabelsString = strings.Join(labelsArray, ",")
}

config.AboutAPIEnabled = helpers.FeatureGateEnabled(
Expand Down
3 changes: 3 additions & 0 deletions pkg/registration/spoke/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ type SpokeAgentOptions struct {
MaxCustomClusterClaims int
ReservedClusterClaimSuffixes []string
ClusterAnnotations map[string]string
ClusterLabels map[string]string

RegisterDriverOption *registerfactory.Options
}
Expand Down Expand Up @@ -76,6 +77,8 @@ func (o *SpokeAgentOptions) AddFlags(fs *pflag.FlagSet) {
"A list of suffixes for reserved cluster claims.")
fs.StringToStringVar(&o.ClusterAnnotations, "cluster-annotations", o.ClusterAnnotations, `the annotations with the reserve
prefix "agent.open-cluster-management.io" set on ManagedCluster when creating only, other actors can update it afterwards.`)
fs.StringToStringVar(&o.ClusterLabels, "cluster-labels", o.ClusterLabels, `the labels set on ManagedCluster
when creating only, other actors can update it afterwards.`)

o.RegisterDriverOption.AddFlags(fs)
}
Expand Down
75 changes: 70 additions & 5 deletions pkg/registration/spoke/registration/creating_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package registration

import (
"bytes"
"context"
"fmt"
"time"
Expand Down Expand Up @@ -89,11 +90,14 @@ func (c *managedClusterCreatingController) sync(ctx context.Context, syncCtx fac
managedCluster = decorator(managedCluster)
}

if len(existingCluster.Spec.ManagedClusterClientConfigs) == len(managedCluster.Spec.ManagedClusterClientConfigs) {
Copy link
Member

@qiujian16 qiujian16 Aug 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the reason we only compare size is because the configs can be set on the hub with different cabundle or endpoint. And the agent should trust setting of hub since hub would have more accurate info on how to access spoke apiserver.

@zhujian7 I wonder whether to update the cluster should be returned by the decorator rather than compare in the controller.

Copy link
Member

@zhujian7 zhujian7 Aug 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, make sense, I think we can let the decorator interface return an additional modified(bool) value like the openshift library-go does.

// Check if any updates are needed (ClientConfigs, Labels, or Annotations)
if equalClientConfigs(existingCluster.Spec.ManagedClusterClientConfigs, managedCluster.Spec.ManagedClusterClientConfigs) &&
equalLabels(existingCluster.Labels, managedCluster.Labels) &&
equalAnnotations(existingCluster.Annotations, managedCluster.Annotations) {
return nil
}

// update ManagedClusterClientConfigs in ManagedCluster
// update ManagedCluster (ClientConfigs, Labels, and Annotations)
_, err = c.hubClusterClient.ClusterV1().ManagedClusters().Update(ctx, managedCluster, metav1.UpdateOptions{})
Comment on lines +93 to 101
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Equality gate may churn on ordering; make client-config comparison order-insensitive

The current equalClientConfigs is order-sensitive; different ordering will trigger unnecessary updates. Sort by URL (and CABundle as tiebreaker) before comparing.

-// equalClientConfigs compares two ClientConfig slices for equality
-func equalClientConfigs(configs1, configs2 []clusterv1.ClientConfig) bool {
-	if len(configs1) != len(configs2) {
-		return false
-	}
-	for i, config1 := range configs1 {
-		config2 := configs2[i]
-		if config1.URL != config2.URL {
-			return false
-		}
-		if string(config1.CABundle) != string(config2.CABundle) {
-			return false
-		}
-	}
-	return true
-}
+// equalClientConfigs compares two ClientConfig slices for equality (order-insensitive).
+func equalClientConfigs(configs1, configs2 []clusterv1.ClientConfig) bool {
+	if len(configs1) != len(configs2) {
+		return false
+	}
+	s1 := append([]clusterv1.ClientConfig(nil), configs1...)
+	s2 := append([]clusterv1.ClientConfig(nil), configs2...)
+	sort.Slice(s1, func(i, j int) bool {
+		if s1[i].URL == s1[j].URL {
+			return string(s1[i].CABundle) < string(s1[j].CABundle)
+		}
+		return s1[i].URL < s1[j].URL
+	})
+	sort.Slice(s2, func(i, j int) bool {
+		if s2[i].URL == s2[j].URL {
+			return string(s2[i].CABundle) < string(s2[j].CABundle)
+		}
+		return s2[i].URL < s2[j].URL
+	})
+	for i := range s1 {
+		if s1[i].URL != s2[i].URL || string(s1[i].CABundle) != string(s2[i].CABundle) {
+			return false
+		}
+	}
+	return true
+}

Note: add the following import in this file:

import "sort"
🤖 Prompt for AI Agents 
In pkg/registration/spoke/registration/creating_controller.go around lines 92 to
100, the equalClientConfigs comparison is order-sensitive and can cause
unnecessary updates; modify equalClientConfigs (or pre-normalize its inputs) to
sort the ManagedClusterClientConfig slices by URL and use CABundle as a
tiebreaker before doing element-wise comparison so ordering differences do not
trigger churn, implement sorting with sort.Slice and stable comparison of URL
then CABundle, and add the import "sort" at the top of the file.

// ManagedClusterClientConfigs in ManagedCluster is only allowed updated during bootstrap.
// After bootstrap secret expired, an unauthorized error will be got, skip it
Expand All @@ -112,15 +116,33 @@ func skipUnauthorizedError(err error) error {
return err
}

// AnnotationDecorator set annotations from annotation map to ManagedCluster
func AnnotationDecorator(annotations map[string]string) ManagedClusterDecorator {
return func(cluster *clusterv1.ManagedCluster) *clusterv1.ManagedCluster {
filteredAnnotations := commonhelpers.FilterClusterAnnotations(annotations)
if cluster.Annotations == nil {
cluster.Annotations = make(map[string]string)
}
for key, value := range filteredAnnotations {
cluster.Annotations[key] = value

filteredAnnotations := commonhelpers.FilterClusterAnnotations(annotations)
for k, v := range filteredAnnotations {
cluster.Annotations[k] = v
}

return cluster
}
}

// LabelDecorator set labels from label map to ManagedCluster
func LabelDecorator(labels map[string]string) ManagedClusterDecorator {
return func(cluster *clusterv1.ManagedCluster) *clusterv1.ManagedCluster {
if cluster.Labels == nil {
cluster.Labels = make(map[string]string)
}

for k, v := range labels {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note we should not allowing set ANY label from the spoke. For instance, open-cluster-management.io/clusterset label is to define the label on the managedcluster, it should not be set by agent.

cluster.Labels[k] = v
}

return cluster
}
}
Expand Down Expand Up @@ -148,3 +170,46 @@ func ClientConfigDecorator(externalServerURLs []string, caBundle []byte) Managed
return cluster
}
}

// equalClientConfigs compares two ClientConfig slices for equality
func equalClientConfigs(configs1, configs2 []clusterv1.ClientConfig) bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, clientconfigs can also be updated by user, so we cannot update clientConfigs based on equality.

if len(configs1) != len(configs2) {
return false
}
for i, config1 := range configs1 {
config2 := configs2[i]
if config1.URL != config2.URL {
return false
}
if !bytes.Equal(config1.CABundle, config2.CABundle) {
return false
}
}
return true
}

// equalLabels compares two label maps for equality
func equalLabels(labels1, labels2 map[string]string) bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we cannot do such equal since the label of cluster can also be set by user on hub. So such label setting can only happen when creating the cluster.

If the label exists already on the cluster, the controller cannot update the label or annotation since it might be updated by user on purpose.

if len(labels1) != len(labels2) {
return false
}
for k, v := range labels1 {
if labels2[k] != v {
return false
}
}
return true
}

// equalAnnotations compares two annotation maps for equality
func equalAnnotations(annotations1, annotations2 map[string]string) bool {
if len(annotations1) != len(annotations2) {
return false
}
for k, v := range annotations1 {
if annotations2[k] != v {
return false
}
}
return true
}
Loading
Loading