fix: manually handle lock

Signed-off-by: Samuel K <skevetter@pm.me>

Author: skevetter

.github/workflows/pr-ci.yml

@@ -366,15 +366,15 @@ jobs:
               KUBECONFIG="${KUBECONFIG:-$HOME/.kube/config}" \
               PATH="${PATH}" \
               GOROOT="${GOROOT}" \
-              go test -v -ginkgo.v -timeout 3600s --ginkgo.label-filter="${{ matrix.label }}"
+              go test -v -ginkgo.v -timeout 900s --ginkgo.label-filter="${{ matrix.label }}"
           else
             GH_USERNAME="${GH_USERNAME}" \
               GH_ACCESS_TOKEN="${GH_ACCESS_TOKEN}" \
               KUBECONFIG="${KUBECONFIG:-$HOME/.kube/config}" \
               PATH="${PATH}" \
               GOROOT="${GOROOT}" \
               DOCKER_HOST="npipe:////./pipe/podman-machine-default" \
-              go test -v -ginkgo.v -timeout 3600s --ginkgo.label-filter="${{ matrix.label }}"
+              go test -v -ginkgo.v -timeout 900s --ginkgo.label-filter="${{ matrix.label }}"
           fi
 
       - name: verify docker is installed

cmd/agent/workspace/build.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"os"
 
-	"github.com/sirupsen/logrus"
 	"github.com/skevetter/devpod/cmd/flags"
 	"github.com/skevetter/devpod/pkg/agent"
 	provider2 "github.com/skevetter/devpod/pkg/provider"
@@ -98,15 +97,9 @@ func (cmd *BuildCmd) Run(ctx context.Context) error {
 		}
 
 		if workspaceInfo.CLIOptions.SkipPush {
-			logger.WithFields(logrus.Fields{
-				"imageName": imageName,
-			})
-			logger.Donef("done building image")
+			logger.Donef("done building image %s", imageName)
 		} else {
-			logger.WithFields(logrus.Fields{
-				"imageName": imageName,
-			})
-			logger.Donef("done building and pushing image")
+			logger.Donef("done building and pushing image %s", imageName)
 		}
 	}
 

cmd/agent/workspace/up.go

@@ -229,7 +229,7 @@ func contentFolderExists(path string, log log.Logger) (bool, error) {
 
 func createContentFolder(path string, log log.Logger) error {
 	log.WithFields(logrus.Fields{"path": path}).Debug("create content folder")
-	if err := os.MkdirAll(path, 0o777); err != nil {
+	if err := os.MkdirAll(path, 0o750); err != nil {
 		return fmt.Errorf("make workspace folder: %w", err)
 	}
 	return nil
@@ -307,7 +307,7 @@ func (w *workspaceInitializer) initialize() error {
 		return err
 	}
 
-	w.configureDockerDaemon()
+	w.tryConfigureDockerDaemon()
 	return nil
 }
 
@@ -319,7 +319,7 @@ func (w *workspaceInitializer) setupDaemonIfNeeded() {
 	}
 }
 
-func (w *workspaceInitializer) configureDockerDaemon() {
+func (w *workspaceInitializer) tryConfigureDockerDaemon() {
 	if !w.shouldConfigureDockerDaemon() {
 		w.logger.Debug("skipping configuring docker daemon")
 		return
@@ -392,8 +392,11 @@ func (w *workspaceInitializer) ensureDockerInstalled() (string, error) {
 	}
 
 	if dockerCmd != "docker" {
-		_, err := exec.LookPath(dockerCmd)
-		return "", err
+		path, err := exec.LookPath(dockerCmd)
+		if err != nil {
+			return "", fmt.Errorf("custom docker path %q not found: %w", dockerCmd, err)
+		}
+		return path, nil
 	}
 
 	if w.isDockerInstallDisabled() {
@@ -429,6 +432,8 @@ func (w *workspaceInitializer) prepareWorkspaceContent() error {
 	})
 }
 
+// waitForDocker waits for the Docker installation to complete.
+// Note: This function modifies workspaceInfo.Agent.Docker.Path if Docker was installed.
 func (w *workspaceInitializer) waitForDocker(resultChan <-chan dockerInstallResult) error {
 	result := <-resultChan
 
@@ -465,6 +470,8 @@ type prepareWorkspaceParams struct {
 	log           log.Logger
 }
 
+// prepareWorkspace initializes the workspace content folder and downloads/prepares the workspace source.
+// Note: This function modifies params.workspaceInfo.ContentFolder when platform is enabled with a local folder.
 func prepareWorkspace(params prepareWorkspaceParams) error {
 	if params.workspaceInfo.CLIOptions.Platform.Enabled && params.workspaceInfo.Workspace.Source.LocalFolder != "" {
 		params.workspaceInfo.ContentFolder = agent.GetAgentWorkspaceContentDir(params.workspaceInfo.Origin)
@@ -653,52 +660,40 @@ func prepareImage(workspaceDir, image string) error {
 	return os.WriteFile(filepath.Join(workspaceDir, ".devcontainer.json"), devcontainerConfig, 0o600)
 }
 
+// installDocker installs Docker and returns the path to the docker binary.
+// This function assumes docker does not already exist - the caller should check first.
 func installDocker(log log.Logger) (dockerPath string, err error) {
-	if !command.Exists("docker") {
-		writer := log.Writer(logrus.InfoLevel, false)
-		defer func() { _ = writer.Close() }()
-
-		log.Debug("Installing Docker")
-
-		dockerPath, err = dockerinstall.Install(writer, writer)
-	} else {
-		dockerPath = "docker"
-	}
-	return dockerPath, err
+	writer := log.Writer(logrus.InfoLevel, false)
+	defer func() { _ = writer.Close() }()
+	log.Debug("installing Docker")
+	return dockerinstall.Install(writer, writer)
 }
 
 func configureDockerDaemon(ctx context.Context, log log.Logger) error {
 	log.Info("configuring docker daemon")
 
-	daemonConfig := []byte(`{
-		"features": {
-			"containerd-snapshotter": true
-		}
-	}`)
-
-	if err := writeDockerDaemonConfig(daemonConfig); err != nil {
+	if err := mergeDockerDaemonConfig(); err != nil {
 		return err
 	}
 
 	return reloadDockerDaemon(ctx)
 }
 
-func writeDockerDaemonConfig(config []byte) error {
-	rootlessErr := tryWriteRootlessDockerConfig(config)
+func mergeDockerDaemonConfig() error {
+	rootlessErr := tryMergeRootlessDockerConfig()
 	if rootlessErr == nil {
 		return nil
 	}
 
-	// #nosec G306 -- daemon.json needs to be readable by docker daemon
-	rootErr := os.WriteFile("/etc/docker/daemon.json", config, 0644)
+	rootErr := tryMergeRootDockerConfig()
 	if rootErr == nil {
 		return nil
 	}
 
 	return fmt.Errorf("failed to write docker daemon config (rootless: %v, root: %v)", rootlessErr, rootErr)
 }
 
-func tryWriteRootlessDockerConfig(config []byte) error {
+func tryMergeRootlessDockerConfig() error {
 	homeDir, err := util.UserHomeDir()
 	if err != nil {
 		return err
@@ -709,10 +704,60 @@ func tryWriteRootlessDockerConfig(config []byte) error {
 		return err
 	}
 
+	configPath := filepath.Join(dockerConfigDir, "daemon.json")
+	return mergeContainerdSnapshotterConfig(configPath)
+}
+
+func tryMergeRootDockerConfig() error {
+	return mergeContainerdSnapshotterConfig("/etc/docker/daemon.json")
+}
+
+func mergeContainerdSnapshotterConfig(configPath string) error {
+	existingConfig := make(map[string]any)
+	data, err := os.ReadFile(configPath)
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
+		return fmt.Errorf("read existing config: %w", err)
+	}
+
+	if len(data) > 0 {
+		if err := json.Unmarshal(data, &existingConfig); err != nil {
+			return fmt.Errorf("parse existing config: %w", err)
+		}
+	}
+
+	features, ok := existingConfig["features"].(map[string]any)
+	if !ok {
+		features = make(map[string]any)
+		existingConfig["features"] = features
+	}
+	features["containerd-snapshotter"] = true
+
+	mergedData, err := json.MarshalIndent(existingConfig, "", "  ")
+	if err != nil {
+		return fmt.Errorf("marshal config: %w", err)
+	}
+
+	if err := os.MkdirAll(filepath.Dir(configPath), 0755); err != nil {
+		return fmt.Errorf("create config directory: %w", err)
+	}
+
 	// #nosec G306 -- daemon.json needs to be readable by docker daemon
-	return os.WriteFile(filepath.Join(dockerConfigDir, "daemon.json"), config, 0644)
+	if err := os.WriteFile(configPath, mergedData, 0644); err != nil {
+		return fmt.Errorf("write config: %w", err)
+	}
+
+	return nil
 }
 
 func reloadDockerDaemon(ctx context.Context) error {
-	return exec.CommandContext(ctx, "pkill", "-HUP", "dockerd").Run()
+	err := exec.CommandContext(ctx, "pkill", "-HUP", "dockerd").Run()
+	if err != nil {
+		// pkill returns exit code 1 if no processes matched
+		var exitErr *exec.ExitError
+		if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 {
+			return nil // No dockerd process found, nothing to reload
+		}
+		return err
+	}
+	return nil
 }

e2e/tests/integration/integration.go

@@ -7,22 +7,20 @@ import (
 	"os/exec"
 
 	"github.com/onsi/ginkgo/v2"
-	"github.com/onsi/gomega"
 	"github.com/skevetter/devpod/e2e/framework"
 )
 
 var _ = ginkgo.Describe("[integration]: devpod provider ssh test suite", ginkgo.Ordered, func() {
 	ginkgo.Context("testing provider integration", ginkgo.Label("integration"), ginkgo.Ordered, func() {
 		var initialDir string
-		ctx := context.Background()
 
 		ginkgo.BeforeEach(func() {
 			var err error
 			initialDir, err = os.Getwd()
 			framework.ExpectNoError(err)
 		})
 
-		ginkgo.It("should generate ssh keypairs", func() {
+		ginkgo.It("should generate ssh keypairs", func(ctx context.Context) {
 			sshDir := os.Getenv("HOME") + "/.ssh"
 			if _, err := os.Stat(sshDir); os.IsNotExist(err) {
 				err = os.MkdirAll(sshDir, 0700)
@@ -32,19 +30,19 @@ var _ = ginkgo.Describe("[integration]: devpod provider ssh test suite", ginkgo.
 			_, err := os.Stat(os.Getenv("HOME") + "/.ssh/id_rsa")
 			if err != nil {
 				fmt.Println("generating ssh keys")
-				cmd := exec.Command("ssh-keygen", "-q", "-t", "rsa", "-N", "", "-f", os.Getenv("HOME")+"/.ssh/id_rsa")
+				cmd := exec.CommandContext(ctx, "ssh-keygen", "-q", "-t", "rsa", "-N", "", "-f", os.Getenv("HOME")+"/.ssh/id_rsa")
 				err = cmd.Run()
 				framework.ExpectNoError(err)
 
-				cmd = exec.Command("ssh-keygen", "-y", "-f", os.Getenv("HOME")+"/.ssh/id_rsa")
+				cmd = exec.CommandContext(ctx, "ssh-keygen", "-y", "-f", os.Getenv("HOME")+"/.ssh/id_rsa")
 				output, err := cmd.Output()
 				framework.ExpectNoError(err)
 
 				err = os.WriteFile(os.Getenv("HOME")+"/.ssh/id_rsa.pub", output, 0600)
 				framework.ExpectNoError(err)
 			}
 
-			cmd := exec.Command("ssh-keygen", "-y", "-f", os.Getenv("HOME")+"/.ssh/id_rsa")
+			cmd := exec.CommandContext(ctx, "ssh-keygen", "-y", "-f", os.Getenv("HOME")+"/.ssh/id_rsa")
 			publicKey, err := cmd.Output()
 			framework.ExpectNoError(err)
 
@@ -63,7 +61,7 @@ var _ = ginkgo.Describe("[integration]: devpod provider ssh test suite", ginkgo.
 			}
 		})
 
-		ginkgo.It("should add provider to devpod", func() {
+		ginkgo.It("should add provider to devpod", func(ctx context.Context) {
 			f := framework.NewDefaultFramework(initialDir + "/bin")
 			// ensure we don't have the ssh provider present
 			err := f.DevPodProviderDelete(ctx, "ssh")
@@ -81,12 +79,11 @@ var _ = ginkgo.Describe("[integration]: devpod provider ssh test suite", ginkgo.
 			framework.ExpectNoError(err)
 		})
 
-		ginkgo.It("should run commands to workspace via ssh", func() {
-			cmd := exec.Command("ssh", "testdata.devpod", "echo", "test")
-			output, err := cmd.Output()
+		ginkgo.It("should run commands to workspace via ssh", func(ctx context.Context) {
+			f := framework.NewDefaultFramework(initialDir + "/bin")
+			out, err := f.DevPodSSH(ctx, "testdata", "echo test")
 			framework.ExpectNoError(err)
-
-			gomega.Expect(output).To(gomega.Equal([]byte("test\n")))
+			framework.ExpectEqual(out, "test\n")
 		})
 
 		ginkgo.It("should cleanup devpod workspace", func(ctx context.Context) {

pkg/client/clientimplementation/daemonclient/client.go

@@ -257,8 +257,8 @@ func (c *client) Ping(ctx context.Context, writer io.Writer) error {
 
 	for range 10 {
 		timeoutCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
-		defer cancel()
 		result, err := c.tsClient.Ping(timeoutCtx, *ip, tailcfg.PingDisco)
+		cancel()
 		if err != nil {
 			return err
 		}

pkg/client/clientimplementation/workspace_client.go

@@ -505,20 +505,7 @@ func (s *workspaceClient) Stop(ctx context.Context, opt client.StopOptions) erro
 }
 
 func (s *workspaceClient) Command(ctx context.Context, commandOptions client.CommandOptions) (err error) {
-	s.m.Lock()
-	defer s.m.Unlock()
-
-	environ, err := provider.ToEnvironmentWithBinaries(provider.EnvironmentOptions{
-		Context:   s.workspace.Context,
-		Workspace: s.workspace,
-		Machine:   s.machine,
-		Options:   s.devPodConfig.ProviderOptions(s.config.Name),
-		Config:    s.config,
-		ExtraEnv: map[string]string{
-			provider.CommandEnv: commandOptions.Command,
-		},
-		Log: s.log,
-	})
+	environ, err := s.buildEnvironment(commandOptions.Command)
 	if err != nil {
 		return err
 	}
@@ -534,6 +521,23 @@ func (s *workspaceClient) Command(ctx context.Context, commandOptions client.Com
 	})
 }
 
+func (s *workspaceClient) buildEnvironment(command string) ([]string, error) {
+	s.m.Lock()
+	defer s.m.Unlock()
+
+	return provider.ToEnvironmentWithBinaries(provider.EnvironmentOptions{
+		Context:   s.workspace.Context,
+		Workspace: s.workspace,
+		Machine:   s.machine,
+		Options:   s.devPodConfig.ProviderOptions(s.config.Name),
+		Config:    s.config,
+		ExtraEnv: map[string]string{
+			provider.CommandEnv: command,
+		},
+		Log: s.log,
+	})
+}
+
 func (s *workspaceClient) Status(ctx context.Context, options client.StatusOptions) (client.Status, error) {
 	s.m.Lock()
 	defer s.m.Unlock()