diff --git a/pkg/apis/serving/v1alpha1/revision_types.go b/pkg/apis/serving/v1alpha1/revision_types.go index 99dc49105409..cc6bf8bc983d 100644 --- a/pkg/apis/serving/v1alpha1/revision_types.go +++ b/pkg/apis/serving/v1alpha1/revision_types.go @@ -199,7 +199,6 @@ const ( var revCondSet = duckv1alpha1.NewLivingConditionSet( RevisionConditionResourcesAvailable, RevisionConditionContainerHealthy, - RevisionConditionActive, RevisionConditionBuildSucceeded, ) diff --git a/pkg/apis/serving/v1alpha1/revision_types_test.go b/pkg/apis/serving/v1alpha1/revision_types_test.go index 4945b9a90813..182b9d120619 100644 --- a/pkg/apis/serving/v1alpha1/revision_types_test.go +++ b/pkg/apis/serving/v1alpha1/revision_types_test.go @@ -501,7 +501,6 @@ func TestTypicalFlowWithSuspendResume(t *testing.T) { checkConditionOngoingRevision(r.Status, RevisionConditionResourcesAvailable, t) checkConditionOngoingRevision(r.Status, RevisionConditionContainerHealthy, t) checkConditionOngoingRevision(r.Status, RevisionConditionReady, t) - checkConditionOngoingRevision(r.Status, RevisionConditionActive, t) // Enter a Ready state. r.Status.MarkActive() @@ -527,9 +526,7 @@ func TestTypicalFlowWithSuspendResume(t *testing.T) { if got := checkConditionFailedRevision(r.Status, RevisionConditionActive, t); got == nil || got.Reason != want { t.Errorf("MarkInactive = %v, want %v", got, want) } - if got := checkConditionFailedRevision(r.Status, RevisionConditionReady, t); got == nil || got.Reason != want { - t.Errorf("MarkInactive = %v, want %v", got, want) - } + checkConditionSucceededRevision(r.Status, RevisionConditionReady, t) // From an Inactive state, start to activate the revision. want = "Activating" @@ -539,9 +536,7 @@ func TestTypicalFlowWithSuspendResume(t *testing.T) { if got := checkConditionOngoingRevision(r.Status, RevisionConditionActive, t); got == nil || got.Reason != want { t.Errorf("MarkInactive = %v, want %v", got, want) } - if got := checkConditionOngoingRevision(r.Status, RevisionConditionReady, t); got == nil || got.Reason != want { - t.Errorf("MarkInactive = %v, want %v", got, want) - } + checkConditionSucceededRevision(r.Status, RevisionConditionReady, t) // From the activating state, simulate the transition back to readiness. r.Status.MarkActive() diff --git a/pkg/reconciler/v1alpha1/revision/reconcile_resources.go b/pkg/reconciler/v1alpha1/revision/reconcile_resources.go index 22f50b6dd50f..e9aadf9732b3 100644 --- a/pkg/reconciler/v1alpha1/revision/reconcile_resources.go +++ b/pkg/reconciler/v1alpha1/revision/reconcile_resources.go @@ -64,7 +64,7 @@ func (c *Reconciler) reconcileDeployment(ctx context.Context, rev *v1alpha1.Revi // Now that we have a Deployment, determine whether there is any relevant // status to surface in the Revision. - if hasDeploymentTimedOut(deployment) { + if hasDeploymentTimedOut(deployment) && !rev.Status.IsActivationRequired() { rev.Status.MarkProgressDeadlineExceeded(fmt.Sprintf( "Unable to create pods for more than %d seconds.", resources.ProgressDeadlineSeconds)) c.Recorder.Eventf(rev, corev1.EventTypeNormal, "ProgressDeadlineExceeded", @@ -187,7 +187,7 @@ func (c *Reconciler) reconcileService(ctx context.Context, rev *v1alpha1.Revisio // TODO(mattmoor): How to ensure this only fires once? c.Recorder.Eventf(rev, corev1.EventTypeNormal, "RevisionReady", "Revision becomes ready upon endpoint %q becoming ready", serviceName) - } else { + } else if !rev.Status.IsActivationRequired() { // If the endpoints is NOT ready, then check whether it is taking unreasonably // long to become ready and if so mark our revision as having timed out waiting // for the Service to become ready. diff --git a/pkg/reconciler/v1alpha1/revision/table_test.go b/pkg/reconciler/v1alpha1/revision/table_test.go index 6484f0ad8300..256d942e7125 100644 --- a/pkg/reconciler/v1alpha1/revision/table_test.go +++ b/pkg/reconciler/v1alpha1/revision/table_test.go @@ -48,10 +48,6 @@ import ( var ( conditionsOnFailure = duckv1alpha1.Conditions{{ - Type: "Active", - Status: "Unknown", - Severity: "Error", - }, { Type: "BuildSucceeded", Status: "True", Severity: "Error", @@ -76,7 +72,7 @@ var ( Type: "Active", Status: "Unknown", Reason: "Deploying", - Severity: "Error", + Severity: "Info", }, { Type: "BuildSucceeded", Status: "True", @@ -364,7 +360,7 @@ func TestReconcile(t *testing.T) { Type: "Active", Status: "Unknown", Reason: "Deploying", - Severity: "Error", + Severity: "Info", }, { Type: "BuildSucceeded", Status: "True", @@ -410,9 +406,8 @@ func TestReconcile(t *testing.T) { LogURL: "http://logger.io/test-uid", Conditions: duckv1alpha1.Conditions{{ Type: "Active", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", + Status: "True", + Severity: "Info", }, { Type: "BuildSucceeded", Status: "True", @@ -428,15 +423,27 @@ func TestReconcile(t *testing.T) { Reason: "Deploying", Severity: "Error", }, { - Type: "Ready", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", + Type: "Ready", + Status: "Unknown", + Reason: "Deploying", // The LTT defaults and is long enough ago that we expire waiting // on the Endpoints to become ready. + Severity: "Error", + }}, + }), + addKPAStatus( + kpa("foo", "endpoint-created-timeout", "busybox"), + kpav1alpha1.PodAutoscalerStatus{ + Conditions: duckv1alpha1.Conditions{{ + Type: "Active", + Status: "True", + Severity: "Info", + }, { + Type: "Ready", + Status: "True", + Severity: "Error", }}, }), - kpa("foo", "endpoint-created-timeout", "busybox"), deploy("foo", "endpoint-created-timeout", "busybox"), svc("foo", "endpoint-created-timeout", "busybox"), endpoints("foo", "endpoint-created-timeout", "busybox"), @@ -450,9 +457,8 @@ func TestReconcile(t *testing.T) { LogURL: "http://logger.io/test-uid", Conditions: duckv1alpha1.Conditions{{ Type: "Active", - Status: "Unknown", - Reason: "Deploying", - Severity: "Error", + Status: "True", + Severity: "Info", }, { Type: "BuildSucceeded", Status: "True", @@ -495,7 +501,7 @@ func TestReconcile(t *testing.T) { Type: "Active", Status: "Unknown", Reason: "Deploying", - Severity: "Error", + Severity: "Info", }, { Type: "BuildSucceeded", Status: "True", @@ -523,7 +529,7 @@ func TestReconcile(t *testing.T) { Conditions: duckv1alpha1.Conditions{{ Type: "Active", Status: "True", - Severity: "Error", + Severity: "Info", }, { Type: "Ready", Status: "True", @@ -544,7 +550,7 @@ func TestReconcile(t *testing.T) { Conditions: duckv1alpha1.Conditions{{ Type: "Active", Status: "True", - Severity: "Error", + Severity: "Info", }, { Type: "BuildSucceeded", Status: "True", @@ -579,7 +585,7 @@ func TestReconcile(t *testing.T) { Type: "Active", Status: "Unknown", Reason: "Deploying", - Severity: "Error", + Severity: "Info", }, { Type: "BuildSucceeded", Status: "True", @@ -606,7 +612,7 @@ func TestReconcile(t *testing.T) { Status: "Unknown", Reason: "Something", Message: "This is something longer", - Severity: "Error", + Severity: "Info", }, { Type: "Ready", Status: "Unknown", @@ -631,7 +637,7 @@ func TestReconcile(t *testing.T) { Status: "Unknown", Reason: "Something", Message: "This is something longer", - Severity: "Error", + Severity: "Info", }, { Type: "BuildSucceeded", Status: "True", @@ -642,9 +648,7 @@ func TestReconcile(t *testing.T) { Severity: "Error", }, { Type: "Ready", - Status: "Unknown", - Reason: "Something", - Message: "This is something longer", + Status: "True", Severity: "Error", }, { Type: "ResourcesAvailable", @@ -667,7 +671,7 @@ func TestReconcile(t *testing.T) { Type: "Active", Status: "Unknown", Reason: "Deploying", - Severity: "Error", + Severity: "Info", }, { Type: "BuildSucceeded", Status: "True", @@ -694,7 +698,7 @@ func TestReconcile(t *testing.T) { Status: "False", Reason: "NoTraffic", Message: "This thing is inactive.", - Severity: "Error", + Severity: "Info", }, { Type: "Ready", Status: "False", @@ -718,7 +722,7 @@ func TestReconcile(t *testing.T) { Status: "False", Reason: "NoTraffic", Message: "This thing is inactive.", - Severity: "Error", + Severity: "Info", }, { Type: "BuildSucceeded", Status: "True", @@ -730,9 +734,8 @@ func TestReconcile(t *testing.T) { Severity: "Error", }, { Type: "Ready", - Status: "False", - Reason: "NoTraffic", - Message: "This thing is inactive.", + Status: "Unknown", + Reason: "Deploying", Severity: "Error", }, { Type: "ResourcesAvailable", @@ -796,7 +799,7 @@ func TestReconcile(t *testing.T) { Type: "Active", Status: "Unknown", Reason: "Deploying", - Severity: "Error", + Severity: "Info", }, { Type: "BuildSucceeded", Status: "True", @@ -809,7 +812,7 @@ func TestReconcile(t *testing.T) { }, { Type: "Ready", Status: "Unknown", - Reason: "Deploying", + Reason: "Updating", Severity: "Error", }, { Type: "ResourcesAvailable", @@ -838,7 +841,7 @@ func TestReconcile(t *testing.T) { Conditions: duckv1alpha1.Conditions{{ Type: "Active", Status: "Unknown", - Severity: "Error", + Severity: "Info", }, { Type: "BuildSucceeded", Status: "True", @@ -920,7 +923,7 @@ func TestReconcile(t *testing.T) { Type: "Active", Status: "Unknown", Reason: "Deploying", - Severity: "Error", + Severity: "Info", }, { Type: "BuildSucceeded", Status: "True", @@ -963,10 +966,6 @@ func TestReconcile(t *testing.T) { v1alpha1.RevisionStatus{ LogURL: "http://logger.io/test-uid", Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "Unknown", - Severity: "Error", - }, { Type: "BuildSucceeded", Status: "Unknown", Severity: "Error", @@ -997,8 +996,9 @@ func TestReconcile(t *testing.T) { addBuild(rev("foo", "running-build", "busybox"), "the-build"), build("foo", "the-build", duckv1alpha1.Condition{ - Type: duckv1alpha1.ConditionSucceeded, - Status: corev1.ConditionUnknown, + Type: duckv1alpha1.ConditionSucceeded, + Status: corev1.ConditionUnknown, + Severity: "Error", }), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ @@ -1007,10 +1007,6 @@ func TestReconcile(t *testing.T) { v1alpha1.RevisionStatus{ LogURL: "http://logger.io/test-uid", Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "Unknown", - Severity: "Error", - }, { Type: "BuildSucceeded", Status: "Unknown", Reason: "Building", @@ -1063,8 +1059,9 @@ func TestReconcile(t *testing.T) { }}, }), build("foo", "the-build", duckv1alpha1.Condition{ - Type: duckv1alpha1.ConditionSucceeded, - Status: corev1.ConditionTrue, + Type: duckv1alpha1.ConditionSucceeded, + Status: corev1.ConditionTrue, + Severity: "Error", }), }, WantCreates: []metav1.Object{ @@ -1084,7 +1081,7 @@ func TestReconcile(t *testing.T) { Type: "Active", Status: "Unknown", Reason: "Deploying", - Severity: "Error", + Severity: "Info", }, { Type: "BuildSucceeded", Status: "True", @@ -1124,7 +1121,7 @@ func TestReconcile(t *testing.T) { Type: "Active", Status: "Unknown", Reason: "Deploying", - Severity: "Error", + Severity: "Info", }, { Type: "BuildSucceeded", Status: "True", @@ -1187,10 +1184,11 @@ func TestReconcile(t *testing.T) { }}, }), build("foo", "the-build", duckv1alpha1.Condition{ - Type: duckv1alpha1.ConditionSucceeded, - Status: corev1.ConditionFalse, - Reason: "SomeReason", - Message: "This is why the build failed.", + Type: duckv1alpha1.ConditionSucceeded, + Status: corev1.ConditionFalse, + Reason: "SomeReason", + Message: "This is why the build failed.", + Severity: "Error", }), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ @@ -1199,10 +1197,6 @@ func TestReconcile(t *testing.T) { v1alpha1.RevisionStatus{ LogURL: "http://logger.io/test-uid", Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "Unknown", - Severity: "Error", - }, { Type: "BuildSucceeded", Status: "False", Reason: "SomeReason", @@ -1240,7 +1234,7 @@ func TestReconcile(t *testing.T) { Conditions: duckv1alpha1.Conditions{{ Type: "Active", Status: "Unknown", - Severity: "Error", + Severity: "Info", }, { Type: "ResourcesAvailable", Status: "Unknown", @@ -1264,10 +1258,11 @@ func TestReconcile(t *testing.T) { }}, }), build("foo", "the-build", duckv1alpha1.Condition{ - Type: duckv1alpha1.ConditionSucceeded, - Status: corev1.ConditionFalse, - Reason: "SomeReason", - Message: "This is why the build failed.", + Type: duckv1alpha1.ConditionSucceeded, + Status: corev1.ConditionFalse, + Reason: "SomeReason", + Message: "This is why the build failed.", + Severity: "Error", }), }, Key: "foo/failed-build-stable", @@ -1373,10 +1368,6 @@ func TestReconcileWithVarLogEnabled(t *testing.T) { ServiceName: svc("foo", "create-configmap-failure", "busybox").Name, LogURL: "http://logger.io/test-uid", Conditions: duckv1alpha1.Conditions{{ - Type: "Active", - Status: "Unknown", - Severity: "Error", - }, { Type: "BuildSucceeded", Status: "True", Severity: "Error", diff --git a/test/e2e/autoscale_test.go b/test/e2e/autoscale_test.go index 866276e8f085..5071bbccceda 100644 --- a/test/e2e/autoscale_test.go +++ b/test/e2e/autoscale_test.go @@ -178,8 +178,8 @@ func setup(t *testing.T) *testContext { if err != nil { t.Fatalf("Configuration %s was not updated with the new revision: %v", names.Config, err) } - deploymentName := - config.Status.LatestCreatedRevisionName + "-deployment" + names.Revision = config.Status.LatestCreatedRevisionName + deploymentName := names.Revision + "-deployment" route, err := clients.ServingClient.Routes.Get(names.Route, metav1.GetOptions{}) if err != nil { t.Fatalf("Error fetching Route %s: %v", names.Route, err) @@ -260,6 +260,12 @@ func assertScaleDown(ctx *testContext) { ctx.t.Fatalf("Waiting for Pod.List to have no non-Evicted pods: %v", err) } + time.Sleep(10 * time.Second) + ctx.logger.Info("The Revision should remain ready after scaling to zero.") + if err := test.CheckRevisionState(ctx.clients.ServingClient, ctx.names.Revision, test.IsRevisionReady); err != nil { + ctx.t.Fatalf("The Revision %s did not stay Ready after scaling down to zero: %v", ctx.names.Revision, err) + } + ctx.logger.Infof("Scaled down.") }