Add ability to configure storage to scorecard (operator-framework#5028)

* Add initial implementation of storage configuration support
* add exec and gather logic
* remove config labels for storage and replace with the v1alpha3 storage spec to hold the mountPath, mountPath in the spec triggers storage to be added
* Bump operator-framework/api dependency
* Regenerate samples

Co-authored-by: jmccormick2001 <[email protected]>

Signed-off-by: Fabian von Feilitzsch <[email protected]>

Author: fabianvf

Diff for: go.mod

@@ -12,7 +12,7 @@ require (
 	github.com/maxbrunsfeld/counterfeiter/v6 v6.2.2
 	github.com/onsi/ginkgo v1.16.4
 	github.com/onsi/gomega v1.13.0
-	github.com/operator-framework/api v0.10.0
+	github.com/operator-framework/api v0.10.2
 	github.com/operator-framework/java-operator-plugins v0.0.0-20210708174638-463fb91f3d5e
 	github.com/operator-framework/operator-lib v0.5.0
 	github.com/operator-framework/operator-registry v1.17.4

Diff for: go.sum

@@ -822,8 +822,9 @@ github.com/openzipkin/zipkin-go v0.1.6/go.mod h1:QgAqvLzwWbR/WpD4A3cGpPtJrZXNIiJ
 github.com/openzipkin/zipkin-go v0.2.1/go.mod h1:NaW6tEwdmWMaCDZzg8sh+IBNOxHMPnhQw8ySjnjRyN4=
 github.com/openzipkin/zipkin-go v0.2.2/go.mod h1:NaW6tEwdmWMaCDZzg8sh+IBNOxHMPnhQw8ySjnjRyN4=
 github.com/operator-framework/api v0.7.1/go.mod h1:L7IvLd/ckxJEJg/t4oTTlnHKAJIP/p51AvEslW3wYdY=
-github.com/operator-framework/api v0.10.0 h1:TaxbgbrV8D3wnKNyrImZ2zjQVVHMQRc7piWLDmlGoEE=
 github.com/operator-framework/api v0.10.0/go.mod h1:tV0BUNvly7szq28ZPBXhjp1Sqg5yHCOeX19ui9K4vjI=
+github.com/operator-framework/api v0.10.2 h1:fo8Bhyx1v46NdJIz2rUzfzNUpe1KDNPtVpHVpuxZnk0=
+github.com/operator-framework/api v0.10.2/go.mod h1:tV0BUNvly7szq28ZPBXhjp1Sqg5yHCOeX19ui9K4vjI=
 github.com/operator-framework/java-operator-plugins v0.0.0-20210708174638-463fb91f3d5e h1:LMsT59IJqaLn7kD6DnZFy0IouRufXyJHTT+mXQrl9Ps=
 github.com/operator-framework/java-operator-plugins v0.0.0-20210708174638-463fb91f3d5e/go.mod h1:sGKGELFkUeRqElcyvyPC89bC76YnCL7MPMa13P0AQcw=
 github.com/operator-framework/operator-lib v0.5.0 h1:Jmhz/WjcstEyBBM9IFUiHEgKg5bd43uF4ej/ZY2S0rM=

Diff for: internal/cmd/operator-sdk/scorecard/cmd.go

@@ -49,6 +49,7 @@ type scorecardCmd struct {
 	list           bool
 	skipCleanup    bool
 	waitTime       time.Duration
+	testOutput     string
 }
 
 func NewCmd() *cobra.Command {
@@ -85,6 +86,8 @@ If the argument holds an image tag, it must be present remotely.`,
 		"Disable resource cleanup after tests are run")
 	scorecardCmd.Flags().DurationVarP(&c.waitTime, "wait-time", "w", 30*time.Second,
 		"seconds to wait for tests to complete. Example: 35s")
+	scorecardCmd.Flags().StringVarP(&c.testOutput, "test-output", "t", "test-output",
+		"Test output directory.")
 
 	return scorecardCmd
 }
@@ -196,11 +199,12 @@ func (c *scorecardCmd) run() (err error) {
 			ServiceAccount: c.serviceAccount,
 			Namespace:      scorecard.GetKubeNamespace(c.kubeconfig, c.namespace),
 			BundlePath:     c.bundle,
+			TestOutput:     c.testOutput,
 			BundleMetadata: metadata,
 		}
 
 		// Only get the client if running tests.
-		if runner.Client, err = scorecard.GetKubeClient(c.kubeconfig); err != nil {
+		if runner.Client, runner.RESTConfig, err = scorecard.GetKubeClient(c.kubeconfig); err != nil {
 			return fmt.Errorf("error getting kubernetes client: %w", err)
 		}
 

Diff for: internal/scorecard/kubeclient.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/operator-framework/operator-sdk/internal/util/k8sutil"
 	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 	cruntime "sigs.k8s.io/controller-runtime/pkg/client/config"
 )
@@ -30,24 +31,24 @@ import (
 // - in-cluster connection for when the sdk is run within a cluster instead of
 //   the command line
 // TODO(joelanford): migrate scorecard use `internal/operator.Configuration`
-func GetKubeClient(kubeconfig string) (client kubernetes.Interface, err error) {
+func GetKubeClient(kubeconfig string) (client kubernetes.Interface, config *rest.Config, err error) {
 
 	if kubeconfig != "" {
 		os.Setenv(k8sutil.KubeConfigEnvVar, kubeconfig)
 	}
 
-	config, err := cruntime.GetConfig()
+	config, err = cruntime.GetConfig()
 	if err != nil {
-		return client, err
+		return client, config, err
 	}
 
 	// create the clientset
 	clientset, err := kubernetes.NewForConfig(config)
 	if err != nil {
-		return client, err
+		return client, config, err
 	}
 
-	return clientset, err
+	return clientset, config, err
 }
 
 // GetKubeNamespace returns the kubernetes namespace to use

Diff for: internal/scorecard/scorecard.go

@@ -26,6 +26,7 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/rest"
 
 	registryutil "github.com/operator-framework/operator-sdk/internal/registry"
 )
@@ -47,8 +48,10 @@ type PodTestRunner struct {
 	Namespace      string
 	ServiceAccount string
 	BundlePath     string
+	TestOutput     string
 	BundleMetadata registryutil.Labels
 	Client         kubernetes.Interface
+	RESTConfig     *rest.Config
 
 	configMapName string
 }
@@ -75,6 +78,7 @@ func (o Scorecard) Run(ctx context.Context) (testOutput v1alpha3.TestList, err e
 		if len(tests) == 0 {
 			continue
 		}
+		tests = o.setTestDefaults(tests)
 
 		output := make(chan v1alpha3.Test, len(tests))
 		if stage.Parallel {
@@ -107,6 +111,15 @@ func (o Scorecard) Run(ctx context.Context) (testOutput v1alpha3.TestList, err e
 	return testOutput, err
 }
 
+func (o Scorecard) setTestDefaults(tests []v1alpha3.TestConfiguration) []v1alpha3.TestConfiguration {
+	for i := range tests {
+		if len(tests[i].Storage.Spec.MountPath.Path) == 0 {
+			tests[i].Storage.Spec.MountPath.Path = o.Config.Storage.Spec.MountPath.Path
+		}
+	}
+	return tests
+}
+
 func (o Scorecard) runStageParallel(ctx context.Context, tests []v1alpha3.TestConfiguration, results chan<- v1alpha3.Test) {
 	var wg sync.WaitGroup
 	for _, t := range tests {
@@ -172,6 +185,7 @@ func (r *PodTestRunner) Initialize(ctx context.Context) error {
 	if err != nil {
 		return fmt.Errorf("error creating ConfigMap %w", err)
 	}
+
 	return nil
 
 }
@@ -187,6 +201,7 @@ func (r FakeTestRunner) Cleanup(ctx context.Context) error {
 
 // Cleanup deletes pods and configmap resources from this test run
 func (r PodTestRunner) Cleanup(ctx context.Context) (err error) {
+
 	err = r.deletePods(ctx, r.configMapName)
 	if err != nil {
 		return err
@@ -195,13 +210,20 @@ func (r PodTestRunner) Cleanup(ctx context.Context) (err error) {
 	if err != nil {
 		return err
 	}
+
 	return nil
 }
 
 // RunTest executes a single test
 func (r PodTestRunner) RunTest(ctx context.Context, test v1alpha3.TestConfiguration) (*v1alpha3.TestStatus, error) {
+
 	// Create a Pod to run the test
 	podDef := getPodDefinition(r.configMapName, test, r)
+
+	if test.Storage.Spec.MountPath.Path != "" {
+		addStorageToPod(podDef, test.Storage.Spec.MountPath.Path)
+	}
+
 	pod, err := r.Client.CoreV1().Pods(r.Namespace).Create(ctx, podDef, metav1.CreateOptions{})
 	if err != nil {
 		return nil, err
@@ -212,6 +234,14 @@ func (r PodTestRunner) RunTest(ctx context.Context, test v1alpha3.TestConfigurat
 		return nil, err
 	}
 
+	// gather test output if necessary
+	if test.Storage.Spec.MountPath.Path != "" {
+		err := gatherTestOutput(r, test.Labels["suite"], test.Labels["test"], pod.Name, test.Storage.Spec.MountPath.Path)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	return r.getTestStatus(ctx, pod), nil
 }
 
@@ -239,9 +269,14 @@ func (r PodTestRunner) waitForTestToComplete(ctx context.Context, p *v1.Pod) (er
 		if err != nil {
 			return true, fmt.Errorf("error getting pod %s %w", p.Name, err)
 		}
-		if tmp.Status.Phase == v1.PodSucceeded || tmp.Status.Phase == v1.PodFailed {
-			return true, nil
+		for _, s := range tmp.Status.ContainerStatuses {
+			if s.Name == "scorecard-test" {
+				if s.State.Terminated != nil {
+					return true, nil
+				}
+			}
 		}
+
 		return false, nil
 	})