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

[augerctl] Support shortname and check namespace for built-in resource #109

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wzshiming
Copy link
Member

@wzshiming wzshiming commented Aug 28, 2024

https://github.com/kubernetes/kubernetes/blob/eebc897e4fd8bf26a69d322c8dbcdf4da475934e/pkg/registry/apps/deployment/storage/storage.go#L122-L124

The short names of the built-in resources are hard-coded into the function, and there's no elegant way to get them, so the implementation here is a script that gets all the built-in resource information from the apiserver.

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wzshiming
Once this PR has been reviewed and has the lgtm label, please assign wenjiaswe for approval. 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

@wzshiming wzshiming force-pushed the feat/shortname branch 20 times, most recently from cca0d04 to 7e9e20c Compare August 29, 2024 06:02
@wzshiming wzshiming changed the title [WIP] [augerctl] Support shortname and check namespace for built-in resource [augerctl] Support shortname and check namespace for built-in resource Aug 29, 2024
@wzshiming
Copy link
Member Author

/cc @jmhbnz
/cc @siyuanfoundation

@k8s-ci-robot k8s-ci-robot requested a review from jmhbnz August 29, 2024 06:22
"k8s.io/client-go/tools/clientcmd"
)

func main() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you document what this script does?
It is hard to decipher based on the names

Copy link
Member Author

Choose a reason for hiding this comment

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

The ./pkg/wellknown/resources.go file is generated by this script, which lists all the aliases of the built-in resources.

@@ -62,10 +62,10 @@ TODO
List a single service with namespace `default` and name `kubernetes`

``` bash
augerctl get services -n default kubernetes
augerctl get svc -n default kubernetes
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would the short name replace the long one? Shouldn't both be supported?

Copy link
Member Author

Choose a reason for hiding this comment

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

Both uses are supported, but I think the descriptions of both uses are a bit verbose.

@siyuanfoundation
Copy link
Contributor

Thank you @wzshiming for enriching the functionality of auger.
Can you provide more information about the use cases the pr helps and how it works?
And it is also very important to have well written comments/documentation for a collaborative project. Can you improve on that?

Thanks!

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

Successfully merging this pull request may close these issues.

3 participants