pkg/test: add in-cluster/in-image testing support (#469)

* pkg/test: add in-cluster/in-image testing support

Issue #435

* *: change the way the dockerfiles are used

* commands/.../test: create new test subcommands

* .travis.yml: update travis.yml to match test subcommands

* commands/.../test/cluster.go: return errors instead of cmderror

The cluster test has defers that need to run on error, which
would not run if cmdError is used due to its use of os.Exit().
Instead, we can just return an error, which allows the defer
functions to run.

* *: add tests for incluster test image and fix bugs

This adds tests for the new incluster image testing mode as well as
fixes some bugs that this test exposed

* commands/.../test/cluster.go: handle pending phase

Test could hang forever if the pod remaining in the 'Pending' phase,
which would occur if it cannot pull the image. This sets a
configurable pending timeout with a default of 60 seconds

* .travis.yml: enable go testing of ./commands/...

* pkg/generator: remove docker_build.sh generation

* pkg/generator/generator.go: change defaultExecFileMode to 0755

* commands/.../test: use structs for test flags/vars

Author: AlexNPavel

.travis.yml

@@ -25,13 +25,14 @@ install:
 
 script:
 - make install
+- go test ./commands/...
 - go test ./pkg/...
 - go test ./test/e2e/...
 - cd test/test-framework
 # test framework with defaults
-- operator-sdk test -t .
+- operator-sdk test local .
 # test operator-sdk test flags
-- operator-sdk test -t . -g deploy/crd.yaml -n deploy/namespace-init.yaml -f "-parallel 1" -k $HOME/.kube/config
+- operator-sdk test local . -g deploy/crd.yaml -n deploy/namespace-init.yaml -f "-parallel 1" -k $HOME/.kube/config
 # go back to project root
 - cd ../..
 - go vet ./...

commands/operator-sdk/cmd/build.go

@@ -15,17 +15,30 @@
 package cmd
 
 import (
+	"bytes"
+	"errors"
 	"fmt"
+	"io/ioutil"
+	"log"
 	"os"
 	"os/exec"
 
+	"github.com/operator-framework/operator-sdk/commands/operator-sdk/cmd/cmdutil"
 	cmdError "github.com/operator-framework/operator-sdk/commands/operator-sdk/error"
+	"github.com/operator-framework/operator-sdk/pkg/generator"
 
+	"github.com/ghodss/yaml"
 	"github.com/spf13/cobra"
 )
 
+var (
+	namespacedManBuild string
+	testLocationBuild  string
+	enableTests        bool
+)
+
 func NewBuildCmd() *cobra.Command {
-	return &cobra.Command{
+	buildCmd := &cobra.Command{
 		Use:   "build <image>",
 		Short: "Compiles code and builds artifacts",
 		Long: `The operator-sdk build command compiles the code, builds the executables,
@@ -42,12 +55,86 @@ For example:
 `,
 		Run: buildFunc,
 	}
+	buildCmd.Flags().BoolVarP(&enableTests, "enable-tests", "e", false, "Enable in-cluster testing by adding test binary to the image")
+	buildCmd.Flags().StringVarP(&testLocationBuild, "test-location", "t", "./test/e2e", "Location of tests")
+	buildCmd.Flags().StringVarP(&namespacedManBuild, "namespaced", "n", "deploy/operator.yaml", "Path of namespaced resources for tests")
+	return buildCmd
+}
+
+/*
+ * verifyDeploymentImages checks image names of pod 0 in deployments found in the provided yaml file.
+ * This is done because e2e tests require a namespaced manifest file to configure a namespace with
+ * required resources. This function is intended to identify if a user used a different image name
+ * for their operator in the provided yaml, which would result in the testing of the wrong operator
+ * image. As it is possible for a namespaced yaml to have multiple deployments (such as the vault
+ * operator, which depends on the etcd-operator), this is just a warning, not a fatal error.
+ */
+func verifyDeploymentImage(yamlFile []byte, imageName string) error {
+	warningMessages := ""
+	yamlSplit := bytes.Split(yamlFile, []byte("\n---\n"))
+	for _, yamlSpec := range yamlSplit {
+		yamlMap := make(map[string]interface{})
+		err := yaml.Unmarshal(yamlSpec, &yamlMap)
+		if err != nil {
+			log.Fatal("Could not unmarshal yaml namespaced spec")
+		}
+		kind, ok := yamlMap["kind"].(string)
+		if !ok {
+			log.Fatal("Yaml manifest file contains a 'kind' field that is not a string")
+		}
+		if kind == "Deployment" {
+			// this is ugly and hacky; we should probably make this cleaner
+			nestedMap, ok := yamlMap["spec"].(map[string]interface{})
+			if !ok {
+				continue
+			}
+			nestedMap, ok = nestedMap["template"].(map[string]interface{})
+			if !ok {
+				continue
+			}
+			nestedMap, ok = nestedMap["spec"].(map[string]interface{})
+			if !ok {
+				continue
+			}
+			containersArray, ok := nestedMap["containers"].([]interface{})
+			if !ok {
+				continue
+			}
+			for _, item := range containersArray {
+				image, ok := item.(map[string]interface{})["image"].(string)
+				if !ok {
+					continue
+				}
+				if image != imageName {
+					warningMessages = fmt.Sprintf("%s\nWARNING: Namespace manifest contains a deployment with image %v, which does not match the name of the image being built: %v", warningMessages, image, imageName)
+				}
+			}
+		}
+	}
+	if warningMessages == "" {
+		return nil
+	}
+	return errors.New(warningMessages)
+}
+
+func renderTestManifest(image string) {
+	namespacedBytes, err := ioutil.ReadFile(namespacedManBuild)
+	if err != nil {
+		log.Fatalf("could not read namespaced manifest: %v", err)
+	}
+	if err = generator.RenderTestYaml(cmdutil.GetConfig(), image); err != nil {
+		log.Fatalf("failed to generate deploy/test-pod.yaml: (%v)", err)
+	}
+	err = verifyDeploymentImage(namespacedBytes, image)
+	// the error from verifyDeploymentImage is just a warning, not fatal error
+	if err != nil {
+		fmt.Printf("%v\n", err)
+	}
 }
 
 const (
-	build       = "./tmp/build/build.sh"
-	dockerBuild = "./tmp/build/docker_build.sh"
-	configYaml  = "./config/config.yaml"
+	build      = "./tmp/build/build.sh"
+	configYaml = "./config/config.yaml"
 )
 
 func buildFunc(cmd *cobra.Command, args []string) {
@@ -56,18 +143,38 @@ func buildFunc(cmd *cobra.Command, args []string) {
 	}
 
 	bcmd := exec.Command(build)
+	bcmd.Env = append(os.Environ(), fmt.Sprintf("TEST_LOCATION=%v", testLocationBuild))
+	bcmd.Env = append(bcmd.Env, fmt.Sprintf("ENABLE_TESTS=%v", enableTests))
 	o, err := bcmd.CombinedOutput()
 	if err != nil {
 		cmdError.ExitWithError(cmdError.ExitError, fmt.Errorf("failed to build: (%v)", string(o)))
 	}
 	fmt.Fprintln(os.Stdout, string(o))
 
 	image := args[0]
-	dbcmd := exec.Command(dockerBuild)
-	dbcmd.Env = append(os.Environ(), fmt.Sprintf("IMAGE=%v", image))
+	baseImageName := image
+	if enableTests {
+		baseImageName += "-intermediate"
+	}
+	dbcmd := exec.Command("docker", "build", ".", "-f", "tmp/build/Dockerfile", "-t", baseImageName)
 	o, err = dbcmd.CombinedOutput()
 	if err != nil {
-		cmdError.ExitWithError(cmdError.ExitError, fmt.Errorf("failed to output build image %v: (%v)", image, string(o)))
+		if enableTests {
+			cmdError.ExitWithError(cmdError.ExitError, fmt.Errorf("failed to build intermediate image for %s image: (%s)", image, string(o)))
+		} else {
+			cmdError.ExitWithError(cmdError.ExitError, fmt.Errorf("failed to output build image %s: (%s)", image, string(o)))
+		}
 	}
 	fmt.Fprintln(os.Stdout, string(o))
+
+	if enableTests {
+		testDbcmd := exec.Command("docker", "build", ".", "-f", "tmp/build/test-framework/Dockerfile", "-t", image, "--build-arg", "NAMESPACEDMAN="+namespacedManBuild, "--build-arg", "BASEIMAGE="+baseImageName)
+		o, err = testDbcmd.CombinedOutput()
+		if err != nil {
+			cmdError.ExitWithError(cmdError.ExitError, fmt.Errorf("failed to output build image %v: (%v)", image, string(o)))
+		}
+		fmt.Fprintln(os.Stdout, string(o))
+		// create test-pod.yaml as well as check image name of deployments in namespaced manifest
+		renderTestManifest(image)
+	}
 }

commands/operator-sdk/cmd/build_test.go

@@ -0,0 +1,116 @@
+// Copyright 2018 The Operator-SDK 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 cmd
+
+import "testing"
+
+var memcachedNamespaceManExample = `apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: memcached-operator
+
+---
+
+kind: Role
+apiVersion: rbac.authorization.k8s.io/v1beta1
+metadata:
+  name: memcached-operator
+rules:
+- apiGroups:
+  - cache.example.com
+  resources:
+  - "*"
+  verbs:
+  - "*"
+- apiGroups:
+  - ""
+  resources:
+  - pods
+  - services
+  - endpoints
+  - persistentvolumeclaims
+  - events
+  - configmaps
+  - secrets
+  verbs:
+  - "*"
+- apiGroups:
+  - apps
+  resources:
+  - deployments
+  - daemonsets
+  - replicasets
+  - statefulsets
+  verbs:
+  - "*"
+
+---
+
+kind: RoleBinding
+apiVersion: rbac.authorization.k8s.io/v1beta1
+metadata:
+  name: memcached-operator
+subjects:
+- kind: ServiceAccount
+  name: memcached-operator
+roleRef:
+  kind: Role
+  name: memcached-operator
+  apiGroup: rbac.authorization.k8s.io
+
+---
+
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: memcached-operator
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      name: memcached-operator
+  template:
+    metadata:
+      labels:
+        name: memcached-operator
+    spec:
+      serviceAccountName: memcached-operator
+      containers:
+        - name: memcached-operator
+          image: quay.io/coreos/operator-sdk-dev:test-framework-operator
+          ports:
+          - containerPort: 60000
+            name: metrics
+          command:
+          - memcached-operator
+          imagePullPolicy: Always
+          env:
+            - name: WATCH_NAMESPACE
+              valueFrom:
+                fieldRef:
+                  fieldPath: metadata.namespace
+            - name: OPERATOR_NAME
+              value: "memcached-operator"
+
+`
+
+func TestVerifyDeploymentImage(t *testing.T) {
+	if err := verifyDeploymentImage([]byte(memcachedNamespaceManExample), "quay.io/coreos/operator-sdk-dev:test-framework-operator"); err != nil {
+		t.Fatalf("verifyDeploymentImage incorrectly reported an error: %v", err)
+	}
+	if err := verifyDeploymentImage([]byte(memcachedNamespaceManExample), "different-image-name"); err == nil {
+		t.Fatal("verifyDeploymentImage did not report an error on an incorrect manifest")
+	}
+}

commands/operator-sdk/cmd/test.go

@@ -15,82 +15,20 @@
 package cmd
 
 import (
-	"io/ioutil"
-	"log"
-	"os"
-	"strings"
-
-	"github.com/operator-framework/operator-sdk/pkg/test"
+	"github.com/operator-framework/operator-sdk/commands/operator-sdk/cmd/test"
 
 	"github.com/spf13/cobra"
 )
 
-var (
-	testLocation           string
-	kubeconfig             string
-	globalManifestPath     string
-	namespacedManifestPath string
-	goTestFlags            string
-)
-
 func NewTestCmd() *cobra.Command {
 	testCmd := &cobra.Command{
-		Use:   "test --test-location <path to tests directory> [flags]",
-		Short: "Run End-To-End tests",
-		Run:   testFunc,
-	}
-	defaultKubeConfig := ""
-	homedir, ok := os.LookupEnv("HOME")
-	if ok {
-		defaultKubeConfig = homedir + "/.kube/config"
+		Use:   "test",
+		Short: "Tests the operator",
+		Long: `The test command has subcommands that can test the operator locally or from within a cluster.
+`,
 	}
-	testCmd.Flags().StringVarP(&testLocation, "test-location", "t", "", "Location of test files (e.g. ./test/e2e/)")
-	testCmd.MarkFlagRequired("test-location")
-	testCmd.Flags().StringVarP(&kubeconfig, "kubeconfig", "k", defaultKubeConfig, "Kubeconfig path")
-	testCmd.Flags().StringVarP(&globalManifestPath, "global-init", "g", "deploy/crd.yaml", "Path to manifest for Global resources (e.g. CRD manifest)")
-	testCmd.Flags().StringVarP(&namespacedManifestPath, "namespaced-init", "n", "", "Path to manifest for per-test, namespaced resources (e.g. RBAC and Operator manifest)")
-	testCmd.Flags().StringVarP(&goTestFlags, "go-test-flags", "f", "", "Additional flags to pass to go test")
 
+	testCmd.AddCommand(cmdtest.NewTestLocalCmd())
+	testCmd.AddCommand(cmdtest.NewTestClusterCmd())
 	return testCmd
 }
-
-func testFunc(cmd *cobra.Command, args []string) {
-	// if no namespaced manifest path is given, combine deploy/sa.yaml, deploy/rbac.yaml and deploy/operator.yaml
-	if namespacedManifestPath == "" {
-		os.Mkdir("deploy/test", os.FileMode(int(0775)))
-		namespacedManifestPath = "deploy/test/namespace-manifests.yaml"
-		sa, err := ioutil.ReadFile("deploy/sa.yaml")
-		if err != nil {
-			log.Fatalf("could not find sa manifest: %v", err)
-		}
-		rbac, err := ioutil.ReadFile("deploy/rbac.yaml")
-		if err != nil {
-			log.Fatalf("could not find rbac manifest: %v", err)
-		}
-		operator, err := ioutil.ReadFile("deploy/operator.yaml")
-		if err != nil {
-			log.Fatalf("could not find operator manifest: %v", err)
-		}
-		combined := append(sa, []byte("\n---\n")...)
-		combined = append(combined, rbac...)
-		combined = append(combined, []byte("\n---\n")...)
-		combined = append(combined, operator...)
-		err = ioutil.WriteFile(namespacedManifestPath, combined, os.FileMode(int(0664)))
-		if err != nil {
-			log.Fatalf("could not create temporary namespaced manifest file: %v", err)
-		}
-		defer func() {
-			err := os.Remove(namespacedManifestPath)
-			if err != nil {
-				log.Fatalf("could not delete temporary namespace manifest file")
-			}
-		}()
-	}
-	testArgs := []string{"test", testLocation + "/..."}
-	testArgs = append(testArgs, "-"+test.KubeConfigFlag, kubeconfig)
-	testArgs = append(testArgs, "-"+test.NamespacedManPathFlag, namespacedManifestPath)
-	testArgs = append(testArgs, "-"+test.GlobalManPathFlag, globalManifestPath)
-	testArgs = append(testArgs, "-"+test.ProjRootFlag, mustGetwd())
-	testArgs = append(testArgs, strings.Split(goTestFlags, " ")...)
-	execCmd(os.Stdout, "go", testArgs...)
-}