From 65210f7f4ec09a61bc088e216328f6f5d9496a55 Mon Sep 17 00:00:00 2001 From: Nader Ziada Date: Mon, 17 Sep 2018 15:07:42 -0400 Subject: [PATCH 1/3] add conformance test for lables --- test/conformance/labels_test.go | 130 ++++++++++++++++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 test/conformance/labels_test.go diff --git a/test/conformance/labels_test.go b/test/conformance/labels_test.go new file mode 100644 index 000000000000..5d19589c8249 --- /dev/null +++ b/test/conformance/labels_test.go @@ -0,0 +1,130 @@ +// +build e2e + +/* +Copyright 2018 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 ( + "testing" + + "github.com/knative/pkg/test/logging" + "github.com/knative/serving/pkg/apis/serving/v1alpha1" + serviceresourcenames "github.com/knative/serving/pkg/reconciler/v1alpha1/service/resources/names" + "github.com/knative/serving/test" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestLabelsPropagation(t *testing.T) { + clients := setup(t) + + // Add test case specific name to its own logger. + logger := logging.GetContextLogger("TestLabelsPropagation") + + var imagePath = test.ImagePath("helloworld") + + var names test.ResourceNames + names.Service = test.AppendRandomString("pizzaplanet-service", logger) + + defer tearDown(clients, names) + test.CleanupOnInterrupt(func() { tearDown(clients, names) }, logger) + + logger.Info("Creating a new Service") + svc, err := test.CreateLatestService(logger, clients, names, imagePath) + if err != nil { + t.Fatalf("Failed to create Service: %v", err) + } + names.Route = serviceresourcenames.Route(svc) + names.Config = serviceresourcenames.Configuration(svc) + + logger.Info("The Service will be updated with the name of the Revision once it is created") + var revisionName string + err = test.WaitForServiceState(clients.ServingClient, names.Service, func(s *v1alpha1.Service) (bool, error) { + if s.Status.LatestCreatedRevisionName != names.Revision { + revisionName = s.Status.LatestCreatedRevisionName + return true, nil + } + return false, nil + }, "ServiceUpdatedWithRevision") + revisionName, err = waitForServiceLatestCreatedRevision(clients, names) + if err != nil { + t.Fatalf("Service %s was not updated with the new revision: %v", names.Service, err) + } + names.Revision = revisionName + + logger.Info("When the Service reports as Ready, everything should be ready.") + if err := test.WaitForServiceState(clients.ServingClient, 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) + } + + logger.Info("Validate Labels on Revision Object") + revision, err := clients.ServingClient.Revisions.Get(names.Revision, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Error fetching Revision %s: %v", names.Revision, err) + } + + if revConfigurationLabel, ok := revision.Labels["serving.knative.dev/configuration"]; ok { + if revConfigurationLabel != names.Config { + t.Fatalf("Expect confguration name in revision label %q but got %q ", names.Config, revConfigurationLabel) + } + } else { + t.Fatalf("Failed to get configuration name from Revision label") + } + if revServiceLabel, ok := revision.Labels["serving.knative.dev/service"]; ok { + if revServiceLabel != names.Service { + t.Fatalf("Expect Service name in revision label %q but got %q ", names.Service, revServiceLabel) + } + } else { + t.Fatalf("Failed to get Service name from Revision label") + } + + logger.Info("Validate Labels on Configuration Object") + config, err := clients.ServingClient.Configs.Get(names.Config, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Error fetching Configuration %s: %v", names.Config, err) + } + + if configServiceLabel, ok := config.Labels["serving.knative.dev/service"]; ok { + if configServiceLabel != names.Service { + t.Fatalf("Expect Service name in configuration label %q but got %q ", names.Service, configServiceLabel) + } + } else { + t.Fatalf("Failed to get service name from Configuration label") + } + + if configRouteLabel, ok := config.Labels["serving.knative.dev/route"]; ok { + if configRouteLabel != names.Route { + t.Fatalf("Expect Route name in configuration label %q but got %q ", names.Route, configRouteLabel) + } + } else { + t.Fatalf("Failed to get route name from Configuration label") + } + + logger.Info("Validate Labels on Route Object") + route, err := clients.ServingClient.Routes.Get(names.Route, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Error fetching Route %s: %v", names.Route, err) + } + + if routeServiceLabel, ok := route.Labels["serving.knative.dev/service"]; ok { + if routeServiceLabel != names.Service { + t.Fatalf("Expect Service name in Route label %q but got %q ", names.Service, routeServiceLabel) + } + } else { + t.Fatalf("Failed to get service name from Route label") + } + +} From 286036d75a1300dda69cdc292e5be7e0a32e634a Mon Sep 17 00:00:00 2001 From: Nader Ziada Date: Wed, 19 Sep 2018 15:12:40 -0400 Subject: [PATCH 2/3] simplify test, update spec doc --- docs/spec/spec.md | 12 +++--- test/conformance/labels_test.go | 69 ++++++++++----------------------- 2 files changed, 26 insertions(+), 55 deletions(-) diff --git a/docs/spec/spec.md b/docs/spec/spec.md index 49580325ea76..0899f964e37a 100644 --- a/docs/spec/spec.md +++ b/docs/spec/spec.md @@ -52,7 +52,7 @@ metadata: name: my-service namespace: default labels: - knative.dev/type: ... # +optional convention: function|app + knative.dev/service: ... # name of the service # system generated meta uid: ... @@ -113,7 +113,9 @@ kind: Configuration metadata: name: my-service namespace: default - + labels: + knative.dev/service: ... # name of the service + knative.dev/route: ... # name of the route # system generated meta uid: ... resourceVersion: ... # used for optimistic concurrency control @@ -222,10 +224,8 @@ metadata: name: myservice-a1e34 # system generated namespace: default labels: - knative.dev/configuration: ... # to list configurations/revisions by service - knative.dev/type: "function" # convention, one of "function" or "app" - knative.dev/revision: ... # generated revision name - knative.dev/revisionUID: ... # generated revision UID + knative.dev/configuration: ... # name of the configuration + knative.dev/service: ... # name of the service annotations: knative.dev/configurationGeneration: ... # generation of configuration that created this Revision # system generated meta diff --git a/test/conformance/labels_test.go b/test/conformance/labels_test.go index 5d19589c8249..5c9fca560ea1 100644 --- a/test/conformance/labels_test.go +++ b/test/conformance/labels_test.go @@ -22,7 +22,6 @@ import ( "testing" "github.com/knative/pkg/test/logging" - "github.com/knative/serving/pkg/apis/serving/v1alpha1" serviceresourcenames "github.com/knative/serving/pkg/reconciler/v1alpha1/service/resources/names" "github.com/knative/serving/test" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -34,11 +33,9 @@ func TestLabelsPropagation(t *testing.T) { // Add test case specific name to its own logger. logger := logging.GetContextLogger("TestLabelsPropagation") + names := test.ResourceNames{Service: test.AppendRandomString("pizzaplanet-service", logger)} var imagePath = test.ImagePath("helloworld") - var names test.ResourceNames - names.Service = test.AppendRandomString("pizzaplanet-service", logger) - defer tearDown(clients, names) test.CleanupOnInterrupt(func() { tearDown(clients, names) }, logger) @@ -51,80 +48,54 @@ func TestLabelsPropagation(t *testing.T) { names.Config = serviceresourcenames.Configuration(svc) logger.Info("The Service will be updated with the name of the Revision once it is created") - var revisionName string - err = test.WaitForServiceState(clients.ServingClient, names.Service, func(s *v1alpha1.Service) (bool, error) { - if s.Status.LatestCreatedRevisionName != names.Revision { - revisionName = s.Status.LatestCreatedRevisionName - return true, nil - } - return false, nil - }, "ServiceUpdatedWithRevision") - revisionName, err = waitForServiceLatestCreatedRevision(clients, names) + names.Revision, err = waitForServiceLatestCreatedRevision(clients, names) if err != nil { t.Fatalf("Service %s was not updated with the new revision: %v", names.Service, err) } - names.Revision = revisionName logger.Info("When the Service reports as Ready, everything should be ready.") if err := test.WaitForServiceState(clients.ServingClient, 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) } + // Now that the serive is ready, we can validate the Labels on the Revision, Configuration + // and Route objects + // see spec here: https://github.com/knative/serving/blob/master/docs/spec/spec.md#revision + logger.Info("Validate Labels on Revision Object") revision, err := clients.ServingClient.Revisions.Get(names.Revision, metav1.GetOptions{}) if err != nil { - t.Fatalf("Error fetching Revision %s: %v", names.Revision, err) + t.Errorf("Error fetching Revision %s: %v", names.Revision, err) } - if revConfigurationLabel, ok := revision.Labels["serving.knative.dev/configuration"]; ok { - if revConfigurationLabel != names.Config { - t.Fatalf("Expect confguration name in revision label %q but got %q ", names.Config, revConfigurationLabel) - } - } else { - t.Fatalf("Failed to get configuration name from Revision label") + if revision.Labels["serving.knative.dev/configuration"] != names.Config { + t.Errorf("Expect confguration name in revision label %q but got %q ", names.Config, revision.Labels["serving.knative.dev/configuration"]) } - if revServiceLabel, ok := revision.Labels["serving.knative.dev/service"]; ok { - if revServiceLabel != names.Service { - t.Fatalf("Expect Service name in revision label %q but got %q ", names.Service, revServiceLabel) - } - } else { - t.Fatalf("Failed to get Service name from Revision label") + if revision.Labels["serving.knative.dev/service"] != names.Service { + t.Errorf("Expect Service name in revision label %q but got %q ", names.Service, revision.Labels["serving.knative.dev/service"]) } logger.Info("Validate Labels on Configuration Object") config, err := clients.ServingClient.Configs.Get(names.Config, metav1.GetOptions{}) if err != nil { - t.Fatalf("Error fetching Configuration %s: %v", names.Config, err) + t.Errorf("Error fetching Configuration %s: %v", names.Config, err) } - if configServiceLabel, ok := config.Labels["serving.knative.dev/service"]; ok { - if configServiceLabel != names.Service { - t.Fatalf("Expect Service name in configuration label %q but got %q ", names.Service, configServiceLabel) - } - } else { - t.Fatalf("Failed to get service name from Configuration label") + if config.Labels["serving.knative.dev/service"] != names.Service { + t.Errorf("Expect Service name in configuration label %q but got %q ", names.Service, config.Labels["serving.knative.dev/service"]) } - - if configRouteLabel, ok := config.Labels["serving.knative.dev/route"]; ok { - if configRouteLabel != names.Route { - t.Fatalf("Expect Route name in configuration label %q but got %q ", names.Route, configRouteLabel) - } - } else { - t.Fatalf("Failed to get route name from Configuration label") + if config.Labels["serving.knative.dev/route"] != names.Route { + t.Errorf("Expect Route name in configuration label %q but got %q ", names.Route, config.Labels["serving.knative.dev/route"]) } logger.Info("Validate Labels on Route Object") route, err := clients.ServingClient.Routes.Get(names.Route, metav1.GetOptions{}) if err != nil { - t.Fatalf("Error fetching Route %s: %v", names.Route, err) + t.Errorf("Error fetching Route %s: %v", names.Route, err) } - if routeServiceLabel, ok := route.Labels["serving.knative.dev/service"]; ok { - if routeServiceLabel != names.Service { - t.Fatalf("Expect Service name in Route label %q but got %q ", names.Service, routeServiceLabel) - } - } else { - t.Fatalf("Failed to get service name from Route label") - } + if route.Labels["serving.knative.dev/service"] != names.Service { + t.Errorf("Expect Service name in Route label %q but got %q ", names.Service, route.Labels["serving.knative.dev/service"]) + } } From 95b42d96d38789a39a53b2983550f422040246f6 Mon Sep 17 00:00:00 2001 From: Nader Ziada Date: Thu, 20 Sep 2018 15:45:28 -0400 Subject: [PATCH 3/3] minor changes for comments --- docs/spec/spec.md | 10 +++++----- test/conformance/labels_test.go | 11 +++++------ 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/docs/spec/spec.md b/docs/spec/spec.md index 0899f964e37a..e42c9740f682 100644 --- a/docs/spec/spec.md +++ b/docs/spec/spec.md @@ -52,7 +52,7 @@ metadata: name: my-service namespace: default labels: - knative.dev/service: ... # name of the service + knative.dev/service: ... # name of the Service automatically filled in # system generated meta uid: ... @@ -114,8 +114,8 @@ metadata: name: my-service namespace: default labels: - knative.dev/service: ... # name of the service - knative.dev/route: ... # name of the route + knative.dev/service: ... # name of the Service automatically filled in + knative.dev/route: ... # name of the Route automatically filled in # system generated meta uid: ... resourceVersion: ... # used for optimistic concurrency control @@ -224,8 +224,8 @@ metadata: name: myservice-a1e34 # system generated namespace: default labels: - knative.dev/configuration: ... # name of the configuration - knative.dev/service: ... # name of the service + knative.dev/configuration: ... # name of the Configuration automatically filled in + knative.dev/service: ... # name of the Service automatically filled in annotations: knative.dev/configurationGeneration: ... # generation of configuration that created this Revision # system generated meta diff --git a/test/conformance/labels_test.go b/test/conformance/labels_test.go index 5c9fca560ea1..15fecea318de 100644 --- a/test/conformance/labels_test.go +++ b/test/conformance/labels_test.go @@ -58,7 +58,7 @@ func TestLabelsPropagation(t *testing.T) { t.Fatalf("The Service %s was not marked as Ready to serve traffic to Revision %s: %v", names.Service, names.Revision, err) } - // Now that the serive is ready, we can validate the Labels on the Revision, Configuration + // Now that the Serive is ready, we can validate the Labels on the Revision, Configuration // and Route objects // see spec here: https://github.com/knative/serving/blob/master/docs/spec/spec.md#revision @@ -69,10 +69,10 @@ func TestLabelsPropagation(t *testing.T) { } if revision.Labels["serving.knative.dev/configuration"] != names.Config { - t.Errorf("Expect confguration name in revision label %q but got %q ", names.Config, revision.Labels["serving.knative.dev/configuration"]) + t.Errorf("Expect Confguration name in Revision label %q but got %q ", names.Config, revision.Labels["serving.knative.dev/configuration"]) } if revision.Labels["serving.knative.dev/service"] != names.Service { - t.Errorf("Expect Service name in revision label %q but got %q ", names.Service, revision.Labels["serving.knative.dev/service"]) + t.Errorf("Expect Service name in Revision label %q but got %q ", names.Service, revision.Labels["serving.knative.dev/service"]) } logger.Info("Validate Labels on Configuration Object") @@ -82,10 +82,10 @@ func TestLabelsPropagation(t *testing.T) { } if config.Labels["serving.knative.dev/service"] != names.Service { - t.Errorf("Expect Service name in configuration label %q but got %q ", names.Service, config.Labels["serving.knative.dev/service"]) + t.Errorf("Expect Service name in Configuration label %q but got %q ", names.Service, config.Labels["serving.knative.dev/service"]) } if config.Labels["serving.knative.dev/route"] != names.Route { - t.Errorf("Expect Route name in configuration label %q but got %q ", names.Route, config.Labels["serving.knative.dev/route"]) + t.Errorf("Expect Route name in Configuration label %q but got %q ", names.Route, config.Labels["serving.knative.dev/route"]) } logger.Info("Validate Labels on Route Object") @@ -95,7 +95,6 @@ func TestLabelsPropagation(t *testing.T) { } if route.Labels["serving.knative.dev/service"] != names.Service { - t.Errorf("Expect Service name in Route label %q but got %q ", names.Service, route.Labels["serving.knative.dev/service"]) } }