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

feat:add a label for kcc resource type #2452

Closed
wants to merge 2 commits into from

Conversation

xiaoweim
Copy link
Contributor

@xiaoweim xiaoweim commented Aug 8, 2024

This PR adds a default label to KCC resource object specifying the resource type.

I would like to have a label in concord usage tables that helps me attribute the resource usage to the exact KCC resource. In order to concord to be able to get this information, we first need a label in KCC so that I can later add a label in concord with this information.

@xiaoweim xiaoweim force-pushed the add_kcc_label branch 10 times, most recently from 0220667 to 267dc65 Compare August 12, 2024 22:20
@xiaoweim xiaoweim force-pushed the add_kcc_label branch 4 times, most recently from 27d016f to 6a2c7a0 Compare August 13, 2024 04:46
@xiaoweim
Copy link
Contributor Author

/assign @cheftako
/assign @justinsb

@yuwenma
Copy link
Collaborator

yuwenma commented Aug 13, 2024

Do we want to add the Kind name in the KCC metadata labels as well? This is a duplicate of the Kind field itself, my concern is that this could bug some users.

Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from cheftako. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

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

@justinsb
Copy link
Collaborator

Yeah, can you describe more what we're trying to achieve here. AIUI the GCP resource implies the KCC resource, so I'm not sure what we're adding? We have the managed-by-cnrm GCP label already if we want to know if the resource is managed by KCC

@xiaoweim
Copy link
Contributor Author

Yeah, can you describe more what we're trying to achieve here. AIUI the GCP resource implies the KCC resource, so I'm not sure what we're adding? We have the managed-by-cnrm GCP label already if we want to know if the resource is managed by KCC

Essentially I would like to have a label in concord usage tables that helps me attribute the resource usage to the exact KCC resource. In order to concord to be able to get this information, we first need a label in KCC so that I can later add a label in concord with this information.

@xiaoweim
Copy link
Contributor Author

Do we want to add the Kind name in the KCC metadata labels as well? This is a duplicate of the Kind field itself, my concern is that this could bug some users.

So I am adding this label to pass this information to concord usage tables, not sure if KCC metadata label is needed or can do the same?

@xiaoweim xiaoweim force-pushed the add_kcc_label branch 5 times, most recently from 577c50b to a603d52 Compare August 15, 2024 23:10
@xiaoweim xiaoweim force-pushed the add_kcc_label branch 2 times, most recently from 8691a99 to c408b97 Compare August 16, 2024 05:04
@cheftako
Copy link
Collaborator

Seems like some types (Eg CloudSQLInstance) are not being labelled today.

@cheftako
Copy link
Collaborator

Otherwise looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants