surface deployment timeout - route active

modify rest of ut

Update net-kourier nightly (knative#14605)

bumping knative.dev/net-kourier 6e4d79d...fb8da93:
  > fb8da93 upgrade to latest dependencies (# 1151)

Signed-off-by: Knative Automation <[email protected]>

Update net-gateway-api nightly (knative#14602)

bumping knative.dev/net-gateway-api a8d56a3...0aa321a:
  > 0aa321a upgrade to latest dependencies (# 579)

Signed-off-by: Knative Automation <[email protected]>

upgrade to latest dependencies (knative#14608)

bumping knative.dev/pkg 5c9b7a8...35011d4:
  > 35011d4 upgrade to latest dependencies (# 2892)
bumping knative.dev/networking 18529fd...e0bee34:
  > e0bee34 upgrade to latest dependencies (# 889)
bumping knative.dev/hack 8834794...5deadde:
  > 5deadde 🐛 Set latest release only when publishing to Github (# 346)
bumping knative.dev/caching c642577...b3781bc:
  > b3781bc upgrade to latest dependencies (# 806)

Signed-off-by: Knative Automation <[email protected]>

Update net-contour nightly (knative#14612)

bumping knative.dev/net-contour 467a573...d2054f2:
  > d2054f2 upgrade to latest dependencies (# 999)

Signed-off-by: Knative Automation <[email protected]>

Update net-certmanager nightly (knative#14611)

bumping knative.dev/net-certmanager 11e6219...8b2a470:
  > 8b2a470 upgrade to latest dependencies (# 625)
  > 2248405 upgrade to latest dependencies (# 624)

Signed-off-by: Knative Automation <[email protected]>

Update net-kourier nightly (knative#14613)

bumping knative.dev/net-kourier fb8da93...ad58d90:
  > ad58d90 upgrade to latest dependencies (# 1155)

Signed-off-by: Knative Automation <[email protected]>

Update net-istio nightly (knative#14614)

bumping knative.dev/net-istio 7f77e97...e3db912:
  > e3db912 upgrade to latest dependencies (# 1209)
  > 1e021c8 upgrade to latest dependencies (# 1208)

Signed-off-by: Knative Automation <[email protected]>

Update net-gateway-api nightly (knative#14616)

bumping knative.dev/net-gateway-api 0aa321a...29bf0b9:
  > 29bf0b9 upgrade to latest dependencies (# 581)
  > cd26216 upgrade to latest dependencies (# 580)

Signed-off-by: Knative Automation <[email protected]>

Surface cpu and mem requests forbidden errors (and other ones too) in KSVC creation (knative#14453)

* reconciling the revision so the deployment status is propagated when there is something wrong (like low resources request and limits), since it was just beign propagated when the revision status was nil and the deployment existed

trying to surface replicaset creation errors

* added revision condition active to revision live condition set so is not nil after the revision is created

* removing RevisionConditionActive from Revision ConditionSet since we can have Ready Inactive Revisions (scale to zero) * added docs and tests for this and the replicaset failure propagation

* fixing lint

* adjusted e2e and unit tests for the replica failure erros propagation, improved propagation logic + left todos regarding revision conditionset

* removed todo from revision lifecycle since the discussion has settled

* added test case for revision fast failures when it comes to replicaset failing to create

* fixed resource quota error, now it never waits for progress deadline and it fails fast, so removing the bits where it can go one way or another in the e2e resource_quota_error_test

* finishing the replicaset deployment status failure bubbling up to the revision table test

* removed unused test methods from revision testing package

* adding condition to wait for container creation in the resource quota service creation test

* with some istio cases this could fail with progress deadline exceeded error so adding that case too

* Update resource_quota_error_test.go

* formated the test file

fix deploy-replicaset-failure UT

rename tests

remove coment

remove comment

wip

want go correct

need fix ScaleTargetInitialized

only extra patch

patch scale to 0

more test helper changes

new tc

ut

WithPubService

activation failure is unreachable

copy inactive condition

comments

comment

comment

refactor computeActiveCondition

allow scale to 0 with no metrics if unreachable

remoce comment

add UT: initial scale zero: with ready pods

change to 1 ready pod

replicas 1

change logic order

readd activeThreshold

change Failed message

scaler_test handle no metrics case

mend

silly typo

fix UT

shortten test cases

scale to 0 if unreachable comment + logic

remove controlls podinformer as its not needed

revert computeActiveCondition refactor

use reachability variables

init scale 0 stays no traffic

undo imports reorder

add comments

pa change Inactive reason to Unreachable

go.mod

@@ -32,10 +32,10 @@ require (
 	k8s.io/code-generator v0.27.6
 	k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f
 	k8s.io/utils v0.0.0-20230209194617-a36077c30491
-	knative.dev/caching v0.0.0-20231101191025-c6425778e5ba
-	knative.dev/hack v0.0.0-20231107173840-883479423aaa
-	knative.dev/networking v0.0.0-20231103063604-18529fd26a8b
-	knative.dev/pkg v0.0.0-20231107094615-5c9b7a8d8265
+	knative.dev/caching v0.0.0-20231108204433-b3781bc47aeb
+	knative.dev/hack v0.0.0-20231109190034-5deaddeb51a7
+	knative.dev/networking v0.0.0-20231108061732-e0bee342a97e
+	knative.dev/pkg v0.0.0-20231108014432-35011d423d4b
 	sigs.k8s.io/yaml v1.4.0
 )
 

go.sum

@@ -926,14 +926,14 @@ k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f h1:2kWPakN3i/k81b0gvD5C5F
 k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f/go.mod h1:byini6yhqGC14c3ebc/QwanvYwhuMWF6yz2F8uwW8eg=
 k8s.io/utils v0.0.0-20230209194617-a36077c30491 h1:r0BAOLElQnnFhE/ApUsg3iHdVYYPBjNSSOMowRZxxsY=
 k8s.io/utils v0.0.0-20230209194617-a36077c30491/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
-knative.dev/caching v0.0.0-20231101191025-c6425778e5ba h1:wMfEfoiu+yfpKG79k9MUhY4ww8p3YiuqQOt0QfgqMoE=
-knative.dev/caching v0.0.0-20231101191025-c6425778e5ba/go.mod h1:36UniA7tdm8pj9dPAt44dneazLbcQqraNSftnOUYRks=
-knative.dev/hack v0.0.0-20231107173840-883479423aaa h1:XVFqW2a8xvyo231TbVSD3i+580pVej9LbTSmYex6d1g=
-knative.dev/hack v0.0.0-20231107173840-883479423aaa/go.mod h1:yk2OjGDsbEnQjfxdm0/HJKS2WqTLEFg/N6nUs6Rqx3Q=
-knative.dev/networking v0.0.0-20231103063604-18529fd26a8b h1:bimYPtsVmXBGjA0dARsoyNFKtREIVceVScnKdsHmqjo=
-knative.dev/networking v0.0.0-20231103063604-18529fd26a8b/go.mod h1:XKuKrS5QCQ3LkPeOYBZqyUxseBIBLpujxttejIBjoCk=
-knative.dev/pkg v0.0.0-20231107094615-5c9b7a8d8265 h1:wFDUSmvSQF48tUCIUIFKMOxq9jpV+vXf5l+RZYxYyt4=
-knative.dev/pkg v0.0.0-20231107094615-5c9b7a8d8265/go.mod h1:P3m1Mg/FJjmr9oFHfWcoUbJLSlBi/hgwakobPxyqrZ4=
+knative.dev/caching v0.0.0-20231108204433-b3781bc47aeb h1:9kuTebXS3SuSxWLGr/5eplg8qu+xn9y/CjFHgVBBo2Q=
+knative.dev/caching v0.0.0-20231108204433-b3781bc47aeb/go.mod h1:owQX47ghEY9OIaxvoTZ9KyJTfEiwLgwY94tyHoUlLUU=
+knative.dev/hack v0.0.0-20231109190034-5deaddeb51a7 h1:HXf7M7n9jwn+Hp904r0HXRSymf+DLXSciFpXVpCg+Bs=
+knative.dev/hack v0.0.0-20231109190034-5deaddeb51a7/go.mod h1:yk2OjGDsbEnQjfxdm0/HJKS2WqTLEFg/N6nUs6Rqx3Q=
+knative.dev/networking v0.0.0-20231108061732-e0bee342a97e h1:IFZuDN6IA3lzGt3zVgQ1VbTPSdDcCkdvD0SmxZ3blBM=
+knative.dev/networking v0.0.0-20231108061732-e0bee342a97e/go.mod h1:cu01aODvz01sLC80d7Md6M8pSFi7RMurQnCubeeVH40=
+knative.dev/pkg v0.0.0-20231108014432-35011d423d4b h1:WrDo9M6vkJ4xnTBodWOT2koXjKqr7dOqJH4RWBq4kBw=
+knative.dev/pkg v0.0.0-20231108014432-35011d423d4b/go.mod h1:zkycL49skv31nWKvT1XAsSMFO6mUu33Qhpv0xDvdVGY=
 pgregory.net/rapid v1.1.0 h1:CMa0sjHSru3puNx+J0MIAuiiEV4N0qj8/cMWGBBCsjw=
 rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=
 rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0=

pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go

@@ -188,11 +188,11 @@ func (pa *PodAutoscaler) IsReady() bool {
 }
 
 func (pa *PodAutoscaler) IsUnreachable() bool {
-	return pa.Spec.Reachability == "Unreachable"
+	return pa.Spec.Reachability == ReachabilityUnreachable
 }
 
 func (pa *PodAutoscaler) IsReachable() bool {
-	return pa.Spec.Reachability == "Reachable"
+	return pa.Spec.Reachability == ReachabilityReachable
 }
 
 // IsActive returns true if the pod autoscaler has finished scaling.

pkg/apis/serving/v1/revision_helpers.go

@@ -19,6 +19,7 @@ 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"
@@ -143,3 +144,9 @@ 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,6 +20,7 @@ import (
 	"testing"
 	"time"
 
+	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 
@@ -267,3 +268,45 @@ 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

@@ -51,6 +51,7 @@ const (
 	ReasonProgressDeadlineExceeded = "ProgressDeadlineExceeded"
 )
 
+// RevisionConditionActive is not part of the RevisionConditionSet because we can have Inactive Ready Revisions (scale to zero)
 var revisionCondSet = apis.NewLivingConditionSet(
 	RevisionConditionResourcesAvailable,
 	RevisionConditionContainerHealthy,
@@ -171,7 +172,6 @@ func (rs *RevisionStatus) PropagateDeploymentStatus(original *appsv1.DeploymentS
 func (rs *RevisionStatus) PropagateAutoscalerStatus(ps *autoscalingv1alpha1.PodAutoscalerStatus) {
 	// Reflect the PA status in our own.
 	cond := ps.GetCondition(autoscalingv1alpha1.PodAutoscalerConditionReady)
-
 	rs.ActualReplicas = nil
 	if ps.ActualScale != nil && *ps.ActualScale >= 0 {
 		rs.ActualReplicas = ps.ActualScale

pkg/autoscaler/scaling/autoscaler.go

@@ -163,7 +163,7 @@ func (a *autoscaler) Scale(logger *zap.SugaredLogger, now time.Time) ScaleResult
 		observedStableValue, observedPanicValue, err = a.metricClient.StableAndPanicConcurrency(metricKey, now)
 	}
 
-	if err != nil && spec.Reachable {
+	if err != nil {
 		if errors.Is(err, metrics.ErrNoData) {
 			logger.Debug("No data to scale on yet")
 		} else {

pkg/reconciler/autoscaling/kpa/kpa.go

@@ -270,15 +270,19 @@ func computeActiveCondition(ctx context.Context, pa *autoscalingv1alpha1.PodAuto
 		if pa.Status.IsActivating() && minReady > 0 {
 			// We only ever scale to zero while activating if we fail to activate within the progress deadline.
 			pa.Status.MarkInactive("TimedOut", "The target could not be activated.")
-		} else if !pa.Status.IsInactive() {
+		} else if pa.Status.IsInactive() {
+			// If the pa is scaled to 0 because "Failed", we dont want to overwrite the condition.
+			// and it needs to be copied to avoid NewObservedGenFailure
+			cond := pa.Status.GetCondition("Active")
+			pa.Status.MarkInactive(cond.Reason, cond.Message)
+		} else {
 			pa.Status.MarkInactive(noTrafficReason, "The target is not receiving traffic.")
 		}
 
 	case pc.ready < minReady:
 		if pc.want > 0 || !pa.Status.IsInactive() {
 			if pa.IsUnreachable() {
-				pa.Status.MarkInactive(
-					"Failed", "The target failed.")
+				pa.Status.MarkInactive("Unreachable", "The target does not have an active routing state.")
 			} else {
 				pa.Status.MarkActivating(
 					"Queued", "Requests to the target are being buffered as resources are provisioned.")
@@ -292,6 +296,7 @@ func computeActiveCondition(ctx context.Context, pa *autoscalingv1alpha1.PodAuto
 			// because we cannot go through one iteration of reconciliation without setting
 			// some status.
 			if pa.Status.IsInactive() {
+				// If the pa is scaled to 0 because "Failed", we dont want to overwrite the condition.
 				cond := pa.Status.GetCondition("Active")
 				pa.Status.MarkInactive(cond.Reason, cond.Message)
 			} else {
@@ -303,6 +308,7 @@ func computeActiveCondition(ctx context.Context, pa *autoscalingv1alpha1.PodAuto
 		if pc.want > 0 || !pa.Status.IsInactive() {
 			pa.Status.MarkActive()
 		}
+
 	}
 }