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
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.

Author: bobcatfish

test/conformance/route_test.go

@@ -27,6 +27,7 @@ import (
 
 	"encoding/json"
 
+	"github.com/knative/serving/pkg/apis/serving"
 	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
 	"github.com/knative/serving/test"
 	"github.com/mattbaird/jsonpatch"
@@ -61,17 +62,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 +93,16 @@ 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, 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", names.Revision, expectedGeneration, a)
+			}
+			return true, nil
+		}
+		return true, fmt.Errorf("Expected Revision %s to be annotated with generation %s but there was no annotation", names.Revision, expectedGeneration)
+	})
 	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 +183,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 +198,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

@@ -24,6 +24,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/knative/serving/pkg/apis/serving"
 	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
 	serviceresourcenames "github.com/knative/serving/pkg/controller/service/resources/names"
 	"github.com/knative/serving/test"
@@ -52,18 +53,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 +73,16 @@ 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, 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", names.Revision, expectedGeneration, a)
+			}
+			return true, nil
+		}
+		return true, fmt.Errorf("Expected Revision %s to be annotated with generation %s but there was no annotation", names.Revision, expectedGeneration)
+	})
 	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 +170,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 +188,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)
 		}