Refactor e2e framework to remove globals (#2037)

* refactor e2e framework to remove globals
* pkg/test/context.go: initialize TestCtx kubeclient
* cmd/operator-sdk/test/local.go: use go1.13 error wrapping to check test failure reason

Author: joelanford

cmd/operator-sdk/test/local.go

@@ -15,6 +15,7 @@
 package test
 
 import (
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -225,6 +226,10 @@ func testLocalGoFunc(cmd *cobra.Command, args []string) error {
 		TestBinaryArgs: testArgs,
 	}
 	if err := projutil.GoTest(opts); err != nil {
+		var exitErr *exec.ExitError
+		if errors.As(err, &exitErr) {
+			os.Exit(exitErr.ExitCode())
+		}
 		return fmt.Errorf("failed to build test binary: (%v)", err)
 	}
 	log.Info("Local operator test successfully completed.")

internal/util/projutil/exec.go

@@ -29,7 +29,7 @@ func ExecCmd(cmd *exec.Cmd) error {
 	cmd.Stderr = os.Stderr
 	log.Debugf("Running %#v", cmd.Args)
 	if err := cmd.Run(); err != nil {
-		return fmt.Errorf("failed to exec %#v: %v", cmd.Args, err)
+		return fmt.Errorf("failed to exec %#v: %w", cmd.Args, err)
 	}
 	return nil
 }

pkg/test/context.go

@@ -21,13 +21,20 @@ import (
 	"time"
 
 	log "github.com/sirupsen/logrus"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/restmapper"
 )
 
 type TestCtx struct {
 	id         string
 	cleanupFns []cleanupFn
 	namespace  string
 	t          *testing.T
+
+	namespacedManPath string
+	client            *frameworkClient
+	kubeclient        kubernetes.Interface
+	restMapper        *restmapper.DeferredDiscoveryRESTMapper
 }
 
 type CleanupOptions struct {
@@ -38,7 +45,7 @@ type CleanupOptions struct {
 
 type cleanupFn func() error
 
-func NewTestCtx(t *testing.T) *TestCtx {
+func (f *Framework) newTestCtx(t *testing.T) *TestCtx {
 	var prefix string
 	if t != nil {
 		// TestCtx is used among others for namespace names where '/' is forbidden
@@ -56,12 +63,26 @@ func NewTestCtx(t *testing.T) *TestCtx {
 	}
 
 	id := prefix + "-" + strconv.FormatInt(time.Now().Unix(), 10)
+
+	var namespace string
+	if f.singleNamespaceMode {
+		namespace = f.Namespace
+	}
 	return &TestCtx{
-		id: id,
-		t:  t,
+		id:                id,
+		t:                 t,
+		namespace:         namespace,
+		namespacedManPath: *f.NamespacedManPath,
+		client:            f.Client,
+		kubeclient:        f.KubeClient,
+		restMapper:        f.restMapper,
 	}
 }
 
+func NewTestCtx(t *testing.T) *TestCtx {
+	return Global.newTestCtx(t)
+}
+
 func (ctx *TestCtx) GetID() string {
 	return ctx.id
 }

pkg/test/framework.go

@@ -15,40 +15,43 @@
 package test
 
 import (
+	"bytes"
 	goctx "context"
+	"flag"
 	"fmt"
+	"io/ioutil"
 	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
 	"sync"
+	"testing"
 	"time"
 
 	// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 
+	"github.com/operator-framework/operator-sdk/internal/scaffold"
 	k8sInternal "github.com/operator-framework/operator-sdk/internal/util/k8sutil"
+	"github.com/operator-framework/operator-sdk/internal/util/projutil"
+	"github.com/operator-framework/operator-sdk/pkg/k8sutil"
 
+	log "github.com/sirupsen/logrus"
 	extscheme "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/scheme"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/serializer"
 	"k8s.io/apimachinery/pkg/util/wait"
 	cached "k8s.io/client-go/discovery/cached"
 	"k8s.io/client-go/kubernetes"
 	cgoscheme "k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/restmapper"
+	"k8s.io/client-go/tools/clientcmd"
 	dynclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 var (
 	// Global framework struct
 	Global *Framework
-	// mutex for AddToFrameworkScheme
-	mutex = sync.Mutex{}
-	// whether to run tests in a single namespace
-	singleNamespace *bool
-	// decoder used by createFromYaml
-	dynamicDecoder runtime.Decoder
-	// restMapper for the dynamic client
-	restMapper *restmapper.DeferredDiscoveryRESTMapper
 )
 
 type Framework struct {
@@ -59,52 +62,102 @@ type Framework struct {
 	NamespacedManPath *string
 	Namespace         string
 	LocalOperator     bool
+
+	projectRoot         string
+	singleNamespaceMode bool
+	globalManPath       string
+	localOperatorArgs   string
+	kubeconfigPath      string
+	restMapper          *restmapper.DeferredDiscoveryRESTMapper
+
+	schemeMutex sync.Mutex
 }
 
-func setup(kubeconfigPath, namespacedManPath *string, localOperator bool) error {
-	namespace := ""
-	if *singleNamespace {
-		namespace = os.Getenv(TestNamespaceEnv)
-	}
-	var err error
-	var kubeconfig *rest.Config
-	var kcNamespace string
-	kubeconfig, kcNamespace, err = k8sInternal.GetKubeconfigAndNamespace(*kubeconfigPath)
-	if *singleNamespace && namespace == "" {
-		namespace = kcNamespace
-	}
+type frameworkOpts struct {
+	projectRoot         string
+	kubeconfigPath      string
+	globalManPath       string
+	namespacedManPath   string
+	localOperator       bool
+	singleNamespaceMode bool
+	isLocalOperator     bool
+	localOperatorArgs   string
+}
+
+const (
+	ProjRootFlag          = "root"
+	KubeConfigFlag        = "kubeconfig"
+	NamespacedManPathFlag = "namespacedMan"
+	GlobalManPathFlag     = "globalMan"
+	SingleNamespaceFlag   = "singleNamespace"
+	LocalOperatorFlag     = "localOperator"
+	LocalOperatorArgs     = "localOperatorArgs"
+
+	TestNamespaceEnv = "TEST_NAMESPACE"
+)
+
+func (opts *frameworkOpts) addToFlagSet(flagset *flag.FlagSet) {
+	flagset.StringVar(&opts.projectRoot, ProjRootFlag, "", "path to project root")
+	flagset.StringVar(&opts.namespacedManPath, NamespacedManPathFlag, "", "path to rbac manifest")
+	flagset.BoolVar(&opts.isLocalOperator, LocalOperatorFlag, false, "enable if operator is running locally (not in cluster)")
+	flagset.StringVar(&opts.kubeconfigPath, KubeConfigFlag, "", "path to kubeconfig")
+	flagset.StringVar(&opts.globalManPath, GlobalManPathFlag, "", "path to operator manifest")
+	flagset.BoolVar(&opts.singleNamespaceMode, SingleNamespaceFlag, false, "enable single namespace mode")
+	flagset.StringVar(&opts.localOperatorArgs, LocalOperatorArgs, "", "flags that the operator needs (while using --up-local). example: \"--flag1 value1 --flag2=value2\"")
+}
+
+func newFramework(opts *frameworkOpts) (*Framework, error) {
+	kubeconfig, kcNamespace, err := k8sInternal.GetKubeconfigAndNamespace(opts.kubeconfigPath)
 	if err != nil {
-		return fmt.Errorf("failed to build the kubeconfig: %v", err)
+		return nil, fmt.Errorf("failed to build the kubeconfig: %v", err)
+	}
+
+	namespace := kcNamespace
+	if opts.singleNamespaceMode {
+		testNamespace := os.Getenv(TestNamespaceEnv)
+		if testNamespace != "" {
+			namespace = testNamespace
+		}
 	}
+
 	kubeclient, err := kubernetes.NewForConfig(kubeconfig)
 	if err != nil {
-		return fmt.Errorf("failed to build the kubeclient: %v", err)
+		return nil, fmt.Errorf("failed to build the kubeclient: %v", err)
 	}
+
 	scheme := runtime.NewScheme()
 	if err := cgoscheme.AddToScheme(scheme); err != nil {
-		return fmt.Errorf("failed to add cgo scheme to runtime scheme: (%v)", err)
+		return nil, fmt.Errorf("failed to add cgo scheme to runtime scheme: (%v)", err)
 	}
 	if err := extscheme.AddToScheme(scheme); err != nil {
-		return fmt.Errorf("failed to add api extensions scheme to runtime scheme: (%v)", err)
+		return nil, fmt.Errorf("failed to add api extensions scheme to runtime scheme: (%v)", err)
 	}
+
 	cachedDiscoveryClient := cached.NewMemCacheClient(kubeclient.Discovery())
-	restMapper = restmapper.NewDeferredDiscoveryRESTMapper(cachedDiscoveryClient)
+	restMapper := restmapper.NewDeferredDiscoveryRESTMapper(cachedDiscoveryClient)
 	restMapper.Reset()
+
 	dynClient, err := dynclient.New(kubeconfig, dynclient.Options{Scheme: scheme, Mapper: restMapper})
 	if err != nil {
-		return fmt.Errorf("failed to build the dynamic client: %v", err)
+		return nil, fmt.Errorf("failed to build the dynamic client: %v", err)
 	}
-	dynamicDecoder = serializer.NewCodecFactory(scheme).UniversalDeserializer()
-	Global = &Framework{
+	framework := &Framework{
 		Client:            &frameworkClient{Client: dynClient},
 		KubeConfig:        kubeconfig,
 		KubeClient:        kubeclient,
 		Scheme:            scheme,
-		NamespacedManPath: namespacedManPath,
+		NamespacedManPath: &opts.namespacedManPath,
 		Namespace:         namespace,
-		LocalOperator:     localOperator,
+		LocalOperator:     opts.isLocalOperator,
+
+		projectRoot:         opts.projectRoot,
+		singleNamespaceMode: opts.singleNamespaceMode,
+		globalManPath:       opts.globalManPath,
+		localOperatorArgs:   opts.localOperatorArgs,
+		kubeconfigPath:      opts.kubeconfigPath,
+		restMapper:          restMapper,
 	}
-	return nil
+	return framework, nil
 }
 
 type addToSchemeFunc func(*runtime.Scheme) error
@@ -119,33 +172,118 @@ type addToSchemeFunc func(*runtime.Scheme) error
 // by the time this function is called. If the CRD takes more than 5 seconds to
 // become ready, this function throws an error
 func AddToFrameworkScheme(addToScheme addToSchemeFunc, obj runtime.Object) error {
-	mutex.Lock()
-	defer mutex.Unlock()
-	err := addToScheme(Global.Scheme)
+	return Global.addToScheme(addToScheme, obj)
+}
+
+func (f *Framework) addToScheme(addToScheme addToSchemeFunc, obj runtime.Object) error {
+	f.schemeMutex.Lock()
+	defer f.schemeMutex.Unlock()
+
+	err := addToScheme(f.Scheme)
 	if err != nil {
 		return err
 	}
-	restMapper.Reset()
-	dynClient, err := dynclient.New(Global.KubeConfig, dynclient.Options{Scheme: Global.Scheme, Mapper: restMapper})
+	f.restMapper.Reset()
+	dynClient, err := dynclient.New(f.KubeConfig, dynclient.Options{Scheme: f.Scheme, Mapper: f.restMapper})
 	if err != nil {
 		return fmt.Errorf("failed to initialize new dynamic client: (%v)", err)
 	}
 	err = wait.PollImmediate(time.Second, time.Second*10, func() (done bool, err error) {
-		if *singleNamespace {
-			err = dynClient.List(goctx.TODO(), obj, dynclient.InNamespace(Global.Namespace))
+		if f.singleNamespaceMode {
+			err = dynClient.List(goctx.TODO(), obj, dynclient.InNamespace(f.Namespace))
 		} else {
 			err = dynClient.List(goctx.TODO(), obj, dynclient.InNamespace("default"))
 		}
 		if err != nil {
-			restMapper.Reset()
+			f.restMapper.Reset()
 			return false, nil
 		}
-		Global.Client = &frameworkClient{Client: dynClient}
+		f.Client = &frameworkClient{Client: dynClient}
 		return true, nil
 	})
 	if err != nil {
 		return fmt.Errorf("failed to build the dynamic client: %v", err)
 	}
-	dynamicDecoder = serializer.NewCodecFactory(Global.Scheme).UniversalDeserializer()
 	return nil
 }
+
+func (f *Framework) runM(m *testing.M) (int, error) {
+	// setup context to use when setting up crd
+	ctx := f.newTestCtx(nil)
+	defer ctx.Cleanup()
+
+	// go test always runs from the test directory; change to project root
+	err := os.Chdir(f.projectRoot)
+	if err != nil {
+		return 0, fmt.Errorf("failed to change directory to project root: %v", err)
+	}
+
+	// create crd
+	globalYAML, err := ioutil.ReadFile(f.globalManPath)
+	if err != nil {
+		return 0, fmt.Errorf("failed to read global resource manifest: %v", err)
+	}
+	err = ctx.createFromYAML(globalYAML, true, &CleanupOptions{TestContext: ctx})
+	if err != nil {
+		return 0, fmt.Errorf("failed to create resource(s) in global resource manifest: %v", err)
+	}
+
+	if !f.LocalOperator {
+		return m.Run(), nil
+	}
+
+	// start local operator before running tests
+	outBuf := &bytes.Buffer{}
+	localCmd, err := f.setupLocalCommand()
+	if err != nil {
+		return 0, fmt.Errorf("failed to setup local command: %v", err)
+	}
+	localCmd.Stdout = outBuf
+	localCmd.Stderr = outBuf
+
+	err = localCmd.Start()
+	if err != nil {
+		return 0, fmt.Errorf("failed to run operator locally: %v", err)
+	}
+	log.Info("Started local operator")
+
+	// run the tests
+	exitCode := m.Run()
+
+	// kill the local operator and print its logs
+	err = localCmd.Process.Kill()
+	if err != nil {
+		log.Warn("Failed to stop local operator process")
+	}
+	fmt.Printf("\n------ Local operator output ------\n%s\n", outBuf.String())
+	return exitCode, nil
+}
+
+func (f *Framework) setupLocalCommand() (*exec.Cmd, error) {
+	projectName := filepath.Base(projutil.MustGetwd())
+	outputBinName := filepath.Join(scaffold.BuildBinDir, projectName+"-local")
+	opts := projutil.GoCmdOptions{
+		BinName:     outputBinName,
+		PackagePath: filepath.Join(scaffold.ManagerDir, scaffold.CmdFile),
+	}
+	if err := projutil.GoBuild(opts); err != nil {
+		return nil, fmt.Errorf("failed to build local operator binary: %s", err)
+	}
+
+	args := []string{}
+	if f.localOperatorArgs != "" {
+		args = append(args, strings.Split(f.localOperatorArgs, " ")...)
+	}
+
+	localCmd := exec.Command(outputBinName, args...)
+
+	if f.kubeconfigPath != "" {
+		localCmd.Env = append(os.Environ(), fmt.Sprintf("%v=%v", k8sutil.KubeConfigEnvVar, f.kubeconfigPath))
+	} else {
+		// we can hardcode index 0 as that is the highest priority kubeconfig to be loaded and will always
+		// be populated by NewDefaultClientConfigLoadingRules()
+		localCmd.Env = append(os.Environ(), fmt.Sprintf("%v=%v", k8sutil.KubeConfigEnvVar, clientcmd.NewDefaultClientConfigLoadingRules().Precedence[0]))
+	}
+	localCmd.Env = append(localCmd.Env, fmt.Sprintf("%v=%v", k8sutil.WatchNamespaceEnvVar, f.Namespace))
+	return localCmd, nil
+}