Assert against Revision generation annotation instead of config spec

In #475 I removed assertions
against Configuration.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
undid 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

@@ -19,7 +19,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.")
 	if err := test.WaitForRouteState(clients.Routes, names.Route, test.IsRouteReady, "RouteIsReady"); err != nil {
 		t.Fatalf("The Route %s was not marked as Ready to serve traffic to Revision %s: %v", names.Route, names.Revision, err)
@@ -95,6 +91,11 @@ func assertResourcesUpdatedWhenRevisionIsReady(t *testing.T, logger *zap.Sugared
 	if err != nil {
 		t.Fatalf("Revision %s did not become ready to serve traffic: %v", names.Revision, err)
 	}
+	logger.Infof("The Revision will be annotated with the generation")
+	err = test.CheckRevisionState(clients.Revisions, names.Revision, test.IsRevisionAtExpectedGeneration(expectedGeneration))
+	if err != nil {
+		t.Fatalf("Revision %s did not have an expected annotation with generation %s: %v", names.Revision, expectedGeneration, err)
+	}
 	logger.Infof("Updates the Configuration that the Revision is ready")
 	err = test.CheckConfigurationState(clients.Configs, names.Config, func(c *v1alpha1.Configuration) (bool, error) {
 		return c.Status.LatestReadyRevisionName == names.Revision, nil
@@ -175,7 +176,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)
@@ -190,5 +191,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

@@ -20,7 +20,6 @@ package conformance
 
 import (
 	"encoding/json"
-	"fmt"
 	"strings"
 	"testing"
 
@@ -52,18 +51,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, test.EventuallyMatchesBody(expectedText), "WaitForEndpointToServeText")
 	if err != nil {
@@ -75,7 +71,11 @@ func assertServiceResourcesUpdated(t *testing.T, logger *zap.SugaredLogger, clie
 	if err := test.CheckRevisionState(clients.Revisions, names.Revision, test.IsRevisionReady); err != nil {
 		t.Fatalf("Revision %s did not become ready to serve traffic: %v", names.Revision, err)
 	}
-
+	logger.Infof("The Revision will be annotated with the generation")
+	err = test.CheckRevisionState(clients.Revisions, names.Revision, test.IsRevisionAtExpectedGeneration(expectedGeneration))
+	if err != nil {
+		t.Fatalf("Revision %s did not have an expected annotation with generation %s: %v", names.Revision, expectedGeneration, err)
+	}
 	logger.Info("The Service's latestReadyRevisionName should match the Configuration's")
 	err = test.CheckConfigurationState(clients.Configs, names.Config, func(c *v1alpha1.Configuration) (bool, error) {
 		return c.Status.LatestReadyRevisionName == names.Revision, nil
@@ -163,7 +163,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 {
@@ -181,7 +181,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/spoof/spoof.go

@@ -103,6 +103,10 @@ func New(kubeClientset *kubernetes.Clientset, logger *zap.SugaredLogger, domain
 			return nil, err
 		}
 
+		if len(ingress.Status.LoadBalancer.Ingress) != 1 {
+			return nil, 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 nil, fmt.Errorf("Expected ingress loadbalancer IP for %s to be set, instead was empty", ingressName)
 		}

test/states.go

@@ -21,6 +21,7 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 
+	"github.com/knative/serving/pkg/apis/serving"
 	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
 )
 
@@ -113,3 +114,18 @@ func IsConfigRevisionCreationFailed(c *v1alpha1.Configuration) (bool, error) {
 	}
 	return false, nil
 }
+
+// IsRevisionAtExpectedGeneration returns a function that will check if the annotations
+// on the revision include an annotation for the generation and that the annotation is
+// set to the expected value.
+func IsRevisionAtExpectedGeneration(expectedGeneration string) func(r *v1alpha1.Revision) (bool, error) {
+	return func(r *v1alpha1.Revision) (bool, error) {
+		if a, ok := r.Annotations[serving.ConfigurationGenerationAnnotationKey]; ok {
+			if a != expectedGeneration {
+				return true, fmt.Errorf("Expected Revision %s to be annotated with generation %s but was %s instead", r.Name, expectedGeneration, a)
+			}
+			return true, nil
+		}
+		return true, fmt.Errorf("Expected Revision %s to be annotated with generation %s but there was no annotation", r.Name, expectedGeneration)
+	}
+}