Skip to content

Add one more case to cover @latest as part of the traffic split. #3104

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

Merged
knative-prow-robot merged 14 commits into from
Feb 8, 2019
Merged

Add one more case to cover @latest as part of the traffic split. #3104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
300294e
Add one more case to cover @latest as part of the traffic split.
vagababov Feb 6, 2019
b78802b
update the test
vagababov Feb 6, 2019
de411cc
intermediate checkpoint state
vagababov Feb 6, 2019
b5ec907
checkpoint
vagababov Feb 6, 2019
be53e8d
checkpoint
vagababov Feb 6, 2019
a5f8f48
make sure stuff works
vagababov Feb 7, 2019
9d2cfc5
restore util.go
vagababov Feb 7, 2019
af8aa0b
update comment
vagababov Feb 7, 2019
dc646bc
Update test/conformance/service_test.go
Feb 7, 2019
d163e9b
Update test/conformance/util.go
Feb 7, 2019
85e0061
address comments
vagababov Feb 7, 2019
53aff46
Merge branch 'more-itests' of github.com:vagababov/serving into more-…
vagababov Feb 7, 2019
73cb859
Merge branch 'master' into more-itests
vagababov Feb 8, 2019
09e7c9f
merge
vagababov Feb 8, 2019
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
131 changes: 120 additions & 11 deletions test/conformance/service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,13 +301,36 @@ func TestRunLatestService(t *testing.T) {
}
}

// TestReleaseService creates a Service in runLatest mode and then updates it to release mode. Once in release mode the test
// goes through Update/Validate to try different possible configurations for a release service.
// Currently tests for the following combinations
func waitForDesiredTrafficShape(sName string, want map[string]v1alpha1.TrafficTarget, clients *test.Clients, logger *logging.BaseLogger) error {
return test.WaitForServiceState(
clients.ServingClient, sName, func(s *v1alpha1.Service) (bool, error) {
// IsServiceReady never returns an error.
if ok, _ := test.IsServiceReady(s); !ok {
return false, nil
}
// Match the traffic shape.
got := map[string]v1alpha1.TrafficTarget{}
for _, tt := range s.Status.Traffic {
got[tt.Name] = tt
}
if !cmp.Equal(got, want) {
logger.Info("For service %s traffic shape mismatch: (-got, +want)", sName, cmp.Diff(got, want))
return false, nil
}
return true, nil
}, "Verify Service Trafic Shape",
)
}

// TestReleaseService creates a Service in `release` mode with the only revision
// being `@latest`. Once this succeeded, the test goes through Update/Validate to
// try different possible configurations for a release service.
// Currently tests for the following combinations:
// 1. One Revision Specified, current == latest
// 2. One Revision Specified, current != latset
// 3. Two Revisions Specified, 50% rollout, candidate == latest
// 4. Two Revisions Specified, 50% rollout, candidate != latest
// 5. Two Revisions Specified, 50% rollout, candidate != latest, latest referred to as `@latest`.
func TestReleaseService(t *testing.T) {
// Create Initial Service
clients := setup(t)
Expand All @@ -332,18 +355,34 @@ func TestReleaseService(t *testing.T) {
if err != nil {
t.Fatalf("Failed to create initial Service %v: %v", names.Service, err)
}

logger.Info("Validating service shape.")
if err := validateReleaseServiceShape(objects); err != nil {
t.Fatalf("Release shape incorrect: %v", err)
}
revisions := []string{names.Revision}

// One Revision Specified, current == latest.
logger.Info("Updating Service to ReleaseType using lastCreatedRevision")
// 1. One Revision Specified, current == latest.
logger.Info("1. Updating Service to ReleaseType using lastCreatedRevision")
objects.Service, err = test.PatchReleaseService(logger, clients, objects.Service, revisions, 0)
if err != nil {
t.Fatalf("Service %s was not updated to release: %v", names.Service, err)
}
desiredTrafficShape := map[string]v1alpha1.TrafficTarget{
"current": v1alpha1.TrafficTarget{
Name: "current",
RevisionName: objects.Config.Status.LatestReadyRevisionName,
Percent: 100,
},
"latest": v1alpha1.TrafficTarget{
Name: "latest",
RevisionName: objects.Config.Status.LatestReadyRevisionName,
},
}
logger.Info("Waiting for Service to become ready with the new shape.")
if err := waitForDesiredTrafficShape(names.Service, desiredTrafficShape, clients, logger); err != nil {
t.Fatal("Service never obtained expected shape")
}

logger.Info("Service traffic should go to the first revision and be available on two names traffic targets: 'current' and 'latest'")
if err := validateDomains(logger, clients,
Expand All @@ -354,8 +393,8 @@ func TestReleaseService(t *testing.T) {
t.Fatal(err)
}

// One Revision Specified, current != latest.
logger.Info("Updating the Service Spec with a new image")
// 2. One Revision Specified, current != latest.
logger.Info("2. Updating the Service Spec with a new image")
if _, err := test.PatchServiceImage(logger, clients, objects.Service, releaseImagePath2); err != nil {
t.Fatalf("Patch update for Service %s with new image %s failed: %v", names.Service, releaseImagePath2, err)
}
Expand All @@ -366,6 +405,16 @@ func TestReleaseService(t *testing.T) {
}
revisions = append(revisions, names.Revision)

// Also verify traffic is in the correct shape.
desiredTrafficShape["latest"] = v1alpha1.TrafficTarget{
Copy link
Copy Markdown

@dgerd dgerd Feb 7, 2019

Choose a reason for hiding this comment

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

For a different PR, but what do you think about making the latest, candidate, and current strings constants in service_types.go since they are part of the API spec.

Copy link
Copy Markdown
Contributor Author

@vagababov vagababov Feb 7, 2019

Choose a reason for hiding this comment

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

Sounds like a good idea. But this would change probably hundreds of files, so let's do it separately.

Name: "latest",
RevisionName: names.Revision,
}
logger.Info("Waiting for Service to become ready with the new shape.")
if err := waitForDesiredTrafficShape(names.Service, desiredTrafficShape, clients, logger); err != nil {
t.Fatal("Service never obtained expected shape")
}

logger.Info("Since the Service is using release the Route will not be updated, but new revision will be available at 'latest'")
if err := validateDomains(logger, clients,
names.Domain,
Expand All @@ -375,12 +424,33 @@ func TestReleaseService(t *testing.T) {
t.Fatal(err)
}

// Two Revisions Specified, 50% rollout, candidate == latest.
logger.Info("Updating Service to split traffic between two revisions using Release mode")
// 3. Two Revisions Specified, 50% rollout, candidate == latest.
logger.Info("3. Updating Service to split traffic between two revisions using Release mode")
if objects.Service, err = test.PatchReleaseService(logger, clients, objects.Service, revisions, 50); err != nil {
t.Fatalf("Service %s was not updated to release: %v", names.Service, err)
}

desiredTrafficShape = map[string]v1alpha1.TrafficTarget{
"current": v1alpha1.TrafficTarget{
Name: "current",
RevisionName: revisions[0],
Percent: 50,
},
"candidate": v1alpha1.TrafficTarget{
Name: "candidate",
RevisionName: revisions[1],
Percent: 50,
},
"latest": v1alpha1.TrafficTarget{
Name: "latest",
RevisionName: revisions[1],
},
}
logger.Info("Waiting for Service to become ready with the new shape.")
if err := waitForDesiredTrafficShape(names.Service, desiredTrafficShape, clients, logger); err != nil {
t.Fatal("Service never obtained expected shape")
}

logger.Info("Traffic should be split between the two revisions and available on three named traffic targets, 'current', 'candidate', and 'latest'")
if err := validateDomains(logger, clients,
names.Domain,
Expand All @@ -390,11 +460,24 @@ func TestReleaseService(t *testing.T) {
t.Fatal(err)
}

// Two Revisions Specified, 50% rollout, candidate != latest.
logger.Info("Updating the Service Spec with a new image")
// 4. Two Revisions Specified, 50% rollout, candidate != latest.
logger.Info("4. Updating the Service Spec with a new image")
if _, err := test.PatchServiceImage(logger, clients, objects.Service, releaseImagePath3); err != nil {
t.Fatalf("Patch update for Service %s with new image %s failed: %v", names.Service, releaseImagePath3, err)
}
logger.Info("Since the Service was updated a new Revision will be created")
if names.Revision, err = test.WaitForServiceLatestRevision(clients, names); err != nil {
t.Fatalf("The Service %s was not updated with new revision %s: %v", names.Service, names.Revision, err)
}

desiredTrafficShape["latest"] = v1alpha1.TrafficTarget{
Name: "latest",
RevisionName: names.Revision,
}
logger.Info("Waiting for Service to become ready with the new shape.")
if err := waitForDesiredTrafficShape(names.Service, desiredTrafficShape, clients, logger); err != nil {
t.Fatal("Service never obtained expected shape")
}

logger.Info("Traffic should remain between the two images, and the new revision should be available on the named traffic target 'latest'")
if err := validateDomains(logger, clients,
Expand All @@ -404,6 +487,32 @@ func TestReleaseService(t *testing.T) {
[]string{expectedThirdRev, expectedSecondRev, expectedFirstRev}); err != nil {
t.Fatal(err)
}

// Now update the service to use `@latest` as candidate.
revisions[1] = v1alpha1.ReleaseLatestRevisionKeyword
logger.Info("5. Updating Service to split traffic between two `current` and `@latest`")
if objects.Service, err = test.PatchReleaseService(logger, clients, objects.Service, revisions, 50); err != nil {
t.Fatalf("Service %s was not updated to release: %v", names.Service, err)
}

// `candidate` now points to the latest.
desiredTrafficShape["candidate"] = v1alpha1.TrafficTarget{
Name: "candidate",
RevisionName: names.Revision,
Percent: 50,
}
logger.Info("Waiting for Service to become ready with the new shape.")
if err := waitForDesiredTrafficShape(names.Service, desiredTrafficShape, clients, logger); err != nil {
t.Fatal("Service never obtained expected shape")
}

if err := validateDomains(logger, clients,
names.Domain,
[]string{expectedFirstRev, expectedThirdRev},
[]string{"latest", "candidate", "current"},
[]string{expectedThirdRev, expectedThirdRev, expectedFirstRev}); err != nil {
t.Fatal(err)
}
}

// TODO(jonjohnsonjr): Examples of deploying from source.
4 changes: 2 additions & 2 deletions test/crd_checks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func CheckConfigurationState(client *ServingClients, name string, inState func(r
}

// WaitForRevisionState polls the status of the Revision called name
// from client every interval until inState returns `true` indicating it
// from client every `interval` until `inState` returns `true` indicating it
// is done, returns an error or timeout. desc will be used to name the metric
// that is emitted to track how long it took for name to get into the state checked by inState.
func WaitForRevisionState(client *ServingClients, name string, inState func(r *v1alpha1.Revision) (bool, error), desc string) error {
Expand Down Expand Up @@ -163,7 +163,7 @@ func CheckRevisionState(client *ServingClients, name string, inState func(r *v1a
}

// WaitForServiceState polls the status of the Service called name
// from client every interval until inState returns `true` indicating it
// from client every `interval` until `inState` returns `true` indicating it
// is done, returns an error or timeout. desc will be used to name the metric
// that is emitted to track how long it took for name to get into the state checked by inState.
func WaitForServiceState(client *ServingClients, name string, inState func(s *v1alpha1.Service) (bool, error), desc string) error {
Expand Down
7 changes: 4 additions & 3 deletions test/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ func validateCreatedServiceStatus(clients *Clients, names *ResourceNames) error
})
}

func getResourceObjects(clients *Clients, names ResourceNames) (*ResourceObjects, error) {
// GetResourceObjects obtains the services resources from the k8s API server.
func GetResourceObjects(clients *Clients, names ResourceNames) (*ResourceObjects, error) {
routeObject, err := clients.ServingClient.Routes.Get(names.Route, metav1.GetOptions{})
if err != nil {
return nil, err
Expand Down Expand Up @@ -130,7 +131,7 @@ func CreateReleaseServiceWithLatest(
}

logger.Info("Getting latest objects Created by Service.")
return getResourceObjects(clients, *names)
return GetResourceObjects(clients, *names)
}

// CreateRunLatestServiceReady creates a new RunLatest Service in state 'Ready'. This function expects Service and Image name passed in through 'names'.
Expand Down Expand Up @@ -163,7 +164,7 @@ func CreateRunLatestServiceReady(logger *logging.BaseLogger, clients *Clients, n
}

logger.Info("Getting latest objects Created by Service.")
return getResourceObjects(clients, *names)
return GetResourceObjects(clients, *names)
}

// CreateReleaseService creates a service in namespace with the name names.Service and names.Image,
Expand Down