API Server & kubectl logic for Vertical Pod Autoscaler #64286

kgrygiel · 2018-05-24T19:32:08Z

What this PR does / why we need it:
Adds API Server and kubectl logic to handle Vertical Pod Autoscalers.

Special notes for your reviewer:
This is follow up to PR #63797 that adds VPA to the autoscaling v2beta1 API.
/cc @mwielgus
/cc @wojtek-t

Release note:

Support for Vertical Pod Autoscalers in the API Server and kubectl.

liggitt · 2018-05-25T04:25:39Z

pkg/printers/internalversion/describe.go

+
+// Describe describes the vertical pod autoscaler object.
+func (d *VerticalPodAutoscalerDescriber) Describe(namespace, name string, describerSettings printers.DescriberSettings) (string, error) {
+	vpa, err := d.client.Autoscaling().VerticalPodAutoscalers(namespace).Get(name, metav1.GetOptions{})


Verify this actually works. If the resource is not in the preferred API group (which for autoscaling is v1) I think you have to use the external client (see cronjobs)

Probably need a test for this as well

Thanks! Changed to use the external client.

k8s-ci-robot · 2018-06-04T15:13:39Z

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.

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 your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: [email protected]

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.

mwielgus · 2018-06-07T17:15:58Z

Adding status/approved-for-milestone as a sig autoscaling leader.

k8s-github-robot · 2018-06-07T17:16:46Z

[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process

@kgrygiel @wojtek-t

Pull Request Labels

sig/autoscaling sig/cluster-lifecycle: Pull Request will be escalated to these SIGs if needed.
priority/critical-urgent: Never automatically move pull request out of a release milestone; continually escalate to contributor and SIG through all available channels.
kind/feature: New functionality.

Help

jberkus · 2018-06-07T17:34:29Z

/hold

This is a 6K line patch you're delivering in the middle of code freeze. When we talked about this, you said that this PR was just for tracking and wasn't going to merge. Please discuss this on #sig-release on Slack.

liggitt · 2018-06-07T17:54:44Z

these two comments are in direct opposition:

#63797 (comment)

The risk associated with this PR is minimal. It is the api that is handled by an opt-in, independent component.

#64286 (comment):

This has to be merged to not leave the autoscaling api in half-working/half-broken state.

if #63797 destabilized things and requires large follow-ups like this, we should revert it asap and bring it in in 1.12

janetkuo · 2018-06-07T19:19:19Z

pkg/printers/internalversion/describe.go

+		return "", err
+	}
+
+	return describeVerticalPodAutoscaler(internalVpa, events, d)


You only use internal vpa. Therefore you don't need external client or conversion.

janetkuo · 2018-06-07T19:31:10Z

pkg/printers/internalversion/printers.go

+	verticalPodAutoscalerColumnDefinitions := []metav1beta1.TableColumnDefinition{
+		{Name: "Name", Type: "string", Format: "name", Description: metav1.ObjectMeta{}.SwaggerDoc()["name"]},
+		{Name: "Selector", Type: "string", Description: autoscalingv2beta1.VerticalPodAutoscalerSpec{}.SwaggerDoc()["selector"]},
+		{Name: "UpdateMode", Type: "string", Description: autoscalingv2beta1.PodUpdatePolicy{}.SwaggerDoc()["updateMode"]},


Selector, and even UpdateMode could be priority 1 (omitted by default, shown when -o wide).

kubectl get is useful when it prints the current status of a resource. Is there something in vpa status that you can add here? For example, deployment printer prints its desired and current replicas; pod printer prints its current ready containers and restart counts. A column doesn't need to map to a status field, it could be aggregated information you gather from several fields.

mwielgus · 2018-06-07T19:56:08Z

As discussed on slack we agreed to postpone it to 1.12.

As of the comment of direct opposition - iVPA follows the same model as Ingress. The API itself is official, however various parties may provide slightly different implementation that fits to their needs. So it is both core and independent at the same time. Sorry if it wasn't crystal clear.

The exact communication with #63797 was that there will be a follow up PR. @jberkus understood that the #63797 is that follow up PR. The miscommunication was resolved on slack.

@jberkus

Automatic merge from submit-queue (batch tested with PRs 64836, 64890). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Revert: Add Vertical Pod Autoscaler to autoscaling/v2beta1 Reverts #63797 per [discussion](https://kubernetes.slack.com/archives/C09R1LV8S/p1528400528000615) with @jberkus and @mwielgus The scope of the follow-ups required in #64286 was not well understood when the API PR was merged, and we are now past code freeze for 1.11 This can be reopened against 1.12 ```release-note NONE ```

@jberkus

Automatic merge from submit-queue (batch tested with PRs 64836, 64890). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Revert: Add Vertical Pod Autoscaler to autoscaling/v2beta1 Reverts kubernetes/kubernetes#63797 per [discussion](https://kubernetes.slack.com/archives/C09R1LV8S/p1528400528000615) with @jberkus and @mwielgus The scope of the follow-ups required in kubernetes/kubernetes#64286 was not well understood when the API PR was merged, and we are now past code freeze for 1.11 This can be reopened against 1.12 ```release-note NONE ``` Kubernetes-commit: 169df7434155cbbc22f1532cba8e0a9588e29ad8

@jberkus

Automatic merge from submit-queue (batch tested with PRs 64836, 64890). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Revert: Add Vertical Pod Autoscaler to autoscaling/v2beta1 Reverts kubernetes/kubernetes#63797 per [discussion](https://kubernetes.slack.com/archives/C09R1LV8S/p1528400528000615) with @jberkus and @mwielgus The scope of the follow-ups required in kubernetes/kubernetes#64286 was not well understood when the API PR was merged, and we are now past code freeze for 1.11 This can be reopened against 1.12 ```release-note NONE ``` Kubernetes-commit: 169df7434155cbbc22f1532cba8e0a9588e29ad8

k8s-ci-robot · 2018-06-08T22:32:10Z

@kgrygiel: PR needs rebase.

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 · 2018-08-22T01:25:17Z

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.

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 your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: [email protected]

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.

wojtek-t · 2018-10-17T14:24:19Z

This is no longer needed. Closing.

k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label May 24, 2018

k8s-ci-robot requested review from mwielgus and wojtek-t May 24, 2018 19:32

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 24, 2018

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/new-api labels May 24, 2018

kgrygiel force-pushed the vpa-api branch from 6516717 to abe2a0f Compare May 24, 2018 21:54

liggitt reviewed May 25, 2018

View reviewed changes

kgrygiel force-pushed the vpa-api branch from abe2a0f to 136ed08 Compare May 28, 2018 21:33

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 28, 2018

kgrygiel force-pushed the vpa-api branch from 136ed08 to adfa93c Compare May 28, 2018 22:29

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 28, 2018

kgrygiel force-pushed the vpa-api branch 3 times, most recently from a9b6d61 to 3ddac91 Compare June 4, 2018 15:10

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. area/kubeadm sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. labels Jun 4, 2018

k8s-github-robot added the kind/old-docs label Jun 4, 2018

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 4, 2018

kgrygiel force-pushed the vpa-api branch from 3ddac91 to 56c8bdb Compare June 5, 2018 08:37

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jun 5, 2018

k8s-github-robot removed the kind/old-docs label Jun 5, 2018

kgrygiel force-pushed the vpa-api branch from 56c8bdb to 94abe53 Compare June 5, 2018 11:19

k8s-github-robot added milestone/needs-approval and removed milestone/incomplete-labels labels Jun 7, 2018

mwielgus added sig/autoscaling Categorizes an issue or PR as relevant to SIG Autoscaling. and removed sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. labels Jun 7, 2018

mwielgus added the status/approved-for-milestone label Jun 7, 2018

k8s-github-robot removed the milestone/needs-approval label Jun 7, 2018

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 7, 2018

janetkuo reviewed Jun 7, 2018

View reviewed changes

mwielgus modified the milestones: v1.11, next-candidate Jun 7, 2018

liggitt mentioned this pull request Jun 7, 2018

Revert: Add Vertical Pod Autoscaler to autoscaling/v2beta1 #64890

Merged

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 8, 2018

k8s-ci-robot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Aug 22, 2018

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/new-api labels Aug 22, 2018

k8s-ci-robot removed the status/approved-for-milestone label Sep 21, 2018

wojtek-t closed this Oct 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API Server & kubectl logic for Vertical Pod Autoscaler #64286

API Server & kubectl logic for Vertical Pod Autoscaler #64286

kgrygiel commented May 24, 2018

liggitt May 25, 2018

kgrygiel Jun 6, 2018

k8s-ci-robot commented Jun 4, 2018

mwielgus commented Jun 7, 2018

k8s-github-robot commented Jun 7, 2018

jberkus commented Jun 7, 2018

liggitt commented Jun 7, 2018

janetkuo Jun 7, 2018

janetkuo Jun 7, 2018 •

edited

Loading

mwielgus commented Jun 7, 2018

k8s-ci-robot commented Jun 8, 2018

k8s-ci-robot commented Aug 22, 2018

wojtek-t commented Oct 17, 2018

API Server & kubectl logic for Vertical Pod Autoscaler #64286

API Server & kubectl logic for Vertical Pod Autoscaler #64286

Conversation

kgrygiel commented May 24, 2018

liggitt May 25, 2018

Choose a reason for hiding this comment

kgrygiel Jun 6, 2018

Choose a reason for hiding this comment

k8s-ci-robot commented Jun 4, 2018

mwielgus commented Jun 7, 2018

k8s-github-robot commented Jun 7, 2018

jberkus commented Jun 7, 2018

liggitt commented Jun 7, 2018

janetkuo Jun 7, 2018

Choose a reason for hiding this comment

janetkuo Jun 7, 2018 • edited Loading

Choose a reason for hiding this comment

mwielgus commented Jun 7, 2018

k8s-ci-robot commented Jun 8, 2018

k8s-ci-robot commented Aug 22, 2018

wojtek-t commented Oct 17, 2018

janetkuo Jun 7, 2018 •

edited

Loading