From b4b049d05061da1d7a84314e7010fb216cb37b32 Mon Sep 17 00:00:00 2001 From: Matt Moore Date: Fri, 2 Nov 2018 20:48:11 +0000 Subject: [PATCH] Have Activator key off of Active not Ready. This adjusts the condition that the activator keys off of to determine whether it can start forwarding requests to the Revision. In the current world, this change is just splitting hairs because these conditions end up being largely the same; however, this is relevant cleanup if we want to shift to a world where `Active=False` does not trigger `Ready=False` because requests to the activator would incorrectly see an `IsReady()` Revision and incorrectly forward traffic. Peripherally related to: https://github.com/knative/serving/issues/2394 --- pkg/activator/revision.go | 14 +++++++------- pkg/activator/revision_test.go | 3 ++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/pkg/activator/revision.go b/pkg/activator/revision.go index 6825f13594e1..3099af77d2dc 100644 --- a/pkg/activator/revision.go +++ b/pkg/activator/revision.go @@ -78,8 +78,8 @@ func (r *revisionActivator) activateRevision(namespace, name string) (*v1alpha1. serviceName, configurationName := getServiceAndConfigurationLabels(revision) r.reporter.ReportRequest(namespace, serviceName, configurationName, name, 1.0) - // Wait for the revision to be ready - if !revision.Status.IsReady() { + // Wait for the revision to not require activation. + if revision.Status.IsActivationRequired() { wi, err := r.knaClient.ServingV1alpha1().Revisions(rev.namespace).Watch(metav1.ListOptions{ FieldSelector: fmt.Sprintf("metadata.name=%s", rev.name), }) @@ -88,24 +88,24 @@ func (r *revisionActivator) activateRevision(namespace, name string) (*v1alpha1. } defer wi.Stop() ch := wi.ResultChan() - RevisionReady: + RevisionActive: for { select { case <-time.After(r.readyTimout): // last chance to check - if revision.Status.IsReady() { - break RevisionReady + if !revision.Status.IsActivationRequired() { + break RevisionActive } return nil, errors.New("Timeout waiting for revision to become ready") case event := <-ch: if revision, ok := event.Object.(*v1alpha1.Revision); ok { - if !revision.Status.IsReady() { + if revision.Status.IsActivationRequired() { logger.Info("Revision is not yet ready") continue } else { logger.Info("Revision is ready") } - break RevisionReady + break RevisionActive } else { return nil, fmt.Errorf("Unexpected result type for revision: %v", event) } diff --git a/pkg/activator/revision_test.go b/pkg/activator/revision_test.go index b12c28e09605..0d843134d6b6 100644 --- a/pkg/activator/revision_test.go +++ b/pkg/activator/revision_test.go @@ -210,8 +210,9 @@ func (b *revisionBuilder) withReady(ready bool) *revisionBuilder { if ready { b.revision.Status.MarkContainerHealthy() b.revision.Status.MarkResourcesAvailable() + b.revision.Status.MarkActive() } else { - b.revision.Status.MarkContainerMissing("reasonz") + b.revision.Status.MarkInactive("reasonz", "detailz") } return b }