Skip to content

[VPA] Enable multiple revive rules - 4 #9155

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.

Sign up for GitHub

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

Open
mayuka-c wants to merge 3 commits into
base: master
Choose a base branch
from
+240 −238
Open

[VPA] Enable multiple revive rules - 4 #9155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f2b9f99
[VPA] Enable multiple revive rules - 4
mayuka-c Feb 1, 2026
18a813a
run codegen
mayuka-c Feb 1, 2026
ab624d8
Lint fix
mayuka-c Feb 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions vertical-pod-autoscaler/.golangci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ linters:
- name: empty-lines
- name: use-errors-new
- name: early-return
- name: redefines-builtin-id
- name: redundant-import-alias

# Below lists the rules disabled

Expand Down
4 changes: 2 additions & 2 deletions vertical-pod-autoscaler/pkg/admission-controller/resource/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package resource
import (
"context"

v1 "k8s.io/api/admission/v1"
apiv1 "k8s.io/api/admission/v1"
Copy link
Member

@adrianmoisey adrianmoisey Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be admissionv1 ?

Copy link
Contributor Author

@mayuka-c mayuka-c Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops my bad, I will update it

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/metrics/admission"
Expand All @@ -41,5 +41,5 @@ type Handler interface {
// DisallowIncorrectObjects returns whether incorrect objects (eg. unparsable, not passing validations) should be disallowed by Admission Server.
DisallowIncorrectObjects() bool
// GetPatches returns patches for given AdmissionRequest
GetPatches(context.Context, *v1.AdmissionRequest) ([]PatchRecord, error)
GetPatches(context.Context, *apiv1.AdmissionRequest) ([]PatchRecord, error)
}
4 changes: 2 additions & 2 deletions vertical-pod-autoscaler/pkg/admission-controller/resource/pod/handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"github.com/stretchr/testify/assert"
admissionv1 "k8s.io/api/admission/v1"
apiv1 "k8s.io/api/core/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Copy link
Contributor Author

@mayuka-c mayuka-c Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the comments here, have enabled the "redundant-import-alias" rule

"k8s.io/apimachinery/pkg/runtime"

resource_admission "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource"
Expand Down Expand Up @@ -184,7 +184,7 @@ func TestGetPatches(t *testing.T) {
fvm := &fakeVpaMatcher{vpa: tc.vpa}
h := NewResourceHandler(fppp, fvm, tc.calculators)
patches, err := h.GetPatches(context.Background(), &admissionv1.AdmissionRequest{
Resource: v1.GroupVersionResource{
Resource: metav1.GroupVersionResource{
Version: "v1",
},
Namespace: tc.namespace,
Expand Down
26 changes: 13 additions & 13 deletions vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import (
"errors"
"fmt"

v1 "k8s.io/api/admission/v1"
corev1 "k8s.io/api/core/v1"
Copy link
Member

@adrianmoisey adrianmoisey Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm taking a look at the kubernetes repo, and it seems that this package is often called corev1, any reason why this changes to apiv1 ?

Copy link
Contributor Author

@mayuka-c mayuka-c Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I looked across the repo, most of the places we have used apiv1, so I have used this to make it consistent.
PLMK if we want to have corev1

Copy link
Member

@adrianmoisey adrianmoisey Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

corev1 would be great, thanks

Copy link
Contributor Author

@mayuka-c mayuka-c Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WIll be making the changes, Thanks!

admissionv1 "k8s.io/api/admission/v1"
apiv1 "k8s.io/api/core/v1"
apires "k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/klog/v2"
Expand Down Expand Up @@ -67,8 +67,8 @@ func (h *resourceHandler) DisallowIncorrectObjects() bool {
}

// GetPatches builds patches for VPA in given admission request.
func (h *resourceHandler) GetPatches(_ context.Context, ar *v1.AdmissionRequest) ([]resource.PatchRecord, error) {
raw, isCreate := ar.Object.Raw, ar.Operation == v1.Create
func (h *resourceHandler) GetPatches(_ context.Context, ar *admissionv1.AdmissionRequest) ([]resource.PatchRecord, error) {
raw, isCreate := ar.Object.Raw, ar.Operation == admissionv1.Create
vpa, err := parseVPA(raw)
if err != nil {
return nil, err
Expand Down Expand Up @@ -158,18 +158,18 @@ func ValidateVPA(vpa *vpa_types.VerticalPodAutoscaler, isCreate bool) error {
return fmt.Errorf("unexpected Mode value %s", *mode)
}
}
for resource, min := range policy.MinAllowed {
if err := validateResourceResolution(resource, min); err != nil {
for resource, minAllowed := range policy.MinAllowed {
if err := validateResourceResolution(resource, minAllowed); err != nil {
return fmt.Errorf("minAllowed: %v", err)
}
max, found := policy.MaxAllowed[resource]
if found && max.Cmp(min) < 0 {
maxAllowed, found := policy.MaxAllowed[resource]
if found && maxAllowed.Cmp(minAllowed) < 0 {
return fmt.Errorf("max resource for %v is lower than min", resource)
}
}

for resource, max := range policy.MaxAllowed {
if err := validateResourceResolution(resource, max); err != nil {
for resource, maxAllowed := range policy.MaxAllowed {
if err := validateResourceResolution(resource, maxAllowed); err != nil {
return fmt.Errorf("maxAllowed: %v", err)
}
}
Expand All @@ -193,11 +193,11 @@ func ValidateVPA(vpa *vpa_types.VerticalPodAutoscaler, isCreate bool) error {
return nil
}

func validateResourceResolution(name corev1.ResourceName, val apires.Quantity) error {
func validateResourceResolution(name apiv1.ResourceName, val apires.Quantity) error {
switch name {
case corev1.ResourceCPU:
case apiv1.ResourceCPU:
return validateCPUResolution(val)
case corev1.ResourceMemory:
case apiv1.ResourceMemory:
return validateMemoryResolution(val)
}
return nil
Expand Down
6 changes: 3 additions & 3 deletions vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/matcher_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"github.com/stretchr/testify/assert"
"go.uber.org/mock/gomock"
appsv1 "k8s.io/api/apps/v1"
v1 "k8s.io/api/autoscaling/v1"
autoscalingv1 "k8s.io/api/autoscaling/v1"
core "k8s.io/api/core/v1"
meta "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
Expand Down Expand Up @@ -51,12 +51,12 @@ func TestGetMatchingVpa(t *testing.T) {
Namespace: "default",
},
}
targetRef := &v1.CrossVersionObjectReference{
targetRef := &autoscalingv1.CrossVersionObjectReference{
Kind: sts.Kind,
Name: sts.Name,
APIVersion: sts.APIVersion,
}
targetRefWithNoMatches := &v1.CrossVersionObjectReference{
targetRefWithNoMatches := &autoscalingv1.CrossVersionObjectReference{
Kind: "ReplicaSet",
Name: "rs",
APIVersion: "apps/v1",
Expand Down
20 changes: 10 additions & 10 deletions vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ package v1

import (
autoscaling "k8s.io/api/autoscaling/v1"
v1 "k8s.io/api/core/v1"
apiv1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
Expand Down Expand Up @@ -127,7 +127,7 @@ type EvictionRequirement struct {
// Resources is a list of one or more resources that the condition applies
// to. If more than one resource is given, the EvictionRequirement is fulfilled
// if at least one resource meets `changeRequirement`.
Resources []v1.ResourceName `json:"resources" protobuf:"bytes,1,name=resources"`
Resources []apiv1.ResourceName `json:"resources" protobuf:"bytes,1,name=resources"`
ChangeRequirement EvictionChangeRequirement `json:"changeRequirement" protobuf:"bytes,2,name=changeRequirement"`
}

Expand Down Expand Up @@ -210,17 +210,17 @@ type ContainerResourcePolicy struct {
// Specifies the minimal amount of resources that will be recommended
// for the container. The default is no minimum.
// +optional
MinAllowed v1.ResourceList `json:"minAllowed,omitempty" protobuf:"bytes,3,rep,name=minAllowed,casttype=ResourceList,castkey=ResourceName"`
MinAllowed apiv1.ResourceList `json:"minAllowed,omitempty" protobuf:"bytes,3,rep,name=minAllowed,casttype=ResourceList,castkey=ResourceName"`
// Specifies the maximum amount of resources that will be recommended
// for the container. The default is no maximum.
// +optional
MaxAllowed v1.ResourceList `json:"maxAllowed,omitempty" protobuf:"bytes,4,rep,name=maxAllowed,casttype=ResourceList,castkey=ResourceName"`
MaxAllowed apiv1.ResourceList `json:"maxAllowed,omitempty" protobuf:"bytes,4,rep,name=maxAllowed,casttype=ResourceList,castkey=ResourceName"`

// Specifies the type of recommendations that will be computed
// (and possibly applied) by VPA.
// If not specified, the default of [ResourceCPU, ResourceMemory] will be used.
// +patchStrategy=merge
ControlledResources *[]v1.ResourceName `json:"controlledResources,omitempty" patchStrategy:"merge" protobuf:"bytes,5,rep,name=controlledResources"`
ControlledResources *[]apiv1.ResourceName `json:"controlledResources,omitempty" patchStrategy:"merge" protobuf:"bytes,5,rep,name=controlledResources"`

// Specifies which resource values should be controlled.
// The default is "RequestsAndLimits".
Expand Down Expand Up @@ -298,17 +298,17 @@ type RecommendedContainerResources struct {
// Name of the container.
ContainerName string `json:"containerName,omitempty" protobuf:"bytes,1,opt,name=containerName"`
// Recommended amount of resources. Observes ContainerResourcePolicy.
Target v1.ResourceList `json:"target" protobuf:"bytes,2,rep,name=target,casttype=ResourceList,castkey=ResourceName"`
Target apiv1.ResourceList `json:"target" protobuf:"bytes,2,rep,name=target,casttype=ResourceList,castkey=ResourceName"`
// Minimum recommended amount of resources. Observes ContainerResourcePolicy.
// This amount is not guaranteed to be sufficient for the application to operate in a stable way, however
// running with less resources is likely to have significant impact on performance/availability.
// +optional
LowerBound v1.ResourceList `json:"lowerBound,omitempty" protobuf:"bytes,3,rep,name=lowerBound,casttype=ResourceList,castkey=ResourceName"`
LowerBound apiv1.ResourceList `json:"lowerBound,omitempty" protobuf:"bytes,3,rep,name=lowerBound,casttype=ResourceList,castkey=ResourceName"`
// Maximum recommended amount of resources. Observes ContainerResourcePolicy.
// Any resources allocated beyond this value are likely wasted. This value may be larger than the maximum
// amount of application is actually capable of consuming.
// +optional
UpperBound v1.ResourceList `json:"upperBound,omitempty" protobuf:"bytes,4,rep,name=upperBound,casttype=ResourceList,castkey=ResourceName"`
UpperBound apiv1.ResourceList `json:"upperBound,omitempty" protobuf:"bytes,4,rep,name=upperBound,casttype=ResourceList,castkey=ResourceName"`
// The most recent recommended resources target computed by the autoscaler
// for the controlled pods, based only on actual resource usage, not taking
// into account the ContainerResourcePolicy.
Expand All @@ -317,7 +317,7 @@ type RecommendedContainerResources struct {
// or higher that MaxAllowed).
// Used only as status indication, will not affect actual resource assignment.
// +optional
UncappedTarget v1.ResourceList `json:"uncappedTarget,omitempty" protobuf:"bytes,5,opt,name=uncappedTarget"`
UncappedTarget apiv1.ResourceList `json:"uncappedTarget,omitempty" protobuf:"bytes,5,opt,name=uncappedTarget"`
}

// VerticalPodAutoscalerConditionType are the valid conditions of
Expand Down Expand Up @@ -348,7 +348,7 @@ type VerticalPodAutoscalerCondition struct {
// type describes the current condition
Type VerticalPodAutoscalerConditionType `json:"type" protobuf:"bytes,1,name=type"`
// status is the status of the condition (True, False, Unknown)
Status v1.ConditionStatus `json:"status" protobuf:"bytes,2,name=status"`
Status apiv1.ConditionStatus `json:"status" protobuf:"bytes,2,name=status"`
// lastTransitionTime is the last time the condition transitioned from
// one status to another
// +optional
Expand Down
16 changes: 8 additions & 8 deletions vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ limitations under the License.
package v1beta1

import (
v1 "k8s.io/api/core/v1"
apiv1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -131,11 +131,11 @@ type ContainerResourcePolicy struct {
// Specifies the minimal amount of resources that will be recommended
// for the container. The default is no minimum.
// +optional
MinAllowed v1.ResourceList `json:"minAllowed,omitempty" protobuf:"bytes,3,rep,name=minAllowed,casttype=ResourceList,castkey=ResourceName"`
MinAllowed apiv1.ResourceList `json:"minAllowed,omitempty" protobuf:"bytes,3,rep,name=minAllowed,casttype=ResourceList,castkey=ResourceName"`
// Specifies the maximum amount of resources that will be recommended
// for the container. The default is no maximum.
// +optional
MaxAllowed v1.ResourceList `json:"maxAllowed,omitempty" protobuf:"bytes,4,rep,name=maxAllowed,casttype=ResourceList,castkey=ResourceName"`
MaxAllowed apiv1.ResourceList `json:"maxAllowed,omitempty" protobuf:"bytes,4,rep,name=maxAllowed,casttype=ResourceList,castkey=ResourceName"`
}

const (
Expand Down Expand Up @@ -187,17 +187,17 @@ type RecommendedContainerResources struct {
// Name of the container.
ContainerName string `json:"containerName,omitempty" protobuf:"bytes,1,opt,name=containerName"`
// Recommended amount of resources. Observes ContainerResourcePolicy.
Target v1.ResourceList `json:"target" protobuf:"bytes,2,rep,name=target,casttype=ResourceList,castkey=ResourceName"`
Target apiv1.ResourceList `json:"target" protobuf:"bytes,2,rep,name=target,casttype=ResourceList,castkey=ResourceName"`
// Minimum recommended amount of resources. Observes ContainerResourcePolicy.
// This amount is not guaranteed to be sufficient for the application to operate in a stable way, however
// running with less resources is likely to have significant impact on performance/availability.
// +optional
LowerBound v1.ResourceList `json:"lowerBound,omitempty" protobuf:"bytes,3,rep,name=lowerBound,casttype=ResourceList,castkey=ResourceName"`
LowerBound apiv1.ResourceList `json:"lowerBound,omitempty" protobuf:"bytes,3,rep,name=lowerBound,casttype=ResourceList,castkey=ResourceName"`
// Maximum recommended amount of resources. Observes ContainerResourcePolicy.
// Any resources allocated beyond this value are likely wasted. This value may be larger than the maximum
// amount of application is actually capable of consuming.
// +optional
UpperBound v1.ResourceList `json:"upperBound,omitempty" protobuf:"bytes,4,rep,name=upperBound,casttype=ResourceList,castkey=ResourceName"`
UpperBound apiv1.ResourceList `json:"upperBound,omitempty" protobuf:"bytes,4,rep,name=upperBound,casttype=ResourceList,castkey=ResourceName"`
// The most recent recommended resources target computed by the autoscaler
// for the controlled pods, based only on actual resource usage, not taking
// into account the ContainerResourcePolicy.
Expand All @@ -206,7 +206,7 @@ type RecommendedContainerResources struct {
// or higher that MaxAllowed).
// Used only as status indication, will not affect actual resource assignment.
// +optional
UncappedTarget v1.ResourceList `json:"uncappedTarget,omitempty" protobuf:"bytes,5,opt,name=uncappedTarget"`
UncappedTarget apiv1.ResourceList `json:"uncappedTarget,omitempty" protobuf:"bytes,5,opt,name=uncappedTarget"`
}

// VerticalPodAutoscalerConditionType are the valid conditions of
Expand All @@ -231,7 +231,7 @@ type VerticalPodAutoscalerCondition struct {
// type describes the current condition
Type VerticalPodAutoscalerConditionType `json:"type" protobuf:"bytes,1,name=type"`
// status is the status of the condition (True, False, Unknown)
Status v1.ConditionStatus `json:"status" protobuf:"bytes,2,name=status"`
Status apiv1.ConditionStatus `json:"status" protobuf:"bytes,2,name=status"`
// lastTransitionTime is the last time the condition transitioned from
// one status to another
// +optional
Expand Down
16 changes: 8 additions & 8 deletions vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta2/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package v1beta2

import (
autoscaling "k8s.io/api/autoscaling/v1"
v1 "k8s.io/api/core/v1"
apiv1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -150,11 +150,11 @@ type ContainerResourcePolicy struct {
// Specifies the minimal amount of resources that will be recommended
// for the container. The default is no minimum.
// +optional
MinAllowed v1.ResourceList `json:"minAllowed,omitempty" protobuf:"bytes,3,rep,name=minAllowed,casttype=ResourceList,castkey=ResourceName"`
MinAllowed apiv1.ResourceList `json:"minAllowed,omitempty" protobuf:"bytes,3,rep,name=minAllowed,casttype=ResourceList,castkey=ResourceName"`
// Specifies the maximum amount of resources that will be recommended
// for the container. The default is no maximum.
// +optional
MaxAllowed v1.ResourceList `json:"maxAllowed,omitempty" protobuf:"bytes,4,rep,name=maxAllowed,casttype=ResourceList,castkey=ResourceName"`
MaxAllowed apiv1.ResourceList `json:"maxAllowed,omitempty" protobuf:"bytes,4,rep,name=maxAllowed,casttype=ResourceList,castkey=ResourceName"`
}

const (
Expand Down Expand Up @@ -207,17 +207,17 @@ type RecommendedContainerResources struct {
// Name of the container.
ContainerName string `json:"containerName,omitempty" protobuf:"bytes,1,opt,name=containerName"`
// Recommended amount of resources. Observes ContainerResourcePolicy.
Target v1.ResourceList `json:"target" protobuf:"bytes,2,rep,name=target,casttype=ResourceList,castkey=ResourceName"`
Target apiv1.ResourceList `json:"target" protobuf:"bytes,2,rep,name=target,casttype=ResourceList,castkey=ResourceName"`
// Minimum recommended amount of resources. Observes ContainerResourcePolicy.
// This amount is not guaranteed to be sufficient for the application to operate in a stable way, however
// running with less resources is likely to have significant impact on performance/availability.
// +optional
LowerBound v1.ResourceList `json:"lowerBound,omitempty" protobuf:"bytes,3,rep,name=lowerBound,casttype=ResourceList,castkey=ResourceName"`
LowerBound apiv1.ResourceList `json:"lowerBound,omitempty" protobuf:"bytes,3,rep,name=lowerBound,casttype=ResourceList,castkey=ResourceName"`
// Maximum recommended amount of resources. Observes ContainerResourcePolicy.
// Any resources allocated beyond this value are likely wasted. This value may be larger than the maximum
// amount of application is actually capable of consuming.
// +optional
UpperBound v1.ResourceList `json:"upperBound,omitempty" protobuf:"bytes,4,rep,name=upperBound,casttype=ResourceList,castkey=ResourceName"`
UpperBound apiv1.ResourceList `json:"upperBound,omitempty" protobuf:"bytes,4,rep,name=upperBound,casttype=ResourceList,castkey=ResourceName"`
// The most recent recommended resources target computed by the autoscaler
// for the controlled pods, based only on actual resource usage, not taking
// into account the ContainerResourcePolicy.
Expand All @@ -226,7 +226,7 @@ type RecommendedContainerResources struct {
// or higher that MaxAllowed).
// Used only as status indication, will not affect actual resource assignment.
// +optional
UncappedTarget v1.ResourceList `json:"uncappedTarget,omitempty" protobuf:"bytes,5,opt,name=uncappedTarget"`
UncappedTarget apiv1.ResourceList `json:"uncappedTarget,omitempty" protobuf:"bytes,5,opt,name=uncappedTarget"`
}

// VerticalPodAutoscalerConditionType are the valid conditions of
Expand Down Expand Up @@ -257,7 +257,7 @@ type VerticalPodAutoscalerCondition struct {
// type describes the current condition
Type VerticalPodAutoscalerConditionType `json:"type" protobuf:"bytes,1,name=type"`
// status is the status of the condition (True, False, Unknown)
Status v1.ConditionStatus `json:"status" protobuf:"bytes,2,name=status"`
Status apiv1.ConditionStatus `json:"status" protobuf:"bytes,2,name=status"`
// lastTransitionTime is the last time the condition transitioned from
// one status to another
// +optional
Expand Down
Loading