Merge pull request #6712 from voelzmo/fix/vpa-updater-npe-ownerref-check

Fix NPE in vpa-updater when Pod owner isn't scalable

vertical-pod-autoscaler/pkg/target/controller_fetcher/controller_fetcher_test.go

@@ -389,6 +389,16 @@ func TestControllerFetcher(t *testing.T) {
 			expectedKey:   nil,
 			expectedError: fmt.Errorf("Unhandled targetRef v1 / Node / node, last error node is not a valid owner"),
 		},
+		{
+			name: "custom resource with no scale subresource",
+			key: &ControllerKeyWithAPIVersion{
+				ApiVersion: "Foo/Foo", ControllerKey: ControllerKey{
+					Name: "bah", Kind: "Foo", Namespace: testNamespace},
+			},
+			objects:       []runtime.Object{},
+			expectedKey:   nil, // Pod owner does not support scale subresource so should return nil"
+			expectedError: nil,
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			f := simpleControllerFetcher()

vertical-pod-autoscaler/pkg/utils/vpa/api.go

@@ -153,6 +153,9 @@ func GetControllingVPAForPod(pod *core.Pod, vpas []*VpaWithSelector, ctrlFetcher
 		klog.Errorf("fail to get pod controller: pod=%s err=%s", pod.Name, err.Error())
 		return nil
 	}
+	if parentController == nil {
+		return nil
+	}
 
 	var controlling *VpaWithSelector
 	var controllingVpa *vpa_types.VerticalPodAutoscaler

vertical-pod-autoscaler/pkg/utils/vpa/api_test.go

@@ -139,6 +139,15 @@ func TestPodMatchesVPA(t *testing.T) {
 	}
 }
 
+type NilControllerFetcher struct{}
+
+// FindTopMostWellKnownOrScalable returns the same key for that fake implementation
+func (f NilControllerFetcher) FindTopMostWellKnownOrScalable(_ *controllerfetcher.ControllerKeyWithAPIVersion) (*controllerfetcher.ControllerKeyWithAPIVersion, error) {
+	return nil, nil
+}
+
+var _ controllerfetcher.ControllerFetcher = &NilControllerFetcher{}
+
 func TestGetControllingVPAForPod(t *testing.T) {
 	isController := true
 	pod := test.Pod().WithName("test-pod").AddContainer(test.Container().WithName(containerName).WithCPURequest(resource.MustParse("1")).WithMemRequest(resource.MustParse("100M")).Get()).Get()
@@ -171,6 +180,12 @@ func TestGetControllingVPAForPod(t *testing.T) {
 		{nonMatchingVPA, parseLabelSelector("app = other")},
 	}, &controllerfetcher.FakeControllerFetcher{})
 	assert.Equal(t, vpaA, chosen.Vpa)
+
+	// For some Pods (which are *not* under VPA), controllerFetcher.FindTopMostWellKnownOrScalable will return `nil`, e.g. when the Pod owner is a custom resource, which doesn't implement the /scale subresource
+	// See pkg/target/controller_fetcher/controller_fetcher_test.go:393 for testing this behavior
+	// This test case makes sure that GetControllingVPAForPod will just return `nil` in that case as well
+	chosen = GetControllingVPAForPod(pod, []*VpaWithSelector{{vpaA, parseLabelSelector("app = testingApp")}}, &NilControllerFetcher{})
+	assert.Nil(t, chosen)
 }
 
 func TestGetContainerResourcePolicy(t *testing.T) {