Skip to content
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.

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

Apply all labels that klusterlet has to all the resources it will create and keep labels in sync #467

Closed
EricaJ6 opened this issue May 17, 2024 · 12 comments
Assignees
Labels
enhancement New feature or request

Comments

@EricaJ6
Copy link

EricaJ6 commented May 17, 2024

Describe the enhancement
We observed that resources created by klusterlet do not have labels that klusterlet has. For example, klusterlet-agent in the open-cluster-management-agent namespace is missing labels. We’re using these labels for cost tracking purposes. As discussed with @qiujian16, and agreed to apply all labels that klusterlet has to all the resources it will create. Additionally, if labels on the klusterlet got changed (either updated or added new ones), all the resources created by it should be in sync as well.

@EricaJ6 EricaJ6 added the enhancement New feature or request label May 17, 2024
@qiujian16
Copy link
Member

@zhiweiyin318 will you be able to take a look at this one?

@zhiweiyin318
Copy link
Member

@EricaJ6 @qiujian16 I have some questions about this requirement.

  1. just double check that we need to sync the label to the all agent resources or only deployments since the description mentioned cost tracking purposes.
  2. do we need delete the label from the agent if it is removed from the klusterlet ? the reason why I ask is currently the map update strategy is merge.

@EricaJ6
Copy link
Author

EricaJ6 commented May 22, 2024

Thanks @zhiweiyin318! For your questions,

  1. We should sync the labels to all resources created by the klusterlet. The goal is all resources should have the necessary labels. I looked into the above PR, it looks like currently, only deployments have labels added, could you please add labels to all resources (such as spec. template. metadata.labels and so on)?
  2. If the label got removed from the klusterlet, it should also be removed from all resources created by the klusterlet.

@zhiweiyin318
Copy link
Member

@EricaJ6 currently there are 2 labels app: klusterlet-agent and createdBy: klusterlet on the deployment created by klusterlet.
if we add the 2 labels to all agent resources, not sure if can meet your requirements for cost tracking purpose?
if the 2 labels can meet your requirements, I think it is not necessary to sync labels from klusterlet.

@EricaJ6
Copy link
Author

EricaJ6 commented May 23, 2024

@zhiweiyin318 Other than the 2 default labels, we are adding additional customization labels to klusterlet, and the organization is using these customized labels for cost-tracking purpose. That's why we would like to sync the labels to all resources created by the klusterlet. Because of the use of additional customization labels, the 2 default labels do not meet the requirements.

@zhiweiyin318
Copy link
Member

@EricaJ6 I synced the labels to all agent resources in the PR, but currently we cannot remove the label from agent resouces after the label is deleted on the klusterlet. Because we use the apply function in the openshift/library-go lib to apply resources, and it uses merge to handle labels and annotations.

func MergeMap(modified *bool, existing *map[string]string, required map[string]string) {

so if want to delete the removed labels on the agent resources , need to remove it from the agent resources manually .

@EricaJ6
Copy link
Author

EricaJ6 commented May 27, 2024

Thanks @zhiweiyin318 on sharing the information and updating the PR!

  1. To make sure all pods created by the deployments also have the labels, how about adding labels to the spec.template.metadata.labels of the *deployment.yaml files as well?
  2. Will clusteradm repo also needs to be updated for synced labels to take effect?

@zhiweiyin318
Copy link
Member

@EricaJ6

  1. Added labels for pods.
  2. I don't think clusteradm repo need to be updated since the latest image will take affect after the PR is merged. you can add labels to klusterlet after the cluster is joined to the hub.

@EricaJ6
Copy link
Author

EricaJ6 commented May 28, 2024

@zhiweiyin318

  1. The pods' labels look good!
  2. Is there a timeline for when the PR will be released? We'll try it out.

@zhiweiyin318
Copy link
Member

the feature will be included into release v0.14.0. you can try it using the latest images after the PR is merged.

@zhiweiyin318
Copy link
Member

@EricaJ6 The PR is merged. please use the latest images to take a try.

@EricaJ6
Copy link
Author

EricaJ6 commented Jun 4, 2024

Thanks @zhiweiyin318 for the updates! I will try it once it is released.

@qiujian16 qiujian16 moved this from In Progress to Done in OCM releases Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

No branches or pull requests

3 participants