Moving envvars_test from e2e to conformance (#2602)

Change moves the envvars_test.go from e2e to conformance folder to use the test as a
conformance test for the environment variables requirement specified in
the run-time contract (https://github.com/knative/serving/blob/master/docs/runtime-contract.md)
The test image is also changed to be more generic and provide a server
that can be used to get information about the environment under which
the container runs. Currently the iamge only exposes environment
variables defined inside the container by exposing "/envvars" to fetch all the
environment variables. The image folder name is also changed to "environment" to
indicate this semantic change.

Change also adds test/conformance/constants.go to define a way to have
constants that can be shared between test-images and tests.

Change also adds test/conformance/runtime_contract_types.go to define the runtime contract as go types.

test/conformance/envvars_test.go

@@ -0,0 +1,77 @@
+// +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 (
+	"encoding/json"
+	"strconv"
+	"testing"
+
+	"github.com/knative/pkg/test/logging"
+	"github.com/knative/serving/test"
+)
+
+//TestShouldEnvVars verifies environment variables that are declared as "SHOULD be set" in runtime-contract
+func TestShouldEnvVars(t *testing.T) {
+	logger := logging.GetContextLogger("TestShouldEnvVars")
+	var names test.ResourceNames
+	resp, err := fetchEnvInfo(t, logger, test.EnvImageEnvVarsPath, &names)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var respValues ShouldEnvvars
+	if err := json.Unmarshal(resp, &respValues); err != nil {
+		t.Fatalf("Failed to unmarshall response : %v", err)
+	}
+
+	expectedValues := ShouldEnvvars {
+		Service: names.Service,
+		Configuration : names.Config,
+		Revision : names.Revision,
+	}
+	if respValues != expectedValues {
+		t.Fatalf("Received response failed to match execpted response. Received: %v Expected: %v", respValues, expectedValues)
+	}
+}
+
+//TestMustEnvVars verifies environment variables that are declared as "MUST be set" in runtime-contract
+func TestMustEnvVars(t *testing.T) {
+	logger := logging.GetContextLogger("TestMustEnvVars")
+	var names test.ResourceNames
+	resp, err := fetchEnvInfo(t, logger, test.EnvImageEnvVarsPath, &names)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var respValues MustEnvvars
+	if err := json.Unmarshal(resp, &respValues); err != nil {
+		t.Fatalf("Failed to unmarshall response : %v", err)
+	}
+
+	expectedValues := MustEnvvars {
+		// The port value needs to match the port exposed by the test-image.
+		// We currently control them by using a common constant, but any change needs synchronization between this check
+		// and the value used by the test-image.
+		Port: strconv.Itoa(test.EnvImageServerPort),
+	}
+	if respValues != expectedValues {
+		t.Fatalf("Received response failed to match execpted response. Received: %v Expected: %v", respValues, expectedValues)
+	}
+}

test/e2e/envvars_test.go → test/conformance/runtime_conformance_helper.go

@@ -16,7 +16,9 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package e2e
+//runtime_conformance_helper.go contains helper methods used by conformance tests that verify runtime-contract.
+
+package conformance
 
 import (
 	"fmt"
@@ -25,33 +27,31 @@ import (
 
 	pkgTest "github.com/knative/pkg/test"
 	"github.com/knative/pkg/test/logging"
+	"github.com/knative/pkg/test/spoof"
 	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
 	serviceresourcenames "github.com/knative/serving/pkg/reconciler/v1alpha1/service/resources/names"
 	"github.com/knative/serving/test"
+	"github.com/pkg/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-func TestEnvVars(t *testing.T) {
-	clients := Setup(t)
-
-	//add test case specific name to its own logger
-	logger := logging.GetContextLogger("TestEnvVars")
-
-	var names test.ResourceNames
-	names.Service = test.AppendRandomString("yashiki", logger)
-	names.Image = "envvars"
-
-	test.CleanupOnInterrupt(func() { TearDown(clients, names, logger) }, logger)
-	defer TearDown(clients, names, logger)
+//fetchEnvInfo creates the service using test_images/environment and fetches environment info defined inside the container dictated by urlPath.
+func fetchEnvInfo(t *testing.T, logger *logging.BaseLogger, urlPath string, names* test.ResourceNames) ([]byte, error) {
+	clients := setup(t)
 
 	logger.Info("Creating a new Service")
-	svc, err := test.CreateLatestService(logger, clients, names)
+	names.Service = test.AppendRandomString("yashiki", logger)
+	names.Image = "environment"
+	svc, err := test.CreateLatestService(logger, clients, *names)
 	if err != nil {
-		t.Fatalf("Failed to create Service: %v", err)
+		return nil, errors.New(fmt.Sprintf("Failed to create Service: %v", err))
 	}
 	names.Route = serviceresourcenames.Route(svc)
 	names.Config = serviceresourcenames.Configuration(svc)
 
+	test.CleanupOnInterrupt(func() { tearDown(clients, *names) }, logger)
+	defer tearDown(clients, *names)
+
 	var revisionName string
 	logger.Info("The Service will be updated with the name of the Revision once it is created")
 	err = test.WaitForServiceState(clients.ServingClient, names.Service, func(s *v1alpha1.Service) (bool, error) {
@@ -62,36 +62,42 @@ func TestEnvVars(t *testing.T) {
 		return false, nil
 	}, "ServiceUpdatedWithRevision")
 	if err != nil {
-		t.Fatalf("Service %s was not updated with the new revision: %v", names.Service, err)
+		return nil, errors.New(fmt.Sprintf("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)
+		return nil, errors.New(fmt.Sprintf("The Service %s was not marked as Ready to serve traffic to Revision %s: %v", names.Service, names.Revision, err))
 	}
 
 	logger.Infof("When the Revision can have traffic routed to it, the Route is marked as Ready.")
 	if err := test.WaitForRouteState(clients.ServingClient, names.Route, test.IsRouteReady, "RouteIsReady"); err != nil {
-		t.Fatalf("The Route %s was not marked as Ready to serve traffic: %v", names.Route, err)
+		return nil, errors.New(fmt.Sprintf("The Route %s was not marked as Ready to serve traffic: %v", names.Route, err))
 	}
 
 	route, err := clients.ServingClient.Routes.Get(names.Route, metav1.GetOptions{})
 	if err != nil {
-		t.Fatalf("Error fetching Route %s: %v", names.Route, err)
+		return nil, errors.New(fmt.Sprintf("Error fetching Route %s: %v", names.Route, err))
 	}
-	domain := route.Status.Domain
 
-	envVarsExpectedOutput := fmt.Sprintf("Here are our env vars service:%s - configuration:%s - revision:%s", names.Service, names.Config, names.Revision)
-
-	_, err = pkgTest.WaitForEndpointState(
+	url := route.Status.Domain + urlPath
+	resp, err := pkgTest.WaitForEndpointState(
 		clients.KubeClient,
 		logger,
-		domain,
-		pkgTest.Retrying(pkgTest.MatchesBody(envVarsExpectedOutput), http.StatusNotFound),
+		url,
+		pkgTest.Retrying(func(resp *spoof.Response) (bool, error) {
+			if resp.StatusCode ==  http.StatusOK {
+				return true, nil
+			}
+
+			return true, errors.New(string(resp.Body))
+		}, http.StatusNotFound),
 		"EnvVarsServesText",
 		test.ServingFlags.ResolvableDomain)
 	if err != nil {
-		t.Fatalf("The endpoint for Route %s at domain %s didn't serve the expected text \"%s\": %v", names.Route, domain, envVarsExpectedOutput, err)
+		return nil, errors.New(fmt.Sprintf("Failed before reaching desired state : %v", err))
 	}
-}
+
+	return resp.Body, nil
+}

test/conformance/runtime_contact_types.go

@@ -0,0 +1,33 @@
+// +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
+
+//runtime_constract_types.go defines types that encapsulate run-time contract requirements as specified here: https://github.com/knative/serving/blob/master/docs/runtime-contract.md
+
+//ShouldEnvvars defines the environment variables that "SHOULD" be set.
+type ShouldEnvvars struct {
+	Service string `json:"K_SERVICE"`
+	Configuration string `json:"K_CONFIGURATION"`
+	Revision string `json:"K_REVISION"`
+}
+
+//MustEnvvars defines environment variables that "MUST" be set.
+type MustEnvvars struct {
+	Port string `json:"PORT"`
+}

test/image_constants.go

@@ -0,0 +1,26 @@
+/*
+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 test
+
+//image_constants.go defines constants that are shared between test-images and conformance tests
+
+//EnvImageServerPort is the port on which the environment test-image server starts.
+// TODO: Modify this port number after https://github.com/knative/serving/issues/2258 is fixed for a stricter verification.
+const EnvImageServerPort = 8080
+
+//EnvImageEnvVarsPath path exposed by environment test-image to fetch environment variables.
+const EnvImageEnvVarsPath = "/envvars"

test/test_images/environment/README.md

@@ -0,0 +1,15 @@
+# Environment test image
+
+This directory contains the test image used to retrieve environment information under which the container runs. This is used by conformance tests to verify Knative [run-time contract](/docs/runtime-contract.md)
+
+The image contains a simple Go webserver, `environment.go`, which by default, listens on port defined in the constant [EnvImageServerPort](/test/conformance/constants.go).
+
+Currently the server exposes:
+
+* /envvars : To provide a JSON payload containing all the environment variables set inside the container
+
+## Building
+
+For details about building and adding new images, see the [section about test
+images](/test/README.md#test-images).
+

test/test_images/envvars/envvars.go → test/test_images/environment/environment.go

@@ -17,26 +17,36 @@ limitations under the License.
 package main
 
 import (
+	"encoding/json"
 	"flag"
 	"fmt"
 	"log"
 	"net/http"
 	"os"
+	"strings"
 
 	"github.com/knative/serving/test"
 )
 
-func handler(w http.ResponseWriter, r *http.Request) {
-	log.Print("Env vars test app received a request.")
-	svc := os.Getenv("K_SERVICE")
-	cfg := os.Getenv("K_CONFIGURATION")
-	rev := os.Getenv("K_REVISION")
-	fmt.Fprintf(w, "Here are our env vars service:%s - configuration:%s - revision:%s", svc, cfg, rev)
+func envvarsHandler(w http.ResponseWriter, r *http.Request) {
+	envvars := make(map[string]string)
+	for _, pair := range os.Environ() {
+		tokens := strings.Split(pair, "=")
+		envvars[tokens[0]] = tokens[1]
+	}
+
+	if resp, err := json.Marshal(envvars); err != nil {
+		fmt.Fprintf(w, fmt.Sprintf("error building response : %v", err))
+	} else {
+		fmt.Fprintf(w, string(resp))
+	}
 }
 
 func main() {
 	flag.Parse()
-	log.Print("Env vars test app started.")
-
-	test.ListenAndServeGracefully(":8080", handler)
+	log.Print("Environment test app started.")
+	test.ListenAndServeGracefullyWithPattern(fmt.Sprintf(":%d", test.EnvImageServerPort), map[string]func(w http.ResponseWriter, r *http.Request) {
+		test.EnvImageEnvVarsPath : envvarsHandler,
+	})
 }
+

test/util.go

@@ -10,7 +10,7 @@ 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.
 */
-// util.go provides shared utilities methods across knative serving test
+
 package test
 
 import (
@@ -25,6 +25,8 @@ import (
 	"github.com/knative/pkg/test/logging"
 )
 
+// util.go provides shared utilities methods across knative serving test
+
 // LogResourceObject logs the resource object with the resource name and value
 func LogResourceObject(logger *logging.BaseLogger, value ResourceObjects) {
 	// Log the route object
@@ -40,18 +42,28 @@ func ImagePath(name string) string {
 	return fmt.Sprintf("%s/%s:%s", ServingFlags.DockerRepo, name, ServingFlags.Tag)
 }
 
-// ListenAndServeGracefully creates an HTTP server, listens on the defined address
-// and handles incoming requests to "/" with the given function.
-// It blocks until SIGTERM is received and the underlying server has shutdown gracefully.
+// ListenAndServeGracefully calls into ListenAndServeGracefullyWithPattern
+// by passing handler to handle requests for "/"
 func ListenAndServeGracefully(addr string, handler func(w http.ResponseWriter, r *http.Request)) {
+	ListenAndServeGracefullyWithPattern(addr, map[string]func(w http.ResponseWriter, r *http.Request){
+		"/" : handler,
+	})
+}
+
+// ListenAndServeGracefullyWithPattern creates an HTTP server, listens on the defined address
+// and handles incoming requests specified on pattern(path) with the given handlers.
+// It blocks until SIGTERM is received and the underlying server has shutdown gracefully.
+func ListenAndServeGracefullyWithPattern(addr string, handlers map[string]func(w http.ResponseWriter, r *http.Request)) {
 	m := http.NewServeMux()
-	m.HandleFunc("/", handler)
-	server := http.Server{Addr: addr, Handler: m}
+	for pattern, handler := range handlers {
+		m.HandleFunc(pattern, handler)
+	}
 
+	server := http.Server{Addr: addr, Handler: m}
 	go server.ListenAndServe()
 
 	sigTermChan := make(chan os.Signal)
 	signal.Notify(sigTermChan, syscall.SIGTERM)
 	<-sigTermChan
 	server.Shutdown(context.Background())
-}
+}