Add e2e test for rollback scenario

test/crd.go

@@ -35,6 +35,7 @@ type ResourceNames struct {
 	TrafficTarget string
 	URL           *url.URL
 	Image         string
+	BYOName       string
 }
 
 // AppendRandomString will generate a random string that begins with prefix. This is useful

test/e2e/image_pull_error_test.go

@@ -100,7 +100,7 @@ func TestImagePullError(t *testing.T) {
 // Wrote our own thing so that we can pass in an image by digest.
 // knative/pkg/test.ImagePath currently assumes there's a tag, which fails to parse.
 func createLatestService(t *testing.T, clients *test.Clients, names test.ResourceNames) (*v1alpha1.Service, error) {
-	opt := v1alpha1testing.WithInlineConfigSpec(*v1a1test.ConfigurationSpec(names.Image))
+	opt := v1alpha1testing.WithInlineConfigSpec(*v1a1test.ConfigurationSpec(names.Image, nil /* byoName */))
 	service := v1alpha1testing.ServiceWithoutNamespace(names.Service, opt)
 	v1a1test.LogResourceObject(t, v1a1test.ResourceObjects{Service: service})
 	return clients.ServingAlphaClient.Services.Create(service)

test/e2e/rollback_byo_test.go

@@ -0,0 +1,174 @@
+// +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 e2e
+
+import (
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"knative.dev/pkg/ptr"
+	pkgTest "knative.dev/pkg/test"
+	"knative.dev/pkg/test/logstream"
+	v1 "knative.dev/serving/pkg/apis/serving/v1"
+	"knative.dev/serving/pkg/apis/serving/v1alpha1"
+	. "knative.dev/serving/pkg/testing/v1alpha1"
+	"knative.dev/serving/test"
+	v1a1test "knative.dev/serving/test/v1alpha1"
+)
+
+func TestRollbackBYOName(t *testing.T) {
+	t.Parallel()
+	cancel := logstream.Start(t)
+	defer cancel()
+
+	clients := Setup(t)
+
+	serviceName := test.ObjectNameForTest(t)
+	byoNameOld := serviceName + "-byo-foo"
+	byoNameNew := serviceName + "-byo-foo-new"
+	names := test.ResourceNames{
+		Service: serviceName,
+		Image:   "helloworld",
+		BYOName: byoNameOld,
+	}
+
+	test.CleanupOnInterrupt(func() { test.TearDown(clients, names) })
+	defer test.TearDown(clients, names)
+
+	withTrafficSpecOld := WithInlineRouteSpec(v1alpha1.RouteSpec{
+		Traffic: []v1alpha1.TrafficTarget{
+			{
+				TrafficTarget: v1.TrafficTarget{
+					RevisionName: byoNameOld,
+					Percent:      ptr.Int64(100),
+				},
+			},
+		},
+	})
+	withTrafficSpecNew := WithInlineRouteSpec(v1alpha1.RouteSpec{
+		Traffic: []v1alpha1.TrafficTarget{
+			{
+				TrafficTarget: v1.TrafficTarget{
+					RevisionName: byoNameNew,
+					Percent:      ptr.Int64(100),
+				},
+			},
+		},
+	})
+
+	t.Logf("Creating a new Service with byo config name %q.", byoNameOld)
+	resources, err := v1a1test.CreateRunLatestServiceReady(t, clients, &names, withTrafficSpecOld)
+	if err != nil {
+		t.Fatalf("Failed to create initial Service: %v: %v", names.Service, err)
+	}
+	url := resources.Route.Status.URL.URL()
+	if _, err = pkgTest.WaitForEndpointState(
+		clients.KubeClient,
+		t.Logf,
+		url,
+		v1a1test.RetryingRouteInconsistency(pkgTest.MatchesAllOf(pkgTest.IsStatusOK, pkgTest.MatchesBody(test.HelloWorldText))),
+		"HelloWorldServesText",
+		test.ServingFlags.ResolvableDomain); err != nil {
+		t.Fatalf("The endpoint %s for Route %s didn't serve the expected text %q: %v", url, names.Route, test.HelloWorldText, err)
+	}
+
+	revisionName := resources.Revision.ObjectMeta.Name
+	if revisionName != byoNameOld {
+		t.Fatalf("Expect configuration name in revision label %q but got %q ", byoNameOld, revisionName)
+	}
+
+	// Update service to use a new byo name
+	t.Logf("Updating the Service to a new revision with a new byo name %q.", byoNameNew)
+	newSvc := resources.Service.DeepCopy()
+	so := WithInlineConfigSpec(*v1a1test.ConfigurationSpec(pkgTest.ImagePath(test.PizzaPlanet1), &byoNameNew))
+	so(newSvc)
+	withTrafficSpecNew(newSvc)
+	svc, err := v1a1test.PatchService(t, clients, resources.Service, newSvc)
+	resources.Service = svc
+	if err != nil {
+		t.Fatalf("Patch update for Service (new byo name %q) failed: %v", byoNameNew, err)
+	}
+
+	t.Log("Since the Service was updated a new Revision will be created and the Service will be updated")
+	newRevision, err := v1a1test.WaitForServiceLatestRevision(clients, names)
+	if err != nil {
+		t.Fatalf("Service %s was not updated with the Revision for new byo name %s: %v", names.Service, byoNameNew, err)
+	}
+	if newRevision != byoNameNew {
+		t.Fatalf("Expect configuration name in revision label %q but got %q ", byoNameNew, newRevision)
+	}
+
+	// Now, rollback to the first RevisionSpec
+	rollbackSvc := resources.Service.DeepCopy()
+	rollbackSvc.Spec = v1alpha1.ServiceSpec{
+		DeprecatedRelease: &v1alpha1.ReleaseType{
+			Revisions:      []string{byoNameOld, byoNameNew},
+			RolloutPercent: 0,
+			Configuration: v1alpha1.ConfigurationSpec{
+				Template: &v1alpha1.RevisionTemplateSpec{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: byoNameNew,
+					},
+					Spec: v1alpha1.RevisionSpec{
+						DeprecatedContainer: &corev1.Container{
+							Image: pkgTest.ImagePath(test.PizzaPlanet1),
+						},
+					},
+				},
+			},
+		},
+	}
+	svc, err = v1a1test.PatchService(t, clients, resources.Service, rollbackSvc)
+	resources.Service = svc
+	if err != nil {
+		t.Fatalf("Patch update for Service (rollback to byo name %q) failed: %v", byoNameOld, err)
+	}
+
+	t.Logf("We are rolling back to the previous revision (byoNameOld %q).", byoNameOld)
+	// Wait for the route to become ready, and check that the traffic split between the byoNameOld
+	// and byoNameNew is 100 and 0, respectively
+	err = v1a1test.WaitForServiceState(clients.ServingAlphaClient, names.Service, func(s *v1alpha1.Service) (bool, error) {
+		for _, tr := range s.Status.Traffic {
+			if tr.RevisionName == byoNameOld {
+				if tr.Tag != "current" || tr.Percent == nil || *tr.Percent != 100 {
+					return false, nil
+				}
+			}
+			if tr.RevisionName == byoNameNew {
+				if tr.Percent != nil && *tr.Percent != 0 {
+					return false, nil
+				}
+			}
+		}
+		return true, nil
+	}, "ServiceRollbackRevision")
+	if err != nil {
+		t.Fatalf("Service %s was not rolled back with byo name %s: %v", names.Service, byoNameOld, err)
+	}
+
+	// Verify that the latest ready revision and latest created revision are both byoNameNew,
+	// which means no new revision is created in the rollback
+	err = v1a1test.WaitForServiceState(clients.ServingAlphaClient, names.Service, func(s *v1alpha1.Service) (bool, error) {
+		return (s.Status.LatestReadyRevisionName == byoNameNew && s.Status.LatestCreatedRevisionName == byoNameNew), nil
+	}, "ServiceNoNewRevisionCreated")
+	if err != nil {
+		t.Fatalf("Service %s was not rolled back with byo name %s: %v", names.Service, byoNameOld, err)
+	}
+}

test/v1alpha1/configuration.go

@@ -81,8 +81,8 @@ func WaitForConfigLatestRevision(clients *test.Clients, names test.ResourceNames
 
 // ConfigurationSpec returns the spec of a configuration to be used throughout different
 // CRD helpers.
-func ConfigurationSpec(imagePath string) *v1alpha1.ConfigurationSpec {
-	return &v1alpha1.ConfigurationSpec{
+func ConfigurationSpec(imagePath string, byoName *string) *v1alpha1.ConfigurationSpec {
+	cs := v1alpha1.ConfigurationSpec{
 		Template: &v1alpha1.RevisionTemplateSpec{
 			Spec: v1alpha1.RevisionSpec{
 				RevisionSpec: v1.RevisionSpec{
@@ -95,6 +95,12 @@ func ConfigurationSpec(imagePath string) *v1alpha1.ConfigurationSpec {
 			},
 		},
 	}
+	if byoName != nil {
+		cs.Template.ObjectMeta = metav1.ObjectMeta{
+			Name: *byoName,
+		}
+	}
+	return &cs
 }
 
 // LegacyConfigurationSpec returns the spec of a configuration to be used throughout different
@@ -119,7 +125,7 @@ func Configuration(names test.ResourceNames, fopt ...v1alpha1testing.ConfigOptio
 		ObjectMeta: metav1.ObjectMeta{
 			Name: names.Config,
 		},
-		Spec: *ConfigurationSpec(ptest.ImagePath(names.Image)),
+		Spec: *ConfigurationSpec(ptest.ImagePath(names.Image), nil /* byoName */),
 	}
 
 	for _, opt := range fopt {

test/v1alpha1/service.go

@@ -268,10 +268,10 @@ func WaitForServiceLatestRevision(clients *test.Clients, names test.ResourceName
 }
 
 // LatestService returns a Service object in namespace with the name names.Service
-// that uses the image specified by names.Image.
+// that uses the image specified by names.Image and name specified in names.BYOName.
 func LatestService(names test.ResourceNames, fopt ...rtesting.ServiceOption) *v1alpha1.Service {
 	a := append([]rtesting.ServiceOption{
-		rtesting.WithInlineConfigSpec(*ConfigurationSpec(ptest.ImagePath(names.Image))),
+		rtesting.WithInlineConfigSpec(*ConfigurationSpec(ptest.ImagePath(names.Image), &names.BYOName)),
 	}, fopt...)
 	return rtesting.ServiceWithoutNamespace(names.Service, a...)
 }