From d29539919f8bd4d74bec0cd3e3a43c44237eab06 Mon Sep 17 00:00:00 2001 From: Matty Evans Date: Fri, 17 Jan 2025 10:44:37 +1000 Subject: [PATCH 1/6] refactor(build): amd/arm builds --- docker-compose.yml | 2 +- install.bats | 43 ++++- install.sh | 12 +- internal/sidecar/docker.go | 52 +++++- internal/sidecar/docker_test.go | 257 ++++++++++++++++----------- internal/sidecar/mock/docker.mock.go | 29 +++ 6 files changed, 277 insertions(+), 118 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 4a93c51..b511352 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,7 +1,7 @@ services: sentry: container_name: contributoor - image: ethpandaops/contributoor:${CONTRIBUTOOR_VERSION} + image: ethpandaops/contributoor:${CONTRIBUTOOR_VERSION}-${CONTRIBUTOOR_ARCH_SUFFIX} entrypoint: ["/usr/local/bin/sentry"] command: ["--config=/config/config.yaml"] extra_hosts: diff --git a/install.bats b/install.bats index 96bab27..be60621 100644 --- a/install.bats +++ b/install.bats @@ -492,7 +492,12 @@ EOF echo "$output" | grep -q "Checksum mismatch" } -@test "setup_docker_contributoor pulls image" { +@test "setup_docker_contributoor pulls amd64 image" { + # Set required variables + ARCH="amd64" + PLATFORM="linux" + CONTRIBUTOOR_VERSION="1.0.0" + # Mock docker commands function docker() { case "$1" in @@ -500,6 +505,10 @@ EOF return 0 ;; "pull") + if [[ "$2" != "ethpandaops/contributoor:${CONTRIBUTOOR_VERSION}-amd64" ]]; then + echo "Invalid image tag: $2" + return 1 + fi return 0 ;; esac @@ -508,6 +517,38 @@ EOF export -f docker run setup_docker_contributoor + echo "Status: $status" + echo "Output: $output" + [ "$status" -eq 0 ] +} + +@test "setup_docker_contributoor pulls arm64 image" { + # Set required variables + ARCH="arm64" + PLATFORM="linux" + CONTRIBUTOOR_VERSION="1.0.0" + + # Mock docker commands + function docker() { + case "$1" in + "system") + return 0 + ;; + "pull") + if [[ "$2" != "ethpandaops/contributoor:${CONTRIBUTOOR_VERSION}-arm64v8" ]]; then + echo "Invalid image tag: $2" + return 1 + fi + return 0 + ;; + esac + } + + export -f docker + + run setup_docker_contributoor + echo "Status: $status" + echo "Output: $output" [ "$status" -eq 0 ] } diff --git a/install.sh b/install.sh index 2e15a9d..f5f04f6 100755 --- a/install.sh +++ b/install.sh @@ -238,11 +238,19 @@ setup_installer() { setup_docker_contributoor() { docker system prune -f >/dev/null 2>&1 || true - docker pull "ethpandaops/contributoor:${CONTRIBUTOOR_VERSION}" >/dev/null 2>&1 & + # Map architecture to Docker image suffix + local arch_suffix + case "$ARCH" in + amd64) arch_suffix="amd64" ;; + arm64) arch_suffix="arm64v8" ;; + *) fail "Unsupported architecture for Docker: $ARCH" ;; + esac + + docker pull "ethpandaops/contributoor:${CONTRIBUTOOR_VERSION}-${arch_suffix}" >/dev/null 2>&1 & spinner $! wait $! [ $? -ne 0 ] && fail "Failed to pull docker image" - success "Pulled docker image: ethpandaops/contributoor:${CONTRIBUTOOR_VERSION}" + success "Pulled docker image: ethpandaops/contributoor:${CONTRIBUTOOR_VERSION}-${arch_suffix}" } setup_binary_contributoor() { diff --git a/internal/sidecar/docker.go b/internal/sidecar/docker.go index b9d688a..4d94ea2 100644 --- a/internal/sidecar/docker.go +++ b/internal/sidecar/docker.go @@ -17,6 +17,10 @@ import ( type DockerSidecar interface { SidecarRunner + // GetArchSuffix returns the architecture suffix for the docker image. + GetArchSuffix() (string, error) + // GetComposeEnv returns the environment variables for docker-compose. + GetComposeEnv() []string } // dockerSidecar is a basic service for interacting with the docker container. @@ -81,7 +85,7 @@ func (s *dockerSidecar) Start() error { args := append(s.getComposeArgs(), "up", "-d", "--pull", "always") cmd := exec.Command("docker", args...) - cmd.Env = s.getComposeEnv() + cmd.Env = s.GetComposeEnv() if output, err := cmd.CombinedOutput(); err != nil { return fmt.Errorf("failed to start containers: %w\nOutput: %s", err, string(output)) @@ -103,7 +107,7 @@ func (s *dockerSidecar) Stop() error { "--timeout", "30") cmd := exec.Command("docker", args...) - cmd.Env = s.getComposeEnv() + cmd.Env = s.GetComposeEnv() if output, err := cmd.CombinedOutput(); err != nil { // Don't return error here, try our fallback. @@ -128,7 +132,7 @@ func (s *dockerSidecar) IsRunning() (bool, error) { // versions, then this will return a non running state. args := append(s.getComposeArgs(), "ps", "--format", "{{.State}}") cmd := exec.Command("docker", args...) - cmd.Env = s.getComposeEnv() + cmd.Env = s.GetComposeEnv() output, err := cmd.Output() if err == nil { @@ -172,7 +176,12 @@ func (s *dockerSidecar) Update() error { func (s *dockerSidecar) updateSidecar() error { cfg := s.sidecarCfg.Get() - image := fmt.Sprintf("%s:%s", s.installerCfg.DockerImage, cfg.Version) + archSuffix, err := s.GetArchSuffix() + if err != nil { + return err + } + + image := fmt.Sprintf("%s:%s%s", s.installerCfg.DockerImage, cfg.Version, archSuffix) cmd := exec.Command("docker", "pull", image) if output, err := cmd.CombinedOutput(); err != nil { @@ -215,13 +224,41 @@ func validateComposePath(path string) error { return nil } -func (s *dockerSidecar) getComposeEnv() []string { +// GetArchSuffix determines the Docker image architecture suffix based on system architecture. +func (s *dockerSidecar) GetArchSuffix() (string, error) { + cmd := exec.Command("uname", "-m") + + output, err := cmd.Output() + if err != nil { + return "", fmt.Errorf("failed to determine system architecture: %w", err) + } + + switch strings.TrimSpace(string(output)) { + case "x86_64", "amd64": + return "amd64", nil + case "arm64", "aarch64", "arm64e": + return "arm64v8", nil + default: + return "", fmt.Errorf("unsupported architecture: %s", string(output)) + } +} + +// GetComposeEnv returns the environment variables for docker-compose. +func (s *dockerSidecar) GetComposeEnv() []string { cfg := s.sidecarCfg.Get() + archSuffix, err := s.GetArchSuffix() + if err != nil { + s.logger.Errorf("%v", err) + + return nil + } + env := append( os.Environ(), fmt.Sprintf("CONTRIBUTOOR_CONFIG_PATH=%s", filepath.Dir(s.configPath)), fmt.Sprintf("CONTRIBUTOOR_VERSION=%s", cfg.Version), + fmt.Sprintf("CONTRIBUTOOR_ARCH_SUFFIX=%s", archSuffix), ) // Add docker network if using docker @@ -263,7 +300,7 @@ func (s *dockerSidecar) Logs(tailLines int, follow bool) error { } cmd := exec.Command("docker", args...) - cmd.Env = s.getComposeEnv() + cmd.Env = s.GetComposeEnv() cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr cmd.Dir = filepath.Dir(s.composePath) @@ -283,6 +320,9 @@ func (s *dockerSidecar) getComposeArgs() []string { additionalArgs = append(additionalArgs, "-f", s.composeNetworkPath) } + fmt.Printf("additionalArgs: %v\n", additionalArgs) + fmt.Printf("composePath: %s\n", s.composePath) + return append([]string{"compose", "-f", s.composePath}, additionalArgs...) } diff --git a/internal/sidecar/docker_test.go b/internal/sidecar/docker_test.go index 1b470f3..e449508 100644 --- a/internal/sidecar/docker_test.go +++ b/internal/sidecar/docker_test.go @@ -3,10 +3,10 @@ package sidecar_test import ( "context" "fmt" - "io" "os" "os/exec" "path/filepath" + "strings" "testing" "time" @@ -27,21 +27,13 @@ services: sentry: container_name: contributoor image: busybox - command: ["sh", "-c", "while true; do echo 'Container is running'; sleep 1; done"] + command: ["sh", "-c", "while true; do echo 'Container is running'; sleep 0.1; done"] healthcheck: test: ["CMD-SHELL", "ps aux | grep -v grep | grep 'sleep' || exit 1"] - interval: 1s - timeout: 1s - retries: 3 - start_period: 1s - networks: - - contributoor - -networks: - contributoor: - name: ${CONTRIBUTOOR_DOCKER_NETWORK:-contributoor} - driver: bridge - external: true + interval: 100ms + timeout: 100ms + retries: 2 + start_period: 100ms ` const composePortsFile = ` @@ -79,13 +71,13 @@ func TestDockerService_Integration(t *testing.T) { tmpDir = t.TempDir() logger = logrus.New() cfg = &config.Config{ - Version: "latest", + Version: "", ContributoorDirectory: tmpDir, RunMethod: config.RunMethod_RUN_METHOD_DOCKER, } ) - // Create mock config manager + // Create mock config manager. ctrl := gomock.NewController(t) defer ctrl.Finish() @@ -94,14 +86,22 @@ func TestDockerService_Integration(t *testing.T) { mockSidecarConfig.EXPECT().Get().Return(cfg).AnyTimes() mockSidecarConfig.EXPECT().GetConfigPath().Return(filepath.Join(tmpDir, "config.yaml")).AnyTimes() + // Write out compose files first. + 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, os.WriteFile(filepath.Join(tmpDir, "docker-compose.network.yml"), []byte(composeNetworkFile), 0644)) + + // Change working directory to our test directory before creating DockerSidecar. + require.NoError(t, os.Chdir(tmpDir)) + container, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{ ContainerRequest: testcontainers.ContainerRequest{ Image: "docker:dind", ExposedPorts: []string{fmt.Sprintf("%d/tcp", port)}, Privileged: true, WaitingFor: wait.ForAll( - wait.ForLog("Daemon has completed initialization").WithStartupTimeout(2*time.Minute), - wait.ForListeningPort(nat.Port(fmt.Sprintf("%d/tcp", port))).WithStartupTimeout(2*time.Minute), + wait.ForLog("Daemon has completed initialization").WithStartupTimeout(30*time.Second), + wait.ForListeningPort(nat.Port(fmt.Sprintf("%d/tcp", port))).WithStartupTimeout(30*time.Second), ), Env: map[string]string{ "DOCKER_TLS_CERTDIR": "", @@ -130,60 +130,39 @@ func TestDockerService_Integration(t *testing.T) { 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())) t.Setenv("CONTRIBUTOOR_CONFIG_PATH", tmpDir) - // Change working directory to our test directory. - require.NoError(t, os.Chdir(tmpDir)) - // Helper function for container health check. - checkContainerHealth := func(t *testing.T, ds sidecar.DockerSidecar) { + checkContainerHealth := func(t *testing.T, ds sidecar.DockerSidecar, expectRunning bool) { t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - - var lastLogs []byte - for { - select { - case <-ctx.Done(): - // Get logs only once at timeout - logs, err := container.Logs(context.Background()) - if err == nil { - logBytes, _ := io.ReadAll(logs) - lastLogs = logBytes - } - t.Fatalf("timeout waiting for docker-in-docker container to become healthy\nLast logs: %s", string(lastLogs)) - default: - running, err := ds.IsRunning() - require.NoError(t, err) - if running { - return - } - time.Sleep(time.Second) - } - } + // Single check if container is running - docker-compose defines a healthcheck which will cover us. + running, err := ds.IsRunning() + require.NoError(t, err) + require.Equal(t, expectRunning, running, "Container running state does not match expected state") } - 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, ds) - - // Verify the container is using the default network + verifyContainerNetwork := func(t *testing.T, ds sidecar.DockerSidecar, network string) { + t.Helper() cmd := exec.Command("docker", "container", "inspect", "--format", "{{range $net,$v := .NetworkSettings.Networks}}{{printf \"%s\" $net}}{{end}}", "contributoor") output, err := cmd.Output() require.NoError(t, err) - require.Contains(t, string(output), "contributoor", "Container should be connected to default network") + require.Contains(t, string(output), network, "Container is connected to incorrect network") + } + + t.Run("lifecycle_without_metrics", func(t *testing.T) { + // Boot up the container. + require.NoError(t, ds.Start()) + checkContainerHealth(t, ds, true) + // Verify the container is using the default network. + verifyContainerNetwork(t, ds, "_default") + + // Cleanup require.NoError(t, ds.Stop()) - running, err := ds.IsRunning() - require.NoError(t, err) - require.False(t, running) + checkContainerHealth(t, ds, false) }) - t.Run("lifecycle_with_metrics", func(t *testing.T) { cfgWithMetrics := &config.Config{ Version: "latest", @@ -194,30 +173,19 @@ func TestDockerService_Integration(t *testing.T) { 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, os.WriteFile(filepath.Join(tmpDir, "docker-compose.network.yml"), []byte(composeNetworkFile), 0644)) - + // Boot up the container. require.NoError(t, ds.Start()) - checkContainerHealth(t, ds) + checkContainerHealth(t, ds, true) - // Verify the container is using the default network - cmd := exec.Command("docker", "container", "inspect", "--format", "{{range $net,$v := .NetworkSettings.Networks}}{{printf \"%s\" $net}}{{end}}", "contributoor") - output, err := cmd.Output() - require.NoError(t, err) - require.Contains(t, string(output), "contributoor", "Container should be connected to default network") + // Verify the container is using the default network. + verifyContainerNetwork(t, ds, "_default") + // Cleanup. require.NoError(t, ds.Stop()) - running, err := ds.IsRunning() - require.NoError(t, err) - require.False(t, running) + checkContainerHealth(t, ds, false) }) t.Run("lifecycle_with_external_container", func(t *testing.T) { - // Write out compose file. - require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "docker-compose.yml"), []byte(composeFile), 0644)) - // Start a container directly with docker (not via compose) of the same name. This mimics // a container the installer isn't aware of. cmd := exec.Command("docker", "run", "-d", "--name", "contributoor", "busybox", @@ -226,37 +194,28 @@ func TestDockerService_Integration(t *testing.T) { require.NoError(t, err, "failed to start container: %s", string(output)) // IsRunning should detect the external container. - running, err := ds.IsRunning() - require.NoError(t, err) - require.True(t, running, "IsRunning should detect externally started container") + checkContainerHealth(t, ds, true) // Stop should be able to handle the external container. require.NoError(t, ds.Stop()) // Verify container is stopped. - running, err = ds.IsRunning() - require.NoError(t, err) - require.False(t, running, "Container should be stopped") + checkContainerHealth(t, ds, false) // Finally, test normal compose lifecycle works after cleaning up external container. require.NoError(t, ds.Start()) - checkContainerHealth(t, ds) + checkContainerHealth(t, ds, true) - // Verify the container is using the default network - cmd = exec.Command("docker", "container", "inspect", "--format", "{{range $net,$v := .NetworkSettings.Networks}}{{printf \"%s\" $net}}{{end}}", "contributoor") - output, err = cmd.Output() - require.NoError(t, err) - require.Contains(t, string(output), "contributoor", "Container should be connected to default network") + // Verify the container is using the default network. + verifyContainerNetwork(t, ds, "_default") + // Cleanup. require.NoError(t, ds.Stop()) - - running, err = ds.IsRunning() - require.NoError(t, err) - require.False(t, running) + checkContainerHealth(t, ds, false) }) t.Run("lifecycle_with_custom_network", func(t *testing.T) { - // Create a custom network first + // Create a custom network first. customNetwork := "test_network" cmd := exec.Command("docker", "network", "create", customNetwork) require.NoError(t, cmd.Run()) @@ -269,7 +228,7 @@ func TestDockerService_Integration(t *testing.T) { DockerNetwork: customNetwork, } - // Create new mock and DockerSidecar instance for this test + // Create new mock and DockerSidecar instance for this test. mockSidecarConfigCustom := mock.NewMockConfigManager(ctrl) mockSidecarConfigCustom.EXPECT().Get().Return(cfgWithNetwork).AnyTimes() mockSidecarConfigCustom.EXPECT().GetConfigPath().Return(filepath.Join(tmpDir, "config.yaml")).AnyTimes() @@ -277,23 +236,105 @@ func TestDockerService_Integration(t *testing.T) { dsCustom, err := sidecar.NewDockerSidecar(logger, mockSidecarConfigCustom, mockInstallerConfig) require.NoError(t, err) - // Write out compose file - composeFilePath := filepath.Join(tmpDir, "docker-compose.yml") - require.NoError(t, os.WriteFile(composeFilePath, []byte(composeFile), 0644)) - require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "docker-compose.network.yml"), []byte(composeNetworkFile), 0644)) - + // Boot up the container. require.NoError(t, dsCustom.Start()) - checkContainerHealth(t, dsCustom) + checkContainerHealth(t, dsCustom, true) - // Verify the container is using the custom network - verifyCmd := exec.Command("docker", "container", "inspect", "--format", "{{range $net,$v := .NetworkSettings.Networks}}{{printf \"%s\" $net}}{{end}}", "contributoor") - verifyOutput, err := verifyCmd.Output() - require.NoError(t, err) - require.Contains(t, string(verifyOutput), customNetwork, "Container should be connected to custom network") + // Verify the container is using the custom network. + verifyContainerNetwork(t, dsCustom, customNetwork) + // Cleanup. require.NoError(t, dsCustom.Stop()) - running, err := dsCustom.IsRunning() - require.NoError(t, err) - require.False(t, running) + checkContainerHealth(t, dsCustom, false) }) } + +func TestGetComposeEnv(t *testing.T) { + logger := logrus.New() + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + tests := []struct { + name string + config *config.Config + expectedEnvVars map[string]string + }{ + { + name: "basic config", + config: &config.Config{ + Version: "latest", + ContributoorDirectory: t.TempDir(), + RunMethod: config.RunMethod_RUN_METHOD_DOCKER, + }, + expectedEnvVars: map[string]string{ + "CONTRIBUTOOR_VERSION": "latest", + }, + }, + { + name: "with metrics", + config: &config.Config{ + Version: "v1.0.0", + ContributoorDirectory: t.TempDir(), + RunMethod: config.RunMethod_RUN_METHOD_DOCKER, + MetricsAddress: "0.0.0.0:9090", + }, + expectedEnvVars: map[string]string{ + "CONTRIBUTOOR_VERSION": "v1.0.0", + "CONTRIBUTOOR_METRICS_ADDRESS": "0.0.0.0", + "CONTRIBUTOOR_METRICS_PORT": "9090", + }, + }, + { + name: "with docker network", + config: &config.Config{ + Version: "v1.0.0", + ContributoorDirectory: t.TempDir(), + RunMethod: config.RunMethod_RUN_METHOD_DOCKER, + DockerNetwork: "custom_network", + }, + expectedEnvVars: map[string]string{ + "CONTRIBUTOOR_VERSION": "v1.0.0", + "CONTRIBUTOOR_DOCKER_NETWORK": "custom_network", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockSidecarConfig := mock.NewMockConfigManager(mockCtrl) + mockInstallerConfig := installer.NewConfig() + + // Set up mock expectations before creating DockerSidecar + mockSidecarConfig.EXPECT().Get().Return(tt.config).AnyTimes() + mockSidecarConfig.EXPECT().GetConfigPath().Return(filepath.Join(tt.config.ContributoorDirectory, "config.yaml")).AnyTimes() + + ds, err := sidecar.NewDockerSidecar(logger, mockSidecarConfig, mockInstallerConfig) + require.NoError(t, err) + + env := ds.GetComposeEnv() + require.NotNil(t, env) + + // Convert env slice to map for easier testing + envMap := make(map[string]string) + for _, e := range env { + parts := strings.SplitN(e, "=", 2) + if len(parts) == 2 { + envMap[parts[0]] = parts[1] + } + } + + // Check arch suffix is present and valid + archSuffix := envMap["CONTRIBUTOOR_ARCH_SUFFIX"] + require.Contains(t, []string{"amd64", "arm64v8"}, archSuffix, "Architecture suffix should be either amd64 or arm64v8") + + // Check config path is set and points to correct directory + configPath := envMap["CONTRIBUTOOR_CONFIG_PATH"] + require.Equal(t, filepath.Dir(filepath.Join(tt.config.ContributoorDirectory, "config.yaml")), configPath) + + // Check all expected env vars are present with correct values + for k, v := range tt.expectedEnvVars { + require.Equal(t, v, envMap[k], "Environment variable %s has incorrect value", k) + } + }) + } +} diff --git a/internal/sidecar/mock/docker.mock.go b/internal/sidecar/mock/docker.mock.go index 9c65983..52f99cf 100644 --- a/internal/sidecar/mock/docker.mock.go +++ b/internal/sidecar/mock/docker.mock.go @@ -38,6 +38,35 @@ func (m *MockDockerSidecar) EXPECT() *MockDockerSidecarMockRecorder { return m.recorder } +// GetArchSuffix mocks base method. +func (m *MockDockerSidecar) GetArchSuffix() (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetArchSuffix") + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetArchSuffix indicates an expected call of GetArchSuffix. +func (mr *MockDockerSidecarMockRecorder) GetArchSuffix() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetArchSuffix", reflect.TypeOf((*MockDockerSidecar)(nil).GetArchSuffix)) +} + +// GetComposeEnv mocks base method. +func (m *MockDockerSidecar) GetComposeEnv() []string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetComposeEnv") + ret0, _ := ret[0].([]string) + return ret0 +} + +// GetComposeEnv indicates an expected call of GetComposeEnv. +func (mr *MockDockerSidecarMockRecorder) GetComposeEnv() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetComposeEnv", reflect.TypeOf((*MockDockerSidecar)(nil).GetComposeEnv)) +} + // IsRunning mocks base method. func (m *MockDockerSidecar) IsRunning() (bool, error) { m.ctrl.T.Helper() From 8c4f145aa25189dc862063cb7d339a8dac88f0b2 Mon Sep 17 00:00:00 2001 From: Matty Evans Date: Fri, 17 Jan 2025 10:49:24 +1000 Subject: [PATCH 2/6] refactor: Remove debug print statements --- internal/sidecar/docker.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/sidecar/docker.go b/internal/sidecar/docker.go index 4d94ea2..1076278 100644 --- a/internal/sidecar/docker.go +++ b/internal/sidecar/docker.go @@ -320,9 +320,6 @@ func (s *dockerSidecar) getComposeArgs() []string { additionalArgs = append(additionalArgs, "-f", s.composeNetworkPath) } - fmt.Printf("additionalArgs: %v\n", additionalArgs) - fmt.Printf("composePath: %s\n", s.composePath) - return append([]string{"compose", "-f", s.composePath}, additionalArgs...) } From 33a0a064e9d0a7499862352a89733b387893db8c Mon Sep 17 00:00:00 2001 From: Matty Evans Date: Fri, 17 Jan 2025 10:53:20 +1000 Subject: [PATCH 3/6] test: Add file writing and directory change in test --- internal/sidecar/docker_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/sidecar/docker_test.go b/internal/sidecar/docker_test.go index e449508..a23d889 100644 --- a/internal/sidecar/docker_test.go +++ b/internal/sidecar/docker_test.go @@ -304,6 +304,14 @@ func TestGetComposeEnv(t *testing.T) { mockSidecarConfig := mock.NewMockConfigManager(mockCtrl) mockInstallerConfig := installer.NewConfig() + // Write out compose files first + require.NoError(t, os.WriteFile(filepath.Join(tt.config.ContributoorDirectory, "docker-compose.yml"), []byte(composeFile), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tt.config.ContributoorDirectory, "docker-compose.ports.yml"), []byte(composePortsFile), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tt.config.ContributoorDirectory, "docker-compose.network.yml"), []byte(composeNetworkFile), 0644)) + + // Change working directory to test directory + require.NoError(t, os.Chdir(tt.config.ContributoorDirectory)) + // Set up mock expectations before creating DockerSidecar mockSidecarConfig.EXPECT().Get().Return(tt.config).AnyTimes() mockSidecarConfig.EXPECT().GetConfigPath().Return(filepath.Join(tt.config.ContributoorDirectory, "config.yaml")).AnyTimes() From 495511cb74092cc6cc1dde8d560d47389c0a0686 Mon Sep 17 00:00:00 2001 From: Matty Evans Date: Fri, 17 Jan 2025 11:04:27 +1000 Subject: [PATCH 4/6] refactor: Remove unnecessary docker system prune command --- install.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/install.sh b/install.sh index de4a4e0..ae03ba9 100755 --- a/install.sh +++ b/install.sh @@ -235,8 +235,6 @@ setup_installer() { } setup_docker_contributoor() { - docker system prune -f >/dev/null 2>&1 || true - # Map architecture to Docker image suffix local arch_suffix case "$ARCH" in From 543098c1cfb411cd58109cb8a33a9d368c7b9263 Mon Sep 17 00:00:00 2001 From: Matty Evans Date: Fri, 17 Jan 2025 11:18:09 +1000 Subject: [PATCH 5/6] refactor: Update Docker image tag and remove unused code --- docker-compose.yml | 2 +- install.bats | 36 ++-------------------------- install.sh | 12 ++-------- internal/sidecar/docker.go | 42 ++++----------------------------- internal/sidecar/docker_test.go | 4 ---- 5 files changed, 9 insertions(+), 87 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index b511352..4a93c51 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,7 +1,7 @@ services: sentry: container_name: contributoor - image: ethpandaops/contributoor:${CONTRIBUTOOR_VERSION}-${CONTRIBUTOOR_ARCH_SUFFIX} + image: ethpandaops/contributoor:${CONTRIBUTOOR_VERSION} entrypoint: ["/usr/local/bin/sentry"] command: ["--config=/config/config.yaml"] extra_hosts: diff --git a/install.bats b/install.bats index 27c8111..f6b85de 100644 --- a/install.bats +++ b/install.bats @@ -491,40 +491,8 @@ EOF echo "$output" | grep -q "Checksum mismatch" } -@test "setup_docker_contributoor pulls amd64 image" { +@test "setup_docker_contributoor pulls image" { # Set required variables - ARCH="amd64" - PLATFORM="linux" - CONTRIBUTOOR_VERSION="1.0.0" - - # Mock docker commands - function docker() { - case "$1" in - "system") - return 0 - ;; - "pull") - if [[ "$2" != "ethpandaops/contributoor:${CONTRIBUTOOR_VERSION}-amd64" ]]; then - echo "Invalid image tag: $2" - return 1 - fi - return 0 - ;; - esac - } - - export -f docker - - run setup_docker_contributoor - echo "Status: $status" - echo "Output: $output" - [ "$status" -eq 0 ] -} - -@test "setup_docker_contributoor pulls arm64 image" { - # Set required variables - ARCH="arm64" - PLATFORM="linux" CONTRIBUTOOR_VERSION="1.0.0" # Mock docker commands @@ -534,7 +502,7 @@ EOF return 0 ;; "pull") - if [[ "$2" != "ethpandaops/contributoor:${CONTRIBUTOOR_VERSION}-arm64v8" ]]; then + if [[ "$2" != "ethpandaops/contributoor:${CONTRIBUTOOR_VERSION}" ]]; then echo "Invalid image tag: $2" return 1 fi diff --git a/install.sh b/install.sh index ae03ba9..22a9dcf 100755 --- a/install.sh +++ b/install.sh @@ -235,19 +235,11 @@ setup_installer() { } setup_docker_contributoor() { - # Map architecture to Docker image suffix - local arch_suffix - case "$ARCH" in - amd64) arch_suffix="amd64" ;; - arm64) arch_suffix="arm64v8" ;; - *) fail "Unsupported architecture for Docker: $ARCH" ;; - esac - - docker pull "ethpandaops/contributoor:${CONTRIBUTOOR_VERSION}-${arch_suffix}" >/dev/null 2>&1 & + docker pull "ethpandaops/contributoor:${CONTRIBUTOOR_VERSION}" >/dev/null 2>&1 & spinner $! wait $! [ $? -ne 0 ] && fail "Failed to pull docker image" - success "Pulled docker image: ethpandaops/contributoor:${CONTRIBUTOOR_VERSION}-${arch_suffix}" + success "Pulled docker image: ethpandaops/contributoor:${CONTRIBUTOOR_VERSION}" } setup_binary_contributoor() { diff --git a/internal/sidecar/docker.go b/internal/sidecar/docker.go index 1076278..a21550d 100644 --- a/internal/sidecar/docker.go +++ b/internal/sidecar/docker.go @@ -17,9 +17,6 @@ import ( type DockerSidecar interface { SidecarRunner - // GetArchSuffix returns the architecture suffix for the docker image. - GetArchSuffix() (string, error) - // GetComposeEnv returns the environment variables for docker-compose. GetComposeEnv() []string } @@ -174,14 +171,10 @@ func (s *dockerSidecar) Update() error { // updateSidecar updates the docker image to the specified version. func (s *dockerSidecar) updateSidecar() error { - cfg := s.sidecarCfg.Get() - - archSuffix, err := s.GetArchSuffix() - if err != nil { - return err - } - - image := fmt.Sprintf("%s:%s%s", s.installerCfg.DockerImage, cfg.Version, archSuffix) + var ( + cfg = s.sidecarCfg.Get() + image = fmt.Sprintf("%s:%s", s.installerCfg.DockerImage, cfg.Version) + ) cmd := exec.Command("docker", "pull", image) if output, err := cmd.CombinedOutput(); err != nil { @@ -224,41 +217,14 @@ func validateComposePath(path string) error { return nil } -// GetArchSuffix determines the Docker image architecture suffix based on system architecture. -func (s *dockerSidecar) GetArchSuffix() (string, error) { - cmd := exec.Command("uname", "-m") - - output, err := cmd.Output() - if err != nil { - return "", fmt.Errorf("failed to determine system architecture: %w", err) - } - - switch strings.TrimSpace(string(output)) { - case "x86_64", "amd64": - return "amd64", nil - case "arm64", "aarch64", "arm64e": - return "arm64v8", nil - default: - return "", fmt.Errorf("unsupported architecture: %s", string(output)) - } -} - // GetComposeEnv returns the environment variables for docker-compose. func (s *dockerSidecar) GetComposeEnv() []string { cfg := s.sidecarCfg.Get() - archSuffix, err := s.GetArchSuffix() - if err != nil { - s.logger.Errorf("%v", err) - - return nil - } - env := append( os.Environ(), fmt.Sprintf("CONTRIBUTOOR_CONFIG_PATH=%s", filepath.Dir(s.configPath)), fmt.Sprintf("CONTRIBUTOOR_VERSION=%s", cfg.Version), - fmt.Sprintf("CONTRIBUTOOR_ARCH_SUFFIX=%s", archSuffix), ) // Add docker network if using docker diff --git a/internal/sidecar/docker_test.go b/internal/sidecar/docker_test.go index a23d889..a60fc5a 100644 --- a/internal/sidecar/docker_test.go +++ b/internal/sidecar/docker_test.go @@ -331,10 +331,6 @@ func TestGetComposeEnv(t *testing.T) { } } - // Check arch suffix is present and valid - archSuffix := envMap["CONTRIBUTOOR_ARCH_SUFFIX"] - require.Contains(t, []string{"amd64", "arm64v8"}, archSuffix, "Architecture suffix should be either amd64 or arm64v8") - // Check config path is set and points to correct directory configPath := envMap["CONTRIBUTOOR_CONFIG_PATH"] require.Equal(t, filepath.Dir(filepath.Join(tt.config.ContributoorDirectory, "config.yaml")), configPath) From 7c5c3c97e7fee7abc5e9cffea66a8265eaaf8996 Mon Sep 17 00:00:00 2001 From: Matty Evans Date: Fri, 17 Jan 2025 11:19:51 +1000 Subject: [PATCH 6/6] refactor: Remove unused MockDockerSidecar method --- internal/sidecar/mock/docker.mock.go | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/internal/sidecar/mock/docker.mock.go b/internal/sidecar/mock/docker.mock.go index 52f99cf..2c288fc 100644 --- a/internal/sidecar/mock/docker.mock.go +++ b/internal/sidecar/mock/docker.mock.go @@ -38,21 +38,6 @@ func (m *MockDockerSidecar) EXPECT() *MockDockerSidecarMockRecorder { return m.recorder } -// GetArchSuffix mocks base method. -func (m *MockDockerSidecar) GetArchSuffix() (string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetArchSuffix") - ret0, _ := ret[0].(string) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetArchSuffix indicates an expected call of GetArchSuffix. -func (mr *MockDockerSidecarMockRecorder) GetArchSuffix() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetArchSuffix", reflect.TypeOf((*MockDockerSidecar)(nil).GetArchSuffix)) -} - // GetComposeEnv mocks base method. func (m *MockDockerSidecar) GetComposeEnv() []string { m.ctrl.T.Helper()