Merge pull request #9155 from mayuka-c/add-revive-rules-part4

[VPA] Enable multiple revive rules - 4

Author: k8s-ci-robot

hack/verify-spelling.sh

@@ -32,4 +32,5 @@ git ls-files --full-name \
     | grep -v cluster-autoscaler/cloudprovider/ionoscloud/ionos-cloud-sdk-go \
     | grep -v cluster-autoscaler/cloudprovider/volcengine/volcengine-go-sdk \
     | grep -v cluster-autoscaler/cloudprovider/civo \
+    | grep -v vertical-pod-autoscaler/.golangci.yaml \
     | xargs misspell -error -o stderr

vertical-pod-autoscaler/.golangci.yaml

@@ -3,6 +3,7 @@ linters:
   enable:
     - forbidigo
     - modernize
+    - importas
     - revive
   settings:
     forbidigo:
@@ -12,8 +13,32 @@ linters:
       analyze-types: true
     modernize:
       disable:
-        # Disabling omitzero linter as it catches isssues with API types, which would be best if kube-api-lint can handle
+        # Disabling omitzero linter as it catches issues with API types, which would be best if kube-api-lint can handle
         - omitzero
+
+    importas:
+      alias:
+        # Inspired by Kubernetes' import alias conventions for consistency:
+        # https://github.com/kubernetes/kubernetes/blob/master/hack/.import-aliases
+        - pkg: "k8s.io/api/admission/v1"
+          alias: admissionv1
+        - pkg: k8s.io/api/apps/v1
+          alias: appsv1
+        - pkg: "k8s.io/api/autoscaling/v1"
+          alias: autoscalingv1
+        - pkg: k8s.io/api/batch/v1
+          alias: batchv1
+        - pkg: "k8s.io/api/core/v1"
+          alias: corev1
+        - pkg: k8s.io/api/policy/v1
+          alias: policyv1
+        - pkg: "k8s.io/apimachinery/pkg/apis/meta/v1"
+          alias: metav1
+        - pkg: k8s.io/client-go/kubernetes/typed/core/v1
+          alias: typedcorev1
+        - pkg: k8s.io/client-go/listers/core/v1
+          alias: listersv1
+
     revive:
       enable-all-rules: false # Disabling all rules for now and enabling each one later
       rules:
@@ -31,6 +56,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
 
@@ -65,6 +92,7 @@ linters:
         # Flags exported funcs returning unexported types; disabled for intentional encapsulation in internal APIs.
         - name: unexported-return
           disabled: true
+
 formatters:
   enable:
     - goimports

vertical-pod-autoscaler/common/flags.go

@@ -21,7 +21,7 @@ package common
 import (
 	"flag"
 
-	apiv1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/klog/v2"
 )
 
@@ -42,7 +42,7 @@ func InitCommonFlags() *CommonFlags {
 	flag.Float64Var(&cf.KubeApiQps, "kube-api-qps", 50.0, "QPS limit when making requests to Kubernetes apiserver")
 	flag.Float64Var(&cf.KubeApiBurst, "kube-api-burst", 100.0, "QPS burst limit when making requests to Kubernetes apiserver")
 	flag.BoolVar(&cf.EnableProfiling, "profiling", false, "Is debug/pprof endpoint enabled")
-	flag.StringVar(&cf.VpaObjectNamespace, "vpa-object-namespace", apiv1.NamespaceAll, "Specifies the namespace to search for VPA objects. Leave empty to include all namespaces. If provided, the garbage collector will only clean this namespace.")
+	flag.StringVar(&cf.VpaObjectNamespace, "vpa-object-namespace", corev1.NamespaceAll, "Specifies the namespace to search for VPA objects. Leave empty to include all namespaces. If provided, the garbage collector will only clean this namespace.")
 	flag.StringVar(&cf.IgnoredVpaObjectNamespaces, "ignored-vpa-object-namespaces", "", "A comma-separated list of namespaces to ignore when searching for VPA objects. Leave empty to avoid ignoring any namespaces. These namespaces will not be cleaned by the garbage collector.")
 	return cf
 }

vertical-pod-autoscaler/pkg/admission-controller/resource/handler.go

@@ -19,7 +19,7 @@ package resource
 import (
 	"context"
 
-	v1 "k8s.io/api/admission/v1"
+	admissionv1 "k8s.io/api/admission/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/metrics/admission"
@@ -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, *admissionv1.AdmissionRequest) ([]PatchRecord, error)
 }

vertical-pod-autoscaler/pkg/admission-controller/resource/pod/handler_test.go

@@ -24,8 +24,8 @@ 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"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 
 	resource_admission "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource"
@@ -38,15 +38,15 @@ type fakePodPreProcessor struct {
 	err error
 }
 
-func (fpp *fakePodPreProcessor) Process(pod apiv1.Pod) (apiv1.Pod, error) {
+func (fpp *fakePodPreProcessor) Process(pod corev1.Pod) (corev1.Pod, error) {
 	return pod, fpp.err
 }
 
 type fakeVpaMatcher struct {
 	vpa *vpa_types.VerticalPodAutoscaler
 }
 
-func (m *fakeVpaMatcher) GetMatchingVPA(_ context.Context, _ *apiv1.Pod) *vpa_types.VerticalPodAutoscaler {
+func (m *fakeVpaMatcher) GetMatchingVPA(_ context.Context, _ *corev1.Pod) *vpa_types.VerticalPodAutoscaler {
 	return m.vpa
 }
 
@@ -59,7 +59,7 @@ func (*fakePatchCalculator) PatchResourceTarget() patch.PatchResourceTarget {
 	return patch.Pod
 }
 
-func (c *fakePatchCalculator) CalculatePatches(_ *apiv1.Pod, _ *vpa_types.VerticalPodAutoscaler) (
+func (c *fakePatchCalculator) CalculatePatches(_ *corev1.Pod, _ *vpa_types.VerticalPodAutoscaler) (
 	[]resource_admission.PatchRecord, error) {
 	return c.patches, c.err
 }
@@ -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,

vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/calculator.go

@@ -17,7 +17,7 @@ limitations under the License.
 package patch
 
 import (
-	core "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 
 	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource"
 	vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1"
@@ -38,7 +38,7 @@ const (
 
 // Calculator is capable of calculating required patches for pod.
 type Calculator interface {
-	CalculatePatches(pod *core.Pod, vpa *vpa_types.VerticalPodAutoscaler) ([]resource.PatchRecord, error)
+	CalculatePatches(pod *corev1.Pod, vpa *vpa_types.VerticalPodAutoscaler) ([]resource.PatchRecord, error)
 	// PatchResourceTarget returns the resource this calculator should calculate patches for.
 	PatchResourceTarget() PatchResourceTarget
 }

vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/observed_containers.go

@@ -17,7 +17,7 @@ limitations under the License.
 package patch
 
 import (
-	core "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 
 	resource_admission "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource"
 	vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1"
@@ -26,7 +26,7 @@ import (
 
 type observedContainers struct{}
 
-func (*observedContainers) CalculatePatches(pod *core.Pod, _ *vpa_types.VerticalPodAutoscaler) ([]resource_admission.PatchRecord, error) {
+func (*observedContainers) CalculatePatches(pod *corev1.Pod, _ *vpa_types.VerticalPodAutoscaler) ([]resource_admission.PatchRecord, error) {
 	vpaObservedContainersValue := annotations.GetVpaObservedContainersValue(pod)
 	return []resource_admission.PatchRecord{GetAddAnnotationPatch(annotations.VpaObservedContainersLabel, vpaObservedContainersValue)}, nil
 }

vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/observed_containers_test.go

@@ -21,7 +21,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	core "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 
 	resource_admission "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource"
 	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/annotations"
@@ -38,7 +38,7 @@ func addVpaObservedContainersPatch(containerNames []string) resource_admission.P
 func TestCalculatePatches_ObservedContainers(t *testing.T) {
 	tests := []struct {
 		name          string
-		pod           *core.Pod
+		pod           *corev1.Pod
 		expectedPatch resource_admission.PatchRecord
 	}{
 		{

vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/resource_updates.go

@@ -21,7 +21,7 @@ import (
 	"fmt"
 	"strings"
 
-	core "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 
 	resource_admission "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource"
@@ -57,7 +57,7 @@ func (*resourcesUpdatesPatchCalculator) PatchResourceTarget() PatchResourceTarge
 	return Pod
 }
 
-func (c *resourcesUpdatesPatchCalculator) CalculatePatches(pod *core.Pod, vpa *vpa_types.VerticalPodAutoscaler) ([]resource_admission.PatchRecord, error) {
+func (c *resourcesUpdatesPatchCalculator) CalculatePatches(pod *corev1.Pod, vpa *vpa_types.VerticalPodAutoscaler) ([]resource_admission.PatchRecord, error) {
 	result := []resource_admission.PatchRecord{}
 
 	containersResources, annotationsPerContainer, err := c.recommendationProvider.GetContainersResourcesForPod(pod, vpa)
@@ -110,7 +110,7 @@ func (c *resourcesUpdatesPatchCalculator) CalculatePatches(pod *core.Pod, vpa *v
 	return result, nil
 }
 
-func getContainerPatch(pod *core.Pod, i int, annotationsPerContainer vpa_api_util.ContainerToAnnotationsMap, containerResources vpa_api_util.ContainerResources) ([]resource_admission.PatchRecord, string) {
+func getContainerPatch(pod *corev1.Pod, i int, annotationsPerContainer vpa_api_util.ContainerToAnnotationsMap, containerResources vpa_api_util.ContainerResources) ([]resource_admission.PatchRecord, string) {
 	var patches []resource_admission.PatchRecord
 	// Add empty resources object if missing.
 	requests, limits := resourcehelpers.ContainerRequestsAndLimits(pod.Spec.Containers[i].Name, pod)
@@ -130,7 +130,7 @@ func getContainerPatch(pod *core.Pod, i int, annotationsPerContainer vpa_api_uti
 	return patches, updatesAnnotation
 }
 
-func appendPatchesAndAnnotations(patches []resource_admission.PatchRecord, annotations []string, current core.ResourceList, containerIndex int, resources core.ResourceList, fieldName, resourceName string) ([]resource_admission.PatchRecord, []string) {
+func appendPatchesAndAnnotations(patches []resource_admission.PatchRecord, annotations []string, current corev1.ResourceList, containerIndex int, resources corev1.ResourceList, fieldName, resourceName string) ([]resource_admission.PatchRecord, []string) {
 	// Add empty object if it's missing and we're about to fill it.
 	if current == nil && len(resources) > 0 {
 		patches = append(patches, GetPatchInitializingEmptyResourcesSubfield(containerIndex, fieldName))
@@ -142,7 +142,7 @@ func appendPatchesAndAnnotations(patches []resource_admission.PatchRecord, annot
 	return patches, annotations
 }
 
-func (c *resourcesUpdatesPatchCalculator) applyCPUStartupBoost(container *core.Container, vpa *vpa_types.VerticalPodAutoscaler, containerResources *vpa_api_util.ContainerResources) ([]resource_admission.PatchRecord, error) {
+func (c *resourcesUpdatesPatchCalculator) applyCPUStartupBoost(container *corev1.Container, vpa *vpa_types.VerticalPodAutoscaler, containerResources *vpa_api_util.ContainerResources) ([]resource_admission.PatchRecord, error) {
 	var patches []resource_admission.PatchRecord
 
 	startupBoostPolicy := getContainerStartupBoostPolicy(container, vpa)
@@ -164,7 +164,7 @@ func (c *resourcesUpdatesPatchCalculator) applyCPUStartupBoost(container *core.C
 	return patches, nil
 }
 
-func getContainerStartupBoostPolicy(container *core.Container, vpa *vpa_types.VerticalPodAutoscaler) *vpa_types.StartupBoost {
+func getContainerStartupBoostPolicy(container *corev1.Container, vpa *vpa_types.VerticalPodAutoscaler) *vpa_types.StartupBoost {
 	policy := vpa_api_util.GetContainerResourcePolicy(container.Name, vpa.Spec.ResourcePolicy)
 	startupBoost := vpa.Spec.StartupBoost
 	if policy != nil && policy.StartupBoost != nil {
@@ -226,52 +226,52 @@ func (c *resourcesUpdatesPatchCalculator) calculateBoostedCPU(recommendedCPU, or
 
 // capStartupBoostToContainerLimit makes sure startup boost recommendation is not above current limit for the container for CPU.
 // It attempts to keep the request 1m below the limit to maintain QoS.
-func capStartupBoostToContainerLimit(recommendation core.ResourceList, containerLimits core.ResourceList) {
-	limit, found := containerLimits[core.ResourceCPU]
+func capStartupBoostToContainerLimit(recommendation corev1.ResourceList, containerLimits corev1.ResourceList) {
+	limit, found := containerLimits[corev1.ResourceCPU]
 	if !found {
 		return
 	}
 
-	recommendedValue, found := recommendation[core.ResourceCPU]
+	recommendedValue, found := recommendation[corev1.ResourceCPU]
 	if found && recommendedValue.MilliValue() > limit.MilliValue() {
 		newRecommended := limit.DeepCopy()
 		if limit.Cmp(resource.MustParse("1m")) > 0 {
 			newRecommended.Sub(resource.MustParse("1m"))
 		}
-		recommendation[core.ResourceCPU] = newRecommended
+		recommendation[corev1.ResourceCPU] = newRecommended
 	}
 }
 
-func (c *resourcesUpdatesPatchCalculator) applyControlledCPUResources(container *core.Container, vpa *vpa_types.VerticalPodAutoscaler, containerResources *vpa_api_util.ContainerResources, startupBoostPolicy *vpa_types.StartupBoost) error {
+func (c *resourcesUpdatesPatchCalculator) applyControlledCPUResources(container *corev1.Container, vpa *vpa_types.VerticalPodAutoscaler, containerResources *vpa_api_util.ContainerResources, startupBoostPolicy *vpa_types.StartupBoost) error {
 	controlledValues := vpa_api_util.GetContainerControlledValues(container.Name, vpa.Spec.ResourcePolicy)
 
-	recommendedRequest := containerResources.Requests[core.ResourceCPU]
-	originalRequest := container.Resources.Requests[core.ResourceCPU]
+	recommendedRequest := containerResources.Requests[corev1.ResourceCPU]
+	originalRequest := container.Resources.Requests[corev1.ResourceCPU]
 	boostedRequest, err := c.calculateBoostedCPU(recommendedRequest, originalRequest, startupBoostPolicy)
 	if err != nil {
 		return err
 	}
 
 	if containerResources.Requests == nil {
-		containerResources.Requests = core.ResourceList{}
+		containerResources.Requests = corev1.ResourceList{}
 	}
-	containerResources.Requests[core.ResourceCPU] = *boostedRequest
+	containerResources.Requests[corev1.ResourceCPU] = *boostedRequest
 
 	switch controlledValues {
 	case vpa_types.ContainerControlledValuesRequestsOnly:
 		capStartupBoostToContainerLimit(containerResources.Requests, container.Resources.Limits)
 	case vpa_types.ContainerControlledValuesRequestsAndLimits:
 		if containerResources.Limits == nil {
-			containerResources.Limits = core.ResourceList{}
+			containerResources.Limits = corev1.ResourceList{}
 		}
 		newLimits, _ := vpa_api_util.GetProportionalLimit(
-			container.Resources.Limits,                           // originalLimits
-			container.Resources.Requests,                         // originalRequests
-			core.ResourceList{core.ResourceCPU: *boostedRequest}, // newRequests
-			core.ResourceList{},                                  // defaultLimit
+			container.Resources.Limits,                               // originalLimits
+			container.Resources.Requests,                             // originalRequests
+			corev1.ResourceList{corev1.ResourceCPU: *boostedRequest}, // newRequests
+			corev1.ResourceList{},                                    // defaultLimit
 		)
-		if newLimit, ok := newLimits[core.ResourceCPU]; ok {
-			containerResources.Limits[core.ResourceCPU] = newLimit
+		if newLimit, ok := newLimits[corev1.ResourceCPU]; ok {
+			containerResources.Limits[corev1.ResourceCPU] = newLimit
 		}
 	default:
 		// Do nothing