Skip to content

Commit

Permalink
Have Activator key off of Active not Ready. (#2395)
Browse files Browse the repository at this point in the history
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: #2394
  • Loading branch information
mattmoor authored and knative-prow-robot committed Nov 2, 2018
1 parent bce6ec5 commit cdb319d
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 8 deletions.
14 changes: 7 additions & 7 deletions pkg/activator/revision.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
})
Expand All @@ -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)
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/activator/revision_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit cdb319d

Please sign in to comment.