From 6e0d07db71470d4e7782aa59ffa160cfc20d93f2 Mon Sep 17 00:00:00 2001 From: Victor Agababov Date: Fri, 1 Feb 2019 10:43:45 -0800 Subject: [PATCH 01/13] base addition to the codebase with uts --- .../serving/v1alpha1/service_validation.go | 7 ++ .../v1alpha1/service/resources/route.go | 32 +++-- pkg/reconciler/v1alpha1/service/service.go | 6 + .../v1alpha1/service/service_test.go | 119 ++++++++++++++++++ 4 files changed, 156 insertions(+), 8 deletions(-) diff --git a/pkg/apis/serving/v1alpha1/service_validation.go b/pkg/apis/serving/v1alpha1/service_validation.go index 51f81c1526c6..749636679b58 100644 --- a/pkg/apis/serving/v1alpha1/service_validation.go +++ b/pkg/apis/serving/v1alpha1/service_validation.go @@ -86,6 +86,9 @@ func (m *ManualType) Validate() *apis.FieldError { return nil } +// See #2819. +const releaseLastRevisionKeyword = "@latestRevision" + // Validate validates the fields belonging to ReleaseType func (rt *ReleaseType) Validate() *apis.FieldError { var errs *apis.FieldError @@ -99,6 +102,10 @@ func (rt *ReleaseType) Validate() *apis.FieldError { errs = errs.Also(apis.ErrOutOfBoundsValue(strconv.Itoa(numRevisions), "1", "2", "revisions")) } for i, r := range rt.Revisions { + // Skip over the last revision special keyword. + if r == releaseLastRevisionKeyword { + continue + } if msgs := validation.IsDNS1035Label(r); len(msgs) > 0 { errs = errs.Also(apis.ErrInvalidValue( fmt.Sprintf("not a DNS 1035 label: %v", msgs), apis.CurrentField).ViaFieldIndex("revisions", i)) diff --git a/pkg/reconciler/v1alpha1/service/resources/route.go b/pkg/reconciler/v1alpha1/service/resources/route.go index 9bcb3789bf78..4015a191b1bb 100644 --- a/pkg/reconciler/v1alpha1/service/resources/route.go +++ b/pkg/reconciler/v1alpha1/service/resources/route.go @@ -26,6 +26,9 @@ import ( "github.com/knative/serving/pkg/reconciler/v1alpha1/service/resources/names" ) +// See #2819. +const releaseLastRevisionKeyword = "@latestRevision" + // MakeRoute creates a Route from a Service object. func MakeRoute(service *v1alpha1.Service) (*v1alpha1.Route, error) { c := &v1alpha1.Route{ @@ -44,21 +47,34 @@ func MakeRoute(service *v1alpha1.Service) (*v1alpha1.Route, error) { numRevisions := len(service.Spec.Release.Revisions) // Configure the 'current' route. - currentRevisionName := service.Spec.Release.Revisions[0] ttCurrent := v1alpha1.TrafficTarget{ - Name: "current", - Percent: 100 - rolloutPercent, - RevisionName: currentRevisionName, + Name: "current", + Percent: 100 - rolloutPercent, + } + currentRevisionName := service.Spec.Release.Revisions[0] + + // If the `current` revision refers to the well known name of the last + // known revision, use `Configuration` instead. + // Same for the `candidate` below. + // Part of #2819. + if currentRevisionName == releaseLastRevisionKeyword { + ttCurrent.ConfigurationName = names.Configuration(service) + } else { + ttCurrent.RevisionName = currentRevisionName } c.Spec.Traffic = append(c.Spec.Traffic, ttCurrent) // Configure the 'candidate' route. if numRevisions == 2 { - candidateRevisionName := service.Spec.Release.Revisions[1] ttCandidate := v1alpha1.TrafficTarget{ - Name: "candidate", - Percent: rolloutPercent, - RevisionName: candidateRevisionName, + Name: "candidate", + Percent: rolloutPercent, + } + candidateRevisionName := service.Spec.Release.Revisions[1] + if candidateRevisionName == releaseLastRevisionKeyword { + ttCandidate.ConfigurationName = names.Configuration(service) + } else { + ttCandidate.RevisionName = candidateRevisionName } c.Spec.Traffic = append(c.Spec.Traffic, ttCandidate) } diff --git a/pkg/reconciler/v1alpha1/service/service.go b/pkg/reconciler/v1alpha1/service/service.go index 20500b61aa3b..5a0f27289148 100644 --- a/pkg/reconciler/v1alpha1/service/service.go +++ b/pkg/reconciler/v1alpha1/service/service.go @@ -22,6 +22,7 @@ import ( "reflect" "time" + "github.com/google/go-cmp/cmp" "github.com/knative/pkg/controller" "github.com/knative/pkg/kmp" "github.com/knative/pkg/logging" @@ -221,11 +222,15 @@ func (c *Reconciler) reconcile(ctx context.Context, service *v1alpha1.Service) e // Update our Status based on the state of our underlying Route. ss := &service.Status + fmt.Printf("### RouteStatus: %#v\n\n", route.Status) + fmt.Printf("### ConfigStatus: %#v\n\n", config.Status) ss.PropagateRouteStatus(&route.Status) + fmt.Printf("### ServiceStatus: %#v\n\n", ss) // `manual` is not reconciled. if rc := service.Status.GetCondition(v1alpha1.ServiceConditionRoutesReady); rc != nil && rc.Status == corev1.ConditionTrue { want, got := route.Spec.DeepCopy().Traffic, route.Status.Traffic + fmt.Printf("### Route Traffic diff: %s", cmp.Diff(want, got)) // Replace `configuration` target with its latest ready revision. for idx := range want { if want[idx].ConfigurationName == config.Name { @@ -233,6 +238,7 @@ func (c *Reconciler) reconcile(ctx context.Context, service *v1alpha1.Service) e want[idx].ConfigurationName = "" } } + fmt.Printf("### Route Traffic diff2: %s", cmp.Diff(want, got)) if eq, err := kmp.SafeEqual(got, want); !eq || err != nil { service.Status.MarkRouteNotYetReady() } diff --git a/pkg/reconciler/v1alpha1/service/service_test.go b/pkg/reconciler/v1alpha1/service/service_test.go index 515ec6757da6..58a68975fe51 100644 --- a/pkg/reconciler/v1alpha1/service/service_test.go +++ b/pkg/reconciler/v1alpha1/service/service_test.go @@ -173,6 +173,26 @@ func TestReconcile(t *testing.T) { WantServiceReadyStats: map[string]int{ "foo/pinned3": 1, }, + }, { + Name: "release - with @latestRevision", + Objects: []runtime.Object{ + svc("release", "foo", WithReleaseRollout("@latestRevision")), + }, + Key: "foo/release", + WantCreates: []metav1.Object{ + config("release", "foo", WithReleaseRollout("release-00001")), + route("release", "foo", WithReleaseRollout("@latestRevision")), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: svc("release", "foo", WithReleaseRollout("@latestRevision"), + // The first reconciliation will initialize the status conditions. + WithInitSvcConditions), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "Created", "Created Configuration %q", "release"), + Eventf(corev1.EventTypeNormal, "Created", "Created Route %q", "release"), + Eventf(corev1.EventTypeNormal, "Updated", "Updated Service %q", "release"), + }, }, { Name: "release - create route and service", Objects: []runtime.Object{ @@ -324,6 +344,105 @@ func TestReconcile(t *testing.T) { WantEvents: []string{ Eventf(corev1.EventTypeNormal, "Updated", "Updated Service %q", "release-nr-ts2"), }, + }, { + Name: "release - route and config ready, using @latestRevision", + Objects: []runtime.Object{ + svc("release-ready-lr", "foo", + WithReleaseRollout("@latestRevision"), WithInitSvcConditions), + route("release-ready-lr", "foo", + WithReleaseRollout("@latestRevision"), + RouteReady, WithDomain, WithDomainInternal, WithAddress, WithInitRouteConditions, + WithStatusTraffic([]v1alpha1.TrafficTarget{{ + Name: "current", + RevisionName: "release-ready-lr-00001", + Percent: 100, + }, { + Name: "latest", + RevisionName: "release-ready-lr-00001", + }}...), MarkTrafficAssigned, MarkIngressReady), + config("release-ready-lr", "foo", WithReleaseRollout("release-ready-lr"), WithGeneration(1), + // These turn a Configuration to Ready=true + WithLatestCreated, WithLatestReady), + }, + Key: "foo/release-ready-lr", + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: svc("release-ready-lr", "foo", + WithReleaseRollout("@latestRevision"), + // The delta induced by the config object. + WithReadyConfig("release-ready-lr-00001"), + // The delta induced by route object. + WithReadyRoute, WithSvcStatusDomain, WithSvcStatusAddress, + WithSvcStatusTraffic([]v1alpha1.TrafficTarget{{ + Name: "current", + RevisionName: "release-ready-lr-00001", + Percent: 100, + }, { + Name: "latest", + RevisionName: "release-ready-lr-00001", + }}...), + ), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "Updated", "Updated Service %q", "release-ready-lr"), + }, + WantServiceReadyStats: map[string]int{ + "foo/release-ready-lr": 1, + }, + }, { + Name: "release - route and config ready, traffic split, using @latestRevision", + Objects: []runtime.Object{ + svc("release-ready-lr", "foo", + WithReleaseRolloutAndPercentage( + 42, "release-ready-lr-00001", "@latestRevision"), WithInitSvcConditions), + route("release-ready-lr", "foo", + WithReleaseRolloutAndPercentage( + 42, "release-ready-lr-00001", "@latestRevision"), + RouteReady, WithDomain, WithDomainInternal, WithAddress, WithInitRouteConditions, + WithStatusTraffic([]v1alpha1.TrafficTarget{{ + Name: "current", + RevisionName: "release-ready-lr-00001", + Percent: 58, + }, { + Name: "candidate", + RevisionName: "release-ready-lr-00002", + Percent: 42, + }, { + Name: "latest", + RevisionName: "release-ready-lr-00002", + }}...), MarkTrafficAssigned, MarkIngressReady), + config("release-ready-lr", "foo", WithReleaseRollout("release-ready-lr"), WithGeneration(2), + // These turn a Configuration to Ready=true + WithLatestCreated, WithLatestReady), + }, + Key: "foo/release-ready-lr", + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: svc("release-ready-lr", "foo", + WithReleaseRolloutAndPercentage( + 42, "release-ready-lr-00001", "@latestRevision"), + // The delta induced by the config object. + WithReadyConfig("release-ready-lr-00002"), + // The delta induced by route object. + WithReadyRoute, WithSvcStatusDomain, WithSvcStatusAddress, + WithSvcStatusTraffic([]v1alpha1.TrafficTarget{{ + Name: "current", + RevisionName: "release-ready-lr-00001", + Percent: 58, + }, { + Name: "candidate", + RevisionName: "release-ready-lr-00002", + Percent: 42, + }, { + Name: "latest", + RevisionName: "release-ready-lr-00002", + }}...), + ), + }}, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "Updated", "Updated Service %q", "release-ready-lr"), + }, + WantServiceReadyStats: map[string]int{ + "foo/release-ready-lr": 1, + }, }, { Name: "release - route and config ready, propagate ready, percentage set", Objects: []runtime.Object{ From e1014c3170249db4f65f9cf4852077047641852e Mon Sep 17 00:00:00 2001 From: Victor Agababov Date: Fri, 1 Feb 2019 15:11:36 -0800 Subject: [PATCH 02/13] add unit test to the validation --- .../v1alpha1/service_validation_test.go | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/pkg/apis/serving/v1alpha1/service_validation_test.go b/pkg/apis/serving/v1alpha1/service_validation_test.go index 6365b69d5f42..4562226c17f8 100644 --- a/pkg/apis/serving/v1alpha1/service_validation_test.go +++ b/pkg/apis/serving/v1alpha1/service_validation_test.go @@ -290,6 +290,28 @@ func TestServiceValidation(t *testing.T) { }, }, want: apis.ErrInvalidValue(incorrectDNS1035Label, "spec.release.revisions[0]"), + }, { + name: "valid release -- with @lastRevision", + s: &Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, + Spec: ServiceSpec{ + Release: &ReleaseType{ + Revisions: []string{"s-1-00001", releaseLastRevisionKeyword}, + Configuration: ConfigurationSpec{ + RevisionTemplate: RevisionTemplateSpec{ + Spec: RevisionSpec{ + Container: corev1.Container{ + Image: "hellworld", + }, + }, + }, + }, + }, + }, + }, + want: nil, }, { name: "invalid release -- too few revisions; empty slice", s: &Service{ From 2185590ba94d66afa0c25c454b29d0b7849be18b Mon Sep 17 00:00:00 2001 From: Victor Agababov Date: Fri, 1 Feb 2019 16:22:25 -0800 Subject: [PATCH 03/13] remove debug logging --- pkg/reconciler/v1alpha1/service/service.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pkg/reconciler/v1alpha1/service/service.go b/pkg/reconciler/v1alpha1/service/service.go index 5a0f27289148..20500b61aa3b 100644 --- a/pkg/reconciler/v1alpha1/service/service.go +++ b/pkg/reconciler/v1alpha1/service/service.go @@ -22,7 +22,6 @@ import ( "reflect" "time" - "github.com/google/go-cmp/cmp" "github.com/knative/pkg/controller" "github.com/knative/pkg/kmp" "github.com/knative/pkg/logging" @@ -222,15 +221,11 @@ func (c *Reconciler) reconcile(ctx context.Context, service *v1alpha1.Service) e // Update our Status based on the state of our underlying Route. ss := &service.Status - fmt.Printf("### RouteStatus: %#v\n\n", route.Status) - fmt.Printf("### ConfigStatus: %#v\n\n", config.Status) ss.PropagateRouteStatus(&route.Status) - fmt.Printf("### ServiceStatus: %#v\n\n", ss) // `manual` is not reconciled. if rc := service.Status.GetCondition(v1alpha1.ServiceConditionRoutesReady); rc != nil && rc.Status == corev1.ConditionTrue { want, got := route.Spec.DeepCopy().Traffic, route.Status.Traffic - fmt.Printf("### Route Traffic diff: %s", cmp.Diff(want, got)) // Replace `configuration` target with its latest ready revision. for idx := range want { if want[idx].ConfigurationName == config.Name { @@ -238,7 +233,6 @@ func (c *Reconciler) reconcile(ctx context.Context, service *v1alpha1.Service) e want[idx].ConfigurationName = "" } } - fmt.Printf("### Route Traffic diff2: %s", cmp.Diff(want, got)) if eq, err := kmp.SafeEqual(got, want); !eq || err != nil { service.Status.MarkRouteNotYetReady() } From fede166afeddda599b5d9210ba48aea10677d0d7 Mon Sep 17 00:00:00 2001 From: Victor Agababov Date: Fri, 1 Feb 2019 16:43:16 -0800 Subject: [PATCH 04/13] raise test coverage --- .../v1alpha1/service/resources/route_test.go | 126 ++++++++++++++++++ 1 file changed, 126 insertions(+) diff --git a/pkg/reconciler/v1alpha1/service/resources/route_test.go b/pkg/reconciler/v1alpha1/service/resources/route_test.go index 79b89f16e441..16f3c3529826 100644 --- a/pkg/reconciler/v1alpha1/service/resources/route_test.go +++ b/pkg/reconciler/v1alpha1/service/resources/route_test.go @@ -19,7 +19,9 @@ package resources import ( "testing" + "github.com/google/go-cmp/cmp" "github.com/knative/serving/pkg/apis/serving" + "github.com/knative/serving/pkg/apis/serving/v1alpha1" "github.com/knative/serving/pkg/reconciler/v1alpha1/service/resources/names" ) @@ -159,6 +161,130 @@ func TestRouteReleaseSingleRevision(t *testing.T) { } } +func TestRouteLatestRevisionSplit(t *testing.T) { + const ( + rolloutPercent = 42 + currentPercent = 100 - rolloutPercent + ) + s := createServiceWithRelease(2 /*num revisions*/, rolloutPercent) + s.Spec.Release.Revisions = []string{releaseLastRevisionKeyword, "juicy-revision"} + testConfigName := names.Configuration(s) + r, err := MakeRoute(s) + if err != nil { + t.Errorf("Expected nil for err got %q", err) + } + if got, want := r.Name, testServiceName; got != want { + t.Errorf("Expected %q for service name got %q", want, got) + } + if got, want := r.Namespace, testServiceNamespace; got != want { + t.Errorf("Expected %q for service namespace got %q", want, got) + } + wantT := []v1alpha1.TrafficTarget{{ + Name: "current", + Percent: currentPercent, + ConfigurationName: testConfigName, + }, { + Name: "candidate", + Percent: rolloutPercent, + RevisionName: "juicy-revision", + }, { + Name: "latest", + ConfigurationName: testConfigName, + }} + if got, want := r.Spec.Traffic, wantT; !cmp.Equal(got, want) { + t.Errorf("Traffic mismatch: diff (-got, +want): %s", cmp.Diff(got, want)) + } + expectOwnerReferencesSetCorrectly(t, r.OwnerReferences) + + wantL := map[string]string{ + testLabelKey: testLabelValueRelease, + serving.ServiceLabelKey: testServiceName, + } + if got, want := r.Labels, wantL; !cmp.Equal(got, want) { + t.Errorf("Labels mismatch: diff (-got, +want): %s", cmp.Diff(got, want)) + } +} +func TestRouteLatestRevisionSplitCandidate(t *testing.T) { + const ( + rolloutPercent = 42 + currentPercent = 100 - rolloutPercent + ) + s := createServiceWithRelease(2 /*num revisions*/, rolloutPercent) + s.Spec.Release.Revisions = []string{"squishy-revision", releaseLastRevisionKeyword} + testConfigName := names.Configuration(s) + r, err := MakeRoute(s) + if err != nil { + t.Errorf("Expected nil for err got %q", err) + } + if got, want := r.Name, testServiceName; got != want { + t.Errorf("Expected %q for service name got %q", want, got) + } + if got, want := r.Namespace, testServiceNamespace; got != want { + t.Errorf("Expected %q for service namespace got %q", want, got) + } + wantT := []v1alpha1.TrafficTarget{{ + Name: "current", + Percent: currentPercent, + RevisionName: "squishy-revision", + }, { + Name: "candidate", + Percent: rolloutPercent, + ConfigurationName: testConfigName, + }, { + Name: "latest", + ConfigurationName: testConfigName, + }} + if got, want := r.Spec.Traffic, wantT; !cmp.Equal(got, want) { + t.Errorf("Traffic mismatch: diff (-got, +want): %s", cmp.Diff(got, want)) + } + expectOwnerReferencesSetCorrectly(t, r.OwnerReferences) + + wantL := map[string]string{ + testLabelKey: testLabelValueRelease, + serving.ServiceLabelKey: testServiceName, + } + if got, want := r.Labels, wantL; !cmp.Equal(got, want) { + t.Errorf("Labels mismatch: diff (-got, +want): %s", cmp.Diff(got, want)) + } +} +func TestRouteLatestRevisionNoSplit(t *testing.T) { + s := createServiceWithRelease(1 /*num revisions*/, 0 /*unused*/) + s.Spec.Release.Revisions = []string{releaseLastRevisionKeyword} + testConfigName := names.Configuration(s) + r, err := MakeRoute(s) + + if err != nil { + t.Errorf("Expected nil for err got %q", err) + } + if got, want := r.Name, testServiceName; got != want { + t.Errorf("Expected %q for service name got %q", want, got) + } + if got, want := r.Namespace, testServiceNamespace; got != want { + t.Errorf("Expected %q for service namespace got %q", want, got) + } + // Should have 2 named traffic targets (current, latest) + wantT := []v1alpha1.TrafficTarget{{ + Name: "current", + Percent: 100, + ConfigurationName: testConfigName, + }, { + Name: "latest", + ConfigurationName: testConfigName, + }} + if got, want := r.Spec.Traffic, wantT; !cmp.Equal(got, want) { + t.Errorf("Traffic mismatch: diff (-got, +want): %s", cmp.Diff(got, want)) + } + expectOwnerReferencesSetCorrectly(t, r.OwnerReferences) + + wantL := map[string]string{ + testLabelKey: testLabelValueRelease, + serving.ServiceLabelKey: testServiceName, + } + if got, want := r.Labels, wantL; !cmp.Equal(got, want) { + t.Errorf("Labels mismatch: diff (-got, +want): %s", cmp.Diff(got, want)) + } +} + func TestRouteReleaseTwoRevisions(t *testing.T) { rolloutPercent := 48 currentPercent := 52 From c3894b6a435fb15f785156e464681ee3e5df1db8 Mon Sep 17 00:00:00 2001 From: Dan Gerdesmeier Date: Fri, 1 Feb 2019 22:14:48 -0800 Subject: [PATCH 05/13] Update pkg/apis/serving/v1alpha1/service_validation.go Co-Authored-By: vagababov --- pkg/apis/serving/v1alpha1/service_validation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/apis/serving/v1alpha1/service_validation.go b/pkg/apis/serving/v1alpha1/service_validation.go index 749636679b58..1024709d65d7 100644 --- a/pkg/apis/serving/v1alpha1/service_validation.go +++ b/pkg/apis/serving/v1alpha1/service_validation.go @@ -87,7 +87,7 @@ func (m *ManualType) Validate() *apis.FieldError { } // See #2819. -const releaseLastRevisionKeyword = "@latestRevision" +const releaseLatestRevisionKeyword = "@latestRevision" // Validate validates the fields belonging to ReleaseType func (rt *ReleaseType) Validate() *apis.FieldError { From 1ed0d749585f48d19a8f585f1404c08a05e1ac15 Mon Sep 17 00:00:00 2001 From: Victor Agababov Date: Fri, 1 Feb 2019 22:25:30 -0800 Subject: [PATCH 06/13] fix the const name --- pkg/apis/serving/v1alpha1/service_validation.go | 2 +- pkg/apis/serving/v1alpha1/service_validation_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/apis/serving/v1alpha1/service_validation.go b/pkg/apis/serving/v1alpha1/service_validation.go index 1024709d65d7..72679d3de071 100644 --- a/pkg/apis/serving/v1alpha1/service_validation.go +++ b/pkg/apis/serving/v1alpha1/service_validation.go @@ -103,7 +103,7 @@ func (rt *ReleaseType) Validate() *apis.FieldError { } for i, r := range rt.Revisions { // Skip over the last revision special keyword. - if r == releaseLastRevisionKeyword { + if r == releaseLatestRevisionKeyword { continue } if msgs := validation.IsDNS1035Label(r); len(msgs) > 0 { diff --git a/pkg/apis/serving/v1alpha1/service_validation_test.go b/pkg/apis/serving/v1alpha1/service_validation_test.go index 4562226c17f8..c8ddc843771b 100644 --- a/pkg/apis/serving/v1alpha1/service_validation_test.go +++ b/pkg/apis/serving/v1alpha1/service_validation_test.go @@ -298,7 +298,7 @@ func TestServiceValidation(t *testing.T) { }, Spec: ServiceSpec{ Release: &ReleaseType{ - Revisions: []string{"s-1-00001", releaseLastRevisionKeyword}, + Revisions: []string{"s-1-00001", releaseLatestRevisionKeyword}, Configuration: ConfigurationSpec{ RevisionTemplate: RevisionTemplateSpec{ Spec: RevisionSpec{ From 1465cad2771ea68774b804edd50fb3fb6a794c49 Mon Sep 17 00:00:00 2001 From: Victor Agababov Date: Mon, 4 Feb 2019 10:02:48 -0800 Subject: [PATCH 07/13] update spec.md with the new constant reference --- docs/spec/spec.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/spec/spec.md b/docs/spec/spec.md index 2c855345a143..d0fa2c4c5312 100644 --- a/docs/spec/spec.md +++ b/docs/spec/spec.md @@ -323,6 +323,7 @@ spec: # One of "runLatest", "release", "pinned" (DEPRECATED), or "manual" release: # Ordered list of 1 or 2 revisions. First revision is traffic target # "current" and second revision is traffic target "candidate". + # It is possible to specify `"@latestRevision"` string as a shortcut to the lastest available revision. revisions: ["myservice-00013", "myservice-00015"] rolloutPercent: 50 # Percent [0-99] of traffic to route to "candidate" revision configuration: # serving.knative.dev/v1alpha1.ConfigurationSpec From 7d84da720340d1a9825dc4a537aa2f2997c9c8c3 Mon Sep 17 00:00:00 2001 From: Victor Agababov Date: Mon, 4 Feb 2019 10:35:23 -0800 Subject: [PATCH 08/13] address comments. create a globally known constant --- pkg/apis/serving/v1alpha1/service_types.go | 5 +++++ pkg/apis/serving/v1alpha1/service_validation.go | 5 +---- pkg/apis/serving/v1alpha1/service_validation_test.go | 2 +- pkg/reconciler/v1alpha1/service/resources/route.go | 7 ++----- pkg/reconciler/v1alpha1/service/resources/route_test.go | 6 +++--- 5 files changed, 12 insertions(+), 13 deletions(-) diff --git a/pkg/apis/serving/v1alpha1/service_types.go b/pkg/apis/serving/v1alpha1/service_types.go index 72ed2b73cfe9..ff7304d192ff 100644 --- a/pkg/apis/serving/v1alpha1/service_types.go +++ b/pkg/apis/serving/v1alpha1/service_types.go @@ -128,6 +128,11 @@ type ReleaseType struct { Configuration ConfigurationSpec `json:"configuration,omitempty"` } +// ReleaseLatestRevisionKeyword is a shortcut for usage in the `release` mode +// to refer to the latest created revision. +// See #2819 for details. +const ReleaseLatestRevisionKeyword = "@latestRevision" + // RunLatestType contains the options for always having a route to the latest configuration. See // ServiceSpec for more details. type RunLatestType struct { diff --git a/pkg/apis/serving/v1alpha1/service_validation.go b/pkg/apis/serving/v1alpha1/service_validation.go index 72679d3de071..cd2e40488f49 100644 --- a/pkg/apis/serving/v1alpha1/service_validation.go +++ b/pkg/apis/serving/v1alpha1/service_validation.go @@ -86,9 +86,6 @@ func (m *ManualType) Validate() *apis.FieldError { return nil } -// See #2819. -const releaseLatestRevisionKeyword = "@latestRevision" - // Validate validates the fields belonging to ReleaseType func (rt *ReleaseType) Validate() *apis.FieldError { var errs *apis.FieldError @@ -103,7 +100,7 @@ func (rt *ReleaseType) Validate() *apis.FieldError { } for i, r := range rt.Revisions { // Skip over the last revision special keyword. - if r == releaseLatestRevisionKeyword { + if r == ReleaseLatestRevisionKeyword { continue } if msgs := validation.IsDNS1035Label(r); len(msgs) > 0 { diff --git a/pkg/apis/serving/v1alpha1/service_validation_test.go b/pkg/apis/serving/v1alpha1/service_validation_test.go index c8ddc843771b..297250d57521 100644 --- a/pkg/apis/serving/v1alpha1/service_validation_test.go +++ b/pkg/apis/serving/v1alpha1/service_validation_test.go @@ -298,7 +298,7 @@ func TestServiceValidation(t *testing.T) { }, Spec: ServiceSpec{ Release: &ReleaseType{ - Revisions: []string{"s-1-00001", releaseLatestRevisionKeyword}, + Revisions: []string{"s-1-00001", ReleaseLatestRevisionKeyword}, Configuration: ConfigurationSpec{ RevisionTemplate: RevisionTemplateSpec{ Spec: RevisionSpec{ diff --git a/pkg/reconciler/v1alpha1/service/resources/route.go b/pkg/reconciler/v1alpha1/service/resources/route.go index 4015a191b1bb..7136a0c61524 100644 --- a/pkg/reconciler/v1alpha1/service/resources/route.go +++ b/pkg/reconciler/v1alpha1/service/resources/route.go @@ -26,9 +26,6 @@ import ( "github.com/knative/serving/pkg/reconciler/v1alpha1/service/resources/names" ) -// See #2819. -const releaseLastRevisionKeyword = "@latestRevision" - // MakeRoute creates a Route from a Service object. func MakeRoute(service *v1alpha1.Service) (*v1alpha1.Route, error) { c := &v1alpha1.Route{ @@ -57,7 +54,7 @@ func MakeRoute(service *v1alpha1.Service) (*v1alpha1.Route, error) { // known revision, use `Configuration` instead. // Same for the `candidate` below. // Part of #2819. - if currentRevisionName == releaseLastRevisionKeyword { + if currentRevisionName == v1alpha1.ReleaseLatestRevisionKeyword { ttCurrent.ConfigurationName = names.Configuration(service) } else { ttCurrent.RevisionName = currentRevisionName @@ -71,7 +68,7 @@ func MakeRoute(service *v1alpha1.Service) (*v1alpha1.Route, error) { Percent: rolloutPercent, } candidateRevisionName := service.Spec.Release.Revisions[1] - if candidateRevisionName == releaseLastRevisionKeyword { + if candidateRevisionName == v1alpha1.ReleaseLatestRevisionKeyword { ttCandidate.ConfigurationName = names.Configuration(service) } else { ttCandidate.RevisionName = candidateRevisionName diff --git a/pkg/reconciler/v1alpha1/service/resources/route_test.go b/pkg/reconciler/v1alpha1/service/resources/route_test.go index 16f3c3529826..eb277c98d440 100644 --- a/pkg/reconciler/v1alpha1/service/resources/route_test.go +++ b/pkg/reconciler/v1alpha1/service/resources/route_test.go @@ -167,7 +167,7 @@ func TestRouteLatestRevisionSplit(t *testing.T) { currentPercent = 100 - rolloutPercent ) s := createServiceWithRelease(2 /*num revisions*/, rolloutPercent) - s.Spec.Release.Revisions = []string{releaseLastRevisionKeyword, "juicy-revision"} + s.Spec.Release.Revisions = []string{v1alpha1.ReleaseLatestRevisionKeyword, "juicy-revision"} testConfigName := names.Configuration(s) r, err := MakeRoute(s) if err != nil { @@ -210,7 +210,7 @@ func TestRouteLatestRevisionSplitCandidate(t *testing.T) { currentPercent = 100 - rolloutPercent ) s := createServiceWithRelease(2 /*num revisions*/, rolloutPercent) - s.Spec.Release.Revisions = []string{"squishy-revision", releaseLastRevisionKeyword} + s.Spec.Release.Revisions = []string{"squishy-revision", v1alpha1.ReleaseLatestRevisionKeyword} testConfigName := names.Configuration(s) r, err := MakeRoute(s) if err != nil { @@ -249,7 +249,7 @@ func TestRouteLatestRevisionSplitCandidate(t *testing.T) { } func TestRouteLatestRevisionNoSplit(t *testing.T) { s := createServiceWithRelease(1 /*num revisions*/, 0 /*unused*/) - s.Spec.Release.Revisions = []string{releaseLastRevisionKeyword} + s.Spec.Release.Revisions = []string{v1alpha1.ReleaseLatestRevisionKeyword} testConfigName := names.Configuration(s) r, err := MakeRoute(s) From 4605cff9f512925d0687110a68a963f92e9aaa34 Mon Sep 17 00:00:00 2001 From: Victor Agababov Date: Mon, 4 Feb 2019 10:36:24 -0800 Subject: [PATCH 09/13] fix the test name --- pkg/apis/serving/v1alpha1/service_validation_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/apis/serving/v1alpha1/service_validation_test.go b/pkg/apis/serving/v1alpha1/service_validation_test.go index 297250d57521..75e35c44e2b4 100644 --- a/pkg/apis/serving/v1alpha1/service_validation_test.go +++ b/pkg/apis/serving/v1alpha1/service_validation_test.go @@ -291,7 +291,7 @@ func TestServiceValidation(t *testing.T) { }, want: apis.ErrInvalidValue(incorrectDNS1035Label, "spec.release.revisions[0]"), }, { - name: "valid release -- with @lastRevision", + name: "valid release -- with @latestRevision", s: &Service{ ObjectMeta: metav1.ObjectMeta{ Name: "valid", From 6b269391aa24d351bb8b11e77f4db75bb1ac2c70 Mon Sep 17 00:00:00 2001 From: Victor Agababov Date: Mon, 4 Feb 2019 10:52:44 -0800 Subject: [PATCH 10/13] go with @latest for brevity --- docs/spec/spec.md | 2 +- pkg/apis/serving/v1alpha1/service_types.go | 2 +- .../v1alpha1/service_validation_test.go | 2 +- .../v1alpha1/service/service_test.go | 24 +++++++++---------- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/docs/spec/spec.md b/docs/spec/spec.md index d0fa2c4c5312..5749a42cb5a9 100644 --- a/docs/spec/spec.md +++ b/docs/spec/spec.md @@ -323,7 +323,7 @@ spec: # One of "runLatest", "release", "pinned" (DEPRECATED), or "manual" release: # Ordered list of 1 or 2 revisions. First revision is traffic target # "current" and second revision is traffic target "candidate". - # It is possible to specify `"@latestRevision"` string as a shortcut to the lastest available revision. + # It is possible to specify `"@latest"` string as a shortcut to the lastest available revision. revisions: ["myservice-00013", "myservice-00015"] rolloutPercent: 50 # Percent [0-99] of traffic to route to "candidate" revision configuration: # serving.knative.dev/v1alpha1.ConfigurationSpec diff --git a/pkg/apis/serving/v1alpha1/service_types.go b/pkg/apis/serving/v1alpha1/service_types.go index ff7304d192ff..767bb5a6dd94 100644 --- a/pkg/apis/serving/v1alpha1/service_types.go +++ b/pkg/apis/serving/v1alpha1/service_types.go @@ -131,7 +131,7 @@ type ReleaseType struct { // ReleaseLatestRevisionKeyword is a shortcut for usage in the `release` mode // to refer to the latest created revision. // See #2819 for details. -const ReleaseLatestRevisionKeyword = "@latestRevision" +const ReleaseLatestRevisionKeyword = "@latest" // RunLatestType contains the options for always having a route to the latest configuration. See // ServiceSpec for more details. diff --git a/pkg/apis/serving/v1alpha1/service_validation_test.go b/pkg/apis/serving/v1alpha1/service_validation_test.go index 75e35c44e2b4..52a19398fa80 100644 --- a/pkg/apis/serving/v1alpha1/service_validation_test.go +++ b/pkg/apis/serving/v1alpha1/service_validation_test.go @@ -291,7 +291,7 @@ func TestServiceValidation(t *testing.T) { }, want: apis.ErrInvalidValue(incorrectDNS1035Label, "spec.release.revisions[0]"), }, { - name: "valid release -- with @latestRevision", + name: "valid release -- with @latest", s: &Service{ ObjectMeta: metav1.ObjectMeta{ Name: "valid", diff --git a/pkg/reconciler/v1alpha1/service/service_test.go b/pkg/reconciler/v1alpha1/service/service_test.go index 58a68975fe51..16cb17e38506 100644 --- a/pkg/reconciler/v1alpha1/service/service_test.go +++ b/pkg/reconciler/v1alpha1/service/service_test.go @@ -174,17 +174,17 @@ func TestReconcile(t *testing.T) { "foo/pinned3": 1, }, }, { - Name: "release - with @latestRevision", + Name: "release - with @latest", Objects: []runtime.Object{ - svc("release", "foo", WithReleaseRollout("@latestRevision")), + svc("release", "foo", WithReleaseRollout(v1alpha1.ReleaseLatestRevisionKeyword)), }, Key: "foo/release", WantCreates: []metav1.Object{ config("release", "foo", WithReleaseRollout("release-00001")), - route("release", "foo", WithReleaseRollout("@latestRevision")), + route("release", "foo", WithReleaseRollout(v1alpha1.ReleaseLatestRevisionKeyword)), }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: svc("release", "foo", WithReleaseRollout("@latestRevision"), + Object: svc("release", "foo", WithReleaseRollout(v1alpha1.ReleaseLatestRevisionKeyword), // The first reconciliation will initialize the status conditions. WithInitSvcConditions), }}, @@ -345,12 +345,12 @@ func TestReconcile(t *testing.T) { Eventf(corev1.EventTypeNormal, "Updated", "Updated Service %q", "release-nr-ts2"), }, }, { - Name: "release - route and config ready, using @latestRevision", + Name: "release - route and config ready, using @latest", Objects: []runtime.Object{ svc("release-ready-lr", "foo", - WithReleaseRollout("@latestRevision"), WithInitSvcConditions), + WithReleaseRollout(v1alpha1.ReleaseLatestRevisionKeyword), WithInitSvcConditions), route("release-ready-lr", "foo", - WithReleaseRollout("@latestRevision"), + WithReleaseRollout(v1alpha1.ReleaseLatestRevisionKeyword), RouteReady, WithDomain, WithDomainInternal, WithAddress, WithInitRouteConditions, WithStatusTraffic([]v1alpha1.TrafficTarget{{ Name: "current", @@ -367,7 +367,7 @@ func TestReconcile(t *testing.T) { Key: "foo/release-ready-lr", WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: svc("release-ready-lr", "foo", - WithReleaseRollout("@latestRevision"), + WithReleaseRollout(v1alpha1.ReleaseLatestRevisionKeyword), // The delta induced by the config object. WithReadyConfig("release-ready-lr-00001"), // The delta induced by route object. @@ -389,14 +389,14 @@ func TestReconcile(t *testing.T) { "foo/release-ready-lr": 1, }, }, { - Name: "release - route and config ready, traffic split, using @latestRevision", + Name: "release - route and config ready, traffic split, using @latest", Objects: []runtime.Object{ svc("release-ready-lr", "foo", WithReleaseRolloutAndPercentage( - 42, "release-ready-lr-00001", "@latestRevision"), WithInitSvcConditions), + 42, "release-ready-lr-00001", v1alpha1.ReleaseLatestRevisionKeyword), WithInitSvcConditions), route("release-ready-lr", "foo", WithReleaseRolloutAndPercentage( - 42, "release-ready-lr-00001", "@latestRevision"), + 42, "release-ready-lr-00001", v1alpha1.ReleaseLatestRevisionKeyword), RouteReady, WithDomain, WithDomainInternal, WithAddress, WithInitRouteConditions, WithStatusTraffic([]v1alpha1.TrafficTarget{{ Name: "current", @@ -418,7 +418,7 @@ func TestReconcile(t *testing.T) { WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: svc("release-ready-lr", "foo", WithReleaseRolloutAndPercentage( - 42, "release-ready-lr-00001", "@latestRevision"), + 42, "release-ready-lr-00001", v1alpha1.ReleaseLatestRevisionKeyword), // The delta induced by the config object. WithReadyConfig("release-ready-lr-00002"), // The delta induced by route object. From f4ff69c188d0d30d0b9b71d73a67955dc6d347f0 Mon Sep 17 00:00:00 2001 From: Victor Agababov Date: Mon, 4 Feb 2019 11:29:42 -0800 Subject: [PATCH 11/13] update the conformance test --- test/conformance/service_test.go | 2 +- test/crd.go | 23 +++++++++++++++ test/service.go | 48 ++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 1 deletion(-) diff --git a/test/conformance/service_test.go b/test/conformance/service_test.go index 74c97b621341..2782ce3eb098 100644 --- a/test/conformance/service_test.go +++ b/test/conformance/service_test.go @@ -315,7 +315,7 @@ func TestReleaseService(t *testing.T) { expectedThirdRev = helloWorldText ) - objects, err := test.CreateRunLatestServiceReady(logger, clients, &names, &test.Options{}) + objects, err := test.CreateReleaseServiceWithLatest(logger, clients, &names, &test.Options{}) if err != nil { t.Fatalf("Failed to create initial Service %v: %v", names.Service, err) } diff --git a/test/crd.go b/test/crd.go index 451dd3ca589a..717fce3e44d6 100644 --- a/test/crd.go +++ b/test/crd.go @@ -178,6 +178,29 @@ func LatestService(namespace string, names ResourceNames, options *Options, fopt return svc } +// ReleaseLatestService returns a Release Service object in namespace with the name names.Service +// that uses the image specified by names.Image and `@latest` as the only revision. +func ReleaseLatestService(namespace string, names ResourceNames, options *Options, fopt ...testing.ServiceOption) *v1alpha1.Service { + svc := &v1alpha1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: names.Service, + }, + Spec: v1alpha1.ServiceSpec{ + Release: &v1alpha1.ReleaseType{ + Revisions: []string{"@latest"}, + Configuration: *ConfigurationSpec(ImagePath(names.Image), options), + }, + }, + } + + // Apply any mutations we have been provided. + for _, opt := range fopt { + opt(svc) + } + return svc +} + // ReleaseService returns a Release Service object in namespace with the name names.Service that uses // the image specified by names.Image. It also takes a list of 1-2 revisons and a rolloutPercent to be // used to configure routing diff --git a/test/service.go b/test/service.go index b6d4d4708895..0a52cf955623 100644 --- a/test/service.go +++ b/test/service.go @@ -21,6 +21,7 @@ package test import ( "fmt" + "github.com/davecgh/go-spew/spew" "github.com/knative/pkg/apis/duck" "github.com/knative/pkg/test/logging" "github.com/knative/serving/pkg/apis/serving/v1alpha1" @@ -76,6 +77,7 @@ func getResourceObjects(clients *Clients, names ResourceNames) (*ResourceObjects if err != nil { return nil, err } + spew.Printf("###### Service: %#v\n", serviceObject) configObject, err := clients.ServingClient.Configs.Get(names.Config, metav1.GetOptions{}) if err != nil { @@ -95,6 +97,44 @@ func getResourceObjects(clients *Clients, names ResourceNames) (*ResourceObjects }, nil } +// CreateReleaseServiceWithLatest creates a `Release` service using `@latest` +// as the only revision. +// This function expects `Service` and `Image` name passed in through `names`. +// Names is updated with the `Route` and `Configuration` created by the Service +// and `ResourceObjects` is returned with the `Service`, `Route`, and `Configuration` objects. +// Returns an error if the service does not come up correctly. +func CreateReleaseServiceWithLatest( + logger *logging.BaseLogger, clients *Clients, + names *ResourceNames, options *Options) (*ResourceObjects, error) { + if names.Service == "" || names.Image == "" { + return nil, fmt.Errorf("expected non-empty Service and Image name; got Service = %s, Image = %s", names.Service, names.Image) + } + + logger.Info("Creating a new Service as Release with @latest.") + svc, err := CreateReleaseService(logger, clients, *names, options) + if err != nil { + return nil, err + } + + // Populate Route and Configuration Objects with name + names.Route = serviceresourcenames.Route(svc) + names.Config = serviceresourcenames.Configuration(svc) + + logger.Info("Waiting for Service to transition to Ready.") + if err := WaitForServiceState(clients.ServingClient, names.Service, IsServiceReady, "ServiceIsReady"); err != nil { + return nil, err + } + + logger.Info("Checking to ensure Service Status is populated for Ready service.") + err = validateCreatedServiceStatus(clients, names) + if err != nil { + return nil, err + } + + logger.Info("Getting latest objects Created by Service.") + return getResourceObjects(clients, *names) +} + // CreateRunLatestServiceReady creates a new RunLatest Service in state 'Ready'. This function expects Service and Image name passed in through 'names'. // Names is updated with the Route and Configuration created by the Service and ResourceObjects is returned with the Service, Route, and Configuration objects. // Returns error if the service does not come up correctly. @@ -128,6 +168,14 @@ func CreateRunLatestServiceReady(logger *logging.BaseLogger, clients *Clients, n return getResourceObjects(clients, *names) } +// CreateReleaseService creates a service in namespace with the name names.Service and names.Image, +// configured with `@latest` revision. +func CreateReleaseService(logger *logging.BaseLogger, clients *Clients, names ResourceNames, options *Options, fopt ...testing.ServiceOption) (*v1alpha1.Service, error) { + service := ReleaseLatestService(ServingNamespace, names, options, fopt...) + LogResourceObject(logger, ResourceObjects{Service: service}) + return clients.ServingClient.Services.Create(service) +} + // CreateLatestService creates a service in namespace with the name names.Service and names.Image func CreateLatestService(logger *logging.BaseLogger, clients *Clients, names ResourceNames, options *Options, fopt ...testing.ServiceOption) (*v1alpha1.Service, error) { service := LatestService(ServingNamespace, names, options, fopt...) From 5b856071a99016e46ab8fffc203bccbc3e9c85e0 Mon Sep 17 00:00:00 2001 From: Victor Agababov Date: Mon, 4 Feb 2019 11:54:55 -0800 Subject: [PATCH 12/13] add conformance test, that validates release service creation --- test/conformance/service_test.go | 8 ++++---- test/service.go | 2 -- test/util.go | 9 ++------- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/test/conformance/service_test.go b/test/conformance/service_test.go index 2782ce3eb098..eae799402fb3 100644 --- a/test/conformance/service_test.go +++ b/test/conformance/service_test.go @@ -319,11 +319,11 @@ func TestReleaseService(t *testing.T) { if err != nil { t.Fatalf("Failed to create initial Service %v: %v", names.Service, err) } - firstRevision := names.Revision + revisions := []string{names.Revision} // One Revision Specified, current == latest. logger.Info("Updating Service to ReleaseType using lastCreatedRevision") - objects.Service, err = test.PatchReleaseService(logger, clients, objects.Service, []string{firstRevision}, 0) + 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) } @@ -345,7 +345,7 @@ func TestReleaseService(t *testing.T) { 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) } - secondRevision := names.Revision + revisions = append(revisions, names.Revision) logger.Info("Since the Service is using release the Route will not be updated, but new revision will be available at 'latest'") validateDomains(t, logger, clients, @@ -356,7 +356,7 @@ func TestReleaseService(t *testing.T) { // Two Revisions Specified, 50% rollout, candidate == latest. logger.Info("Updating Service to split traffic between two revisions using Release mode") - if objects.Service, err = test.PatchReleaseService(logger, clients, objects.Service, []string{firstRevision, secondRevision}, 50); err != nil { + 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) } diff --git a/test/service.go b/test/service.go index 0a52cf955623..f829ffb25e37 100644 --- a/test/service.go +++ b/test/service.go @@ -21,7 +21,6 @@ package test import ( "fmt" - "github.com/davecgh/go-spew/spew" "github.com/knative/pkg/apis/duck" "github.com/knative/pkg/test/logging" "github.com/knative/serving/pkg/apis/serving/v1alpha1" @@ -77,7 +76,6 @@ func getResourceObjects(clients *Clients, names ResourceNames) (*ResourceObjects if err != nil { return nil, err } - spew.Printf("###### Service: %#v\n", serviceObject) configObject, err := clients.ServingClient.Configs.Get(names.Config, metav1.GetOptions{}) if err != nil { diff --git a/test/util.go b/test/util.go index 9a8d500bdcff..1cc15eb86910 100644 --- a/test/util.go +++ b/test/util.go @@ -15,10 +15,10 @@ package test import ( "context" - "encoding/json" "fmt" "net/http" + "github.com/davecgh/go-spew/spew" "github.com/knative/pkg/signals" "github.com/knative/pkg/test/logging" ) @@ -27,12 +27,7 @@ import ( // LogResourceObject logs the resource object with the resource name and value func LogResourceObject(logger *logging.BaseLogger, value ResourceObjects) { - // Log the route object - if resourceJSON, err := json.Marshal(value); err != nil { - logger.Infof("Failed to create json from resource object: %v", err) - } else { - logger.Infof("resource %s", string(resourceJSON)) - } + logger.Infof("resource %s", spew.Sdump(value)) } // ImagePath is a helper function to prefix image name with repo and suffix with tag From 1503544f93111aa13bf276790e00eedf14f98af6 Mon Sep 17 00:00:00 2001 From: Victor Agababov Date: Mon, 4 Feb 2019 15:09:59 -0800 Subject: [PATCH 13/13] added more validation to the service shape --- test/conformance/service_test.go | 17 +++++++++++++++++ test/crd.go | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/test/conformance/service_test.go b/test/conformance/service_test.go index eae799402fb3..46cf6d95e79e 100644 --- a/test/conformance/service_test.go +++ b/test/conformance/service_test.go @@ -24,6 +24,7 @@ import ( "strconv" "testing" + "github.com/google/go-cmp/cmp" pkgTest "github.com/knative/pkg/test" "github.com/knative/pkg/test/logging" "github.com/knative/serving/pkg/apis/serving/v1alpha1" @@ -136,6 +137,18 @@ func validateLabelsPropagation(logger *logging.BaseLogger, objects test.Resource return nil } +func validateReleaseServiceShape(objs *test.ResourceObjects) error { + // Check that Spec.Revisions is as expected. + if got, want := objs.Service.Spec.Release.Revisions, []string{v1alpha1.ReleaseLatestRevisionKeyword}; !cmp.Equal(got, want) { + return fmt.Errorf("Spec.Release.Revisions mismatch: diff: %s", cmp.Diff(got, want)) + } + // Traffic should be routed to the lastest created revision. + if got, want := objs.Service.Status.Traffic[0].RevisionName, objs.Config.Status.LatestReadyRevisionName; got != want { + return fmt.Errorf("Status.Traffic[0].RevisionsName = %s, want: %s", got, want) + } + return nil +} + // TestRunLatestService tests both Creation and Update paths of a runLatest service. The test performs a series of Update/Validate steps to ensure that // the service transitions as expected during each step. // Currently the test performs the following updates: @@ -319,6 +332,10 @@ 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. diff --git a/test/crd.go b/test/crd.go index 717fce3e44d6..cf6628cb1d4a 100644 --- a/test/crd.go +++ b/test/crd.go @@ -188,7 +188,7 @@ func ReleaseLatestService(namespace string, names ResourceNames, options *Option }, Spec: v1alpha1.ServiceSpec{ Release: &v1alpha1.ReleaseType{ - Revisions: []string{"@latest"}, + Revisions: []string{v1alpha1.ReleaseLatestRevisionKeyword}, Configuration: *ConfigurationSpec(ImagePath(names.Image), options), }, },