Revamp the integration tests

HACKING.md

@@ -75,10 +75,14 @@ oc create -f manifests/04_cluster-ingress-operator.yaml
 
 ### Integration tests
 
-Integration tests are still very immature. To run them, use the GCP test cluster instructions and then run the tests with:
+Integration tests are still very immature. To run them, start with an OpenShift 4.0 cluster and then run the following,
+substituting for your own details where appropriate. This assumes `KUBECONFIG` is set.
 
 ```
-KUBECONFIG=/path/to/admin.kubeconfig CLUSTER_NAME=your_gcp_cluster_name make test-integration
+REPO=docker.io/username/origin-cluster-ingress-operator make release-local
+
+# Set the manifests directory to the temporary directory reported by `release-local`.
+CLUSTER_NAME=your-cluster-name MANIFESTS=/release-local/output make test-integration
 ```
 
-**Important**: Note that the resources and namespaces used for the test are currently fixed and the tests will clean up after themselves, including deleting the `openshift-cluster-ingress-router` namespace. Don't run these tests in a cluster where data loss is a concern.
+**Important**: Note that these tests will destroy the Cluster Version Operator and any existing Cluster Ingress Operator deployment in the cluster. Don't run these tests in a cluster where data loss is a concern.

Makefile

@@ -28,11 +28,14 @@ $(GOBINDATA_BIN):
 test:
 	go test ./...
 
+release-local:
+	MANIFESTS=$(shell mktemp -d) hack/release-local.sh
+
 test-integration:
-	go test -v -tags integration ./test/integration
+	hack/test-integration.sh
 
 clean:
 	go clean
 	rm -f $(BIN)
 
-.PHONY: all build generate test test-integration clean
+.PHONY: all build generate test test-integration clean release-local

hack/release-local.sh

@@ -0,0 +1,32 @@
+#/bin/bash
+set -euo pipefail
+
+REPO="${REPO:-}"
+MANIFESTS="${MANIFESTS:-}"
+
+if [ -z "$REPO" ]; then echo "REPO is required"; exit 1; fi
+if [ -z "$MANIFESTS" ]; then echo "MANIFESTS is required"; exit 1; fi
+
+TEMP_COMMIT="false"
+test -z "$(git status --porcelain)" || TEMP_COMMIT="true"
+
+if [[ "${TEMP_COMMIT}" == "true" ]]; then
+  git add .
+  git commit -m "Temporary" || true
+fi
+
+REV=$(git rev-parse --short HEAD)
+docker build -t $REPO:$REV -f images/cluster-ingress-operator/Dockerfile .
+docker push $REPO:$REV
+
+if [[ "${TEMP_COMMIT}" == "true" ]]; then
+  git reset --soft HEAD~1
+fi
+
+cp -R manifests/ $MANIFESTS
+cat manifests/02-deployment.yaml | sed "s~openshift/origin-cluster-ingress-operator:latest~$REPO:$REV~" > "$MANIFESTS/02-deployment.yaml"
+
+echo "Pushed $REPO:$REV"
+echo "Install manifests using:"
+echo ""
+echo "oc apply -f $MANIFESTS"

hack/test-integration.sh

@@ -0,0 +1,12 @@
+#!/bin/bash
+set -euo pipefail
+
+CLUSTER_NAME="${CLUSTER_NAME:-}"
+MANIFESTS="${MANIFESTS:-}"
+
+if [ -z "${CLUSTER_NAME}" ]; then echo "CLUSTER_NAME is required"; exit 1; fi
+if [ -z "${MANIFESTS}" ]; then echo "MANIFESTS is required"; exit 1; fi
+
+export WATCH_NAMESPACE="openshift-cluster-ingress-operator"
+export KUBERNETES_CONFIG="${KUBECONFIG}"
+go test -v -tags integration ./test/integration --manifests-dir "${MANIFESTS}" --cluster-name "${CLUSTER_NAME}"

test/assets/app-ingress/route-default.yaml

@@ -4,7 +4,7 @@ metadata:
   name: default
   namespace: cluster-ingress-test
 spec:
-  host: app-ingress-test.apps.CLUSTER_NAME.origin-gce.dev.openshift.com
+  host: app.NAMESPACE.apps.CLUSTER_NAME.devcluster.openshift.com
   to:
     kind: Service
     name: app

test/assets/app-ingress/route-internal.yaml

@@ -6,7 +6,7 @@ metadata:
   labels:
     environment: internal
 spec:
-  host: app-ingress-test.apps.CLUSTER_NAME-internal.origin-gce.dev.openshift.com
+  host: app-ingress-test.apps.CLUSTER_NAME-internal.devcluster.openshift.com
   to:
     kind: Service
     name: app

test/integration/integration_test.go

@@ -3,244 +3,106 @@
 package integration
 
 import (
-	"context"
+	"flag"
 	"fmt"
-	"net/http"
 	"os"
 	"os/exec"
 	"testing"
-	"time"
 
-	stub "github.com/openshift/cluster-ingress-operator/pkg/stub"
-	"github.com/openshift/cluster-ingress-operator/test/manifests"
-
-	sdk "github.com/operator-framework/operator-sdk/pkg/sdk"
-	k8sutil "github.com/operator-framework/operator-sdk/pkg/util/k8sutil"
-
-	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/util/wait"
+	"github.com/sirupsen/logrus"
 )
 
-var testConfig *TestConfig
-
-func TestIntegration(t *testing.T) {
-	testConfig = NewTestConfig(t)
-
-	testConfig.createCRD()
-	defer testConfig.deleteCRD()
-
-	testConfig.startOperator()
+var clusterName = flag.String("cluster-name", "", "cluster name")
+var manifestsDir = flag.String("manifests-dir", "", "manifests directory")
 
-	// Execute subtests
-	t.Run("TestMultipleIngresses", testMultipleIngresses)
+func TestMain(m *testing.M) {
+	flag.Parse()
+	os.Exit(m.Run())
 }
 
-func testMultipleIngresses(t *testing.T) {
-	f := manifests.NewFactory(testConfig.clusterName)
-
-	appNamespace, err := f.AppIngressNamespace()
-	if err != nil {
-		t.Fatal(err)
-	}
-	appDeployment, err := f.AppIngressDeployment()
-	if err != nil {
-		t.Fatal(err)
-	}
-	appService, err := f.AppIngressService()
-	if err != nil {
-		t.Fatal(err)
-	}
-	appRouteDefault, err := f.AppIngressRouteDefault()
-	if err != nil {
-		t.Fatal(err)
-	}
-	appRouteInternal, err := f.AppIngressRouteInternal()
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	clusterIngressDefault, err := f.ClusterIngressDefault()
-	if err != nil {
-		t.Fatal(err)
-	}
-	clusterIngressDefault.Namespace = testConfig.operatorNamespace
-
-	clusterIngressInternal, err := f.ClusterIngressInternal()
-	if err != nil {
-		t.Fatal(err)
-	}
-	clusterIngressInternal.Namespace = testConfig.operatorNamespace
-
-	routerNamespace, err := f.RouterNamespace()
-	if err != nil {
-		t.Fatal(err)
-	}
-	defaultService, err := f.RouterServiceCloud(clusterIngressDefault)
-	if err != nil {
-		t.Fatal(err)
-	}
-	internalService, err := f.RouterServiceCloud(clusterIngressInternal)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	cleanup := func() {
-		leftovers := []sdk.Object{
-			clusterIngressDefault,
-			clusterIngressInternal,
-			routerNamespace,
-			appNamespace,
-		}
-		anyFailed := false
-		for _, o := range leftovers {
-			err := sdk.Delete(o)
-			if err != nil && !errors.IsNotFound(err) {
-				t.Logf("failed to clean up object %#v: %s", o, err)
-				anyFailed = true
-			}
-		}
-		if anyFailed {
-			t.Fatalf("failed to clean up resources")
-		}
-	}
-	defer cleanup()
-
-	err = sdk.Create(appNamespace)
-	if err != nil {
-		t.Fatal(err)
-	}
-	err = sdk.Create(appDeployment)
-	if err != nil {
-		t.Fatal(err)
-	}
-	err = sdk.Create(appService)
-	if err != nil {
-		t.Fatal(err)
-	}
-	err = sdk.Create(appRouteDefault)
-	if err != nil {
-		t.Fatal(err)
-	}
-	err = sdk.Create(appRouteInternal)
-	if err != nil {
-		t.Fatal(err)
-	}
-	err = sdk.Create(clusterIngressDefault)
-	if err != nil {
-		t.Fatal(err)
-	}
-	err = sdk.Create(clusterIngressInternal)
-	if err != nil {
-		t.Fatal(err)
-	}
+func TestIntegration(t *testing.T) {
+	tc := NewTestConfig(t, *clusterName, *manifestsDir)
 
-	for _, service := range []*corev1.Service{defaultService, internalService} {
-		err := wait.Poll(1*time.Second, 2*time.Minute, func() (bool, error) {
-			err := sdk.Get(service)
-			if err != nil {
-				if errors.IsNotFound(err) {
-					return false, nil
-				}
-				return false, err
-			}
-			for _, ingress := range service.Status.LoadBalancer.Ingress {
-				if len(ingress.IP) > 0 {
-					t.Logf("service %s/%s has ingress.IP %s", service.Namespace, service.Name, ingress.IP)
-					return true, nil
-				}
-			}
-			return false, nil
-		})
-		if err != nil {
-			t.Fatalf("timed out waiting for service %s/%s: %s", service.Namespace, service.Name, err)
-		}
-	}
+	tc.setup(t)
+	defer tc.teardown(t)
 
-	client := &http.Client{}
-	for routeHost, ingressIP := range map[string]string{
-		appRouteDefault.Spec.Host:  defaultService.Status.LoadBalancer.Ingress[0].IP,
-		appRouteInternal.Spec.Host: internalService.Status.LoadBalancer.Ingress[0].IP,
-	} {
-		err := wait.Poll(1*time.Second, 2*time.Minute, func() (bool, error) {
-			req, err := http.NewRequest("GET", "http://"+ingressIP, nil)
-			req.Host = routeHost
-			resp, err := client.Do(req)
-			defer resp.Body.Close()
-			if err != nil {
-				return false, err
-			}
-			if resp.StatusCode == http.StatusOK {
-				return true, nil
-			}
-			return false, fmt.Errorf("last response: %s", resp.Status)
-		})
-		if err != nil {
-			t.Fatalf("timed out waiting for route endpoint %q at ingress IP %q: %s", routeHost, ingressIP, err)
-		}
-	}
+	// Execute subtests
+	t.Run("TestDefaultIngress", func(t *testing.T) { testDefaultIngress(t, tc) })
 }
 
 type TestConfig struct {
 	operatorNamespace string
 	clusterName       string
+	manifestsDir      string
 	kubeConfig        string
-
-	t *testing.T
 }
 
-func NewTestConfig(t *testing.T) *TestConfig {
-	config := &TestConfig{t: t}
-
+func NewTestConfig(t *testing.T, clusterName string, manifestsDir string) *TestConfig {
 	// Check prerequisites
 	kubeConfig := os.Getenv("KUBECONFIG")
 	if len(kubeConfig) == 0 {
 		t.Fatalf("KUBECONFIG is required")
 	}
 	// The operator-sdk uses KUBERNETES_CONFIG...
 	os.Setenv("KUBERNETES_CONFIG", kubeConfig)
-	config.kubeConfig = kubeConfig
 
-	clusterName := os.Getenv("CLUSTER_NAME")
 	if len(clusterName) == 0 {
-		t.Fatalf("CLUSTER_NAME is required")
+		t.Fatalf("cluster name is required")
+	}
+
+	if len(manifestsDir) == 0 {
+		t.Fatalf("manifests directory is required")
 	}
-	config.clusterName = clusterName
 
-	namespace, err := k8sutil.GetWatchNamespace()
-	if err != nil {
-		namespace = "default"
-		os.Setenv("WATCH_NAMESPACE", namespace)
+	return &TestConfig{
+		clusterName:       clusterName,
+		kubeConfig:        kubeConfig,
+		manifestsDir:      manifestsDir,
+		operatorNamespace: "openshift-cluster-ingress-operator",
 	}
-	config.operatorNamespace = namespace
+}
+
+func (tc *TestConfig) setup(t *testing.T) {
+	// uninstall the CVO
+	tc.runShellCmdNonFatal(t, `oc patch -n openshift-cluster-version daemonsets/cluster-version-operator --patch '{"spec": {"template": {"spec": {"nodeSelector": {"node-role.kubernetes.io/fake": ""}}}}}'`)
+
+	// uninstall tectonic-ingress
+	tc.runShellCmdNonFatal(t, `oc delete namespaces/openshift-ingress`)
+
+	// uninstall any existing operator
+	tc.uninstallOperator(t)
 
-	return config
+	// reinstall the operator
+	tc.runShellCmdNonFatal(t, fmt.Sprintf(`oc apply -f %s`, tc.manifestsDir))
 }
 
-func (tc *TestConfig) startOperator() {
-	resource := "ingress.openshift.io/v1alpha1"
-	kind := "ClusterIngress"
-	resyncPeriod := 10 * time.Minute
-	tc.t.Logf("Watching %s, %s, %s, %d", resource, kind, tc.operatorNamespace, resyncPeriod)
-	sdk.Watch(resource, kind, tc.operatorNamespace, resyncPeriod)
-	sdk.Handle(stub.NewHandler())
-	go sdk.Run(context.TODO())
+func (tc *TestConfig) uninstallOperator(t *testing.T) {
+	tc.runShellCmdNonFatal(t, `oc delete -n openshift-cluster-ingress-operator --force --grace-period=0 clusteringresses/default`)
+	tc.runShellCmdNonFatal(t, `oc delete namespaces/openshift-cluster-ingress-operator`)
+	tc.runShellCmdNonFatal(t, `oc delete namespaces/openshift-cluster-ingress-router`)
+	tc.runShellCmdNonFatal(t, `oc delete clusterroles/cluster-ingress-operator:operator`)
+	tc.runShellCmdNonFatal(t, `oc delete clusterroles/cluster-ingress:router`)
+	tc.runShellCmdNonFatal(t, `oc delete clusterrolebindings/cluster-ingress-operator:operator`)
+	tc.runShellCmdNonFatal(t, `oc delete clusterrolebindings/cluster-ingress:router`)
+	tc.runShellCmdNonFatal(t, `oc delete customresourcedefinition.apiextensions.k8s.io/clusteringresses.ingress.openshift.io`)
 }
 
-func (tc *TestConfig) createCRD() {
-	tc.runShellCmd(fmt.Sprintf("oc apply -f ../../manifests/00-custom-resource-definition.yaml -n %s", tc.operatorNamespace), "create cluster ingress CRD")
+func (tc *TestConfig) teardown(t *testing.T) {
+	tc.uninstallOperator(t)
 }
 
-func (tc *TestConfig) deleteCRD() {
-	tc.runShellCmd(fmt.Sprintf("oc delete crd clusteringresses.ingress.openshift.io -n %s", tc.operatorNamespace), "delete cluster ingress CRD")
+func (tc *TestConfig) runShellCmdNonFatal(t *testing.T, command string) {
+	tc.runShellCmd(t, command, "", false)
 }
 
-func (tc *TestConfig) runShellCmd(command, msg string) {
+func (tc *TestConfig) runShellCmd(t *testing.T, command string, msg string, failOnError bool) {
 	cmd := []string{"sh", "-c", command}
 	c := exec.Command(cmd[0], cmd[1:]...)
 	c.Env = os.Environ()
 	c.Env = append(c.Env, fmt.Sprintf("KUBECONFIG=%s", tc.kubeConfig))
-	if err := c.Run(); err != nil {
-		tc.t.Fatalf("failed to %s: %v", msg, err)
+	output, err := c.CombinedOutput()
+	if err != nil && failOnError {
+		t.Fatalf("failed to %s: %v", msg, err)
 	}
+	logrus.Infof("cmd output: %s", output)
 }