[release-v1.13] Sync upstream release

pkg/apis/serving/k8s_lifecycle.go

@@ -50,29 +50,42 @@ func TransformDeploymentStatus(ds *appsv1.DeploymentStatus) *duckv1.Status {
 	// The absence of this condition means no failure has occurred. If we find it
 	// below, we'll overwrite this.
 	depCondSet.Manage(s).MarkTrue(DeploymentConditionReplicaSetReady)
+	depCondSet.Manage(s).MarkUnknown(DeploymentConditionProgressing, "Deploying", "")
 
-	for _, cond := range ds.Conditions {
-		// TODO(jonjohnsonjr): Should we care about appsv1.DeploymentAvailable here?
-		switch cond.Type {
-		case appsv1.DeploymentProgressing:
-			switch cond.Status {
-			case corev1.ConditionUnknown:
-				depCondSet.Manage(s).MarkUnknown(DeploymentConditionProgressing, cond.Reason, cond.Message)
-			case corev1.ConditionTrue:
-				depCondSet.Manage(s).MarkTrue(DeploymentConditionProgressing)
-			case corev1.ConditionFalse:
-				depCondSet.Manage(s).MarkFalse(DeploymentConditionProgressing, cond.Reason, cond.Message)
+	conds := []appsv1.DeploymentConditionType{
+		appsv1.DeploymentProgressing,
+		appsv1.DeploymentReplicaFailure,
+	}
+
+	for _, wantType := range conds {
+		for _, cond := range ds.Conditions {
+			if wantType != cond.Type {
+				continue
 			}
-		case appsv1.DeploymentReplicaFailure:
-			switch cond.Status {
-			case corev1.ConditionUnknown:
-				depCondSet.Manage(s).MarkUnknown(DeploymentConditionReplicaSetReady, cond.Reason, cond.Message)
-			case corev1.ConditionTrue:
-				depCondSet.Manage(s).MarkFalse(DeploymentConditionReplicaSetReady, cond.Reason, cond.Message)
-			case corev1.ConditionFalse:
-				depCondSet.Manage(s).MarkTrue(DeploymentConditionReplicaSetReady)
+
+			switch cond.Type {
+			case appsv1.DeploymentProgressing:
+				switch cond.Status {
+				case corev1.ConditionUnknown:
+					depCondSet.Manage(s).MarkUnknown(DeploymentConditionProgressing, cond.Reason, cond.Message)
+				case corev1.ConditionTrue:
+					depCondSet.Manage(s).MarkTrue(DeploymentConditionProgressing)
+				case corev1.ConditionFalse:
+					depCondSet.Manage(s).MarkFalse(DeploymentConditionProgressing, cond.Reason, cond.Message)
+				}
+			case appsv1.DeploymentReplicaFailure:
+				switch cond.Status {
+				case corev1.ConditionUnknown:
+					depCondSet.Manage(s).MarkUnknown(DeploymentConditionReplicaSetReady, cond.Reason, cond.Message)
+				case corev1.ConditionTrue:
+					depCondSet.Manage(s).MarkFalse(DeploymentConditionReplicaSetReady, cond.Reason, cond.Message)
+				case corev1.ConditionFalse:
+					depCondSet.Manage(s).MarkTrue(DeploymentConditionReplicaSetReady)
+				}
 			}
+
 		}
 	}
+
 	return s
 }

pkg/apis/serving/k8s_lifecycle_test.go

@@ -40,12 +40,14 @@ func TestTransformDeploymentStatus(t *testing.T) {
 			Conditions: []apis.Condition{{
 				Type:   DeploymentConditionProgressing,
 				Status: corev1.ConditionUnknown,
+				Reason: "Deploying",
 			}, {
 				Type:   DeploymentConditionReplicaSetReady,
 				Status: corev1.ConditionTrue,
 			}, {
 				Type:   DeploymentConditionReady,
 				Status: corev1.ConditionUnknown,
+				Reason: "Deploying",
 			}},
 		},
 	}, {
@@ -147,7 +149,7 @@ func TestTransformDeploymentStatus(t *testing.T) {
 				Type:    appsv1.DeploymentReplicaFailure,
 				Status:  corev1.ConditionTrue,
 				Reason:  "ReplicaSetReason",
-				Message: "Something bag happened",
+				Message: "Something bad happened",
 			}},
 		},
 		want: &duckv1.Status{
@@ -158,12 +160,45 @@ func TestTransformDeploymentStatus(t *testing.T) {
 				Type:    DeploymentConditionReplicaSetReady,
 				Status:  corev1.ConditionFalse,
 				Reason:  "ReplicaSetReason",
-				Message: "Something bag happened",
+				Message: "Something bad happened",
 			}, {
 				Type:    DeploymentConditionReady,
 				Status:  corev1.ConditionFalse,
 				Reason:  "ReplicaSetReason",
-				Message: "Something bag happened",
+				Message: "Something bad happened",
+			}},
+		},
+	}, {
+		name: "replica failure has priority over progressing",
+		ds: &appsv1.DeploymentStatus{
+			Conditions: []appsv1.DeploymentCondition{{
+				Type:    appsv1.DeploymentReplicaFailure,
+				Status:  corev1.ConditionTrue,
+				Reason:  "ReplicaSetReason",
+				Message: "Something really bad happened",
+			}, {
+				Type:    appsv1.DeploymentProgressing,
+				Status:  corev1.ConditionFalse,
+				Reason:  "ProgressingReason",
+				Message: "Something bad happened",
+			}},
+		},
+		want: &duckv1.Status{
+			Conditions: []apis.Condition{{
+				Type:    DeploymentConditionProgressing,
+				Status:  corev1.ConditionFalse,
+				Reason:  "ProgressingReason",
+				Message: "Something bad happened",
+			}, {
+				Type:    DeploymentConditionReplicaSetReady,
+				Status:  corev1.ConditionFalse,
+				Reason:  "ReplicaSetReason",
+				Message: "Something really bad happened",
+			}, {
+				Type:    DeploymentConditionReady,
+				Status:  corev1.ConditionFalse,
+				Reason:  "ReplicaSetReason",
+				Message: "Something really bad happened",
 			}},
 		},
 	}}

pkg/apis/serving/v1/revision_helpers.go

@@ -19,7 +19,6 @@ package v1
 import (
 	"time"
 
-	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	net "knative.dev/networking/pkg/apis/networking"
 	"knative.dev/pkg/kmeta"
@@ -144,9 +143,3 @@ func (rs *RevisionStatus) IsActivationRequired() bool {
 	c := revisionCondSet.Manage(rs).GetCondition(RevisionConditionActive)
 	return c != nil && c.Status != corev1.ConditionTrue
 }
-
-// IsReplicaSetFailure returns true if the deployment replicaset failed to create
-func (rs *RevisionStatus) IsReplicaSetFailure(deploymentStatus *appsv1.DeploymentStatus) bool {
-	ds := serving.TransformDeploymentStatus(deploymentStatus)
-	return ds != nil && ds.GetCondition(serving.DeploymentConditionReplicaSetReady).IsFalse()
-}

pkg/apis/serving/v1/revision_helpers_test.go

@@ -20,7 +20,6 @@ import (
 	"testing"
 	"time"
 
-	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 
@@ -268,45 +267,3 @@ func TestSetRoutingState(t *testing.T) {
 		t.Error("Expected default value for unparsable annotationm but got:", got)
 	}
 }
-
-func TestIsReplicaSetFailure(t *testing.T) {
-	revisionStatus := RevisionStatus{}
-	cases := []struct {
-		name                string
-		status              appsv1.DeploymentStatus
-		IsReplicaSetFailure bool
-	}{{
-		name:   "empty deployment status should not be a failure",
-		status: appsv1.DeploymentStatus{},
-	}, {
-		name: "Ready deployment status should not be a failure",
-		status: appsv1.DeploymentStatus{
-			Conditions: []appsv1.DeploymentCondition{{
-				Type: appsv1.DeploymentAvailable, Status: corev1.ConditionTrue,
-			}},
-		},
-	}, {
-		name: "ReplicasetFailure true should be a failure",
-		status: appsv1.DeploymentStatus{
-			Conditions: []appsv1.DeploymentCondition{{
-				Type: appsv1.DeploymentReplicaFailure, Status: corev1.ConditionTrue,
-			}},
-		},
-		IsReplicaSetFailure: true,
-	}, {
-		name: "ReplicasetFailure false should not be a failure",
-		status: appsv1.DeploymentStatus{
-			Conditions: []appsv1.DeploymentCondition{{
-				Type: appsv1.DeploymentReplicaFailure, Status: corev1.ConditionFalse,
-			}},
-		},
-	}}
-
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			if got, want := revisionStatus.IsReplicaSetFailure(&tc.status), tc.IsReplicaSetFailure; got != want {
-				t.Errorf("IsReplicaSetFailure = %v, want: %v", got, want)
-			}
-		})
-	}
-}

pkg/apis/serving/v1/revision_lifecycle.go

@@ -170,6 +170,8 @@ func (rs *RevisionStatus) PropagateDeploymentStatus(original *appsv1.DeploymentS
 
 // PropagateAutoscalerStatus propagates autoscaler's status to the revision's status.
 func (rs *RevisionStatus) PropagateAutoscalerStatus(ps *autoscalingv1alpha1.PodAutoscalerStatus) {
+	resUnavailable := rs.GetCondition(RevisionConditionResourcesAvailable).IsFalse()
+
 	// Reflect the PA status in our own.
 	cond := ps.GetCondition(autoscalingv1alpha1.PodAutoscalerConditionReady)
 	rs.ActualReplicas = nil
@@ -183,20 +185,29 @@ func (rs *RevisionStatus) PropagateAutoscalerStatus(ps *autoscalingv1alpha1.PodA
 	}
 
 	if cond == nil {
-		rs.MarkActiveUnknown("Deploying", "")
+		rs.MarkActiveUnknown(ReasonDeploying, "")
+
+		if !resUnavailable {
+			rs.MarkResourcesAvailableUnknown(ReasonDeploying, "")
+		}
 		return
 	}
 
 	// Don't mark the resources available, if deployment status already determined
 	// it isn't so.
-	resUnavailable := rs.GetCondition(RevisionConditionResourcesAvailable).IsFalse()
 	if ps.IsScaleTargetInitialized() && !resUnavailable {
 		// Precondition for PA being initialized is SKS being active and
 		// that implies that |service.endpoints| > 0.
 		rs.MarkResourcesAvailableTrue()
 		rs.MarkContainerHealthyTrue()
 	}
 
+	// Mark resource unavailable if we don't have a Service Name and the deployment is ready
+	// This can happen when we have initial scale set to 0
+	if rs.GetCondition(RevisionConditionResourcesAvailable).IsTrue() && ps.ServiceName == "" {
+		rs.MarkResourcesAvailableUnknown(ReasonDeploying, "")
+	}
+
 	switch cond.Status {
 	case corev1.ConditionUnknown:
 		rs.MarkActiveUnknown(cond.Reason, cond.Message)
@@ -222,14 +233,6 @@ func (rs *RevisionStatus) PropagateAutoscalerStatus(ps *autoscalingv1alpha1.PodA
 		rs.MarkActiveFalse(cond.Reason, cond.Message)
 	case corev1.ConditionTrue:
 		rs.MarkActiveTrue()
-
-		// Precondition for PA being active is SKS being active and
-		// that implies that |service.endpoints| > 0.
-		//
-		// Note: This is needed for backwards compatibility as we're adding the new
-		// ScaleTargetInitialized condition to gate readiness.
-		rs.MarkResourcesAvailableTrue()
-		rs.MarkContainerHealthyTrue()
 	}
 }
 

pkg/apis/serving/v1/revision_lifecycle_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package v1
 
 import (
+	"fmt"
 	"sort"
 	"testing"
 
@@ -536,6 +537,7 @@ func TestPropagateAutoscalerStatus(t *testing.T) {
 
 	// PodAutoscaler becomes ready, making us active.
 	r.PropagateAutoscalerStatus(&autoscalingv1alpha1.PodAutoscalerStatus{
+		ServiceName: "some-service",
 		Status: duckv1.Status{
 			Conditions: duckv1.Conditions{{
 				Type:   autoscalingv1alpha1.PodAutoscalerConditionReady,
@@ -551,6 +553,7 @@ func TestPropagateAutoscalerStatus(t *testing.T) {
 
 	// PodAutoscaler flipping back to Unknown causes Active become ongoing immediately.
 	r.PropagateAutoscalerStatus(&autoscalingv1alpha1.PodAutoscalerStatus{
+		ServiceName: "some-service",
 		Status: duckv1.Status{
 			Conditions: duckv1.Conditions{{
 				Type:   autoscalingv1alpha1.PodAutoscalerConditionReady,
@@ -566,6 +569,7 @@ func TestPropagateAutoscalerStatus(t *testing.T) {
 
 	// PodAutoscaler becoming unready makes Active false, but doesn't affect readiness.
 	r.PropagateAutoscalerStatus(&autoscalingv1alpha1.PodAutoscalerStatus{
+		ServiceName: "some-service",
 		Status: duckv1.Status{
 			Conditions: duckv1.Conditions{{
 				Type:   autoscalingv1alpha1.PodAutoscalerConditionReady,
@@ -582,32 +586,45 @@ func TestPropagateAutoscalerStatus(t *testing.T) {
 	apistest.CheckConditionSucceeded(r, RevisionConditionResourcesAvailable, t)
 }
 
-func TestPAResAvailableNoOverride(t *testing.T) {
-	r := &RevisionStatus{}
-	r.InitializeConditions()
-	apistest.CheckConditionOngoing(r, RevisionConditionReady, t)
-
-	// Deployment determined that something's wrong, e.g. the only pod
-	// has crashed.
-	r.MarkResourcesAvailableFalse("somehow", "somewhere")
+func TestPropagateAutoscalerStatus_NoOverridingResourcesAvailable(t *testing.T) {
+	// Cases to test Ready condition
+	// we fix ScaleTargetInitialized to True
+	cases := []corev1.ConditionStatus{
+		corev1.ConditionTrue,
+		corev1.ConditionFalse,
+		corev1.ConditionUnknown,
+	}
 
-	// PodAutoscaler achieved initial scale.
-	r.PropagateAutoscalerStatus(&autoscalingv1alpha1.PodAutoscalerStatus{
-		Status: duckv1.Status{
-			Conditions: duckv1.Conditions{{
-				Type:   autoscalingv1alpha1.PodAutoscalerConditionReady,
-				Status: corev1.ConditionUnknown,
-			}, {
-				Type:   autoscalingv1alpha1.PodAutoscalerConditionScaleTargetInitialized,
-				Status: corev1.ConditionTrue,
-			}},
-		},
-	})
-	// Verify we did not override this.
-	apistest.CheckConditionFailed(r, RevisionConditionResourcesAvailable, t)
-	cond := r.GetCondition(RevisionConditionResourcesAvailable)
-	if got, notWant := cond.Reason, ReasonProgressDeadlineExceeded; got == notWant {
-		t.Error("PA Status propagation overrode the ResourcesAvailable status")
+	for _, tc := range cases {
+		t.Run(fmt.Sprintf("ready is %v", tc), func(t *testing.T) {
+
+			r := &RevisionStatus{}
+			r.InitializeConditions()
+			apistest.CheckConditionOngoing(r, RevisionConditionReady, t)
+
+			// Deployment determined that something's wrong, e.g. the only pod
+			// has crashed.
+			r.MarkResourcesAvailableFalse("somehow", "somewhere")
+			r.PropagateAutoscalerStatus(&autoscalingv1alpha1.PodAutoscalerStatus{
+				Status: duckv1.Status{
+					Conditions: duckv1.Conditions{{
+						Type:   autoscalingv1alpha1.PodAutoscalerConditionReady,
+						Status: tc,
+					}, {
+						Type:   autoscalingv1alpha1.PodAutoscalerConditionScaleTargetInitialized,
+						Status: corev1.ConditionTrue,
+					}},
+				},
+			})
+
+			// Verify we did not override this.
+			apistest.CheckConditionFailed(r, RevisionConditionResourcesAvailable, t)
+
+			cond := r.GetCondition(RevisionConditionResourcesAvailable)
+			if got, notWant := cond.Reason, ReasonProgressDeadlineExceeded; got == notWant {
+				t.Error("PodAutoscalerStatus propagation overrode the ResourcesAvailable status")
+			}
+		})
 	}
 }
 
@@ -671,6 +688,7 @@ func TestPropagateAutoscalerStatusRace(t *testing.T) {
 
 	// The PodAutoscaler might have been ready but it's scaled down already.
 	r.PropagateAutoscalerStatus(&autoscalingv1alpha1.PodAutoscalerStatus{
+		ServiceName: "some-service",
 		Status: duckv1.Status{
 			Conditions: duckv1.Conditions{{
 				Type:   autoscalingv1alpha1.PodAutoscalerConditionReady,