Merge pull request kubernetes#3498 from jbartosik/no-fetch-nodes

Don't fetch nodes

Author: k8s-ci-robot

vertical-pod-autoscaler/pkg/recommender/input/controller_fetcher/controller_fetcher.go

@@ -45,13 +45,14 @@ import (
 type wellKnownController string
 
 const (
+	cronJob               wellKnownController = "CronJob"
 	daemonSet             wellKnownController = "DaemonSet"
 	deployment            wellKnownController = "Deployment"
+	node                  wellKnownController = "Node"
+	job                   wellKnownController = "Job"
 	replicaSet            wellKnownController = "ReplicaSet"
-	statefulSet           wellKnownController = "StatefulSet"
 	replicationController wellKnownController = "ReplicationController"
-	job                   wellKnownController = "Job"
-	cronJob               wellKnownController = "CronJob"
+	statefulSet           wellKnownController = "StatefulSet"
 )
 
 const (
@@ -251,6 +252,9 @@ func (f *controllerFetcher) isWellKnownOrScalable(key *ControllerKeyWithAPIVersi
 	if f.isWellKnown(key) {
 		return true
 	}
+	if gk, err := key.groupKind(); err != nil && wellKnownController(gk.Kind) == node {
+		return false
+	}
 
 	//if not well known check if it supports scaling
 	groupKind, err := key.groupKind()
@@ -276,11 +280,16 @@ func (f *controllerFetcher) isWellKnownOrScalable(key *ControllerKeyWithAPIVersi
 }
 
 func (f *controllerFetcher) getOwnerForScaleResource(groupKind schema.GroupKind, namespace, name string) (*ControllerKeyWithAPIVersion, error) {
+	if wellKnownController(groupKind.Kind) == node {
+		// Some pods specify nods as their owners. This causes performance problems
+		// in big clusters when VPA tries to get all nodes. We know nodes aren't
+		// valid controllers so we can skip trying to fetch them.
+		return nil, fmt.Errorf("node is not a valid owner")
+	}
 	mappings, err := f.mapper.RESTMappings(groupKind)
 	if err != nil {
 		return nil, err
 	}
-
 	var lastError error
 	for _, mapping := range mappings {
 		groupResource := mapping.Resource.GroupResource()

vertical-pod-autoscaler/pkg/recommender/input/controller_fetcher/controller_fetcher_test.go

@@ -107,25 +107,29 @@ func addController(controller *controllerFetcher, obj runtime.Object) {
 
 func TestControllerFetcher(t *testing.T) {
 	type testCase struct {
+		name          string
 		apiVersion    string
 		key           *ControllerKeyWithAPIVersion
 		objects       []runtime.Object
 		expectedKey   *ControllerKeyWithAPIVersion
 		expectedError error
 	}
-	for i, tc := range []testCase{
+	for _, tc := range []testCase{
 		{
+			name:          "nils",
 			key:           nil,
 			expectedKey:   nil,
 			expectedError: nil,
 		},
 		{
+			name: "deployment doesn't exist",
 			key: &ControllerKeyWithAPIVersion{ControllerKey: ControllerKey{
 				Name: "test-deployment", Kind: "Deployment", Namespace: "test-namesapce"}},
 			expectedKey:   nil,
 			expectedError: fmt.Errorf("Deployment test-namesapce/test-deployment does not exist"),
 		},
 		{
+			name: "deployment no parent",
 			key: &ControllerKeyWithAPIVersion{ControllerKey: ControllerKey{
 				Name: "test-deployment", Kind: "Deployment", Namespace: "test-namesapce"}},
 			objects: []runtime.Object{&appsv1.Deployment{
@@ -142,6 +146,7 @@ func TestControllerFetcher(t *testing.T) {
 			expectedError: nil,
 		},
 		{
+			name: "deployment with parent",
 			key: &ControllerKeyWithAPIVersion{ControllerKey: ControllerKey{
 				Name: "test-rs", Kind: "ReplicaSet", Namespace: "test-namesapce"}},
 			objects: []runtime.Object{&appsv1.Deployment{
@@ -173,6 +178,7 @@ func TestControllerFetcher(t *testing.T) {
 			expectedError: nil,
 		},
 		{
+			name: "StatefulSet",
 			key: &ControllerKeyWithAPIVersion{ControllerKey: ControllerKey{
 				Name: "test-statefulset", Kind: "StatefulSet", Namespace: "test-namesapce"}},
 			objects: []runtime.Object{&appsv1.StatefulSet{
@@ -189,6 +195,7 @@ func TestControllerFetcher(t *testing.T) {
 			expectedError: nil,
 		},
 		{
+			name: "DaemonSet",
 			key: &ControllerKeyWithAPIVersion{ControllerKey: ControllerKey{
 				Name: "test-daemonset", Kind: "DaemonSet", Namespace: "test-namesapce"}},
 			objects: []runtime.Object{&appsv1.DaemonSet{
@@ -205,6 +212,7 @@ func TestControllerFetcher(t *testing.T) {
 			expectedError: nil,
 		},
 		{
+			name: "CronJob",
 			key: &ControllerKeyWithAPIVersion{ControllerKey: ControllerKey{
 				Name: "test-job", Kind: "Job", Namespace: "test-namespace"}},
 			objects: []runtime.Object{&batchv1.Job{
@@ -236,6 +244,7 @@ func TestControllerFetcher(t *testing.T) {
 			expectedError: nil,
 		},
 		{
+			name: "CronJob no parent",
 			key: &ControllerKeyWithAPIVersion{ControllerKey: ControllerKey{
 				Name: "test-cronjob", Kind: "CronJob", Namespace: "test-namespace"}},
 			objects: []runtime.Object{&batchv1beta1.CronJob{
@@ -252,6 +261,7 @@ func TestControllerFetcher(t *testing.T) {
 			expectedError: nil,
 		},
 		{
+			name: "rc no parent",
 			key: &ControllerKeyWithAPIVersion{ControllerKey: ControllerKey{
 				Name: "test-rc", Kind: "ReplicationController", Namespace: "test-namesapce"}},
 			objects: []runtime.Object{&corev1.ReplicationController{
@@ -268,6 +278,7 @@ func TestControllerFetcher(t *testing.T) {
 			expectedError: nil,
 		},
 		{
+			name: "deployment cycle in ownership",
 			key: &ControllerKeyWithAPIVersion{ControllerKey: ControllerKey{
 				Name: "test-deployment", Kind: "Deployment", Namespace: "test-namesapce"}},
 			objects: []runtime.Object{&appsv1.Deployment{
@@ -291,6 +302,7 @@ func TestControllerFetcher(t *testing.T) {
 			expectedError: fmt.Errorf("Cycle detected in ownership chain"),
 		},
 		{
+			name: "deployment, parent with no scale subresource",
 			key: &ControllerKeyWithAPIVersion{ControllerKey: ControllerKey{
 				Name: "test-deployment", Kind: "Deployment", Namespace: "test-namesapce"}},
 			objects: []runtime.Object{&appsv1.Deployment{
@@ -316,6 +328,7 @@ func TestControllerFetcher(t *testing.T) {
 			expectedError: nil,
 		},
 		{
+			name: "deployment, parent not well known with scale subresource",
 			key: &ControllerKeyWithAPIVersion{ControllerKey: ControllerKey{
 				Name: "test-deployment", Kind: "Deployment", Namespace: "test-namesapce"}},
 			objects: []runtime.Object{&appsv1.Deployment{
@@ -340,8 +353,33 @@ func TestControllerFetcher(t *testing.T) {
 				Name: "iCanScale", Kind: "Scale", Namespace: "test-namesapce"}, ApiVersion: "Foo/Foo"}, // Parent supports scale subresource"
 			expectedError: nil,
 		},
+		{
+			name: "pod, parent is node",
+			key: &ControllerKeyWithAPIVersion{ControllerKey: ControllerKey{
+				Name: "test-deployment", Kind: "Deployment", Namespace: "test-namesapce"}},
+			objects: []runtime.Object{&appsv1.Deployment{
+				TypeMeta: metav1.TypeMeta{
+					Kind: "Deployment",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-deployment",
+					Namespace: "test-namesapce",
+					// Parent is a node
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							APIVersion: "v1",
+							Controller: &trueVar,
+							Kind:       "Node",
+							Name:       "node",
+						},
+					},
+				},
+			}},
+			expectedKey:   nil,
+			expectedError: fmt.Errorf("Unhandled targetRef v1 / Node / node, last error node is not a valid owner"),
+		},
 	} {
-		t.Run(fmt.Sprintf("test case %d", i), func(t *testing.T) {
+		t.Run(tc.name, func(t *testing.T) {
 			f := simpleControllerFetcher()
 			for _, obj := range tc.objects {
 				addController(f, obj)