Assert against Revision generation annotation instead of config spec

In #475 I removed assertions
against Configuratoin.Spec.Generation b/c it is a hack and not part of
the knative spec.

In #600 I updated the
conformance tests to assert against the Revision annotation which
contains the generation.

BUT THEN in #778 when I
completely re-wrote the tests to no longer use Ginkgo, I accidentally
until both of those changes, so this commit puts them back 😅.

BONUS: I hit a case where the length of the loadbalancer ingresses was
0 and got a panic, so if that happens again we'll get an informative
error instead.

test/conformance/route_test.go

@@ -18,7 +18,6 @@ limitations under the License.
 package conformance
 
 import (
-	"fmt"
 	"strings"
 	"testing"
 
@@ -61,17 +60,14 @@ func updateConfigWithImage(clients *test.Clients, names test.ResourceNames, imag
 		},
 	}
 	patchBytes, err := json.Marshal(patches)
-	newConfig, err := clients.Configs.Patch(names.Config, types.JSONPatchType, patchBytes, "")
+	_, err = clients.Configs.Patch(names.Config, types.JSONPatchType, patchBytes, "")
 	if err != nil {
 		return err
 	}
-	if newConfig.Spec.Generation != int64(2) {
-		return fmt.Errorf("The spec was updated so the Generation should be 2 but it was actually %d", newConfig.Spec.Generation)
-	}
 	return nil
 }
 
-func assertResourcesUpdatedWhenRevisionIsReady(t *testing.T, logger *zap.SugaredLogger, clients *test.Clients, names test.ResourceNames, expectedText string) {
+func assertResourcesUpdatedWhenRevisionIsReady(t *testing.T, logger *zap.SugaredLogger, clients *test.Clients, names test.ResourceNames, expectedGeneration, expectedText string) {
 	logger.Infof("When the Route reports as Ready, everything should be ready.")
 	err := test.WaitForRouteState(clients.Routes, names.Route, func(r *v1alpha1.Route) (bool, error) {
 		if cond := r.Status.GetCondition(v1alpha1.RouteConditionReady); cond == nil {
@@ -99,7 +95,7 @@ func assertResourcesUpdatedWhenRevisionIsReady(t *testing.T, logger *zap.Sugared
 
 	// We want to verify that the endpoint works as soon as Ready: True, but there are a bunch of other pieces of state that we validate for conformance.
 	logger.Infof("The Revision will be marked as Ready when it can serve traffic")
-	err = test.CheckRevisionState(clients.Revisions, names.Revision, test.IsRevisionReady())
+	err = test.CheckRevisionState(clients.Revisions, names.Revision, test.IsRevisionReady(expectedGeneration))
 	if err != nil {
 		t.Fatalf("Revision %s did not become ready to serve traffic: %v", names.Revision, err)
 	}
@@ -174,7 +170,7 @@ func TestRouteCreation(t *testing.T) {
 	}
 	names.Revision = revisionName
 
-	assertResourcesUpdatedWhenRevisionIsReady(t, logger, clients, names, "What a spaceport!")
+	assertResourcesUpdatedWhenRevisionIsReady(t, logger, clients, names, "1", "What a spaceport!")
 
 	logger.Infof("Updating the Configuration to use a different image")
 	err = updateConfigWithImage(clients, names, imagePaths)
@@ -189,5 +185,5 @@ func TestRouteCreation(t *testing.T) {
 	}
 	names.Revision = revisionName
 
-	assertResourcesUpdatedWhenRevisionIsReady(t, logger, clients, names, "Re-energize yourself with a slice of pepperoni!")
+	assertResourcesUpdatedWhenRevisionIsReady(t, logger, clients, names, "2", "Re-energize yourself with a slice of pepperoni!")
 }

test/conformance/service_test.go

@@ -19,7 +19,6 @@ package conformance
 
 import (
 	"encoding/json"
-	"fmt"
 	"strings"
 	"testing"
 
@@ -51,18 +50,15 @@ func updateServiceWithImage(clients *test.Clients, names test.ResourceNames, ima
 	if err != nil {
 		return err
 	}
-	newService, err := clients.Services.Patch(names.Service, types.JSONPatchType, patchBytes, "")
+	_, err = clients.Services.Patch(names.Service, types.JSONPatchType, patchBytes, "")
 	if err != nil {
 		return err
 	}
-	if newService.Spec.Generation != int64(2) {
-		return fmt.Errorf("The spec was updated so the Generation should be 2 but it was actually %d", newService.Spec.Generation)
-	}
 	return nil
 }
 
 // Shamelessly cribbed from route_test. We expect the Route and Configuration to be ready if the Service is ready.
-func assertServiceResourcesUpdated(t *testing.T, logger *zap.SugaredLogger, clients *test.Clients, names test.ResourceNames, routeDomain, expectedText string) {
+func assertServiceResourcesUpdated(t *testing.T, logger *zap.SugaredLogger, clients *test.Clients, names test.ResourceNames, routeDomain, expectedGeneration, expectedText string) {
 	// TODO(#1178): Remove "Wait" from all checks below this point.
 	err := test.WaitForEndpointState(clients.Kube, logger, test.Flags.ResolvableDomain, routeDomain, namespaceName, names.Route, func(body string) (bool, error) {
 		return body == expectedText, nil
@@ -73,7 +69,7 @@ func assertServiceResourcesUpdated(t *testing.T, logger *zap.SugaredLogger, clie
 
 	// We want to verify that the endpoint works as soon as Ready: True, but there are a bunch of other pieces of state that we validate for conformance.
 	logger.Info("The Revision will be marked as Ready when it can serve traffic")
-	if err := test.CheckRevisionState(clients.Revisions, names.Revision, test.IsRevisionReady()); err != nil {
+	if err := test.CheckRevisionState(clients.Revisions, names.Revision, test.IsRevisionReady(expectedGeneration)); err != nil {
 		t.Fatalf("Revision %s did not become ready to serve traffic: %v", names.Revision, err)
 	}
 
@@ -158,7 +154,7 @@ func TestRunLatestService(t *testing.T) {
 	if err := test.WaitForServiceState(clients.Services, names.Service, test.IsServiceReady(), "ServiceIsReady"); err != nil {
 		t.Fatalf("The Service %s was not marked as Ready to serve traffic to Revision %s: %v", names.Service, names.Revision, err)
 	}
-	assertServiceResourcesUpdated(t, logger, clients, names, routeDomain, "What a spaceport!")
+	assertServiceResourcesUpdated(t, logger, clients, names, routeDomain, "1", "What a spaceport!")
 
 	logger.Info("Updating the Service to use a different image")
 	if err := updateServiceWithImage(clients, names, imagePaths[1]); err != nil {
@@ -176,7 +172,7 @@ func TestRunLatestService(t *testing.T) {
 	if err := test.WaitForServiceState(clients.Services, names.Service, test.IsServiceReady(), "ServiceIsReady"); err != nil {
 		t.Fatalf("The Service %s was not marked as Ready to serve traffic to Revision %s: %v", names.Service, names.Revision, err)
 	}
-	assertServiceResourcesUpdated(t, logger, clients, names, routeDomain, "Re-energize yourself with a slice of pepperoni!")
+	assertServiceResourcesUpdated(t, logger, clients, names, routeDomain, "2", "Re-energize yourself with a slice of pepperoni!")
 }
 
 // TODO(jonjohnsonjr): LatestService roads less traveled.

test/request.go

@@ -93,6 +93,9 @@ func WaitForEndpointState(kubeClientset *kubernetes.Clientset, logger *zap.Sugar
 		if err != nil {
 			return err
 		}
+		if len(ingress.Status.LoadBalancer.Ingress) != 1 {
+			return fmt.Errorf("Expected exactly one ingress load balancer, instead had %d: %s", len(ingress.Status.LoadBalancer.Ingress), ingress.Status.LoadBalancer.Ingress)
+		}
 		if ingress.Status.LoadBalancer.Ingress[0].IP == "" {
 			return fmt.Errorf("Expected ingress loadbalancer IP for %s to be set, instead was empty", ingressName)
 		}

test/states.go

@@ -18,6 +18,7 @@ package test
 import (
 	"fmt"
 
+	"github.com/knative/serving/pkg/apis/serving"
 	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
 	corev1 "k8s.io/api/core/v1"
 )
@@ -48,26 +49,35 @@ func AllRouteTrafficAtRevision(names ResourceNames) func(r *v1alpha1.Route) (boo
 }
 
 // IsRevisionReady will check the status conditions of the revision and return true if the revision is
-// ready to serve traffic. It will return false if the status indicates a state other than deploying
-// or being ready. It will also return false if the type of the condition is unexpected.
-func IsRevisionReady() func(r *v1alpha1.Revision) (bool, error) {
+// ready to serve traffic. It will return false until the status becomes true. If the status is true
+// and the expected generation annotation is not present an error will be returned.
+func IsRevisionReady(generation string) func(r *v1alpha1.Revision) (bool, error) {
 	return func(r *v1alpha1.Revision) (bool, error) {
-		if cond := r.Status.GetCondition(v1alpha1.RevisionConditionReady); cond == nil {
+		cond := r.Status.GetCondition(v1alpha1.RevisionConditionReady)
+		if cond == nil {
 			return false, nil
-		} else {
-			return cond.Status == corev1.ConditionTrue, nil
 		}
+		if cond.Status == corev1.ConditionTrue {
+			if a, ok := r.Annotations[serving.ConfigurationGenerationAnnotationKey]; ok {
+				if a != generation {
+					return true, fmt.Errorf("Expected Revision to be annotated with generation %s but was %s instead", generation, a)
+				}
+				return true, nil
+			}
+			return true, fmt.Errorf("Expected Revision to be annotated with generation %s but there was no annotation", generation)
+		}
+		return false, nil
 	}
 }
 
 // IsServiceReady will check the status conditions of the service and return true if the service is
 // ready. This means that its configurations and routes have all reported ready.
 func IsServiceReady() func(r *v1alpha1.Service) (bool, error) {
 	return func(s *v1alpha1.Service) (bool, error) {
-		if cond := s.Status.GetCondition(v1alpha1.ServiceConditionReady); cond == nil {
+		cond := s.Status.GetCondition(v1alpha1.ServiceConditionReady)
+		if cond == nil {
 			return false, nil
-		} else {
-			return cond.Status == corev1.ConditionTrue, nil
 		}
+		return cond.Status == corev1.ConditionTrue, nil
 	}
 }