✨ sync the labels from klusterlet to agent #475

zhiweiyin318 · 2024-05-22T14:58:56Z

Summary

sync the labels from klusterlet to all agent resouces, the default labels createdBy: klusterlet

Related issue(s)

#467

codecov · 2024-05-27T14:09:55Z

Codecov Report

Attention: Patch coverage is 58.62069% with 12 lines in your changes missing coverage. Please review.

Project coverage is 62.72%. Comparing base (b530125) to head (ea0dd73).

Files	Patch %	Lines
pkg/operator/helpers/helpers.go	15.38%	11 Missing ⚠️
...stermanagercontroller/clustermanager_controller.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #475   +/-   ##
=======================================
  Coverage   62.72%   62.72%           
=======================================
  Files         136      136           
  Lines       11593    11615   +22     
=======================================
+ Hits         7272     7286   +14     
- Misses       3552     3562   +10     
+ Partials      769      767    -2

Flag	Coverage Δ
unit	`62.72% <58.62%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

zhiweiyin318 · 2024-05-28T01:56:30Z

/assign @qiujian16

qiujian16

let's add an integration test.

qiujian16 · 2024-05-28T02:01:14Z

pkg/operator/helpers/helpers.go

@@ -821,3 +822,18 @@ func GetOperatorNamespace() string {
 	}
 	return operatorNamespace
 }
+
+func MergeLabels(destination map[string]string, source map[string]string) map[string]string {


can we just use MergeMap in library-go?

qiujian16 · 2024-05-28T02:02:02Z

pkg/operator/helpers/helpers.go

+}
+
+func KlusterletAgentDefaultLabels() map[string]string {
+	return map[string]string{"createdBy": "klusterlet"}


could be just a var rather than a func?

change this func to this:

func GetKlusterletAgentLabels(klusterlet *operatorapiv1.Klusterlet) map[string]string { labels := klusterlet.GetLabels() if labels == nil { labels = map[string]string{} } labels["klusterletName"] = klusterlet.Name // This label is used to filter in deployment informer labels["createdBy"] = "klusterlet" return labels }

qiujian16 · 2024-05-28T02:02:56Z

...operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller_test.go

@@ -463,6 +463,11 @@ func ensureObject(t *testing.T, object runtime.Object, klusterlet *operatorapiv1
 		return
 	}

+	if !mapCompare(helpers.MergeLabels(helpers.KlusterletAgentDefaultLabels(), klusterlet.GetLabels()), access.GetLabels()) {


I think you can just use deepEqual?

the synced klusterlet labels is a part of the exiting labels, for example the deployments have label app=klusterlet-registration-agent, namespace has the label workload.openshift.io/allowed=management

zhujian7 · 2024-05-28T03:33:09Z

manifests/klusterlet/managed/klusterlet-registration-clusterrole-addon-management.yaml

@@ -7,6 +7,13 @@ apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole
 metadata:
  name: open-cluster-management:{{ .KlusterletName }}-registration:addon-management
+  labels:
+    createdBy: klusterlet


considering there might be multiple klusterlet in hosted mode, should we put the klusterlet as the real klusterlet name?

the label createdBy: klusterlet is used to filter in deployment informer, I added another label klusterletName to set the real klusterlet name.

TBH maybe not necessary to create both label right now...why do we still need createdBy: klusterlet if we have klusterletName label? We can change the filter in deployment informer.

how about to change the label to createdByKlusterlet: <klusterlet name> , in deployment informer, change to check if the key createdByKlusterlet exists?

zhiweiyin318 · 2024-05-31T01:46:17Z

/hold

qiujian16 · 2024-05-31T08:03:00Z

/approve
/lgtm

openshift-ci · 2024-05-31T08:03:07Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qiujian16, zhiweiyin318

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [qiujian16]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

qiujian16 · 2024-06-03T08:32:08Z

@zhiweiyin318 can we merge this?

zhiweiyin318 · 2024-06-04T03:49:12Z

/unhold

Signed-off-by: Zhiwei Yin <zyin@redhat.com>

qiujian16 · 2024-06-04T06:45:51Z

/lgtm

…gement-io#475) Signed-off-by: Zhiwei Yin <zyin@redhat.com>

* sync labels from klusterlet to all agent resources (#475) Signed-off-by: Zhiwei Yin <zyin@redhat.com> * add enable-sync-labels flag to klusterlet operator (#505) Signed-off-by: Zhiwei Yin <zyin@redhat.com> * fix issue that pull secret and ns are synced labels when enable-sync-labels is disabled (#511) Signed-off-by: Zhiwei Yin <zyin@redhat.com> --------- Signed-off-by: Zhiwei Yin <zyin@redhat.com>

openshift-ci bot requested review from xuezhaojun and zhujian7 May 22, 2024 14:59

zhiweiyin318 changed the title ~~✨ wip: sync the labels from klusterlet to agent~~ ✨ WIP: sync the labels from klusterlet to agent May 23, 2024

zhiweiyin318 changed the title ~~✨ WIP: sync the labels from klusterlet to agent~~ WIP: ✨ sync the labels from klusterlet to agent May 23, 2024

openshift-ci bot added the do-not-merge/work-in-progress label May 23, 2024

zhiweiyin318 force-pushed the sync-labels branch from 9a1d140 to 19eca1e Compare May 27, 2024 14:07

zhiweiyin318 force-pushed the sync-labels branch 3 times, most recently from 254b853 to d415cad Compare May 28, 2024 01:06

zhiweiyin318 changed the title ~~WIP: ✨ sync the labels from klusterlet to agent~~ ✨ sync the labels from klusterlet to agent May 28, 2024

openshift-ci bot removed the do-not-merge/work-in-progress label May 28, 2024

openshift-ci bot assigned qiujian16 May 28, 2024

qiujian16 reviewed May 28, 2024

View reviewed changes

zhujian7 reviewed May 28, 2024

View reviewed changes

zhiweiyin318 force-pushed the sync-labels branch 8 times, most recently from 068faaf to d7ff3fc Compare May 30, 2024 16:02

openshift-ci bot added the do-not-merge/hold label May 31, 2024

zhiweiyin318 force-pushed the sync-labels branch from d7ff3fc to a53b6e9 Compare May 31, 2024 02:12

openshift-ci bot added the lgtm label May 31, 2024

openshift-ci bot added the approved label May 31, 2024

openshift-merge-robot added the needs-rebase label Jun 3, 2024

zhiweiyin318 force-pushed the sync-labels branch from a53b6e9 to 57428ae Compare June 4, 2024 03:47

openshift-ci bot removed the lgtm label Jun 4, 2024

openshift-merge-robot removed the needs-rebase label Jun 4, 2024

openshift-ci bot removed the do-not-merge/hold label Jun 4, 2024

sync labels from klusterlet to all agent resources

ea0dd73

Signed-off-by: Zhiwei Yin <zyin@redhat.com>

zhiweiyin318 force-pushed the sync-labels branch from 57428ae to ea0dd73 Compare June 4, 2024 04:21

openshift-ci bot added the lgtm label Jun 4, 2024

openshift-merge-bot bot merged commit 7fd7330 into open-cluster-management-io:main Jun 4, 2024
14 checks passed

zhiweiyin318 deleted the sync-labels branch June 4, 2024 06:50

zhiweiyin318 added a commit to zhiweiyin318/ocm that referenced this pull request Jun 13, 2024

sync labels from klusterlet to all agent resources (open-cluster-mana…

d2f29ee

…gement-io#475) Signed-off-by: Zhiwei Yin <zyin@redhat.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ sync the labels from klusterlet to agent #475

✨ sync the labels from klusterlet to agent #475

zhiweiyin318 commented May 22, 2024 •

edited

Loading

codecov bot commented May 27, 2024 •

edited

Loading

zhiweiyin318 commented May 28, 2024

qiujian16 left a comment

qiujian16 May 28, 2024

zhiweiyin318 May 29, 2024

qiujian16 May 28, 2024

zhiweiyin318 May 29, 2024

qiujian16 May 28, 2024

zhiweiyin318 May 29, 2024

zhujian7 May 28, 2024

zhiweiyin318 May 29, 2024

qiujian16 May 30, 2024

zhiweiyin318 May 30, 2024 •

edited

Loading

zhiweiyin318 commented May 31, 2024

qiujian16 commented May 31, 2024

openshift-ci bot commented May 31, 2024

qiujian16 commented Jun 3, 2024

zhiweiyin318 commented Jun 4, 2024

qiujian16 commented Jun 4, 2024

✨ sync the labels from klusterlet to agent #475

✨ sync the labels from klusterlet to agent #475

Conversation

zhiweiyin318 commented May 22, 2024 • edited Loading

Summary

Related issue(s)

codecov bot commented May 27, 2024 • edited Loading

Codecov Report

zhiweiyin318 commented May 28, 2024

qiujian16 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhiweiyin318 May 30, 2024 • edited Loading

Choose a reason for hiding this comment

zhiweiyin318 commented May 31, 2024

qiujian16 commented May 31, 2024

openshift-ci bot commented May 31, 2024

qiujian16 commented Jun 3, 2024

zhiweiyin318 commented Jun 4, 2024

qiujian16 commented Jun 4, 2024

zhiweiyin318 commented May 22, 2024 •

edited

Loading

codecov bot commented May 27, 2024 •

edited

Loading

zhiweiyin318 May 30, 2024 •

edited

Loading