SIG Autoscaling Charter and sigs.yaml updates #2425
Conversation
k8s-ci-robot
added
size/M
|
@kubernetes/sig-api-machinery-misc I think SIG Autoscaling should probably own the polymorphic scale client (since the primary motivating use case is the HPA controller), but I can be convinced otherwise. WDYT? cc @kubernetes/sig-autoscaling-misc |
k8s-ci-robot
added
the
sig/api-machinery
| Covers development and maintenance of componets for automated scaling in | ||
| Kubernetes. This includes automated vertical and horizontal pod | ||
| autoscaling, initial resource estimation, cluster-proportional system | ||
| component autoscaling, and autoscaling of Kubernetes clusters themselves. |
There was a problem hiding this comment.
Currently cluster proportional autoscaling is not owned by sig-autoscaling:
- addon-resizer is maintained by sig-instrumentation (AFAIK it's primarily used for heapster and metrics-server)
- kube-dns-autoscaler is maintained by sig-networking
Ideally sig-autoscaling should take ownership of this use-case, but we're not there yet.
There was a problem hiding this comment.
oh, do we not own the addon-resizer? I thought it was under our purview from the pre-sig-instrumentation Heapster days, and I didn't think it got transferred over, since it's an autoscaler.
We probably should own those, at any rate. @kubernetes/sig-instrumentation-misc
There was a problem hiding this comment.
addon-resizer is maintained by sig-instrumentation (AFAIK it's primarily used for heapster and metrics-server)
I was not aware of this, and it's also not in our sig components. I was under the impression that sig-autoscaling own it.
There was a problem hiding this comment.
Every kubernetes piece of code needs to be owned by a SIG. I think it makes sense for us to own it, especially since it doesn't fall under the responsibilities of SIG instrumentation normally. @kawych and @x13n can continue to be the ones to work on it, and we can list them as the owners of the subproject, but it'll fall under the purview of sig autoscaling.
There was a problem hiding this comment.
@DirectXMan12’s suggestion sounds good to me.
There was a problem hiding this comment.
@kubernetes/sig-network-misc the cluster-proportional-autoscaler doesn't seem to be listed under sigs.yaml (unless my searching is failing me). I think it makes some amount of sense for it to be a subproject of sig-autoscaling (no change in ownership necessary, but it should fall under a sig umbrella), so I'd appreciate your opinions.
There was a problem hiding this comment.
@DirectXMan12 what you suggested sounds great. Also wanted to point out yet another component that might fall under this same umbrella: cluster-proportional-vertical-autoscaler.
There was a problem hiding this comment.
cool, thanks for the heads up. I'll update sigs.yaml with all of those
There was a problem hiding this comment.
I don't think this SIG owns an incubator project, unless it was created by SIG members who want it to be within the scope of the SIG...
|
|
||
| - Ensuring API interfaces (the scale subresource) are availble and usable | ||
| to enable other SIG to write autoscalable objects, and enable people to | ||
| interact with those interfaces. |
There was a problem hiding this comment.
I'd argue this goes requires a lot of apimachinery know-how, which is not too closely related to anything else we do in sig. I suspect you're the only one who actually knows how the polymorphic scale client actually works.
There was a problem hiding this comment.
Sure, but we're responsible for maintaining the HPA, and the HPA can't work w/o the scale subresource, and the scale subresource's object is part of the autoscaling API group.
@kubernetes/sig-api-machinery-misc any opinions?
There was a problem hiding this comment.
In general, api-machinery owns mechanisms, specific sigs own specific APIs.
In this case, I'd expect api-machinery to own the apiserver code that enables subresources, and autoscaling to own the in-tree scale subresource implementations, the autoscaling/v1 Scale API types, and custom code in the scale client.
There was a problem hiding this comment.
ack, that's more-or-less what I expected
| autoscaling behaves properly. | ||
|
|
||
| - Coordinating with SIG Instrumentation to ensure that metrics APIs are | ||
| suitable for autoscaling on. |
There was a problem hiding this comment.
Can we add coordinating with sig-scheduling to ensure new scheduling features are compatible with cluster autoscaling?
sig-autoscaling/charter.md
Outdated
| ### Out of scope | ||
|
|
||
| - Testing performance at scale (this falls under the purview of [SIG | ||
| Scalability]. |
There was a problem hiding this comment.
We certainly do it for cluster autoscaler. And I don't think sig-scalability is doing any tests for HPA. Maybe we should reconsider? I would argue some level of scalability should be pre-requisite for HPAv2 GA, which would make it very much our problem.
There was a problem hiding this comment.
SIG scalability and SIG autoscaling are often confused in issue requests, and I kind-of wanted to make a clear deliniation. It doesn't mean we can't test our own components, but we're not responsible for general cluster scale testing (e.g. if you can't launch 100000 pods at once, it's not our job to look into why). I'll try to clarify.
There was a problem hiding this comment.
Ah, gotcha, I completely misunderstood what you meant.
k8s-ci-robot
added
sig/instrumentation
DirectXMan12
force-pushed
the
autoscaling/charter
branch
from
20e257c to
ef9235c
Compare
sig-autoscaling/charter.md
Outdated
|
|
||
| ## Scope | ||
|
|
||
| Covers development and maintenance of componets for automated scaling in |
| Covers development and maintenance of componets for automated scaling in | ||
| Kubernetes. This includes automated vertical and horizontal pod | ||
| autoscaling, initial resource estimation, cluster-proportional system | ||
| component autoscaling, and autoscaling of Kubernetes clusters themselves. |
There was a problem hiding this comment.
@DirectXMan12 what you suggested sounds great. Also wanted to point out yet another component that might fall under this same umbrella: cluster-proportional-vertical-autoscaler.
DirectXMan12
force-pushed
the
autoscaling/charter
branch
from
ef9235c to
f459a54
Compare
k8s-ci-robot
added
size/L
|
@DirectXMan12 can you split the sigs.yaml change apart from the charter change? I'd like the sigs.yaml changes in sooner, charter review will take longer |
|
See #2534 |
sigs.yaml
Outdated
| - name: autoscaler | ||
| - name: scale-client | ||
| owners: | ||
| - https://raw.githubusercontent.com/kubernetes/client-go/master/scale/OWNERS # doesn't exist yet |
There was a problem hiding this comment.
api machinery owns the level above this. Is it OK for OWNERS files in this list to overlap? I assume it must be. I don't have a strong opinion about who should own this code at the moment. If we had a client-focussed SIG, they would own it, I think, but we don't.
sigs.yaml
Outdated
| owners: | ||
| - https://raw.githubusercontent.com/kubernetes/metrics/master/OWNERS | ||
| - https://raw.githubusercontent.com/kubernetes/autoscaler/master/OWNERS | ||
| - name: horizntal-pod-autoscaler |
k8s-ci-robot
added
the
needs-rebase
|
/committee steering |
|
@spiffxp: GitHub didn't allow me to assign the following users: brendanburns. Note that only kubernetes members and repo collaborators can be assigned. 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. |
k8s-ci-robot
added
the
committee/steering
|
/assign brendandburns
…On Fri, Aug 31, 2018, 2:38 PM k8s-ci-robot ***@***.***> wrote:
@spiffxp <https://github.com/spiffxp>: GitHub didn't allow me to assign
the following users: brendanburns.
Note that only kubernetes members
<https://github.com/orgs/kubernetes/people> and repo collaborators can be
assigned.
For more information please see the contributor guide
<https://git.k8s.io/community/contributors/guide/#issue-assignment-in-github>
In response to this
<#2425 (comment)>
:
/committee steering
/assign @brendanburns <https://github.com/brendanburns> @michelleN
<https://github.com/michelleN>
per kubernetes/steering#31
<kubernetes/steering#31>
Instructions for interacting with me using PR comments are available here
<https://git.k8s.io/community/contributors/guide/pull-requests.md>. If
you have questions or suggestions related to my behavior, please file an
issue against the kubernetes/test-infra
<https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:>
repository.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2425 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABOYXsU5maLzXIKhfjmw1hRxiikkD4WUks5uWazHgaJpZM4ViaGd>
.
|
k8s-ci-robot
added
size/M
|
@spiffxp @brendanburns rebased. What else has to be done? |
DirectXMan12
force-pushed
the
autoscaling/charter
branch
from
a2c60c1 to
2e903e7
Compare
|
This should fix the verify issues. |
sig-autoscaling/charter.md
Outdated
|
|
||
| ## Scope | ||
|
|
||
| Covers development and maintenance of components for automated scaling in |
There was a problem hiding this comment.
I think that there should be something about "integrated into kubernetes by default" or something like that.
Basically, I want it to be clear that if someone develops a random autoscaler in their own repo, it's not covered by this SIG. I think that's pretty clearly understood by everyone involved, but this text seems to lay claim to all Kubernetes autoscaling
(and the thread below that points to an incubator project as part of the scope makes this distinction even more important to make)
There was a problem hiding this comment.
We also own officially "sponsored" projects. The term "integrated in to Kubernetes by default" is pretty ambiguous, especially in the era of "do it out-of-tree using a CRD" (i.e. vertical pod autoscaler and cluster autoscaler). If someone wants to get their project sponsored by a SIG, SIG Autoscaling is the right place, yet it may not be "integrated into Kubernetes by default".
|
|
||
| #### Code, Binaries and Services | ||
|
|
||
| - Components and utilities that take automated action to scale a component |
There was a problem hiding this comment.
These are all kind of vague, pointers to actual repos/folders?
There was a problem hiding this comment.
I was under the impression that concrete folders is what sigs.yaml was supposed to be for, and that this was more of "conceptually, what do you cover". That being said, I can certainly provide some examples here, or actually links if my impression was incorrection.
There was a problem hiding this comment.
fixed to mention the actual subproject names
| - Components and utilities that take automated action to scale the cluster | ||
| itself | ||
|
|
||
| - Special parts of client-go for interacting with with the scaling |
There was a problem hiding this comment.
Definitely need a pointer into the project here.
There was a problem hiding this comment.
done -- mentioned the specific subproject. Actual folder definitions are in sigs.yaml
This updates the mission statement for SIG Autoscaling to correspond to the scope of SIG Autoscaling in the charter.
DirectXMan12
force-pushed
the
autoscaling/charter
branch
from
2e903e7 to
bfd97a2
Compare
|
@spiffxp typos fixed @spiffxp I don't see any comments from @mwielgus ... what do you mean? |
|
@DirectXMan12 wow yeah where did I get that name from, I meant @MaciekPytel, I think their comments are satisfied? just looking for a confirm |
|
looks like @MaciekPytel has responded to most of the threads that they raised confirming that, but just in case ping @MaciekPytel to confirm. |
|
I'm happy with how my comments were addressed, lgtm. |
|
@spiffxp @brendanburns good to go then? (want to get this merged by KubeCon ;-) ) |
|
/lgtm
…On Wed, Dec 5, 2018, 1:21 PM Solly Ross ***@***.*** wrote:
@spiffxp <https://github.com/spiffxp> @brendanburns
<https://github.com/brendanburns> good to go then?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2425 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABOYXomds4p5qiNM81_zdD-MtzEJBfsKks5u2DjBgaJpZM4ViaGd>
.
|
|
as do we, I poked @brendandburns and @michelleN both during steering meeting today, if you're still hanging tomorrow I can serve as another SC reviewer lgtm |
|
@brendanburns: changing LGTM is restricted to assignees, and only kubernetes/community repo collaborators may be assigned issues. 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. |
|
/lgtm Thanks for working through this! |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: spiffxp 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 |
k8s-ci-robot
added
the
approved
This adds a SIG Autoscaling charter, and updates sigs.yaml to make sure it has the right subprojects assigned to sig autoscaling vs sig instrumentation