-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Exoscale cloud provider overhaul #4247
Conversation
Welcome @falzm! |
05f59b3
to
b9ba3cf
Compare
@aleksandra-malinowska @feiskyer do I need to do something to have this PR reviewed? |
@aleksandra-malinowska @feiskyer can I have eyes on this PR please? 🙏 |
2 similar comments
@aleksandra-malinowska @feiskyer can I have eyes on this PR please? 🙏 |
@aleksandra-malinowska @feiskyer can I have eyes on this PR please? 🙏 |
@mwielgus, @MaciekPytel, @bskiba, @gjtempleton, why this PR isn't reviewed ? It's very useful and looks like to be well done work! |
Hey @PhilippeChepy, the @k8s-ci-robot comments document the process for PRs, including this stanza:
For my part I generally only monitor PRs I've been assigned to at this point, I know some others follow the same workflow. I have to say that as this appears to be a wholesale change to a CloudProvider where I have no experience, I wouldn't be comfortable reviewing it, and we should be looking to find owners for the Exoscale Cloud Provider, to enable you to take ownership of the code for this CloudProvider implementation, and only need wider approval for changes which touch code outside of this. @falzm I'm guessing from your comments in the original PR #3470 that you worked with @pierre-emmanuelJ at the time of implementation? Are both of you willing to take on ownership for this implementation (i.e. the contents of the |
Hi @gjtempleton, thank you for your answer. Please, can you can set ownership of this provider code to: |
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.
/lgtm
/approve
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
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. I understand the commands that are listed here. |
@falzm It looks like the commit's no longer linking to your Github user, can you update the commit to link it so the CLA check passes? |
@PhilippeChepy @7fELF one of you should probably sign off the CLA on behalf of Exoscale at this point, since I'm no longer part of the company. It'd be best if one of you re-authored the commit/PR. |
@PhilippeChepy @7fELF may I ask you to proceed as suggested by @falzm I'll have a look at the CLA |
|
d3a0a17
to
3418d3b
Compare
This change refactors the `exoscale` cloud provider to support both plain Instance Pools and SKS Nodepools scaling.
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.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: falzm, mwielgus 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 |
This change refactors the
exoscale
cloud provider to support bothplain Instance Pools and SKS Nodepools scaling.