-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Hub registration using kubeconfig and labels support #785
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
Hub registration using kubeconfig and labels support #785
Conversation
feat: Grant roles/artifactregistry.reader to created service account …
Thanks for the PR! 🚀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@abhinavrau I had a chance to test this out and unfortunately looks like GKE with kubeconfig does not work.
Registering a non-GKE Cluster. Using current-context to register Hub membership.
ERROR: (gcloud.container.hub.memberships.register) Invalid value for [--context]: --context cannot be used for GKE clusters. Either --gke-uri | --gke-cluster must be specified
Works with a kind cluster so maybe we drop the gke kubeconfig example and just mention in the docs how to to use it with kind? Testing with kind would also probably need some dind setup since all our tests run within a docker image.
specify PROJECT_ID to be more specific Co-authored-by: Bharath KKB <[email protected]>
fix number of arguments check Co-authored-by: Bharath KKB <[email protected]>
@bharathkkb Thanks for response and reviewing the code. I have committed your suggested changes. As far as the example with kubeconfig, I have modified the example to use a kind cluster so users can see how to use the hub module with kubeconfig. I understand it will be difficult to do an integration test with dind but is it necessary for all examples to have an integration test? I see there are some examples that don't have an integration test (e.g. examples/simple_zonal_with_acm) so will it be an issue to the have an example using it even though the build process does not run the example? |
@abhinavrau LGTM, we dont need to have an integration test for every example |
Fix for one of the items in #637.
Please review and let me know if this looks good. I will wait for feedback on the tests as there are no integration tests for hub specifically. See comment on 637