kola: Replace run-ext-bin with kola run -E/--exttest

As I tried to port the ostree tests, I hit various issues
with the `run-ext-bin` model; for example, `kola run` already
supports `--parallel` and cleanly separates tests into
output directories etc.

This reworks things so that one can do e.g.:

`cosa kola run -- --parallel 4 --output-dir tmp/kola -E ~/src/github/ostreedev/ostree/tests/installed/nondestructive/ 'ext.nondestructive.*'`

to run the ostree test suite.  Or of course, run *both* the
ostree exttests plus builtin tests `basic` or whatever.

And another thing I'd like to do soon is teach cosa how to
automatically add `src/config/tests` so that one can seamlessly
e.g. add a change to the manifest and also add a test for it
as one atomic commit.

There's still a lot more to do here; among other things ostree
cleanly separates destructive/nondestructive tests and having
each test run as a new VM is a hit.  But this is a good start
I think.

mantle/cmd/kola/kola.go

@@ -84,22 +84,6 @@ will be ignored.
 		SilenceUsage: true,
 	}
 
-	cmdRunExtBin = &cobra.Command{
-		Use:   "run-ext-bin",
-		Short: "Run an external test (single binary)",
-		Long: `Run an externally defined test
-
-This injects a single binary into the target system and executes it as a systemd
-unit.  The test is considered successful when the service exits normally, and
-failed if the test exits non-zero - but the service being killed by e.g. SIGTERM
-is ignored.  This is intended to allow rebooting the system.
-`,
-
-		Args:    cobra.ExactArgs(1),
-		PreRunE: preRun,
-		RunE:    runRunExtBin,
-	}
-
 	cmdHttpServer = &cobra.Command{
 		Use:   "http-server",
 		Short: "Run a static webserver",
@@ -140,14 +124,15 @@ This can be useful for e.g. serving locally built OSTree repos to qemu.
 	qemuImageDirIsTemp bool
 
 	extDependencyDir string
+	runExternals     []string
 )
 
 func init() {
 	root.AddCommand(cmdRun)
-	root.AddCommand(cmdRunExtBin)
-	cmdRunExtBin.Flags().StringVar(&extDependencyDir, "depdir", "", "Copy (rsync) dir to target, available as ${KOLA_EXT_DATA}")
+	cmdRun.Flags().StringArrayVarP(&runExternals, "exttest", "E", nil, "Externally defined tests (will be found in DIR/tests/kola)")
 
 	root.AddCommand(cmdList)
+	cmdList.Flags().StringArrayVarP(&runExternals, "exttest", "E", nil, "Externally defined tests in directory")
 	cmdList.Flags().BoolVar(&listJSON, "json", false, "format output in JSON")
 	cmdList.Flags().StringVarP(&listPlatform, "platform", "p", "all", "filter output by platform")
 	cmdList.Flags().StringVarP(&listDistro, "distro", "b", "all", "filter output by distro")
@@ -200,6 +185,13 @@ func runRun(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
+	for _, d := range runExternals {
+		err := kola.RegisterExternalTests(d)
+		if err != nil {
+			return err
+		}
+	}
+
 	runErr := kola.RunTests(patterns, kolaPlatform, outputDir, !kola.Options.NoTestExitError)
 
 	// needs to be after RunTests() because harness empties the directory
@@ -210,22 +202,6 @@ func runRun(cmd *cobra.Command, args []string) error {
 	return runErr
 }
 
-func runRunExtBin(cmd *cobra.Command, args []string) error {
-	extbin := args[0]
-
-	outputDir, err := kola.SetupOutputDir(outputDir, kolaPlatform)
-	if err != nil {
-		return err
-	}
-
-	runErr := kola.RunExtBin(kolaPlatform, outputDir, extbin, extDependencyDir)
-	if err := writeProps(); err != nil {
-		return errors.Wrapf(err, "writing properties")
-	}
-
-	return runErr
-}
-
 func writeProps() error {
 	f, err := os.OpenFile(filepath.Join(outputDir, "properties.json"), os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0644)
 	if err != nil {
@@ -348,6 +324,12 @@ func writeProps() error {
 }
 
 func runList(cmd *cobra.Command, args []string) error {
+	for _, d := range runExternals {
+		err := kola.RegisterExternalTests(d)
+		if err != nil {
+			return err
+		}
+	}
 	var testlist []*item
 	for name, test := range register.Tests {
 		item := &item{

mantle/kola/README-kola-ext.md

@@ -10,28 +10,46 @@ repositories, and allow these projects to target Fedora CoreOS
 in e.g. their own CI.  And we also want to run unmodified
 upstream tests, *without rebuilding* the project.
 
-Using kola run-ext-bin
+Using kola run -E/--exttest
 ===
 
-The `kola run-ext-bin` is one way to accomplish this.  The
-core idea is to express your test suite as a binary (plus an optional
-directory of dependencies) that will be uploaded to a CoreOS
-system and run as a systemd unit.
+The `--exttest` (`-E`) argument to `kola run` one way to accomplish this; you
+provide the path to an upstream project git repository.  Tests will be found
+in the `tests/kola` directory.  If this project contains binaries that require
+building, it is assumed that `make` (or equivalent) has already been invoked.
 
-Currently this systemd unit runs with full privileges - tests
+The `tests/kola` directory will be traversed recursively to find tests.
+
+The core idea is to express a test as a single binary (plus an optional
+directory of dependencies named `deps`) that will be uploaded to a CoreOS
+system and run as a systemd unit, along with an optional Ignition config
+named `config.ign`.
+
+Concretely then, an external test directory can have the following content:
+
+- `config.ign` (optional): Ignition config provided
+- `deps` (optional): Directory (or symlink to dir): Will be uploaded and available as `${KOLA_EXT_DATA}`
+- one or more executables: Each executable is its own test, run independently
+  with the Ignition config and/or dependencies provided.
+
+In the case of a test directory with a single executable, the kola test name will be
+`ext.<projname>.<subdirectory>`.  Otherwise, the test will be named `ext.<projname>.<subdirectory>.<executable>`.
+
+Currently the test systemd unit runs with full privileges - tests
 are assumed to be (potentially) destructive and a general assumption
-is tests are run in easily disposable virtual machines.
+is tests are run in easily disposable virtual machines.  A future
+enhancement will support nondestructive tests.
 
 A best practice for doing this is to write your tests in a language
 that compiles to a single binary - Rust and Go for example, but
 there exist for example tools like [PyInstaller](https://realpython.com/pyinstaller-python/#pyinstaller)
-too.
+too.  This way you can usually avoid the need for a `deps` directory.
 
 This mechanism is suitable for testing most userspace components
 of CoreOS; for example, one can have the binary drive a container runtime.
 
-An important feature of `run-ext-bin` is support for rebooting the host system.
-This allows one to easily test OS updates.  To do this, simply invoke the usual
+An important feature of exttests is support for rebooting the host system.
+This allows one to easily test OS updates for example.  To do this, simply invoke the usual
 `reboot` - the test framework will monitor the target systemd unit
 and ignore the case where it exits with `SIGTERM`.
 

mantle/kola/harness.go

@@ -15,7 +15,9 @@
 package kola
 
 import (
+	"encoding/json"
 	"fmt"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 	"regexp"
@@ -24,8 +26,11 @@ import (
 
 	"github.com/coreos/go-semver/semver"
 	"github.com/coreos/pkg/capnslog"
+	"github.com/kballard/go-shellquote"
 	"github.com/pkg/errors"
 
+	ignv3 "github.com/coreos/ignition/v2/config/v3_0"
+	ignv3types "github.com/coreos/ignition/v2/config/v3_0/types"
 	"github.com/coreos/mantle/cosa"
 	"github.com/coreos/mantle/harness"
 	"github.com/coreos/mantle/harness/reporters"
@@ -49,6 +54,7 @@ import (
 	"github.com/coreos/mantle/platform/machine/packet"
 	"github.com/coreos/mantle/platform/machine/unprivqemu"
 	"github.com/coreos/mantle/system"
+	"github.com/coreos/mantle/util"
 )
 
 var (
@@ -152,6 +158,12 @@ var (
 	}
 )
 
+// kolaExtBinDataDir is where data will be stored
+const kolaExtBinDataDir = "/var/opt/kola/extdata"
+
+// kolaExtBinDataEnv is an environment variable pointing to the above
+const kolaExtBinDataEnv = "KOLA_EXT_DATA"
+
 // NativeRunner is a closure passed to all kola test functions and used
 // to run native go functions directly on kola machines. It is necessary
 // glue until kola does introspection.
@@ -436,6 +448,153 @@ func RunUpgradeTests(patterns []string, pltfrm, outputDir string, propagateTestE
 	return runProvidedTests(register.UpgradeTests, patterns, pltfrm, outputDir, propagateTestErrors)
 }
 
+func registerExternalTest(testname, executable, dependencydir, ignition string) error {
+	ignc3, _, err := ignv3.Parse([]byte(ignition))
+	if err != nil {
+		return errors.Wrapf(err, "Parsing config.ign")
+	}
+
+	unitname := "kola-runext.service"
+	remotepath := fmt.Sprintf("/usr/local/bin/kola-runext-%s", filepath.Base(executable))
+	unit := fmt.Sprintf(`[Unit]
+[Service]
+Type=oneshot
+RemainAfterExit=yes
+Environment=%s=%s
+ExecStart=%s
+[Install]
+RequiredBy=multi-user.target
+`, kolaExtBinDataEnv, kolaExtBinDataDir, remotepath)
+	runextconfig := ignv3types.Config{
+		Ignition: ignv3types.Ignition{
+			Version: "3.0.0",
+		},
+		Systemd: ignv3types.Systemd{
+			Units: []ignv3types.Unit{
+				{
+					Name:     unitname,
+					Contents: &unit,
+					Enabled:  util.BoolToPtr(false),
+				},
+			},
+		},
+	}
+
+	finalIgn := ignv3.Merge(ignc3, runextconfig)
+	serializedIgn, err := json.Marshal(finalIgn)
+	if err != nil {
+		return errors.Wrapf(err, "serializing ignition")
+	}
+
+	register.RegisterTest(&register.Test{
+		Name:          testname,
+		ClusterSize:   1, // Hardcoded for now
+		ExternalTest:  executable,
+		DependencyDir: dependencydir,
+
+		Run: func(c cluster.TestCluster) {
+			mach := c.Machines()[0]
+			plog.Debugf("Running kolet")
+			_, stderr, err := mach.SSH(fmt.Sprintf("sudo ./kolet run-test-unit %s", shellquote.Join(unitname)))
+			out, _, suberr := mach.SSH(fmt.Sprintf("sudo systemctl status %s", shellquote.Join(unitname)))
+			if suberr != nil {
+				fmt.Printf("systemctl status %s:\n%s\n", unitname, string(out))
+			}
+			if err != nil {
+				if Options.SSHOnTestFailure {
+					plog.Errorf("dropping to shell: kolet failed: %v: %s", err, stderr)
+					platform.Manhole(mach)
+				} else {
+					c.Fatalf(errors.Wrapf(err, "kolet failed: %s", stderr).Error())
+				}
+			}
+		},
+
+		UserDataV3: conf.Ignition(string(serializedIgn)),
+	})
+
+	return nil
+}
+
+// registerTestDir parses one test directory and registers it as a test
+func registerTestDir(dir, testprefix string, children []os.FileInfo) error {
+	var dependencydir string
+	ignition := `{ "ignition": { "version": "3.0.0" } }`
+	executables := []string{}
+
+	for _, c := range children {
+		isreg := c.Mode().IsRegular()
+		if isreg && (c.Mode().Perm()&0001) > 0 {
+			executables = append(executables, filepath.Join(dir, c.Name()))
+		} else if isreg && c.Name() == "config.ign" {
+			v, err := ioutil.ReadFile(filepath.Join(dir, c.Name()))
+			if err != nil {
+				return errors.Wrapf(err, "reading %s", c.Name())
+			}
+			ignition = string(v)
+		} else if c.IsDir() && c.Name() == "deps" {
+			dependencydir = filepath.Join(dir, c.Name())
+		} else if c.Mode()&os.ModeSymlink != 0 && c.Name() == "deps" {
+			target, err := filepath.EvalSymlinks(filepath.Join(dir, c.Name()))
+			if err != nil {
+				return err
+			}
+			dependencydir = target
+		} else if c.IsDir() {
+			subdir := filepath.Join(dir, c.Name())
+			subchildren, err := ioutil.ReadDir(subdir)
+			if err != nil {
+				return err
+			}
+			subprefix := fmt.Sprintf("%s.%s", testprefix, c.Name())
+			if err := registerTestDir(subdir, subprefix, subchildren); err != nil {
+				return err
+			}
+		}
+	}
+
+	if len(executables) == 1 {
+		err := registerExternalTest(testprefix, executables[0], dependencydir, ignition)
+		if err != nil {
+			return err
+		}
+	} else {
+		for _, executable := range executables {
+			testname := fmt.Sprintf("%s.%s", testprefix, filepath.Base(executable))
+			err := registerExternalTest(testname, executable, dependencydir, ignition)
+			if err != nil {
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
+// RegisterExternalTests iterates over a directory, and finds subdirectories
+// that have exactly one executable binary.
+func RegisterExternalTests(dir string) error {
+	// eval symlinks to turn e.g. src/config into fedora-coreos-config
+	// for the test basename.
+	realdir, err := filepath.EvalSymlinks(dir)
+	if err != nil {
+		return err
+	}
+	basename := fmt.Sprintf("ext.%s", filepath.Base(realdir))
+
+	testsdir := filepath.Join(dir, "tests/kola")
+	children, err := ioutil.ReadDir(testsdir)
+	if err != nil {
+		return errors.Wrapf(err, "reading %s", dir)
+	}
+
+	if err := registerTestDir(testsdir, basename, children); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 // getClusterSemVer returns the CoreOS semantic version via starting a
 // machine and checking
 func getClusterSemver(flight platform.Flight, outputDir string) (*semver.Version, error) {
@@ -543,12 +702,34 @@ func runTest(h *harness.H, t *register.Test, pltfrm string, flight platform.Flig
 	}
 
 	// drop kolet binary on machines
-	if t.NativeFuncs != nil {
+	if t.ExternalTest != "" || t.NativeFuncs != nil {
 		if err := scpKolet(tcluster.Machines(), architecture(pltfrm)); err != nil {
 			h.Fatal(err)
 		}
 	}
 
+	if t.ExternalTest != "" {
+		in, err := os.Open(t.ExternalTest)
+		if err != nil {
+			h.Fatal(err)
+		}
+		defer in.Close()
+		for _, mach := range tcluster.Machines() {
+			remotepath := fmt.Sprintf("/usr/local/bin/kola-runext-%s", filepath.Base(t.ExternalTest))
+			if err := platform.InstallFile(in, mach, remotepath); err != nil {
+				h.Fatal(errors.Wrapf(err, "uploading %s", t.ExternalTest))
+			}
+		}
+	}
+
+	if t.DependencyDir != "" {
+		for _, mach := range tcluster.Machines() {
+			if err := platform.CopyDirToMachine(t.DependencyDir, mach, kolaExtBinDataDir); err != nil {
+				h.Fatal(errors.Wrapf(err, "copying dependencies %s to %s", t.DependencyDir, mach.ID()))
+			}
+		}
+	}
+
 	defer func() {
 		// give some time for the remote journal to be flushed so it can be read
 		// before we run the deferred machine destruction

mantle/kola/register/register.go

@@ -52,6 +52,7 @@ type Test struct {
 	Name                 string // should be unique
 	Run                  func(cluster.TestCluster)
 	NativeFuncs          map[string]NativeFuncWrap
+	ExternalTest         string
 	UserData             *conf.UserData
 	UserDataV3           *conf.UserData
 	ClusterSize          int
@@ -62,6 +63,7 @@ type Test struct {
 	Architectures        []string // whitelist of machine architectures supported -- defaults to all
 	ExcludeArchitectures []string // blacklist of architectures to ignore -- defaults to none
 	Flags                []Flag   // special-case options for this test
+	DependencyDir        string
 
 	// FailFast skips any sub-test that occurs after a sub-test has
 	// failed.