Eliminate TESTBED_CONFIG in favor of tests specifying the executable …

…path

Contributes to open-telemetry/opentelemetry-collector-contrib#873

We had the concept of a testbed config file that was specified via TESTBED_CONFIG
env variable. The idea was that the config file would contain definitions for all
tests to use, including the location of the executable to test.

Time has shown that we never used anything other than the local.yaml config
with one specific location of the executable.

We now have the need to allow tests to individually specify different executable.
This is needed to run certain tests against the "unstable" executable that contains
unstable features (such as Log receivers in the contrib repo).

This change eliminates the concept of testbed config and TESTBED_CONFIG env variable.
Instead the location of the executable is defined in the TestCase with the default
location pointing to the regular "../../bin/otelcol_{{.GOOS}}_{{.GOARCH}}" executable.

Individual tests can now override this by setting ChildProcess.AgentExePath to something
else if needed. We will use this capability in the contrib testbed.

testbed/correctness/traces/runtests.sh

@@ -23,7 +23,7 @@ FAIL_COLOR=$(printf "\033[31mFAIL\033[0m")
 TEST_COLORIZE="${SED} 's/PASS/${PASS_COLOR}/' | ${SED} 's/FAIL/${FAIL_COLOR}/'"
 echo ${TEST_ARGS}
 mkdir -p results
-TESTBED_CONFIG=inprocess.yaml go test -v ${TEST_ARGS} 2>&1 | tee results/testoutput.log | bash -c "${TEST_COLORIZE}"
+go test -v ${TEST_ARGS} 2>&1 | tee results/testoutput.log | bash -c "${TEST_COLORIZE}"
 
 testStatus=${PIPESTATUS[0]}
 

testbed/testbed/child_process.go

@@ -15,6 +15,7 @@
 package testbed
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
 	"io"
@@ -24,8 +25,10 @@ import (
 	"os/exec"
 	"path"
 	"path/filepath"
+	"runtime"
 	"sync"
 	"syscall"
+	"text/template"
 	"time"
 
 	"github.com/shirou/gopsutil/cpu"
@@ -61,6 +64,11 @@ func (rs *ResourceSpec) isSpecified() bool {
 // ChildProcess implements the OtelcolRunner interface as a child process on the same machine executing
 // the test. The process can be monitored and the output of which will be written to a log file.
 type ChildProcess struct {
+	// Path to agent executable. If unset the default executable in
+	// bin/otelcol_{{.GOOS}}_{{.GOARCH}} will be used.
+	// Can be set for example to use the unstable executable for a specific test.
+	AgentExePath string
+
 	// Descriptive name of the process
 	name string
 
@@ -116,7 +124,6 @@ type ChildProcess struct {
 type StartParams struct {
 	Name         string
 	LogFilePath  string
-	Cmd          string
 	CmdArgs      []string
 	resourceSpec *ResourceSpec
 }
@@ -157,21 +164,57 @@ func (cp *ChildProcess) PrepareConfig(configStr string) (configCleanup func(), e
 	return configCleanup, err
 }
 
+func expandExeFileName(exeName string) string {
+	cfgTemplate, err := template.New("").Parse(exeName)
+	if err != nil {
+		log.Fatalf("Template failed to parse exe name %q: %s",
+			exeName, err.Error())
+	}
+
+	templateVars := struct {
+		GOOS   string
+		GOARCH string
+	}{
+		GOOS:   runtime.GOOS,
+		GOARCH: runtime.GOARCH,
+	}
+	var buf bytes.Buffer
+	if err = cfgTemplate.Execute(&buf, templateVars); err != nil {
+		log.Fatalf("Configuration template failed to run on exe name %q: %s",
+			exeName, err.Error())
+	}
+
+	return string(buf.Bytes())
+}
+
 // start a child process.
 //
+// cp.AgentExePath defines the executable to run. If unspecified
+// "../../bin/otelcol_{{.GOOS}}_{{.GOARCH}}" will be used.
+// {{.GOOS}} and {{.GOARCH}} will be expanded to the current OS and ARCH correspondingly.
+//
 // Parameters:
 // name is the human readable name of the process (e.g. "Agent"), used for logging.
 // logFilePath is the file path to write the standard output and standard error of
 // the process to.
-// cmd is the executable to run.
 // cmdArgs is the command line arguments to pass to the process.
 func (cp *ChildProcess) Start(params StartParams) error {
 
 	cp.name = params.Name
 	cp.doneSignal = make(chan struct{})
 	cp.resourceSpec = params.resourceSpec
 
-	log.Printf("Starting %s (%s)", cp.name, params.Cmd)
+	if cp.AgentExePath == "" {
+		var err error
+		cp.AgentExePath = "../../bin/otelcol_{{.GOOS}}_{{.GOARCH}}"
+		if err != nil {
+			return err
+		}
+	}
+	exePath := expandExeFileName(cp.AgentExePath)
+	exePath, err := filepath.Abs(exePath)
+
+	log.Printf("Starting %s (%s)", cp.name, exePath)
 
 	// Prepare log file
 	logFile, err := os.Create(params.LogFilePath)
@@ -194,21 +237,21 @@ func (cp *ChildProcess) Start(params StartParams) error {
 		args = append(args, "--config")
 		args = append(args, cp.configFileName)
 	}
-	cp.cmd = exec.Command(params.Cmd, args...)
+	cp.cmd = exec.Command(exePath, args...)
 
 	// Capture standard output and standard error.
 	stdoutIn, err := cp.cmd.StdoutPipe()
 	if err != nil {
-		return fmt.Errorf("cannot capture stdout of %s: %s", params.Cmd, err.Error())
+		return fmt.Errorf("cannot capture stdout of %s: %s", exePath, err.Error())
 	}
 	stderrIn, err := cp.cmd.StderrPipe()
 	if err != nil {
-		return fmt.Errorf("cannot capture stderr of %s: %s", params.Cmd, err.Error())
+		return fmt.Errorf("cannot capture stderr of %s: %s", exePath, err.Error())
 	}
 
 	// Start the process.
 	if err = cp.cmd.Start(); err != nil {
-		return fmt.Errorf("cannot start executable at %s: %s", params.Cmd, err.Error())
+		return fmt.Errorf("cannot start executable at %s: %s", exePath, err.Error())
 	}
 
 	cp.startTime = time.Now()

testbed/testbed/test_bed.go

@@ -25,111 +25,13 @@
 package testbed
 
 import (
-	"bytes"
-	"errors"
 	"log"
 	"os"
-	"path"
 	"path/filepath"
-	"runtime"
 	"testing"
-	"text/template"
-
-	"go.opentelemetry.io/collector/config"
 )
 
-// GlobalConfig defines test bed configuration.
-type GlobalConfig struct {
-	Agent         string
-	LoadGenerator string `mapstructure:"load-generator"`
-}
-
-var testBedConfig = GlobalConfig{}
-
-const testBedConfigEnvVarName = "TESTBED_CONFIG"
-
-// ErrSkipTests indicates that the tests must be skipped.
-var ErrSkipTests = errors.New("skip tests")
-
-// LoadConfig loads test bed config.
-func LoadConfig() error {
-	// Get the test bed config file location from env variable.
-	testBedConfigFile := os.Getenv(testBedConfigEnvVarName)
-	if testBedConfigFile == "" {
-		log.Printf(testBedConfigEnvVarName + " is not defined, skipping E2E tests.")
-		return ErrSkipTests
-	}
-
-	testBedConfigFile, err := filepath.Abs(testBedConfigFile)
-	if err != nil {
-		log.Fatalf("Cannot resolve file name %q: %s",
-			testBedConfigFile, err.Error())
-	}
-
-	testBedConfigDir := path.Dir(testBedConfigFile)
-
-	// Use templates to expand some selected content on the config file.
-	cfgTemplate, err := template.ParseFiles(testBedConfigFile)
-	if err != nil {
-		log.Fatalf("Template failed to parse config file %q: %s",
-			testBedConfigFile, err.Error())
-	}
-
-	templateVars := struct {
-		GOOS   string
-		GOARCH string
-	}{
-		GOOS:   runtime.GOOS,
-		GOARCH: runtime.GOARCH,
-	}
-	var buf bytes.Buffer
-	if err = cfgTemplate.Execute(&buf, templateVars); err != nil {
-		log.Fatalf("Configuration template failed to run on file %q: %s",
-			testBedConfigFile, err.Error())
-	}
-
-	// Read the config.
-	v := config.NewViper()
-	v.SetConfigType("yaml")
-	if err = v.ReadConfig(bytes.NewBuffer(buf.Bytes())); err != nil {
-		log.Fatalf("Cannot load test bed config from %q: %s",
-			testBedConfigFile, err.Error())
-	}
-
-	if err = v.UnmarshalExact(&testBedConfig); err != nil {
-		log.Fatalf("Cannot load test bed config from %q: %s",
-			testBedConfigFile, err.Error())
-	}
-
-	// Convert relative paths to absolute.
-	testBedConfig.Agent, err = filepath.Abs(path.Join(testBedConfigDir, testBedConfig.Agent))
-	if err != nil {
-		log.Fatalf("Cannot resolve file name %q: %s",
-			testBedConfig.Agent, err.Error())
-	}
-
-	testBedConfig.LoadGenerator, err = filepath.Abs(path.Join(testBedConfig.LoadGenerator))
-	if err != nil {
-		log.Fatalf("Cannot resolve file name %q: %s",
-			testBedConfig.LoadGenerator, err.Error())
-	}
-
-	return nil
-}
-
 func Start(resultsSummary TestResultsSummary) error {
-	// Load the test bed config first.
-	err := LoadConfig()
-
-	if err != nil {
-		if err == ErrSkipTests {
-			// Let the caller know all tests must be skipped.
-			return err
-		}
-		// Any other error while loading the config is fatal.
-		log.Fatalf(err.Error())
-	}
-
 	dir, err := filepath.Abs("results")
 	if err != nil {
 		log.Fatalf(err.Error())
@@ -149,8 +51,7 @@ func DoTestMain(m *testing.M, resultsSummary TestResultsSummary) {
 	// Load the test bed config first.
 	err := Start(resultsSummary)
 
-	if err == ErrSkipTests {
-		// Test bed config is not loaded because the tests are globally skipped.
+	if err != nil {
 		os.Exit(0)
 	}
 

testbed/testbed/test_case.go

@@ -171,7 +171,6 @@ func (tc *TestCase) StartAgent(args ...string) {
 	err := tc.agentProc.Start(StartParams{
 		Name:         "Agent",
 		LogFilePath:  logFileName,
-		Cmd:          testBedConfig.Agent,
 		CmdArgs:      args,
 		resourceSpec: &tc.resourceSpec,
 	})

testbed/tests/runtests.sh

@@ -22,7 +22,7 @@ PASS_COLOR=$(printf "\033[32mPASS\033[0m")
 FAIL_COLOR=$(printf "\033[31mFAIL\033[0m")
 TEST_COLORIZE="${SED} 's/PASS/${PASS_COLOR}/' | ${SED} 's/FAIL/${FAIL_COLOR}/'"
 echo ${TEST_ARGS}
-TESTBED_CONFIG=local.yaml go test -v ${TEST_ARGS} 2>&1 | tee results/testoutput.log | bash -c "${TEST_COLORIZE}"
+go test -v ${TEST_ARGS} 2>&1 | tee results/testoutput.log | bash -c "${TEST_COLORIZE}"
 
 testStatus=${PIPESTATUS[0]}