clean up

knative · Oct 4, 2024 · a302d2f · a302d2f
1 parent 0710696
commit a302d2f
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 32 deletions.
diff --git a/pkg/reconciler/autoscaling/kpa/kpa.go b/pkg/reconciler/autoscaling/kpa/kpa.go
@@ -122,7 +122,7 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pa *autoscalingv1alpha1.
 
 	// Get the appropriate current scale from the metric, and right size
 	// the scaleTargetRef based on it.
-	want, err := c.scaler.scale(ctx, pa, sks, decider.Status.DesiredScale, c.Client, &podCounter)
+	want, err := c.scaler.scale(ctx, pa, sks, decider.Status.DesiredScale, &podCounter)
 	if err != nil {
 		return fmt.Errorf("error scaling target: %w", err)
 	}

diff --git a/pkg/reconciler/autoscaling/kpa/kpa_test.go b/pkg/reconciler/autoscaling/kpa/kpa_test.go
@@ -40,7 +40,6 @@ import (
 	fakepainformer "knative.dev/serving/pkg/client/injection/informers/autoscaling/v1alpha1/podautoscaler/fake"
 	fakerevisioninformer "knative.dev/serving/pkg/client/injection/informers/serving/v1/revision/fake"
 	"knative.dev/serving/pkg/metrics"
-	pkgtesting "knative.dev/serving/pkg/testing"
 
 	networkingclient "knative.dev/networking/pkg/client/injection/client"
 	filteredinformerfactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered"
@@ -295,7 +294,7 @@ func TestReconcile(t *testing.T) {
 	}
 	activeKPAMinScale := func(g, w int32) *autoscalingv1alpha1.PodAutoscaler {
 		return kpa(
-			testNamespace, testRevision, WithPASKSReady, WithTraffic, markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures,
+			testNamespace, testRevision, WithPASKSReady, WithTraffic, markScaleTargetInitialized, WithScaleTargetNoScaleFailures,
 			withScales(g, w), WithReachabilityReachable,
 			withMinScale(defaultScale), WithPAStatusService(testRevision), WithPAMetricsService(privateSvc),
 			WithObservedGeneration(1),
@@ -333,7 +332,7 @@ func TestReconcile(t *testing.T) {
 		Name: "steady state",
 		Key:  key,
 		Objects: []runtime.Object{
-			kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic, pkgtesting.WithScaleTargetNoScaleFailures,
+			kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic, WithScaleTargetNoScaleFailures,
 				markScaleTargetInitialized, WithPAMetricsService(privateSvc),
 				withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)),
 			defaultSKS,
@@ -359,12 +358,12 @@ func TestReconcile(t *testing.T) {
 		},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: kpa(testNamespace, testRevision, WithTraffic,
-				markScaleTargetInitialized, WithPASKSReady, pkgtesting.WithScaleTargetNoScaleFailures,
+				markScaleTargetInitialized, WithPASKSReady, WithScaleTargetNoScaleFailures,
 				WithPAMetricsService(privateSvc), withScales(1, defaultScale),
 				WithPAStatusService(testRevision), WithObservedGeneration(1)),
 		}, {
 			Object: kpa(testNamespace, testRevision, WithTraffic,
-				markScaleTargetInitialized, WithPASKSReady, pkgtesting.WithScaleTargetNoScaleFailures,
+				markScaleTargetInitialized, WithPASKSReady, WithScaleTargetNoScaleFailures,
 				WithPAMetricsService(privateSvc), withScales(1, defaultScale),
 				WithPAStatusService(testRevision), WithObservedGeneration(1)),
 		}},
@@ -411,11 +410,11 @@ func TestReconcile(t *testing.T) {
 		Name: "create metric",
 		Key:  key,
 		Objects: []runtime.Object{
-			kpa(testNamespace, testRevision, WithTraffic, markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures,
+			kpa(testNamespace, testRevision, WithTraffic, markScaleTargetInitialized, WithScaleTargetNoScaleFailures,
 				withScales(1, defaultScale), WithPAStatusService(testRevision)),
 			defaultSKS, defaultDeployment, defaultReady},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
-			Object: kpa(testNamespace, testRevision, WithTraffic, markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures,
+			Object: kpa(testNamespace, testRevision, WithTraffic, markScaleTargetInitialized, WithScaleTargetNoScaleFailures,
 				withScales(1, defaultScale), WithPASKSReady, WithPAStatusService(testRevision),
 				WithPAMetricsService(privateSvc), WithObservedGeneration(1)),
 		}},
@@ -426,7 +425,7 @@ func TestReconcile(t *testing.T) {
 		Name: "scale up deployment",
 		Key:  key,
 		Objects: []runtime.Object{
-			kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic, markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures,
+			kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic, markScaleTargetInitialized, WithScaleTargetNoScaleFailures,
 				WithPAMetricsService(privateSvc), withScales(1, defaultScale), WithPAStatusService(testRevision),
 				WithObservedGeneration(1)),
 			defaultSKS,
@@ -496,7 +495,7 @@ func TestReconcile(t *testing.T) {
 			preciseReady...),
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: kpa(testNamespace, testRevision, withScales(defaultScale, defaultScale),
-				WithPASKSNotReady(""), WithTraffic, markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures,
+				WithPASKSNotReady(""), WithTraffic, markScaleTargetInitialized, WithScaleTargetNoScaleFailures,
 				WithPAMetricsService(privateSvc), WithPAStatusService(testRevision), WithObservedGeneration(1)),
 		}},
 	}, {
@@ -556,7 +555,7 @@ func TestReconcile(t *testing.T) {
 			defaultDeployment, defaultReady},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: kpa(testNamespace, testRevision, WithPASKSReady,
-				WithTraffic, markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures, withMinScale(2), WithPAMetricsService(privateSvc),
+				WithTraffic, markScaleTargetInitialized, WithScaleTargetNoScaleFailures, withMinScale(2), WithPAMetricsService(privateSvc),
 				withScales(1, defaultScale), WithPAStatusService(testRevision), WithReachabilityUnreachable,
 				WithObservedGeneration(1)),
 		}},
@@ -572,7 +571,7 @@ func TestReconcile(t *testing.T) {
 		}, makeReadyPods(2, testNamespace, testRevision)...),
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: kpa(testNamespace, testRevision, WithPASKSReady,
-				WithTraffic, markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures, withMinScale(2), WithPAMetricsService(privateSvc),
+				WithTraffic, markScaleTargetInitialized, WithScaleTargetNoScaleFailures, withMinScale(2), WithPAMetricsService(privateSvc),
 				withScales(2, defaultScale), WithPAStatusService(testRevision), WithReachabilityReachable,
 				WithObservedGeneration(1)),
 		}},
@@ -588,7 +587,7 @@ func TestReconcile(t *testing.T) {
 		}, makeReadyPods(2, testNamespace, testRevision)...),
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: kpa(testNamespace, testRevision, WithPASKSReady,
-				WithTraffic, markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures, withMinScale(2), WithPAMetricsService(privateSvc),
+				WithTraffic, markScaleTargetInitialized, WithScaleTargetNoScaleFailures, withMinScale(2), WithPAMetricsService(privateSvc),
 				withScales(2, defaultScale), WithPAStatusService(testRevision), WithReachabilityUnknown,
 				WithObservedGeneration(1)),
 		}},
@@ -750,7 +749,7 @@ func TestReconcile(t *testing.T) {
 			metric(testNamespace, testRevision),
 			deploy(testNamespace, testRevision), defaultReady},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
-			Object: kpa(testNamespace, testRevision, markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures, withScales(1, 0),
+			Object: kpa(testNamespace, testRevision, markScaleTargetInitialized, WithScaleTargetNoScaleFailures, withScales(1, 0),
 				WithPASKSReady, WithPAMetricsService(privateSvc),
 				WithNoTraffic(noTrafficReason, "The target is not receiving traffic."),
 				WithPAStatusService(testRevision), WithPAMetricsService(privateSvc),
@@ -766,7 +765,7 @@ func TestReconcile(t *testing.T) {
 			decider(testNamespace, testRevision, 0 /* desiredScale */, 0 /* ebc */)),
 		Objects: []runtime.Object{
 			kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic,
-				markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures, withScales(1, 1),
+				markScaleTargetInitialized, WithScaleTargetNoScaleFailures, withScales(1, 1),
 				WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), WithObservedGeneration(1)),
 			defaultSKS,
 			metric(testNamespace, testRevision),
@@ -784,7 +783,7 @@ func TestReconcile(t *testing.T) {
 			metric(testNamespace, testRevision),
 			deploy(testNamespace, testRevision), defaultReady},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
-			Object: kpa(testNamespace, testRevision, markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures, WithPASKSReady, WithPAMetricsService(privateSvc),
+			Object: kpa(testNamespace, testRevision, markScaleTargetInitialized, WithScaleTargetNoScaleFailures, WithPASKSReady, WithPAMetricsService(privateSvc),
 				WithNoTraffic("TimedOut", "The target could not be activated."), withScales(1, 0),
 				WithPAStatusService(testRevision), WithPAMetricsService(privateSvc),
 				WithObservedGeneration(1)),
@@ -860,7 +859,7 @@ func TestReconcile(t *testing.T) {
 			minScalePatch,
 		},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
-			Object: activatingKPAMinScale(underscale, markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures, WithPASKSReady),
+			Object: activatingKPAMinScale(underscale, markScaleTargetInitialized, WithScaleTargetNoScaleFailures, WithPASKSReady),
 		}},
 	}, {
 		// Scale to `minScale` and mark PA "active"
@@ -955,7 +954,7 @@ func TestReconcile(t *testing.T) {
 				-42 /* ebc */)),
 		Objects: []runtime.Object{
 			kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic,
-				markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures, WithPAMetricsService(privateSvc),
+				markScaleTargetInitialized, WithScaleTargetNoScaleFailures, WithPAMetricsService(privateSvc),
 				withScales(1, defaultScale), WithPAStatusService(testRevision), WithObservedGeneration(1)),
 			defaultProxySKS,
 			metric(testNamespace, testRevision),
@@ -967,7 +966,7 @@ func TestReconcile(t *testing.T) {
 			decider(testNamespace, testRevision, defaultScale, /* desiredScale */
 				-18 /* ebc */)),
 		Objects: []runtime.Object{
-			kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic, markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures,
+			kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic, markScaleTargetInitialized, WithScaleTargetNoScaleFailures,
 				WithPAMetricsService(privateSvc), withScales(1, defaultScale),
 				WithPAStatusService(testRevision), WithObservedGeneration(1)),
 			sks(testNamespace, testRevision, WithDeployRef(deployName), WithSKSReady,
@@ -985,7 +984,7 @@ func TestReconcile(t *testing.T) {
 			decider(testNamespace, testRevision, defaultScale, /* desiredScale */
 				1 /* ebc */)),
 		Objects: []runtime.Object{
-			kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic, markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures,
+			kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic, markScaleTargetInitialized, WithScaleTargetNoScaleFailures,
 				WithPAMetricsService(privateSvc), withScales(1, defaultScale),
 				WithPAStatusService(testRevision), WithObservedGeneration(1)),
 			defaultProxySKS,
@@ -1039,7 +1038,7 @@ func TestReconcile(t *testing.T) {
 		}, makeReadyPods(20, testNamespace, testRevision)...),
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic,
-				markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures, withScales(20, 20), withInitialScale(20), WithReachabilityReachable,
+				markScaleTargetInitialized, WithScaleTargetNoScaleFailures, withScales(20, 20), withInitialScale(20), WithReachabilityReachable,
 				WithPAStatusService(testRevision), WithPAMetricsService(privateSvc), WithObservedGeneration(1),
 			),
 		}},
@@ -1065,7 +1064,7 @@ func TestReconcile(t *testing.T) {
 			}),
 		},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
-			Object: kpa(testNamespace, testRevision, markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures,
+			Object: kpa(testNamespace, testRevision, markScaleTargetInitialized, WithScaleTargetNoScaleFailures,
 				WithNoTraffic(noTrafficReason, "The target is not receiving traffic."),
 				withScales(0, -1), WithReachabilityReachable,
 				WithPAMetricsService(privateSvc), WithObservedGeneration(1),
@@ -1135,7 +1134,7 @@ func TestReconcile(t *testing.T) {
 			}),
 		}, makeReadyPods(2, testNamespace, testRevision)...),
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
-			Object: kpa(testNamespace, testRevision, WithTraffic, WithPASKSReady, markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures,
+			Object: kpa(testNamespace, testRevision, WithTraffic, WithPASKSReady, markScaleTargetInitialized, WithScaleTargetNoScaleFailures,
 				withScales(2, 2), WithReachabilityReachable, WithPAStatusService(testRevision),
 				WithPAMetricsService(privateSvc), WithObservedGeneration(1),
 			),
@@ -1147,7 +1146,7 @@ func TestReconcile(t *testing.T) {
 			decider(testNamespace, testRevision, defaultScale, /* desiredScale */
 				1 /* ebc */)),
 		Objects: []runtime.Object{
-			kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic, markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures,
+			kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic, markScaleTargetInitialized, WithScaleTargetNoScaleFailures,
 				WithPAMetricsService(privateSvc), withScales(1, defaultScale),
 				WithPAStatusService(testRevision), WithObservedGeneration(1)),
 			defaultProxySKS,
@@ -1162,7 +1161,7 @@ func TestReconcile(t *testing.T) {
 			decider(testNamespace, testRevision, defaultScale, /* desiredScale */
 				1 /* ebc */)),
 		Objects: []runtime.Object{
-			kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic, markScaleTargetInitialized, pkgtesting.WithScaleTargetNoScaleFailures,
+			kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic, markScaleTargetInitialized, WithScaleTargetNoScaleFailures,
 				WithPAMetricsService(privateSvc), withScales(1, defaultScale),
 				WithPAStatusService(testRevision), WithObservedGeneration(1)),
 			defaultSKS,

diff --git a/pkg/reconciler/autoscaling/kpa/scaler.go b/pkg/reconciler/autoscaling/kpa/scaler.go
@@ -38,7 +38,6 @@ import (
 	"knative.dev/serving/pkg/activator"
 	autoscalingv1alpha1 "knative.dev/serving/pkg/apis/autoscaling/v1alpha1"
 	"knative.dev/serving/pkg/autoscaler/config/autoscalerconfig"
-	clientset "knative.dev/serving/pkg/client/clientset/versioned"
 	"knative.dev/serving/pkg/reconciler/autoscaling/config"
 	kparesources "knative.dev/serving/pkg/reconciler/autoscaling/kpa/resources"
 	aresources "knative.dev/serving/pkg/reconciler/autoscaling/resources"
@@ -332,7 +331,7 @@ func (ks *scaler) applyScale(ctx context.Context, pa *autoscalingv1alpha1.PodAut
 }
 
 // scale attempts to scale the given PA's target reference to the desired scale.
-func (ks *scaler) scale(ctx context.Context, pa *autoscalingv1alpha1.PodAutoscaler, sks *netv1alpha1.ServerlessService, desiredScale int32, client clientset.Interface, podCounter *resources.PodAccessor) (int32, error) {
+func (ks *scaler) scale(ctx context.Context, pa *autoscalingv1alpha1.PodAutoscaler, sks *netv1alpha1.ServerlessService, desiredScale int32, podCounter *resources.PodAccessor) (int32, error) {
 	asConfig := config.FromContext(ctx).Autoscaler
 	logger := logging.FromContext(ctx)
 
@@ -410,10 +409,9 @@ func checkForPodErrorsBeforeScalingDown(logger *zap.SugaredLogger, pod *corev1.P
 						pa.Status.MarkWithScaleFailures(v1.ExitCodeReason(t.ExitCode),
 							v1.RevisionContainerExitingMessage("container exited with no error"))
 						break
-					} else {
-						pa.Status.MarkWithScaleFailures(v1.ExitCodeReason(t.ExitCode), v1.RevisionContainerExitingMessage(t.Message))
-						break
 					}
+					pa.Status.MarkWithScaleFailures(v1.ExitCodeReason(t.ExitCode), v1.RevisionContainerExitingMessage(t.Message))
+					break
 				} else if w := status.State.Waiting; w != nil {
 					logger.Debugf("marking pa as failed: %s: %s", w.Reason, w.Message)
 					pa.Status.MarkWithScaleFailures(w.Reason, w.Message)

diff --git a/pkg/reconciler/autoscaling/kpa/scaler_test.go b/pkg/reconciler/autoscaling/kpa/scaler_test.go
@@ -556,7 +556,7 @@ func TestScaler(t *testing.T) {
 				test.configMutator(cfg)
 			}
 			ctx = config.ToContext(ctx, cfg)
-			desiredScale, err := revisionScaler.scale(ctx, pa, sks, test.scaleTo, fakeservingclient.Get(ctx), nil)
+			desiredScale, err := revisionScaler.scale(ctx, pa, sks, test.scaleTo, nil)
 			if err != nil {
 				t.Error("Scale got an unexpected error:", err)
 			}
@@ -647,7 +647,7 @@ func TestDisableScaleToZero(t *testing.T) {
 			conf := defaultConfig()
 			conf.Autoscaler.EnableScaleToZero = false
 			ctx = config.ToContext(ctx, conf)
-			desiredScale, err := revisionScaler.scale(ctx, pa, nil /*sks doesn't matter in this test*/, test.scaleTo, fakeservingclient.Get(ctx), nil)
+			desiredScale, err := revisionScaler.scale(ctx, pa, nil /*sks doesn't matter in this test*/, test.scaleTo, nil)
 
 			if err != nil {
 				t.Error("Scale got an unexpected error:", err)