-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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
Add conformance tests for .metadata.generateName. #3292
Merged
knative-prow-robot
merged 5 commits into
knative:master
from
brandon-mabey:generate-name-conformance
Mar 19, 2019
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
e46bde2
Add conformance tests for .metadata.generateName.
brandon-mabey 3a164fd
Cleanup generate name conformance tests.
brandon-mabey 1ed5406
Simplify name verification regex for generateName conformance tests.
brandon-mabey ed7d4a0
Change IsStatusOk() -> IsStatusOk in generateName conformance tests.
brandon-mabey 8e48797
Cleanup error messages to match Go style more closely.
brandon-mabey File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,203 @@ | ||
// +build e2e | ||
|
||
/* | ||
Copyright 2019 The Knative Authors | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
package conformance | ||
|
||
import ( | ||
"fmt" | ||
"net/http" | ||
"regexp" | ||
"testing" | ||
|
||
pkgTest "github.com/knative/pkg/test" | ||
"github.com/knative/serving/pkg/apis/serving/v1alpha1" | ||
rtesting "github.com/knative/serving/pkg/reconciler/v1alpha1/testing" | ||
"github.com/knative/serving/test" | ||
) | ||
|
||
func setServiceGenerateName(generateName string) rtesting.ServiceOption { | ||
return func(service *v1alpha1.Service) { | ||
service.ObjectMeta.GenerateName = generateName | ||
} | ||
} | ||
|
||
func setConfigurationGenerateName(generateName string) rtesting.ConfigOption { | ||
return func(config *v1alpha1.Configuration) { | ||
config.ObjectMeta.GenerateName = generateName | ||
} | ||
} | ||
|
||
func setRouteGenerateName(generateName string) rtesting.RouteOption { | ||
return func(route *v1alpha1.Route) { | ||
route.ObjectMeta.GenerateName = generateName | ||
} | ||
} | ||
|
||
// generateNamePrefix returns the object name to be used for testing, shorted to | ||
// 44 characters to avoid #3236, as generateNames longer than 44 characters may cause | ||
// some knative resources to never become ready. | ||
func generateNamePrefix(t *testing.T) string { | ||
generateName := test.ObjectNameForTest(t) + "-" | ||
|
||
generateNameLength := len(generateName) | ||
if generateNameLength > 44 { | ||
generateNameLength = 44 | ||
} | ||
return generateName[0:generateNameLength] | ||
} | ||
|
||
// validateName checks that a name generated using a generateName is valid. It checks | ||
// 1. The generateName is a prefix of the name, but they are not equal | ||
// 2. Any number of valid name characters (alphanumeric, -, and .) are added togenerateName to | ||
// create the value of name. | ||
func validateName(generateName, name string) error { | ||
r := regexp.MustCompile("^" + regexp.QuoteMeta(generateName) + "[a-zA-Z0-9\\-.]+$") | ||
|
||
if !r.MatchString(name) { | ||
return fmt.Errorf("generated name = %q, want to match %q", name, r.String()) | ||
} | ||
return nil | ||
} | ||
|
||
func canServeRequests(t *testing.T, clients *test.Clients, route *v1alpha1.Route) error { | ||
t.Logf("Route %s has a domain set in its status", route.Name) | ||
var domain string | ||
err := test.WaitForRouteState( | ||
clients.ServingClient, | ||
route.Name, | ||
func(r *v1alpha1.Route) (bool, error) { | ||
domain = r.Status.Domain | ||
return domain != "", nil | ||
}, | ||
"RouteDomain", | ||
) | ||
if err != nil { | ||
return fmt.Errorf("route did not get assigned a domain: %v", err) | ||
} | ||
|
||
t.Logf("Route %s can serve the expected data at the endpoint", route.Name) | ||
_, err = pkgTest.WaitForEndpointState( | ||
clients.KubeClient, | ||
t.Logf, | ||
domain, | ||
pkgTest.Retrying(pkgTest.MatchesAllOf(pkgTest.IsStatusOK, pkgTest.MatchesBody(helloWorldText)), http.StatusNotFound), | ||
"WaitForEndpointToServeText", | ||
test.ServingFlags.ResolvableDomain) | ||
if err != nil { | ||
return fmt.Errorf("the endpoint for Route %s at domain %s didn't serve the expected text %q: %v", route.Name, domain, helloWorldText, err) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
// TestServiceGenerateName checks that knative Services MAY request names generated by | ||
// the system using metadata.generateName. It ensures that knative Services created this way can become ready | ||
// and serve requests. | ||
func TestServiceGenerateName(t *testing.T) { | ||
t.Parallel() | ||
clients := setup(t) | ||
|
||
generateName := generateNamePrefix(t) | ||
names := test.ResourceNames{ | ||
Image: helloworld, | ||
} | ||
|
||
// Cleanup on test failure. | ||
test.CleanupOnInterrupt(func() { test.TearDown(clients, names) }) | ||
defer func() { test.TearDown(clients, names) }() | ||
|
||
// Create the service using the generate name field. If the serivce does not become ready this will fail. | ||
t.Logf("Creating new service with generateName %s", generateName) | ||
resources, err := test.CreateRunLatestServiceReady(t, clients, &names, &test.Options{}, setServiceGenerateName(generateName)) | ||
if err != nil { | ||
t.Fatalf("Failed to create service with generateName %s: %v", generateName, err) | ||
} | ||
|
||
// Ensure that the name given to the service is generated from the generateName field. | ||
t.Log("When the service is created, the name is generated using the provided generateName") | ||
if err := validateName(generateName, names.Service); err != nil { | ||
t.Errorf("Illegal name generated for service %s: %v", names.Service, err) | ||
} | ||
|
||
// Ensure that the service can serve requests | ||
err = canServeRequests(t, clients, resources.Route) | ||
if err != nil { | ||
t.Errorf("Service %s could not serve requests: %v", names.Service, err) | ||
} | ||
} | ||
|
||
// TestRouteAndConfiguration checks that both routes and configurations MAY request names generated by | ||
// the system using metadata.generateName. It ensures that routes and configurations created this way both: | ||
// 1. Become ready | ||
// 2. Can serve requests. | ||
func TestRouteAndConfigurationGenerateName(t *testing.T) { | ||
t.Parallel() | ||
clients := setup(t) | ||
|
||
generateName := generateNamePrefix(t) | ||
names := test.ResourceNames{ | ||
Image: helloworld, | ||
} | ||
|
||
test.CleanupOnInterrupt(func() { test.TearDown(clients, names) }) | ||
defer func() { test.TearDown(clients, names) }() | ||
|
||
t.Logf("Creating new configuration with generateName %s", generateName) | ||
config, err := test.CreateConfiguration(t, clients, names, &test.Options{}, setConfigurationGenerateName(generateName)) | ||
if err != nil { | ||
t.Fatalf("Failed to create configuration with generateName %s: %v", generateName, err) | ||
} | ||
names.Config = config.Name | ||
|
||
// Ensure the associated revision is created. This also checks that the configuration becomes ready. | ||
t.Log("The configuration will be updated with the name of the associated Revision once it is created.") | ||
names.Revision, err = test.WaitForConfigLatestRevision(clients, names) | ||
if err != nil { | ||
t.Fatalf("Configuration %s was not updated with the new revision: %v", names.Config, err) | ||
} | ||
|
||
// Ensure that the name given to the configuration is generated from the generate name field. | ||
t.Log("When the configuration is created, the name is generated using the provided generateName") | ||
if err := validateName(generateName, names.Config); err != nil { | ||
t.Errorf("Illegal name generated for configuration %s: %v", names.Config, err) | ||
} | ||
|
||
// Create a route that maps to the revision created by the configuration above | ||
t.Logf("Create new Route with generateName %s", generateName) | ||
route, err := test.CreateRoute(t, clients, names, setRouteGenerateName(generateName)) | ||
if err != nil { | ||
t.Fatalf("Failed to create route with generateName %s: %v", generateName, err) | ||
} | ||
names.Route = route.Name | ||
|
||
t.Log("When the route is created, it will become ready") | ||
if err := test.WaitForRouteState(clients.ServingClient, names.Route, test.IsRouteReady, "RouteIsReady"); err != nil { | ||
t.Fatalf("Error waiting for the route %s to become ready: %v", names.Route, err) | ||
} | ||
|
||
// Ensure that the name given to the route is generated from the generate name field | ||
t.Log("When the route is created, the name is generated using the provided generateName") | ||
if err := validateName(generateName, names.Route); err != nil { | ||
t.Errorf("Illegal name generated for route %s: %v", names.Route, err) | ||
} | ||
|
||
// Ensure that the generated route endpoint can serve requests | ||
if err := canServeRequests(t, clients, route); err != nil { | ||
t.Errorf("Configuration %s with Route %s could not serve requests: %v", names.Config, names.Route, err) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue referenced seems to indicate 49 characters. I assume the text here then is expecting 5 additional from the generateName randomness?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that is exactly it. 49 characters will work with a service with
.metadata.name
set but not.metadata.generateName
set.