Repository: knative/serving
Pull request #8944 (2020-08-06 14:30:09)

Add anti-affinity to our deployments.
Related: https://github.com/knative/pkg/issues/1269


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

This pull-request has been approved by: *<a href="https://github.com/knative/serving/pull/8944#" 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:

- ~~[config/OWNERS](https://github.com/knative/serving/blob/master/config/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":[]} -->
mattmoor: We probably want something like this for the dataplane as well, but probably without as strong an anti-affinity.

Patch:
diff --git a/config/core/deployments/autoscaler.yaml b/config/core/deployments/autoscaler.yaml
index 07cdb843d..b33b4387c 100644
--- a/config/core/deployments/autoscaler.yaml
+++ b/config/core/deployments/autoscaler.yaml
@@ -32,6 +32,17 @@ spec:
         app: autoscaler
         serving.knative.dev/release: devel
     spec:
+      # To avoid node becoming SPOF, spread our replicas to different nodes.
+      affinity:
+        podAntiAffinity:
+          preferredDuringSchedulingIgnoredDuringExecution:
+          - podAffinityTerm:
+              labelSelector:
+                matchLabels:
+                  app: autoscaler
+              topologyKey: kubernetes.io/hostname
+            weight: 100
+
       serviceAccountName: controller
       containers:
       - name: autoscaler
diff --git a/config/core/deployments/controller.yaml b/config/core/deployments/controller.yaml
index d8803ccb3..6ddbd58ec 100644
--- a/config/core/deployments/controller.yaml
+++ b/config/core/deployments/controller.yaml
@@ -31,6 +31,17 @@ spec:
         app: controller
         serving.knative.dev/release: devel
     spec:
+      # To avoid node becoming SPOF, spread our replicas to different nodes.
+      affinity:
+        podAntiAffinity:
+          preferredDuringSchedulingIgnoredDuringExecution:
+          - podAffinityTerm:
+              labelSelector:
+                matchLabels:
+                  app: controller
+              topologyKey: kubernetes.io/hostname
+            weight: 100
+
       serviceAccountName: controller
       containers:
       - name: controller
diff --git a/config/core/deployments/webhook.yaml b/config/core/deployments/webhook.yaml
index aa445431c..836bde85a 100644
--- a/config/core/deployments/webhook.yaml
+++ b/config/core/deployments/webhook.yaml
@@ -33,6 +33,17 @@ spec:
         role: webhook
         serving.knative.dev/release: devel
     spec:
+      # To avoid node becoming SPOF, spread our replicas to different nodes.
+      affinity:
+        podAntiAffinity:
+          preferredDuringSchedulingIgnoredDuringExecution:
+          - podAffinityTerm:
+              labelSelector:
+                matchLabels:
+                  app: webhook
+              topologyKey: kubernetes.io/hostname
+            weight: 100
+
       serviceAccountName: controller
       containers:
       - name: webhook
diff --git a/config/hpa-autoscaling/controller.yaml b/config/hpa-autoscaling/controller.yaml
index 2faf2c3f3..b1c69be94 100644
--- a/config/hpa-autoscaling/controller.yaml
+++ b/config/hpa-autoscaling/controller.yaml
@@ -32,6 +32,17 @@ spec:
         app: autoscaler-hpa
         serving.knative.dev/release: devel
     spec:
+      # To avoid node becoming SPOF, spread our replicas to different nodes.
+      affinity:
+        podAntiAffinity:
+          preferredDuringSchedulingIgnoredDuringExecution:
+          - podAffinityTerm:
+              labelSelector:
+                matchLabels:
+                  app: autoscaler-hpa
+              topologyKey: kubernetes.io/hostname
+            weight: 100
+
       serviceAccountName: controller
       containers:
       - name: autoscaler-hpa
diff --git a/config/namespace-wildcard-certs/controller.yaml b/config/namespace-wildcard-certs/controller.yaml
index 7883420c5..f28e8cd30 100644
--- a/config/namespace-wildcard-certs/controller.yaml
+++ b/config/namespace-wildcard-certs/controller.yaml
@@ -32,6 +32,17 @@ spec:
         app: networking-ns-cert
         serving.knative.dev/release: devel
     spec:
+      # To avoid node becoming SPOF, spread our replicas to different nodes.
+      affinity:
+        podAntiAffinity:
+          preferredDuringSchedulingIgnoredDuringExecution:
+          - podAffinityTerm:
+              labelSelector:
+                matchLabels:
+                  app: networking-ns-cert
+              topologyKey: kubernetes.io/hostname
+            weight: 100
+
       serviceAccountName: controller
       containers:
       - name: networking-nscert
diff --git a/pkg/apis/serving/v1/revision_lifecycle.go b/pkg/apis/serving/v1/revision_lifecycle.go
index c637b0496..c750567c7 100644
--- a/pkg/apis/serving/v1/revision_lifecycle.go
+++ b/pkg/apis/serving/v1/revision_lifecycle.go
@@ -182,6 +182,19 @@ func (rs *RevisionStatus) PropagateAutoscalerStatus(ps *av1alpha1.PodAutoscalerS
 	case corev1.ConditionUnknown:
 		rs.MarkActiveUnknown(cond.Reason, cond.Message)
 	case corev1.ConditionFalse:
+		// Here we have 2 things coming together at the same time:
+		// 1. The ready is False, meaning the revision is scaled to 0
+		// 2. Initial scale was never achieved, which means we failed to progress
+		//    towards initial scale during the progress deadline period and scaled to 0
+		//		failing to activate.
+		// So mark the revision as failed at that point.
+		// See #8922 for details. When we try to scale to 0, we force the Deployment's
+		// Progress status to become `true`, since successful scale down means
+		// progress has been achieved.
+		if !ps.IsScaleTargetInitialized() {
+			rs.MarkResourcesAvailableFalse(ReasonProgressDeadlineExceeded,
+				"Initial scale was never achieved")
+		}
 		rs.MarkActiveFalse(cond.Reason, cond.Message)
 	case corev1.ConditionTrue:
 		rs.MarkActiveTrue()
diff --git a/pkg/reconciler/revision/reconcile_resources.go b/pkg/reconciler/revision/reconcile_resources.go
index 0e72799f1..ee53eb759 100644
--- a/pkg/reconciler/revision/reconcile_resources.go
+++ b/pkg/reconciler/revision/reconcile_resources.go
@@ -175,7 +175,7 @@ func (c *Reconciler) reconcilePA(ctx context.Context, rev *v1.Revision) error {
 func hasDeploymentTimedOut(deployment *appsv1.Deployment) bool {
 	// as per https://kubernetes.io/docs/concepts/workloads/controllers/deployment
 	for _, cond := range deployment.Status.Conditions {
-		// Look for Deployment with status False
+		// Look for a condition with status False
 		if cond.Status != corev1.ConditionFalse {
 			continue
 		}


Test patch:
diff --git a/pkg/apis/serving/v1/revision_lifecycle_test.go b/pkg/apis/serving/v1/revision_lifecycle_test.go
index b917c98b3..719d8cd27 100644
--- a/pkg/apis/serving/v1/revision_lifecycle_test.go
+++ b/pkg/apis/serving/v1/revision_lifecycle_test.go
@@ -575,6 +575,31 @@ func TestPropagateAutoscalerStatus(t *testing.T) {
 	apistest.CheckConditionSucceeded(r, RevisionConditionResourcesAvailable, t)
 }
 
+func TestPropagateAutoscalerStatusNoProgress(t *testing.T) {
+	r := &RevisionStatus{}
+	r.InitializeConditions()
+	apistest.CheckConditionOngoing(r, RevisionConditionReady, t)
+
+	// PodAutoscaler is not ready and initial scale was never attained.
+	r.PropagateAutoscalerStatus(&av1alpha1.PodAutoscalerStatus{
+		Status: duckv1.Status{
+			Conditions: duckv1.Conditions{{
+				Type:   av1alpha1.PodAutoscalerConditionReady,
+				Status: corev1.ConditionFalse,
+			}, {
+				Type:   av1alpha1.PodAutoscalerConditionScaleTargetInitialized,
+				Status: corev1.ConditionUnknown,
+			}},
+		},
+	})
+	apistest.CheckConditionFailed(r, RevisionConditionActive, t)
+	apistest.CheckConditionFailed(r, RevisionConditionResourcesAvailable, t)
+	cond := r.GetCondition(RevisionConditionResourcesAvailable)
+	if got, want := cond.Reason, ReasonProgressDeadlineExceeded; got != want {
+		t.Errorf("Reason = %q, want: %q", got, want)
+	}
+}
+
 func TestPropagateAutoscalerStatusRace(t *testing.T) {
 	r := &RevisionStatus{}
 	r.InitializeConditions()
diff --git a/pkg/reconciler/revision/table_test.go b/pkg/reconciler/revision/table_test.go
index 868c8a46a..fea2d4e08 100644
--- a/pkg/reconciler/revision/table_test.go
+++ b/pkg/reconciler/revision/table_test.go
@@ -237,7 +237,8 @@ func TestReconcile(t *testing.T) {
 				MarkInactive("NoTraffic", "This thing is inactive."),
 				withDefaultContainerStatuses(), withObservedGeneration(1)),
 			pa("foo", "stable-deactivation",
-				WithNoTraffic("NoTraffic", "This thing is inactive."), WithReachabilityUnreachable),
+				WithNoTraffic("NoTraffic", "This thing is inactive."), WithReachabilityUnreachable,
+				WithScaleTargetInitialized),
 			deploy(t, "foo", "stable-deactivation"),
 			image("foo", "stable-deactivation"),
 		},
@@ -297,6 +298,7 @@ func TestReconcile(t *testing.T) {
 				MarkRevisionReady, withObservedGeneration(1)),
 			pa("foo", "pa-inactive",
 				WithNoTraffic("NoTraffic", "This thing is inactive."),
+				WithScaleTargetInitialized,
 				WithReachabilityUnreachable),
 			readyDeploy(deploy(t, "foo", "pa-inactive")),
 			image("foo", "pa-inactive"),
@@ -309,6 +311,28 @@ func TestReconcile(t *testing.T) {
 				MarkInactive("NoTraffic", "This thing is inactive."), withObservedGeneration(1)),
 		}},
 		Key: "foo/pa-inactive",
+	}, {
+		Name: "pa inactive II",
+		// Test propagating the inactivity signal from the pa to the Revision.
+		Objects: []runtime.Object{
+			rev("foo", "pa-inactive",
+				withK8sServiceName("something-in-the-way"), WithLogURL,
+				withObservedGeneration(1)),
+			pa("foo", "pa-inactive",
+				WithNoTraffic("NoTraffic", "This thing is inactive.")),
+			readyDeploy(deploy(t, "foo", "pa-inactive")),
+			image("foo", "pa-inactive"),
+		},
+		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
+			Object: rev("foo", "pa-inactive",
+				WithLogURL, withDefaultContainerStatuses(), MarkDeploying(""),
+				// When we reconcile an "all ready" revision when the PA
+				// is inactive, we should see the following change.
+				MarkInactive("NoTraffic", "This thing is inactive."), withObservedGeneration(1),
+				MarkResourcesUnavailable(v1.ReasonProgressDeadlineExceeded,
+					"Initial scale was never achieved")),
+		}},
+		Key: "foo/pa-inactive",
 	}, {
 		Name: "pa inactive, but has service",
 		// Test propagating the inactivity signal from the pa to the Revision.
@@ -320,6 +344,7 @@ func TestReconcile(t *testing.T) {
 			pa("foo", "pa-inactive",
 				WithNoTraffic("NoTraffic", "This thing is inactive."),
 				WithPAStatusService("pa-inactive-svc"),
+				WithScaleTargetInitialized,
 				WithReachabilityUnreachable),
 			readyDeploy(deploy(t, "foo", "pa-inactive")),
 			image("foo", "pa-inactive"),
diff --git a/third_party/contour-latest/contour.yaml b/third_party/contour-latest/contour.yaml
index 435f8d874..f38c2482d 100644
--- a/third_party/contour-latest/contour.yaml
+++ b/third_party/contour-latest/contour.yaml
@@ -1432,7 +1432,7 @@ spec:
         # if we change this on each version, you can no longer upgrade
         # just by applying the deployment YAML.
         # See #2423, #2395, #2150, and #2030 for earlier questions about this.
-        image: gcr.io/knative-nightly/github.com/projectcontour/contour/cmd/contour@sha256:2a1b03f5c23b80bd81619a2e94e3a3da3ad451ac7de2fb6cbaba810787263025
+        image: gcr.io/knative-nightly/github.com/projectcontour/contour/cmd/contour@sha256:b1a252d5f1b6d4d95554f12f056cc0b4b1b980416721e870917b11916398aeb5
         imagePullPolicy: Always
         command:
         - contour
@@ -1672,7 +1672,7 @@ spec:
         - --contour-key-file=/certs/tls.key
         - --config-path=/config/contour.yaml
         command: ["contour"]
-        image: gcr.io/knative-nightly/github.com/projectcontour/contour/cmd/contour@sha256:2a1b03f5c23b80bd81619a2e94e3a3da3ad451ac7de2fb6cbaba810787263025
+        image: gcr.io/knative-nightly/github.com/projectcontour/contour/cmd/contour@sha256:b1a252d5f1b6d4d95554f12f056cc0b4b1b980416721e870917b11916398aeb5
         imagePullPolicy: Always
         name: contour
         ports:
@@ -1764,7 +1764,7 @@ spec:
         args:
         - envoy
         - shutdown-manager
-        image: gcr.io/knative-nightly/github.com/projectcontour/contour/cmd/contour@sha256:2a1b03f5c23b80bd81619a2e94e3a3da3ad451ac7de2fb6cbaba810787263025
+        image: gcr.io/knative-nightly/github.com/projectcontour/contour/cmd/contour@sha256:b1a252d5f1b6d4d95554f12f056cc0b4b1b980416721e870917b11916398aeb5
         imagePullPolicy: Always
         lifecycle:
           preStop:
@@ -1840,7 +1840,7 @@ spec:
         - --envoy-key-file=/certs/tls.key
         command:
         - contour
-        image: gcr.io/knative-nightly/github.com/projectcontour/contour/cmd/contour@sha256:2a1b03f5c23b80bd81619a2e94e3a3da3ad451ac7de2fb6cbaba810787263025
+        image: gcr.io/knative-nightly/github.com/projectcontour/contour/cmd/contour@sha256:b1a252d5f1b6d4d95554f12f056cc0b4b1b980416721e870917b11916398aeb5
         imagePullPolicy: Always
         name: envoy-initconfig
         volumeMounts:
@@ -3305,7 +3305,7 @@ spec:
         # if we change this on each version, you can no longer upgrade
         # just by applying the deployment YAML.
         # See #2423, #2395, #2150, and #2030 for earlier questions about this.
-        image: gcr.io/knative-nightly/github.com/projectcontour/contour/cmd/contour@sha256:2a1b03f5c23b80bd81619a2e94e3a3da3ad451ac7de2fb6cbaba810787263025
+        image: gcr.io/knative-nightly/github.com/projectcontour/contour/cmd/contour@sha256:b1a252d5f1b6d4d95554f12f056cc0b4b1b980416721e870917b11916398aeb5
         imagePullPolicy: Always
         command:
         - contour
@@ -3545,7 +3545,7 @@ spec:
         - --contour-key-file=/certs/tls.key
         - --config-path=/config/contour.yaml
         command: ["contour"]
-        image: gcr.io/knative-nightly/github.com/projectcontour/contour/cmd/contour@sha256:2a1b03f5c23b80bd81619a2e94e3a3da3ad451ac7de2fb6cbaba810787263025
+        image: gcr.io/knative-nightly/github.com/projectcontour/contour/cmd/contour@sha256:b1a252d5f1b6d4d95554f12f056cc0b4b1b980416721e870917b11916398aeb5
         imagePullPolicy: Always
         name: contour
         ports:
@@ -3637,7 +3637,7 @@ spec:
         args:
         - envoy
         - shutdown-manager
-        image: gcr.io/knative-nightly/github.com/projectcontour/contour/cmd/contour@sha256:2a1b03f5c23b80bd81619a2e94e3a3da3ad451ac7de2fb6cbaba810787263025
+        image: gcr.io/knative-nightly/github.com/projectcontour/contour/cmd/contour@sha256:b1a252d5f1b6d4d95554f12f056cc0b4b1b980416721e870917b11916398aeb5
         imagePullPolicy: Always
         lifecycle:
           preStop:
@@ -3713,7 +3713,7 @@ spec:
         - --envoy-key-file=/certs/tls.key
         command:
         - contour
-        image: gcr.io/knative-nightly/github.com/projectcontour/contour/cmd/contour@sha256:2a1b03f5c23b80bd81619a2e94e3a3da3ad451ac7de2fb6cbaba810787263025
+        image: gcr.io/knative-nightly/github.com/projectcontour/contour/cmd/contour@sha256:b1a252d5f1b6d4d95554f12f056cc0b4b1b980416721e870917b11916398aeb5
         imagePullPolicy: Always
         name: envoy-initconfig
         volumeMounts:
diff --git a/third_party/contour-latest/net-contour.yaml b/third_party/contour-latest/net-contour.yaml
index 47daf8d68..baf176073 100644
--- a/third_party/contour-latest/net-contour.yaml
+++ b/third_party/contour-latest/net-contour.yaml
@@ -35,7 +35,7 @@ metadata:
   namespace: knative-serving
   labels:
     networking.knative.dev/ingress-provider: contour
-    serving.knative.dev/release: "v20200804-3e3e1970"
+    serving.knative.dev/release: "v20200806-cc91b744"
 data:
   _example: |
     ################################
@@ -94,7 +94,7 @@ spec:
       - name: controller
         # This is the Go import path for the binary that is containerized
         # and substituted here.
-        image: gcr.io/knative-nightly/knative.dev/net-contour/cmd/controller@sha256:25e8c2232f4ed6be52e0b39097abcf301cf0b094095764837dab81bfef1ecdb0
+        image: gcr.io/knative-nightly/knative.dev/net-contour/cmd/controller@sha256:643c985b077129fecda9e16281888db9b9b458de2d918a14e9c35c35e78b9ba1
         resources:
           requests:
             cpu: 40m
diff --git a/third_party/kourier-latest/kourier.yaml b/third_party/kourier-latest/kourier.yaml
index 72d66bd9c..2b44a0a17 100644
--- a/third_party/kourier-latest/kourier.yaml
+++ b/third_party/kourier-latest/kourier.yaml
@@ -172,7 +172,7 @@ spec:
         app: 3scale-kourier-control
     spec:
       containers:
-      - image: gcr.io/knative-nightly/knative.dev/net-kourier/cmd/kourier@sha256:1f2d9abba4dd282f643605e4bcda3d04ae50ad223efe0f810f086714e059a65a
+      - image: gcr.io/knative-nightly/knative.dev/net-kourier/cmd/kourier@sha256:f243c2f897883aeee839c44d0ff8a23b147edd0520ebf92472d72a8bfbc7cdaf
         imagePullPolicy: Always
         name: kourier-control
         env: