From 7a3612f3c7e9b8003eb4a6b113d7225db983a998 Mon Sep 17 00:00:00 2001 From: Matty Evans Date: Wed, 15 Jan 2025 15:04:00 +1000 Subject: [PATCH] feat(docker): expose metrics ports only if metricsAddress is set --- .goreleaser.yaml | 1 + docker-compose.ports.yml | 4 ++ docker-compose.yml | 3 +- install.bats | 7 ++- install.sh | 6 ++ internal/sidecar/docker.go | 105 ++++++++++++++++++++++++++------ internal/sidecar/docker_test.go | 67 +++++++++++++------- 7 files changed, 148 insertions(+), 45 deletions(-) create mode 100644 docker-compose.ports.yml diff --git a/.goreleaser.yaml b/.goreleaser.yaml index 1c4552c..b94b6ad 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -30,6 +30,7 @@ archives: - LICENSE* - install.sh - docker-compose.yml + - docker-compose.ports.yml changelog: sort: asc diff --git a/docker-compose.ports.yml b/docker-compose.ports.yml new file mode 100644 index 0000000..dc2ccf7 --- /dev/null +++ b/docker-compose.ports.yml @@ -0,0 +1,4 @@ +services: + sentry: + ports: + - "${CONTRIBUTOOR_METRICS_ADDRESS:-127.0.0.1}:${CONTRIBUTOOR_METRICS_PORT:-9090}:${CONTRIBUTOOR_METRICS_PORT:-9090}" \ No newline at end of file diff --git a/docker-compose.yml b/docker-compose.yml index 14d5a57..13958f4 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,12 +1,11 @@ services: sentry: + container_name: contributoor image: ethpandaops/contributoor:${CONTRIBUTOOR_VERSION} entrypoint: ["/usr/local/bin/sentry"] command: ["--config=/config/config.yaml"] networks: - contributoor - ports: - - "${CONTRIBUTOOR_METRICS_ADDRESS:-127.0.0.1}:${CONTRIBUTOOR_METRICS_PORT:-9090}:${CONTRIBUTOOR_METRICS_PORT:-9090}" extra_hosts: - "host.docker.internal:host-gateway" volumes: diff --git a/install.bats b/install.bats index b86dcaa..a723ea0 100644 --- a/install.bats +++ b/install.bats @@ -395,8 +395,9 @@ EOF touch "$CONTRIBUTOOR_PATH/releases/installer-${INSTALLER_VERSION}/contributoor" chmod +x "$CONTRIBUTOOR_PATH/releases/installer-${INSTALLER_VERSION}/contributoor" - # Create docker-compose.yml + # Create compose files touch "$CONTRIBUTOOR_PATH/releases/installer-${INSTALLER_VERSION}/docker-compose.yml" + touch "$CONTRIBUTOOR_PATH/releases/installer-${INSTALLER_VERSION}/docker-compose.ports.yml" return 0 } @@ -408,9 +409,10 @@ EOF touch "$3" chmod +x "$3" - # Also create docker-compose.yml in the same directory if it's the binary symlink + # Also create compose files in the same directory if it's the binary symlink if [[ "$3" == *"/bin/contributoor" ]]; then cp "$CONTRIBUTOOR_PATH/releases/installer-${INSTALLER_VERSION}/docker-compose.yml" "$(dirname "$3")/docker-compose.yml" + cp "$CONTRIBUTOOR_PATH/releases/installer-${INSTALLER_VERSION}/docker-compose.ports.yml" "$(dirname "$3")/docker-compose.ports.yml" fi fi return 0 @@ -430,6 +432,7 @@ EOF [ -x "$CONTRIBUTOOR_PATH/bin/contributoor" ] [ -f "$CONTRIBUTOOR_PATH/releases/installer-${INSTALLER_VERSION}/docker-compose.yml" ] [ -f "$CONTRIBUTOOR_PATH/bin/docker-compose.yml" ] + [ -f "$CONTRIBUTOOR_PATH/releases/installer-${INSTALLER_VERSION}/docker-compose.ports.yml" ] } @test "setup_installer fails on checksum mismatch" { diff --git a/install.sh b/install.sh index 5edaf6c..eb41858 100755 --- a/install.sh +++ b/install.sh @@ -209,10 +209,16 @@ setup_installer() { success "Extracted archive" chmod +x "$release_dir/contributoor" + [ -f "$release_dir/docker-compose.yml" ] && { chmod 644 "$release_dir/docker-compose.yml" chmod 755 "$release_dir" } || fail "docker-compose.yml not found after extraction" + + [ -f "$release_dir/docker-compose.ports.yml" ] && { + chmod 644 "$release_dir/docker-compose.ports.yml" + chmod 755 "$release_dir" + } || fail "docker-compose.ports.yml not found after extraction" # Create/update symlink rm -f "$CONTRIBUTOOR_BIN/contributoor" # Remove existing symlink or file diff --git a/internal/sidecar/docker.go b/internal/sidecar/docker.go index 3224280..69be38a 100644 --- a/internal/sidecar/docker.go +++ b/internal/sidecar/docker.go @@ -20,11 +20,12 @@ type DockerSidecar interface { // dockerSidecar is a basic service for interacting with the docker container. type dockerSidecar struct { - logger *logrus.Logger - composePath string - configPath string - sidecarCfg ConfigManager - installerCfg *installer.Config + logger *logrus.Logger + composePath string + composePortsPath string + configPath string + sidecarCfg ConfigManager + installerCfg *installer.Config } // NewDockerSidecar creates a new DockerSidecar. @@ -34,23 +35,41 @@ func NewDockerSidecar(logger *logrus.Logger, sidecarCfg ConfigManager, installer return nil, fmt.Errorf("failed to find docker-compose.yml: %w", err) } + composePortsPath, err := findComposePortsFile() + if err != nil { + return nil, fmt.Errorf("failed to find docker-compose.ports.yml: %w", err) + } + if err := validateComposePath(composePath); err != nil { return nil, fmt.Errorf("invalid docker-compose file: %w", err) } + if err := validateComposePath(composePortsPath); err != nil { + return nil, fmt.Errorf("invalid docker-compose.ports file: %w", err) + } + return &dockerSidecar{ - logger: logger, - composePath: filepath.Clean(composePath), - configPath: sidecarCfg.GetConfigPath(), - sidecarCfg: sidecarCfg, - installerCfg: installerCfg, + logger: logger, + composePath: filepath.Clean(composePath), + composePortsPath: filepath.Clean(composePortsPath), + configPath: sidecarCfg.GetConfigPath(), + sidecarCfg: sidecarCfg, + installerCfg: installerCfg, }, nil } // Start starts the docker container using docker-compose. func (s *dockerSidecar) Start() error { - //nolint:gosec // validateComposePath() and filepath.Clean() in-use. - cmd := exec.Command("docker", "compose", "-f", s.composePath, "up", "-d", "--pull", "always") + // If metrics are enabled, append our ports.yml as an additional -f arg. + var additionalArgs []string + if metricsHost, _ := s.sidecarCfg.Get().GetMetricsHostPort(); metricsHost != "" { + additionalArgs = append(additionalArgs, "-f", s.composePortsPath) + } + + args := append([]string{"compose", "-f", s.composePath}, additionalArgs...) + args = append(args, "up", "-d", "--pull", "always") + + cmd := exec.Command("docker", args...) cmd.Env = s.getComposeEnv() if output, err := cmd.CombinedOutput(); err != nil { @@ -188,6 +207,53 @@ func findComposeFile() (string, error) { return "", fmt.Errorf("docker-compose.yml not found") } +// findComposeFile finds the docker-compose file based on the OS. +func findComposePortsFile() (string, error) { + // Get binary directory. + ex, err := os.Executable() + if err != nil { + return "", fmt.Errorf("could not get executable path: %w", err) + } + + binDir := filepath.Dir(ex) + + // Get the actual binary path (resolve symlink). + actualBin, err := filepath.EvalSymlinks(ex) + if err != nil { + return "", fmt.Errorf("could not resolve symlink: %w", err) + } + + releaseDir := filepath.Dir(actualBin) + + // First check release directory (next to actual binary). + composePath := filepath.Join(releaseDir, "docker-compose.ports.yml") + if _, e := os.Stat(composePath); e == nil { + return composePath, nil + } + + // Fallback to bin directory for backward compatibility. + if _, statErr := os.Stat(filepath.Join(binDir, "docker-compose.ports.yml")); statErr == nil { + return filepath.Join(binDir, "docker-compose.ports.yml"), nil + } + + // Try current directory. + cwd, err := os.Getwd() + if err != nil { + return "", fmt.Errorf("could not get working directory: %w", err) + } + + if _, err := os.Stat(filepath.Join(cwd, "docker-compose.ports.yml")); err == nil { + return filepath.Join(cwd, "docker-compose.ports.yml"), nil + } + + // Try repo root + if _, err := os.Stat(filepath.Join(cwd, "..", "..", "docker-compose.ports.yml")); err == nil { + return filepath.Join(cwd, "..", "..", "docker-compose.ports.yml"), nil + } + + return "", fmt.Errorf("docker-compose.ports.yml not found") +} + func validateComposePath(path string) error { // Check if path exists and is a regular file fi, err := os.Stat(path) @@ -223,13 +289,14 @@ func (s *dockerSidecar) getComposeEnv() []string { fmt.Sprintf("CONTRIBUTOOR_VERSION=%s", cfg.Version), ) - // Handle metrics address (always added). - metricsHost, metricsPort := cfg.GetMetricsHostPort() - env = append( - env, - fmt.Sprintf("CONTRIBUTOOR_METRICS_ADDRESS=%s", metricsHost), - fmt.Sprintf("CONTRIBUTOOR_METRICS_PORT=%s", metricsPort), - ) + // Handle metrics address (only added if set). + if metricsHost, metricsPort := cfg.GetMetricsHostPort(); metricsHost != "" { + env = append( + env, + fmt.Sprintf("CONTRIBUTOOR_METRICS_ADDRESS=%s", metricsHost), + fmt.Sprintf("CONTRIBUTOOR_METRICS_PORT=%s", metricsPort), + ) + } // Handle pprof address (only added if set). if pprofHost, pprofPort := cfg.GetPprofHostPort(); pprofHost != "" { diff --git a/internal/sidecar/docker_test.go b/internal/sidecar/docker_test.go index bb7654d..c832aaf 100644 --- a/internal/sidecar/docker_test.go +++ b/internal/sidecar/docker_test.go @@ -33,6 +33,13 @@ services: start_period: 1s ` +const composePortsFile = ` +services: + test: + ports: + - "9090:9090" +` + // TestDockerService_Integration tests the docker sidecar. // We use test-containers to boot an instance of docker-in-docker. // We can then use this to test our docker service in isolation. @@ -96,57 +103,73 @@ func TestDockerService_Integration(t *testing.T) { containerPort, err := container.MappedPort(ctx, nat.Port(fmt.Sprintf("%d/tcp", port))) require.NoError(t, err) - // Create docker service with mock config + // Create docker service with mock config. ds, err := sidecar.NewDockerSidecar(logger, mockSidecarConfig, mockInstallerConfig) require.NoError(t, err) - // Set docker host to test container. + // Set docker host to test container t.Setenv("DOCKER_HOST", fmt.Sprintf("tcp://localhost:%s", containerPort.Port())) - - // Write out dummy compose file. - require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "docker-compose.yml"), []byte(composeFile), 0644)) - - // Change working directory to our test directory so findComposeFile finds our test file. - require.NoError(t, os.Chdir(tmpDir)) - t.Setenv("CONTRIBUTOOR_CONFIG_PATH", tmpDir) - // Run our tests in a real container. - t.Run("lifecycle", func(t *testing.T) { - // Ensure Start() executes as expected. - require.NoError(t, ds.Start()) + // Change working directory to our test directory. + require.NoError(t, os.Chdir(tmpDir)) - // Wait for the container to be healthy. + // Helper function for container health check. + checkContainerHealth := func(t *testing.T) { + t.Helper() ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() for { select { case <-ctx.Done(): - // If we timeout, log the container logs so we get some idea of what went wrong. logs, err := container.Logs(context.Background()) if err == nil { t.Logf("docker-in-docker container logs:\n%s", logs) } - t.Fatal("timeout waiting for docker-in-docker container to become healthy") default: - // Check if the container is running. running, err := ds.IsRunning() require.NoError(t, err) - if running { - goto containerRunning + return } - time.Sleep(time.Second) } } + } + + t.Run("lifecycle_without_metrics", func(t *testing.T) { + // Write out compose file. + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "docker-compose.yml"), []byte(composeFile), 0644)) + + require.NoError(t, ds.Start()) + checkContainerHealth(t) - containerRunning: - // Stop container and verify it's not running anymore. require.NoError(t, ds.Stop()) + running, err := ds.IsRunning() + require.NoError(t, err) + require.False(t, running) + }) + t.Run("lifecycle_with_metrics", func(t *testing.T) { + cfgWithMetrics := &config.Config{ + Version: "latest", + ContributoorDirectory: tmpDir, + RunMethod: config.RunMethod_RUN_METHOD_DOCKER, + MetricsAddress: "0.0.0.0:9090", + } + + mockSidecarConfig.EXPECT().Get().Return(cfgWithMetrics).AnyTimes() + + // Write out compose files. + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "docker-compose.yml"), []byte(composeFile), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "docker-compose.ports.yml"), []byte(composePortsFile), 0644)) + + require.NoError(t, ds.Start()) + checkContainerHealth(t) + + require.NoError(t, ds.Stop()) running, err := ds.IsRunning() require.NoError(t, err) require.False(t, running)