Repository: knative/serving
Pull request #9246 (2020-09-01 03:25:15)

Disable TestInitScaleZero

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/serving/pull/9246#" title="Author self-approved">mattmoor</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fserving).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[test/OWNERS](https://github.com/knative/serving/blob/master/test/OWNERS)~~ [mattmoor]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
vagababov: /lgtm

/Cc @taragu

Patch:
diff --git a/pkg/reconciler/route/resources/labels/labels.go b/pkg/reconciler/route/resources/labels/labels.go
index 3a77f6017..c6cbeee38 100644
--- a/pkg/reconciler/route/resources/labels/labels.go
+++ b/pkg/reconciler/route/resources/labels/labels.go
@@ -22,8 +22,8 @@ import (
 )
 
 // IsObjectLocalVisibility returns whether an ObjectMeta is of cluster-local visibility
-func IsObjectLocalVisibility(meta v1.ObjectMeta) bool {
-	return meta.Labels != nil && meta.Labels[serving.VisibilityLabelKey] != ""
+func IsObjectLocalVisibility(meta *v1.ObjectMeta) bool {
+	return meta.Labels[serving.VisibilityLabelKey] != ""
 }
 
 // SetVisibility sets the visibility on an ObjectMeta
diff --git a/pkg/reconciler/route/traffic/traffic.go b/pkg/reconciler/route/traffic/traffic.go
index c4389dcdc..9f695f6fa 100644
--- a/pkg/reconciler/route/traffic/traffic.go
+++ b/pkg/reconciler/route/traffic/traffic.go
@@ -116,7 +116,7 @@ func (t *Config) GetRevisionTrafficTargets(ctx context.Context, r *v1.Route) ([]
 
 			labels.SetVisibility(meta, t.Visibility[tt.Tag] == netv1alpha1.IngressVisibilityClusterLocal)
 
-			// http is currently the only supported scheme
+			// HTTP is currently the only supported scheme.
 			fullDomain, err := domains.DomainNameFromTemplate(ctx, *meta, hostname)
 			if err != nil {
 				return nil, err
diff --git a/pkg/reconciler/route/visibility/visibility.go b/pkg/reconciler/route/visibility/visibility.go
index d6f8892d1..08665b0bc 100644
--- a/pkg/reconciler/route/visibility/visibility.go
+++ b/pkg/reconciler/route/visibility/visibility.go
@@ -99,7 +99,7 @@ func (b *Resolver) GetVisibility(ctx context.Context, route *v1.Route) (map[stri
 		ttVisibility := netv1alpha1.IngressVisibilityExternalIP
 		// Is there a visibility setting on the placeholder Service?
 		if svc, ok := services[hostname]; ok {
-			if labels.IsObjectLocalVisibility(svc.ObjectMeta) {
+			if labels.IsObjectLocalVisibility(&svc.ObjectMeta) {
 				ttVisibility = netv1alpha1.IngressVisibilityClusterLocal
 			}
 		}


Test patch:
diff --git a/pkg/reconciler/route/resources/labels/labels_test.go b/pkg/reconciler/route/resources/labels/labels_test.go
index 24e16c502..a5f8a68bd 100644
--- a/pkg/reconciler/route/resources/labels/labels_test.go
+++ b/pkg/reconciler/route/resources/labels/labels_test.go
@@ -24,47 +24,83 @@ import (
 	"knative.dev/serving/pkg/apis/serving"
 )
 
-func TestDeleteLabel(t *testing.T) {
+func TestIsObjectLocalVisibility(t *testing.T) {
 	tests := []struct {
-		name     string
-		meta     *v1.ObjectMeta
-		key      string
-		expected v1.ObjectMeta
-	}{
-		{
-			name:     "No labels in object meta",
-			meta:     &v1.ObjectMeta{},
-			key:      "key",
-			expected: v1.ObjectMeta{},
+		name string
+		meta *v1.ObjectMeta
+		want bool
+	}{{
+		name: "nil",
+		meta: &v1.ObjectMeta{},
+	}, {
+		name: "empty labels",
+		meta: &v1.ObjectMeta{
+			Labels: map[string]string{},
+		},
+	}, {
+		name: "no matching labels",
+		meta: &v1.ObjectMeta{
+			Labels: map[string]string{"frankie-goes": "to-hollywood"},
 		},
-		{
-			name: "No matching key",
-			meta: &v1.ObjectMeta{
-				Labels: map[string]string{"some label": "some value"},
-			},
-			key: "unknown",
-			expected: v1.ObjectMeta{
-				Labels: map[string]string{"some label": "some value"},
-			},
+	}, {
+		name: "false",
+		meta: &v1.ObjectMeta{
+			Labels: map[string]string{serving.VisibilityLabelKey: ""},
 		},
-		{
-			name: "Has matching key",
-			meta: &v1.ObjectMeta{
-				Labels: map[string]string{"some label": "some value"},
-			},
-			key: "some label",
-			expected: v1.ObjectMeta{
-				Labels: map[string]string{},
-			},
+	}, {
+		name: "true",
+		meta: &v1.ObjectMeta{
+			Labels: map[string]string{serving.VisibilityLabelKey: "set"},
 		},
+		want: true,
+	}}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if got, want := IsObjectLocalVisibility(test.meta), test.want; got != want {
+				t.Errorf("IsObjectLocalVisibility = %v, want: %v", got, want)
+			}
+		})
 	}
+}
+
+func TestDeleteLabel(t *testing.T) {
+	tests := []struct {
+		name string
+		meta *v1.ObjectMeta
+		key  string
+		want v1.ObjectMeta
+	}{{
+		name: "No labels in object meta",
+		meta: &v1.ObjectMeta{},
+		key:  "key",
+		want: v1.ObjectMeta{},
+	}, {
+		name: "No matching key",
+		meta: &v1.ObjectMeta{
+			Labels: map[string]string{"some label": "some value"},
+		},
+		key: "unknown",
+		want: v1.ObjectMeta{
+			Labels: map[string]string{"some label": "some value"},
+		},
+	}, {
+		name: "Has matching key",
+		meta: &v1.ObjectMeta{
+			Labels: map[string]string{"some label": "some value"},
+		},
+		key: "some label",
+		want: v1.ObjectMeta{
+			Labels: map[string]string{},
+		},
+	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			DeleteLabel(tt.meta, tt.key)
 
-			if !cmp.Equal(tt.expected, *tt.meta) {
+			if !cmp.Equal(tt.want, *tt.meta) {
 				t.Errorf("DeleteLabel (-want, +got) = %v",
-					cmp.Diff(tt.expected, *tt.meta))
+					cmp.Diff(tt.want, *tt.meta))
 			}
 		})
 	}
@@ -76,48 +112,43 @@ func TestSetLabel(t *testing.T) {
 		meta  *v1.ObjectMeta
 		key   string
 		value string
-
-		expected v1.ObjectMeta
-	}{
-		{
-			name:  "No labels in object meta",
-			meta:  &v1.ObjectMeta{},
-			key:   "key",
-			value: "value",
-			expected: v1.ObjectMeta{
-				Labels: map[string]string{"key": "value"},
-			},
+		want  v1.ObjectMeta
+	}{{
+		name:  "No labels in object meta",
+		meta:  &v1.ObjectMeta{},
+		key:   "key",
+		value: "value",
+		want: v1.ObjectMeta{
+			Labels: map[string]string{"key": "value"},
 		},
-		{
-			name: "Empty labels",
-			meta: &v1.ObjectMeta{
-				Labels: map[string]string{},
-			},
-			key:   "key",
-			value: "value",
-			expected: v1.ObjectMeta{
-				Labels: map[string]string{"key": "value"},
-			},
+	}, {
+		name: "Empty labels",
+		meta: &v1.ObjectMeta{
+			Labels: map[string]string{},
 		},
-		{
-			name: "Conflicting labels",
-			meta: &v1.ObjectMeta{
-				Labels: map[string]string{"key": "old value"},
-			},
-			key:   "key",
-			value: "new value",
-			expected: v1.ObjectMeta{
-				Labels: map[string]string{"key": "new value"},
-			},
+		key:   "key",
+		value: "value",
+		want: v1.ObjectMeta{
+			Labels: map[string]string{"key": "value"},
 		},
-	}
+	}, {
+		name: "Conflicting labels",
+		meta: &v1.ObjectMeta{
+			Labels: map[string]string{"key": "old value"},
+		},
+		key:   "key",
+		value: "new value",
+		want: v1.ObjectMeta{
+			Labels: map[string]string{"key": "new value"},
+		},
+	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			SetLabel(tt.meta, tt.key, tt.value)
 
-			if !cmp.Equal(tt.expected, *tt.meta) {
+			if !cmp.Equal(tt.want, *tt.meta) {
 				t.Errorf("DeleteLabel (-want, +got) = %v",
-					cmp.Diff(tt.expected, *tt.meta))
+					cmp.Diff(tt.want, *tt.meta))
 			}
 		})
 	}
@@ -128,17 +159,16 @@ func TestSetVisibility(t *testing.T) {
 		name           string
 		meta           *v1.ObjectMeta
 		isClusterLocal bool
-		expected       v1.ObjectMeta
+		want           v1.ObjectMeta
 	}{{
 		name:           "Set cluster local true",
 		meta:           &v1.ObjectMeta{},
 		isClusterLocal: true,
-		expected:       v1.ObjectMeta{Labels: map[string]string{serving.VisibilityLabelKey: serving.VisibilityClusterLocal}},
+		want:           v1.ObjectMeta{Labels: map[string]string{serving.VisibilityLabelKey: serving.VisibilityClusterLocal}},
 	}, {
-		name:           "Set cluster local false",
-		meta:           &v1.ObjectMeta{Labels: map[string]string{serving.VisibilityLabelKey: serving.VisibilityClusterLocal}},
-		isClusterLocal: false,
-		expected:       v1.ObjectMeta{Labels: map[string]string{}},
+		name: "Set cluster local false",
+		meta: &v1.ObjectMeta{Labels: map[string]string{serving.VisibilityLabelKey: serving.VisibilityClusterLocal}},
+		want: v1.ObjectMeta{Labels: map[string]string{}},
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
diff --git a/test/e2e/initial_scale_test.go b/test/e2e/initial_scale_test.go
index 08f52afd5..63fbb5212 100644
--- a/test/e2e/initial_scale_test.go
+++ b/test/e2e/initial_scale_test.go
@@ -36,6 +36,7 @@ import (
 // the revision level. This test runs after the cluster wide flag allow-zero-initial-scale
 // is set to true.
 func TestInitScaleZero(t *testing.T) {
+	t.Skip("Enable once #9245 is fixed")
 	t.Parallel()
 
 	clients := Setup(t)