Skip to content

Commit

Permalink
Assert against Revision generation annotation instead of config spec
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@bobcatfish
bobcatfish committed Jul 19, 2018
1 parent 9aea1ed commit 896628a
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 15 deletions.
22 changes: 15 additions & 7 deletions test/conformance/route_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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!")
}
23 changes: 15 additions & 8 deletions test/conformance/service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions test/spoof/spoof.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down

0 comments on commit 896628a

Please sign in to comment.