Restructure resize policy naming and set default resize policy values #116119

vinaykul · 2023-02-28T08:16:58Z

What type of PR is this?

/kind cleanup
/kind api-change

What this PR does / why we need it:

This PR restructures resize policy naming and sets default resize policy values.

To better represent intent, ResourceResizePolicy type is renamed to ResourceResizeRestartPolicy and RestartRequired policy value is renamed to RestartContainer
If not specified by the user, RestartNotRequired policy is defaulted for CPU and memory resources.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Please review specific commits, bulk of the changes are just generated files.
Resize policy name restructuring commit: 2d3569b
Set default resize policy commit: 4b5099e

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-triage-robot · 2023-02-28T09:21:41Z

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

cici37 · 2023-02-28T21:09:39Z

/remove-sig api-machinery

vinaykul · 2023-03-01T15:15:06Z

/assign @thockin @derekwaynecarr

…estartNotRequired -> NotRequired

vinaykul · 2023-03-13T21:12:11Z

ping @thockin please review. Latest commit fixes RestartNotRequired -> NotRequired and sets ResizePolicy only for specified resources (which looks much cleaner)

thockin · 2023-03-14T22:28:26Z

Squash or leave it?

thockin

Thanks!

/lgtm
/approve

thockin · 2023-03-14T22:30:25Z

pkg/apis/core/v1/defaults.go

 				}
 			}
 		}
+		if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) &&


Follow up with a test? :)

Yes, a more comprehensive test is warranted, sorry for the miss. I'll bring it with GetPodQoS -> PodQOSClass change.
Having a stiff drink rn :)

@thockin Please review PR #116684 that adds missing unit test for defaulting

k8s-ci-robot · 2023-03-14T22:31:47Z

LGTM label has been added.

Git tree hash: 53d283b5a6e0b57fdbc02c0ba4e16895a201f18e

k8s-ci-robot · 2023-03-14T22:32:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: thockin, vinaykul

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:

~~api/OWNERS~~ [thockin]
~~pkg/api/OWNERS~~ [thockin]
~~pkg/apis/OWNERS~~ [thockin]
~~pkg/generated/openapi/OWNERS~~ [thockin]
~~pkg/kubelet/OWNERS~~ [thockin]
~~staging/src/k8s.io/api/OWNERS~~ [thockin]
~~staging/src/k8s.io/client-go/applyconfigurations/OWNERS~~ [thockin]
~~test/e2e/node/OWNERS~~ [thockin]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

pacoxu · 2023-03-15T00:17:38Z

/release-note-edit

vpa: ResourceResizePolicy type is renamed to ResourceResizeRestartPolicy and RestartRequired policy value is renamed to RestartContainer. If not specified by the user, RestartNotRequired policy defaults for CPU and memory resources.

SergeyKanzhelev · 2023-03-15T04:19:25Z

/release-note-edit

vpa: ResourceResizePolicy type is renamed to ResourceResizeRestartPolicy and RestartRequired policy value is renamed to RestartContainer. If not specified by the user, RestartNotRequired policy defaults for CPU and memory resources.

In terms of release notes, these fields are new in 1.27 and we probably should not call out renaming explicitly - if added to release notes as-is it will be confusing

pacoxu · 2023-03-15T07:25:12Z

/release-note-none

k8s-ci-robot · 2023-03-15T07:25:13Z

@pacoxu: you can only set the release note label to release-note-none if the release-note block in the PR body text is empty or "none".

In response to this:

/release-note-none

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.

pacoxu · 2023-03-15T07:26:15Z

@pacoxu: you can only set the release note label to release-note-none if the release-note block in the PR body text is empty or "none".

@vinaykul would you help to remove the release-note in pr descriptions? 😓 sorry for applying an improper release note here.

vinaykul · 2023-03-16T14:04:17Z

@pacoxu I don't see it in the CHANGELOG. Isn't that where the release notes go? I can attach the fix to the missing unit test PR.

pacoxu · 2023-03-17T02:53:44Z

pacoxu · 2023-03-17T02:55:15Z

I am afraid that it has already been collected to the Changelog:

kubernetes/CHANGELOG/CHANGELOG-1.27.md

Line 232 in 8e01ee7

    
           - Vpa: ResourceResizePolicy type is renamed to ResourceResizeRestartPolicy and RestartRequired policy value is renamed to RestartContainer. If not specified by the user, RestartNotRequired policy defaults for CPU and memory resources. ([#116119](https://github.com/kubernetes/kubernetes/pull/116119), [@vinaykul](https://github.com/vinaykul)) [SIG API Machinery, Apps, Node and Testing]

vinaykul · 2023-03-17T02:56:58Z

/release-note-none

vinaykul · 2023-03-17T02:57:59Z

I am afraid that it has already been collected to the Changelog:

kubernetes/CHANGELOG/CHANGELOG-1.27.md

Line 232 in 8e01ee7

- Vpa: ResourceResizePolicy type is renamed to ResourceResizeRestartPolicy and RestartRequired policy value is renamed to RestartContainer. If not specified by the user, RestartNotRequired policy defaults for CPU and memory resources. ([#116119](https://github.com/kubernetes/kubernetes/pull/116119), [@vinaykul](https://github.com/vinaykul)) [SIG API Machinery, Apps, Node and Testing]

Ah.. I'll piggy back a CHANGELOG update with unit test addition PR

k8s-ci-robot requested review from andrewsykim and aojea February 28, 2023 08:17

vinaykul marked this pull request as draft February 28, 2023 13:35

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 28, 2023

vinaykul marked this pull request as ready for review February 28, 2023 14:04

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 28, 2023

k8s-ci-robot requested review from bobbypage and dchen1107 February 28, 2023 14:05

k8s-ci-robot removed the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Feb 28, 2023

vinaykul force-pushed the restart-free-pod-vertical-scaling-fixes branch from 3a68f30 to 338ebaf Compare March 1, 2023 14:08

k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Mar 1, 2023

vinaykul added 5 commits March 12, 2023 23:11

Restructure naming of resource resize restart policy

8b23497

Restructure naming of resource resize restart policy - generated files

c5130fb

Set default resource resize restart policy to RestartNotRequired

9411050

Set default resize policy only for specified resource types, rename R…

9a805db

…estartNotRequired -> NotRequired

Update generated files

06e1692

vinaykul force-pushed the restart-free-pod-vertical-scaling-fixes branch from b38378d to 06e1692 Compare March 12, 2023 23:47

thockin reviewed Mar 14, 2023

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 14, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 14, 2023

cici37 added this to the v1.27 milestone Mar 15, 2023

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Mar 15, 2023

k8s-ci-robot merged commit 9053b5d into kubernetes:master Mar 15, 2023

vinaykul mentioned this pull request Mar 16, 2023

Add missing unit test for resource resize policy defaulting #116684

Merged

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Mar 17, 2023

Restructure resize policy naming and set default resize policy values #116119

Restructure resize policy naming and set default resize policy values #116119

Uh oh!

Conversation

vinaykul commented Feb 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Uh oh!

k8s-triage-robot commented Feb 28, 2023

Uh oh!

cici37 commented Feb 28, 2023

Uh oh!

vinaykul commented Mar 1, 2023

Uh oh!

vinaykul commented Mar 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thockin commented Mar 14, 2023

Uh oh!

thockin left a comment

Choose a reason for hiding this comment

Uh oh!

thockin Mar 14, 2023

Choose a reason for hiding this comment

Uh oh!

vinaykul Mar 15, 2023

Choose a reason for hiding this comment

Uh oh!

vinaykul Mar 16, 2023

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Mar 14, 2023

Uh oh!

k8s-ci-robot commented Mar 14, 2023

Uh oh!

pacoxu commented Mar 15, 2023

Uh oh!

SergeyKanzhelev commented Mar 15, 2023

Uh oh!

pacoxu commented Mar 15, 2023

Uh oh!

k8s-ci-robot commented Mar 15, 2023

Uh oh!

pacoxu commented Mar 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vinaykul commented Mar 16, 2023

Uh oh!

pacoxu commented Mar 17, 2023

Uh oh!

pacoxu commented Mar 17, 2023

Uh oh!

vinaykul commented Mar 17, 2023

Uh oh!

vinaykul commented Mar 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

vinaykul commented Feb 28, 2023 •

edited

Loading

vinaykul commented Mar 13, 2023 •

edited

Loading

pacoxu commented Mar 15, 2023 •

edited

Loading