Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assert against Revision generation annotation instead of config spec #1428

Merged
merged 1 commit into from
Jul 20, 2018
Merged

Assert against Revision generation annotation instead of config spec #1428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions test/conformance/route_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ limitations under the License.
package conformance

import (
"fmt"
"strings"
"testing"

Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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!")
}
18 changes: 9 additions & 9 deletions test/conformance/service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ package conformance

import (
"encoding/json"
"fmt"
"strings"
"testing"

Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions test/spoof/spoof.go
View file
Open in desktop
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
16 changes: 16 additions & 0 deletions test/states.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay so now this is the only function in states.go that returns a function, i think im okay with that but curious what you think @jonjohnsonjr

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See AllRouteTrafficAtRevision

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well nevermind then sir!

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)
}
}