-
Notifications
You must be signed in to change notification settings - Fork 39.9k
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
auto-scaler types #5492
auto-scaler types #5492
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/. If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA or see this help article on setting the email on your git commits. Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name. |
// MaxAutoScaleCount defines the max replicas that the auto-scaler can use. This value must be | ||
// >= MinAutoScaleCount. | ||
MaxAutoScaleCount int `json:"maxAutoScaleCount,omitempty"` | ||
|
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.
Make these int64
95c023f
to
ed0f730
Compare
@smarterclayton - commit 2 contains updates for almost all the feedback given today. I still need to sync with Dario on the selectors, the refactor for the new etcd layout took longer than expected. |
ed0f730
to
952e5d5
Compare
@pweil- @smarterclayton What's the urgency of this issue? I assume you need this in Openshift now/soon? Possible general approaches to enable you to move faster:
|
We've reprioritized this a bit to get security account / security context sorted out (what Paul is going to be working on soon). I think we can leave this in discussion state until that work gets closed, unless there is someone else actively looking to push it. ----- Original Message -----
|
@pweil- Regarding API, I think we should create a new API prefix for autoscaling for now (item 3 from #5492 (comment) by @bgrant0607). We don't want to modify v1 API as autoscaling will not be a part of v1. Later, when autoscaling is ready, we may reconsider this decision and merge our autoscaling API with the main API. |
@jszczepkowski - I will talk with @smarterclayton about what needs to be cleaned up to get it in a state that can be merged or at least handed over if it will help with the work you're doing since it low priority for me right now. Full disclosure: I should point out that there is a fairly large (intentional) gap in the statistics storage. As in, it was not defined in the proposal. |
All of the concerns I had are probably best resolved with an actual implementation prototype and discussion.
|
@jszczepkowski based on the comment above there is no more clean up to be done. I think the easiest way to keep this going would be to have you cherry-pick the current commit and continue on with the types. I'm assuming we don't want to merge the types until there is a working prototype. If there is anything that would make this easier then please let me know and I'll do my best to get it done ASAP. |
Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist") If this message is too spammy, please complain @ixdy. |
@pweil- @smarterclayton What's your current timeline for horizontal auto-scaling? |
We were hoping to make a push for shortly after 1.0 to define the core types and start prototyping.
|
Closing in favor of #12087 |
Step 1 of auto-scaler implementation. Types and rest impl.
To try and keep these small(ish) I will submit separate PRs for the kubectl wiring and controller implementations.
See proposal at: #2863
@davidopp @bgrant0607 @smarterclayton @jlowdermilk @fbalejko @pmorie PTAL