[cmd/opampsupervisor] Fix merging lists when using local config (open-telemetry#39947)

Author: evan-bradley

.chloggen/opampsupervisor-fix-local-config.yaml

@@ -0,0 +1,27 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
+component: cmd/opampsupervisor
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Fix slice merging when using local configuration files
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [39947]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# If your change doesn't affect end users or the exported elements of any package,
+# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: []

cmd/opampsupervisor/e2e_test.go

@@ -23,6 +23,7 @@ import (
 	"path"
 	"path/filepath"
 	"runtime"
+	"strconv"
 	"strings"
 	"sync/atomic"
 	"testing"
@@ -431,33 +432,38 @@ func TestSupervisorStartsCollectorWithRemoteConfigAndExecParams(t *testing.T) {
 	require.NoError(t, err)
 	t.Cleanup(func() { outputFile.Close() })
 
+	secondHealthcheckPort, err := findRandomPort()
+	require.NoError(t, err)
+
 	// fill env variables passed via parameters which are used in the collector config passed via config_files param
 	s := newSupervisor(t, "exec_config", map[string]string{
-		"url":           server.addr,
-		"storage_dir":   storageDir,
-		"inputLogFile":  inputFile.Name(),
-		"outputLogFile": outputFile.Name(),
+		"url":             server.addr,
+		"storage_dir":     storageDir,
+		"inputLogFile":    inputFile.Name(),
+		"outputLogFile":   outputFile.Name(),
+		"healthcheckPort": strconv.Itoa(secondHealthcheckPort),
 	})
 
 	require.Nil(t, s.Start())
 	defer s.Shutdown()
 
 	waitForSupervisorConnection(server.supervisorConnected, true)
 
-	// check health
-	require.Eventually(t, func() bool {
-		resp, err := http.DefaultClient.Get(fmt.Sprintf("http://localhost:%d", healthcheckPort))
-		if err != nil {
-			t.Logf("Failed healthcheck: %s", err)
-			return false
-		}
-		require.NoError(t, resp.Body.Close())
-		if resp.StatusCode >= 300 || resp.StatusCode < 200 {
-			t.Logf("Got non-2xx status code: %d", resp.StatusCode)
-			return false
-		}
-		return true
-	}, 3*time.Second, 100*time.Millisecond)
+	for _, port := range []int{healthcheckPort, secondHealthcheckPort} {
+		require.Eventually(t, func() bool {
+			resp, err := http.DefaultClient.Get(fmt.Sprintf("http://localhost:%d", port))
+			if err != nil {
+				t.Logf("Failed healthcheck: %s", err)
+				return false
+			}
+			require.NoError(t, resp.Body.Close())
+			if resp.StatusCode >= 300 || resp.StatusCode < 200 {
+				t.Logf("Got non-2xx status code: %d", resp.StatusCode)
+				return false
+			}
+			return true
+		}, 3*time.Second, 100*time.Millisecond)
+	}
 
 	// check that collector uses filelog receiver and file exporter from config passed via config_files param
 	n, err := inputFile.WriteString("{\"body\":\"hello, world\"}\n")

cmd/opampsupervisor/supervisor/commander/commander.go

@@ -11,6 +11,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"slices"
 	"sync/atomic"
 	"time"
 
@@ -72,12 +73,9 @@ func (c *Commander) Start(ctx context.Context) error {
 	}
 	c.logger.Debug("Starting agent", zap.String("agent", c.cfg.Executable))
 
-	if err := c.buildConfigs(); err != nil {
-		return err
-	}
-	c.args = append(c.args, c.cfg.Arguments...)
+	args := slices.Concat(c.args, c.cfg.Arguments)
 
-	c.cmd = exec.CommandContext(ctx, c.cfg.Executable, c.args...) // #nosec G204
+	c.cmd = exec.CommandContext(ctx, c.cfg.Executable, args...) // #nosec G204
 	c.cmd.Env = common.EnvVarMapToEnvMapSlice(c.cfg.Env)
 	c.cmd.SysProcAttr = sysProcAttrs()
 
@@ -88,19 +86,6 @@ func (c *Commander) Start(ctx context.Context) error {
 	return c.startNormal()
 }
 
-func (c *Commander) buildConfigs() error {
-	for _, conf := range c.cfg.ConfigFiles {
-		fileName := filepath.Base(conf)
-		newPath := filepath.Join(c.logsDir, fileName)
-		if err := common.CopyFile(conf, newPath); err != nil {
-			return fmt.Errorf("cannot copy config file '%s' to storage directory: %s", conf, err.Error())
-		}
-		c.args = append(c.args, "--config")
-		c.args = append(c.args, newPath)
-	}
-	return nil
-}
-
 func (c *Commander) Restart(ctx context.Context) error {
 	c.logger.Debug("Restarting agent", zap.String("agent", c.cfg.Executable))
 	if err := c.Stop(ctx); err != nil {

cmd/opampsupervisor/testdata/collector/simple_pipeline_env.yaml

@@ -8,7 +8,12 @@ exporters:
   file:
     path: ${OUT_FILE}
 
+extensions:
+  health_check/e2e:
+    endpoint: "localhost:${HEALTHCHECK_PORT}"
+
 service:
+  extensions: [health_check/e2e]
   pipelines:
     logs:
       receivers: [filelog]

cmd/opampsupervisor/testdata/supervisor/supervisor_exec_config.yaml

@@ -19,3 +19,4 @@ agent:
   env:
     OUT_FILE: '{{.outputLogFile}}'
     IN_FILE: '{{.inputLogFile}}'
+    HEALTHCHECK_PORT: '{{.healthcheckPort}}'