-
Notifications
You must be signed in to change notification settings - Fork 4k
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
[cluster-autoscaler] Support using --cloud-config for clusterapi provider #3203
Conversation
|
thanks Jason! i'll give this a test on monday and let you know how it goes. |
I'm happy to lgtm once this is ready. |
hey Jason, i've just started some testing on this patch and the first thing i tried was to repeat the expected behavior for the autoscaler. i created a joined CAPI cluster and then attempted to start the autoscaler as i have done previously, unfortunately it failed on the kubeconfig detect. i tried the same action against master and it worked, so i am guessing there is a regression or something missed. attaching the output.
i am going to test a multicluster setup next |
quick followup, if i set both setting both flags to the same kubeconfig is working for me on a joined cluster. |
@elmiko 😬 it looks like I accidentally inverted the logic for the fallback previously. It should work now. |
hehe, i found the same when i started to exercise the multi-cluster deployment. thanks for the update! this is working for me when i use the autoscaler like this i also confirmed that it works in joined cluster mode as well. i think we just need a docs adjustment and then i am good to approve. |
@elmiko this should be good to go now, updated the README to cover the use of the |
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.
looks great!
just need to modify the cloud-provider value to clusterapi
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.
awesome! thanks again Jason =)
/approve
/area provider/clusterapi |
@elmiko: The label(s) In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/area provider/cluster-api |
forgot to mention this but, as for the error stuff i tend to side for not wrapping them and trying to use the base errors package. i kinda like the whitespace that the linter suggested though. |
I just got this running on my kind cluster using my other branch, this is all I needed to add to get it to work managing a cluster called 😸 |
/hold cancel |
…ider - Leverage --cloud-config to allow for providing a separate kubeconfig for Cluster API management and workload cluster resources - Allow for fallback to previous behavior when --cloud-config is not specified for backward compatibility - Provides a --clusterapi-cloud-config-authoritative flag to disable the above fallback behavior and allow for both the management and workload cluster clients to use the in-cluster config
6239077
to
150dbde
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: elmiko, MaciekPytel 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:
Approvers can indicate their approval by writing |
Rebased, apologies about the delays. |
/lgtm |
[cluster-autoscaler] Support using --cloud-config for clusterapi provider
[cluster-autoscaler] Support using --cloud-config for clusterapi provider
Fixes: #3196