From 609adac6c8afd94cbb0ac173739096531740236b Mon Sep 17 00:00:00 2001 From: Vincent Hou Date: Thu, 15 May 2025 17:26:33 -0400 Subject: [PATCH 1/4] Revision sets to True when deploy has the minimum number --- pkg/reconciler/revision/reconcile_resources.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/reconciler/revision/reconcile_resources.go b/pkg/reconciler/revision/reconcile_resources.go index cdb90c553794..8089fb120ab6 100644 --- a/pkg/reconciler/revision/reconcile_resources.go +++ b/pkg/reconciler/revision/reconcile_resources.go @@ -120,7 +120,7 @@ func (c *Reconciler) reconcileDeployment(ctx context.Context, rev *v1.Revision) } } - if deployment.Status.ReadyReplicas > 0 { + if deployment.Spec.Replicas != nil && deployment.Status.ReadyReplicas >= *deployment.Spec.Replicas { rev.Status.MarkContainerHealthyTrue() } From 0cb7ff84eeecbafae2446942e398574aaef68cf9 Mon Sep 17 00:00:00 2001 From: Vincent Hou Date: Fri, 16 May 2025 15:58:46 -0400 Subject: [PATCH 2/4] Added the e2e test verification to make sure pod number reaches minScale --- test/e2e/minscale_readiness_test.go | 30 +++++++++++++++++++++++++++++ test/v1/revision.go | 5 +++++ 2 files changed, 35 insertions(+) diff --git a/test/e2e/minscale_readiness_test.go b/test/e2e/minscale_readiness_test.go index 95d6fb5c0e3a..a373fbd35394 100644 --- a/test/e2e/minscale_readiness_test.go +++ b/test/e2e/minscale_readiness_test.go @@ -85,6 +85,21 @@ func TestMinScale(t *testing.T) { revName := latestRevisionName(t, clients, names.Config, "") serviceName := PrivateServiceName(t, clients, revName) + t.Log("Waiting for new revision's container to become healthy with the minScale") + if err := v1test.WaitForRevisionState( + clients.ServingClient, newRevName, v1test.IsRevisionContainerHealthy, "ContainerIsHealthy", + ); err != nil { + t.Fatalf("The container of the Revision %q did not become healthy: %v", newRevName, err) + } + + revision, err := clients.ServingClient.Revisions.Get(context.Background(), revName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("An error occurred getting revision %v, %v", revName, err) + } + if replicas := *revision.Status.ActualReplicas; replicas < minScale { + t.Fatalf("Container is indicated as healthy. Expected actual replicas for revision %v to be %v but got %v", revision.Name, minScale, replicas) + } + t.Log("Waiting for revision to scale to minScale before becoming ready") if lr, err := waitForDesiredScale(clients, serviceName, gte(minScale)); err != nil { t.Fatalf("The revision %q scaled to %d < %d before becoming ready: %v", revName, lr, minScale, err) @@ -118,6 +133,21 @@ func TestMinScale(t *testing.T) { newRevName := latestRevisionName(t, clients, names.Config, revName) newServiceName := PrivateServiceName(t, clients, newRevName) + t.Log("Waiting for new revision's container to become healthy with the minScale") + if err := v1test.WaitForRevisionState( + clients.ServingClient, newRevName, v1test.IsRevisionContainerHealthy, "ContainerIsHealthy", + ); err != nil { + t.Fatalf("The container of the Revision %q did not become healthy: %v", newRevName, err) + } + + revision, err := clients.ServingClient.Revisions.Get(context.Background(), revName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("An error occurred getting revision %v, %v", revName, err) + } + if replicas := *revision.Status.ActualReplicas; replicas < minScale { + t.Fatalf("Container is indicated as healthy. Expected actual replicas for revision %v to be %v but got %v", revision.Name, minScale, replicas) + } + t.Log("Waiting for new revision to scale to minScale after update") if lr, err := waitForDesiredScale(clients, newServiceName, gte(minScale)); err != nil { t.Fatalf("The revision %q scaled to %d < %d after creating route: %v", newRevName, lr, minScale, err) diff --git a/test/v1/revision.go b/test/v1/revision.go index 591ddf94f256..62b5340a2b8e 100644 --- a/test/v1/revision.go +++ b/test/v1/revision.go @@ -79,6 +79,11 @@ func IsRevisionReady(r *v1.Revision) (bool, error) { return r.IsReady(), nil } +// IsRevisionContainerHealthy will indicate whether the revision has marked the container healthy. +func IsRevisionContainerHealthy(r *v1.Revision) (bool, error) { + return r.Status.GetCondition(v1.RevisionConditionContainerHealthy).IsTrue(), nil +} + // IsRevisionFailed will check the status condition sof the revision and return true if the revision is // marked as failed, otherwise it will return false. func IsRevisionFailed(r *v1.Revision) (bool, error) { From 1bbcee757c90c7c652da714782d3f5084cfebe87 Mon Sep 17 00:00:00 2001 From: Vincent Hou Date: Fri, 16 May 2025 16:05:42 -0400 Subject: [PATCH 3/4] Used the correct name for the revision --- test/e2e/minscale_readiness_test.go | 32 ++++++++--------------------- test/v1/revision.go | 5 ++++- 2 files changed, 13 insertions(+), 24 deletions(-) diff --git a/test/e2e/minscale_readiness_test.go b/test/e2e/minscale_readiness_test.go index a373fbd35394..bb2b54136a3a 100644 --- a/test/e2e/minscale_readiness_test.go +++ b/test/e2e/minscale_readiness_test.go @@ -85,11 +85,11 @@ func TestMinScale(t *testing.T) { revName := latestRevisionName(t, clients, names.Config, "") serviceName := PrivateServiceName(t, clients, revName) - t.Log("Waiting for new revision's container to become healthy with the minScale") + t.Log("Waiting for new revision to become ready") if err := v1test.WaitForRevisionState( - clients.ServingClient, newRevName, v1test.IsRevisionContainerHealthy, "ContainerIsHealthy", + clients.ServingClient, revName, v1test.IsRevisionReady, "RevisionIsReady", ); err != nil { - t.Fatalf("The container of the Revision %q did not become healthy: %v", newRevName, err) + t.Fatalf("The Revision %q did not become ready: %v", revName, err) } revision, err := clients.ServingClient.Revisions.Get(context.Background(), revName, metav1.GetOptions{}) @@ -105,19 +105,12 @@ func TestMinScale(t *testing.T) { t.Fatalf("The revision %q scaled to %d < %d before becoming ready: %v", revName, lr, minScale, err) } - t.Log("Waiting for revision to become ready") - if err := v1test.WaitForRevisionState( - clients.ServingClient, revName, v1test.IsRevisionReady, "RevisionIsReady", - ); err != nil { - t.Fatalf("The Revision %q did not become ready: %v", revName, err) - } - t.Log("Holding revision at minScale after becoming ready") if lr, ok := ensureDesiredScale(clients, t, serviceName, gte(minScale)); !ok { t.Fatalf("The revision %q observed scale %d < %d after becoming ready", revName, lr, minScale) } - revision, err := clients.ServingClient.Revisions.Get(context.Background(), revName, metav1.GetOptions{}) + revision, err = clients.ServingClient.Revisions.Get(context.Background(), revName, metav1.GetOptions{}) if err != nil { t.Fatalf("An error occurred getting revision %v, %v", revName, err) } @@ -133,16 +126,16 @@ func TestMinScale(t *testing.T) { newRevName := latestRevisionName(t, clients, names.Config, revName) newServiceName := PrivateServiceName(t, clients, newRevName) - t.Log("Waiting for new revision's container to become healthy with the minScale") + t.Log("Waiting for new revision to become ready") if err := v1test.WaitForRevisionState( - clients.ServingClient, newRevName, v1test.IsRevisionContainerHealthy, "ContainerIsHealthy", + clients.ServingClient, newRevName, v1test.IsRevisionReady, "RevisionIsReady", ); err != nil { - t.Fatalf("The container of the Revision %q did not become healthy: %v", newRevName, err) + t.Fatalf("The Revision %q did not become ready: %v", newRevName, err) } - revision, err := clients.ServingClient.Revisions.Get(context.Background(), revName, metav1.GetOptions{}) + revision, err = clients.ServingClient.Revisions.Get(context.Background(), newRevName, metav1.GetOptions{}) if err != nil { - t.Fatalf("An error occurred getting revision %v, %v", revName, err) + t.Fatalf("An error occurred getting revision %v, %v", newRevName, err) } if replicas := *revision.Status.ActualReplicas; replicas < minScale { t.Fatalf("Container is indicated as healthy. Expected actual replicas for revision %v to be %v but got %v", revision.Name, minScale, replicas) @@ -153,13 +146,6 @@ func TestMinScale(t *testing.T) { t.Fatalf("The revision %q scaled to %d < %d after creating route: %v", newRevName, lr, minScale, err) } - t.Log("Waiting for new revision to become ready") - if err := v1test.WaitForRevisionState( - clients.ServingClient, newRevName, v1test.IsRevisionReady, "RevisionIsReady", - ); err != nil { - t.Fatalf("The Revision %q did not become ready: %v", newRevName, err) - } - t.Log("Holding new revision at minScale after becoming ready") if lr, ok := ensureDesiredScale(clients, t, newServiceName, gte(minScale)); !ok { t.Fatalf("The revision %q observed scale %d < %d after becoming ready", newRevName, lr, minScale) diff --git a/test/v1/revision.go b/test/v1/revision.go index 62b5340a2b8e..3843cd387671 100644 --- a/test/v1/revision.go +++ b/test/v1/revision.go @@ -81,7 +81,10 @@ func IsRevisionReady(r *v1.Revision) (bool, error) { // IsRevisionContainerHealthy will indicate whether the revision has marked the container healthy. func IsRevisionContainerHealthy(r *v1.Revision) (bool, error) { - return r.Status.GetCondition(v1.RevisionConditionContainerHealthy).IsTrue(), nil + if r.Status.GetCondition(v1.RevisionConditionContainerHealthy).IsTrue() { + return true, nil + } + return false, nil } // IsRevisionFailed will check the status condition sof the revision and return true if the revision is From 797b64c1cbf1ae4bdf1738d6e1af74c66eb001b6 Mon Sep 17 00:00:00 2001 From: Vincent Hou Date: Mon, 19 May 2025 11:09:18 -0400 Subject: [PATCH 4/4] Removed the unused function IsRevisionContainerHealthy --- test/v1/revision.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/test/v1/revision.go b/test/v1/revision.go index 3843cd387671..591ddf94f256 100644 --- a/test/v1/revision.go +++ b/test/v1/revision.go @@ -79,14 +79,6 @@ func IsRevisionReady(r *v1.Revision) (bool, error) { return r.IsReady(), nil } -// IsRevisionContainerHealthy will indicate whether the revision has marked the container healthy. -func IsRevisionContainerHealthy(r *v1.Revision) (bool, error) { - if r.Status.GetCondition(v1.RevisionConditionContainerHealthy).IsTrue() { - return true, nil - } - return false, nil -} - // IsRevisionFailed will check the status condition sof the revision and return true if the revision is // marked as failed, otherwise it will return false. func IsRevisionFailed(r *v1.Revision) (bool, error) {