Fix: improve Docker startup and compose validation by austin047 · Pull Request #78 · flatrun/agent

austin047 · 2026-02-18T23:36:41Z

Set DOCKER_HOST from config to environment and verify Docker is reachable at startup
Update config.example deployments_path to ./deployment
Create deployments directory on startup if not exist; fail fast if Docker is unavailable
Use the Default docker_socket from the Docker client env when unset
Validate compose in updateDeployment via Docker Compose config
Add moby/moby docker client dependency for platform socket detection

sourceant · 2026-02-18T23:37:15Z

Code Review Summary

This PR improves Docker integration by verifying reachability at startup, creating necessary directories, and implementing robust Compose validation using compose-go.

🚀 Key Improvements

Switched to compose-go for schema-aware Docker Compose validation.
Added startup check to fail fast if Docker is unreachable.
Automated creation of the deployments directory.

💡 Minor Suggestions

Remove unused _ parameter in ensureDockerReachable.
Cleanup unused os/exec and runtime imports if switching to library-based detection.

🚨 Critical Issues

Repeated use of exec.Command for Docker checks instead of using the already-imported Moby client library.

sourceant

Review complete. See the overview comment for a summary.

sourceant · 2026-02-18T23:37:16Z

cmd/agent/main.go

@@ -42,6 +43,15 @@ func main() {
 		log.Fatalf("Failed to load config from %s: %v", resolvedConfigPath, err)
 	}


Using exec.Command("docker", "info") relies on the docker CLI being installed in the PATH. Since you've added the moby/moby client dependency, it is more efficient and reliable to check connectivity using the library's Ping() or Info() methods instead of spawning a shell process.

Suggested change

}

func ensureDockerReachable(socket string) {

ctx := context.Background()

cli, err := client.NewClientWithOpts(client.WithHost(socket), client.WithAPIVersionNegotiation())

if err != nil {

log.Fatalf("Failed to create Docker client: %v", err)

}

defer cli.Close()

if _, err := cli.Ping(ctx); err != nil {

log.Fatalf("Docker is not reachable: %v", err)

}

}

sourceant · 2026-02-18T23:37:16Z

internal/api/server.go

+	return nil
+}
+
+func validateComposeWithCLI(content string) error {


Closing the file before deleting it is safer on some operating systems (like Windows). Also, consider using defer os.Remove(tmp.Name()) immediately after creation to ensure cleanup even if subsequent steps fail.

Suggested change

func validateComposeWithCLI(content string) error {

tmp, err := os.CreateTemp("", "compose-*.yml")

if err != nil {

log.Printf("Warning: skipping CLI validation, failed to create temp file: %v", err)

return nil

}

defer os.Remove(tmp.Name())

defer tmp.Close()

sourceant

Review complete. See the overview comment for a summary.

sourceant · 2026-02-19T00:49:35Z

cmd/agent/main.go

 		log.Fatalf("Failed to load config from %s: %v", resolvedConfigPath, err)
 	}

+	os.Setenv("DOCKER_HOST", cfg.DockerSocket)


Setting DOCKER_HOST globally via os.Setenv can cause side effects if other parts of the application or third-party libraries expect a different environment. Since you've already added the Moby client, it is better to pass the socket explicitly to constructors rather than relying on global environment variables.

Suggested change

os.Setenv("DOCKER_HOST", cfg.DockerSocket)

ensureDockerReachable(cfg.DockerSocket)

sourceant · 2026-02-19T00:49:35Z

pkg/config/config.go

 	}
 	if cfg.DockerSocket == "" {
-		cfg.DockerSocket = "unix:///var/run/docker.sock"
+		apiClient, err := client.New(client.FromEnv)


Creating a new client just to detect the default host is slightly heavy. Note that client.FromEnv will look at the DOCKER_HOST environment variable first. Ensure this fallback logic matches the expected behavior if the agent is running inside a container.

Suggested change

apiClient, err := client.New(client.FromEnv)

cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation())

if err == nil {

cfg.DockerSocket = cli.DaemonHost()

cli.Close()

} else {

cfg.DockerSocket = "unix:///var/run/docker.sock"

}

sourceant

Review complete. See the overview comment for a summary.

sourceant · 2026-02-20T09:25:05Z

pkg/config/config.go

@@ -98,15 +101,15 @@ type InfrastructureConfig struct {
 }


Relying on docker context inspect via exec.Command adds unnecessary overhead and a dependency on the CLI binary being in the PATH at this specific moment. Since you've added the moby/moby client dependency, it is better to use the library's environment detection logic, which handles context, environment variables, and platform-specific defaults correctly.

Suggested change

}

func detectDockerHost() string {

cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation())

if err != nil {

// Fallback to platform defaults if client creation fails

if runtime.GOOS == "windows" {

return "npipe:////./pipe/docker_engine"

}

return "unix:///var/run/docker.sock"

}

host := cli.DaemonHost()

cli.Close()

return host

}

sourceant · 2026-02-20T09:25:05Z

internal/api/server.go

+		_ = os.Remove(tmpName)
+	}()
+
+	if _, err := tmp.WriteString(content); err != nil {


The current implementation writes to the file, syncs, closes it, and then re-opens it via the docker compose CLI. While functional, it's cleaner to handle errors on WriteString and Sync more strictly if you want to ensure validation occurs. More importantly, since the file is already closed on line 2853, the deferred tmp.Close() on line 2841 will return an error (which is ignored), but it's redundant.

Suggested change

if _, err := tmp.WriteString(content); err != nil {

if _, err := tmp.WriteString(content); err != nil {

return fmt.Errorf("failed to write temp file for validation: %w", err)

}

if err := tmp.Close(); err != nil {

return fmt.Errorf("failed to close temp file: %w", err)

}

nfebe

Nice work, thanks for this a few things to address:

The PR description mentions adding moby/moby but it's not in the diff. Let's use the official Docker Go SDK (github.com/docker/docker/client) and compose-go SDK (github.com/compose-spec/compose-go) instead of shelling out to the CLI they handle socket detection, context resolution, and compose validation natively.
None of the new exec.Command calls have timeouts a hanging Docker daemon blocks the agent indefinitely.
./deployment in the example config should be an absolute path outside the code directory and naming should be consistent with .gitignore.

nfebe · 2026-02-20T11:40:45Z

cmd/agent/main.go

 	_ = apiServer.Stop()
 }

+func ensureDockerReachable(_ string) {


The os.Setenv("DOCKER_HOST") above unconditionally overwrites the env var even when empty, and this function accepts a socket param but ignores it (_ string). There is also no timeout a hanging daemon blocks startup forever.

Let's use the Docker Go SDK instead: client.NewClientWithOpts(client.FromEnv) + client.Ping(ctx) handles host resolution, timeouts via context, and doesn't need the docker binary in PATH.

We can use github.com/compose-spec/compose-go just did not want add to do just one thing

nfebe · 2026-02-20T11:40:45Z

internal/api/server.go

 		}
 	}

+	if err := validateComposeWithCLI(content); err != nil {


Let's replace this with the compose-go SDK (github.com/compose-spec/compose-go/loader). It validates compose files in-process no temp files, no subprocess per request, proper Go error types, and no dependency on docker compose CLI being installed.

As-is this also has no timeout, so a hung daemon blocks the request handler.

nfebe · 2026-02-20T11:40:45Z

config.example.yml

@@ -1,4 +1,4 @@
-deployments_path: /home/nfebe/work/deployments
+deployments_path: ./deployment


./deployment (singular) doesn't match .gitignore which has deployments/ (plural). Also a relative path puts deployment data inside the project directory — something like /var/lib/flatrun/deployments would be a better default.

I think we should stick to ./deployments, /var/lib/flatrun/deployments will require permission. If the user then wants to keep in that directory, they should create it. But for now lets stick to the ./deployments

nfebe · 2026-02-20T11:40:45Z

pkg/config/config.go

 	}
 }

+func detectDockerHost() string {


Running docker context inspect as a side-effect of config.Load() makes config loading slower, silently broken if docker isn't installed, and harder to test.

The Docker Go SDK handles this natively — client.NewClientWithOpts(client.FromEnv) resolves DOCKER_HOST, Docker context, and platform defaults automatically. We can drop detectDockerHost() and dockerContextHost() entirely.

I think if we have to rely on the DOCKER_HOST from client.FromEnv, then we will have to remove the socket from the config. It is kind of misleading in a way, because it doesn't get used in the load config.

austin047 · 2026-02-20T12:32:24Z

The Docker client has been migrated to moby/moby, even though you can still use the previous one. You can check out this moby/moby#50472 , I evaluated it, and we could safely use moby/moby as of this date and when the migration was done. You can have a look and then we settle.

nfebe · 2026-02-21T13:59:33Z

The Docker client has been migrated to moby/moby, even though you can still use the previous one. You can check out this moby/moby#50472 , I evaluated it, and we could safely use moby/moby as of this date and when the migration was done. You can have a look and then we settle.

Thanks for this! But you did not add moby/moby so it will be good if we introduce it.

sourceant

Review complete. See the overview comment for a summary.

sourceant · 2026-02-24T15:54:58Z

cmd/agent/main.go

 	_ = apiServer.Stop()
 }

+func ensureDockerReachable(_ string) {


The previous review suggested using the Moby client library's Ping() or Info() instead of exec.Command("docker", "info"). This implementation still uses the shell command, which depends on the CLI being installed in the PATH and carries more overhead than an API call.

Suggested change

func ensureDockerReachable(_ string) {

func ensureDockerReachable(socket string) {

log.Println("Checking if Docker is reachable...")

ctx := context.Background()

cli, err := client.NewClientWithOpts(client.WithHost(socket), client.WithAPIVersionNegotiation())

if err != nil {

log.Fatalf("Failed to create Docker client: %v", err)

}

defer cli.Close()

if _, err := cli.Ping(ctx); err != nil {

log.Fatalf("Docker is not reachable: %v", err)

}

log.Println("Docker is reachable")

}

sourceant

Review complete. See the overview comment for a summary.

sourceant · 2026-02-24T16:03:48Z

pkg/config/config.go

+	return "unix:///var/run/docker.sock"
+}
+
+func dockerContextHost() string {


Relying on docker context inspect via exec.Command adds unnecessary overhead and a dependency on the CLI binary. Use the Moby client's environment detection logic instead, which is already designed to handle contexts and platform defaults correctly.

Suggested change

func dockerContextHost() string {

func detectDockerHost() string {

cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation())

if err != nil {

if runtime.GOOS == "windows" {

return "npipe:////./pipe/docker_engine"

}

return "unix:///var/run/docker.sock"

}

host := cli.DaemonHost()

cli.Close()

return host

}

austin047 · 2026-02-24T16:22:01Z

We will have to use the CLI to detect the host. The default implementation of the Moby client always falls back to platform-specific paths, and in some cases, the current context is the default OS socket context. For example, on my system the default path is unix:///home/fuh-austin/.docker/desktop/docker.sock. However, when DOCKER_HOST is not set in the environment, the Moby client defaults to using unix:///var/run/docker.sock. This behavior is hardcoded in the SDK.

So for now, using the CLI for this particular operation before setting DOCKER_HOST is the most optimal solution we have at the moment.

fix: improve Docker startup and compose validation

2ad91e8

sourceant bot reviewed Feb 18, 2026

View reviewed changes

austin047 marked this pull request as draft February 18, 2026 23:42

refactor: Use moby client for Docker ping and harden compose validation

6a3e0e6

austin047 changed the title ~~fix: improve Docker startup and compose validation~~ Fix: improve Docker startup and compose validation Feb 19, 2026

austin047 marked this pull request as ready for review February 19, 2026 00:49

sourceant bot reviewed Feb 19, 2026

View reviewed changes

austin047 marked this pull request as draft February 19, 2026 00:49

austin047 added 2 commits February 19, 2026 12:59

refactor: Drop moby client and improve Docker socket detection

0ee7627

fix: Test for docker compose file

0c3283e

austin047 marked this pull request as ready for review February 20, 2026 09:24

sourceant bot reviewed Feb 20, 2026

View reviewed changes

nfebe requested changes Feb 20, 2026

View reviewed changes

feat: Implement compose go

dd4200b

sourceant bot reviewed Feb 24, 2026

View reviewed changes

fix: Remove unused packages

cf82f53

sourceant bot reviewed Feb 24, 2026

View reviewed changes

		@@ -42,6 +43,15 @@ func main() {
		log.Fatalf("Failed to load config from %s: %v", resolvedConfigPath, err)
		}

-	}
+func ensureDockerReachable(socket string) {
+	ctx := context.Background()
+	cli, err := client.NewClientWithOpts(client.WithHost(socket), client.WithAPIVersionNegotiation())
+	if err != nil {
+		log.Fatalf("Failed to create Docker client: %v", err)
+	}
+	defer cli.Close()
+	if _, err := cli.Ping(ctx); err != nil {
+		log.Fatalf("Docker is not reachable: %v", err)
+	}
+}

-func validateComposeWithCLI(content string) error {
+	tmp, err := os.CreateTemp("", "compose-*.yml")
+	if err != nil {
+		log.Printf("Warning: skipping CLI validation, failed to create temp file: %v", err)
+		return nil
+	}
+	defer os.Remove(tmp.Name())
+	defer tmp.Close()

	os.Setenv("DOCKER_HOST", cfg.DockerSocket)
	ensureDockerReachable(cfg.DockerSocket)

-		apiClient, err := client.New(client.FromEnv)
+		cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation())
+		if err == nil {
+			cfg.DockerSocket = cli.DaemonHost()
+			cli.Close()
+		} else {
+			cfg.DockerSocket = "unix:///var/run/docker.sock"
+		}

-	if _, err := tmp.WriteString(content); err != nil {
+	if _, err := tmp.WriteString(content); err != nil {
+		return fmt.Errorf("failed to write temp file for validation: %w", err)
+	}
+	if err := tmp.Close(); err != nil {
+		return fmt.Errorf("failed to close temp file: %w", err)
+	}

		@@ -1,4 +1,4 @@
		deployments_path: /home/nfebe/work/deployments
		deployments_path: ./deployment

Conversation

austin047 commented Feb 18, 2026

Uh oh!

sourceant bot commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Summary

🚀 Key Improvements

💡 Minor Suggestions

🚨 Critical Issues

Uh oh!

sourceant bot left a comment

Choose a reason for hiding this comment

Uh oh!

sourceant bot Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

sourceant bot Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

sourceant bot left a comment

Choose a reason for hiding this comment

Uh oh!

sourceant bot Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

sourceant bot Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

sourceant bot left a comment

Choose a reason for hiding this comment

Uh oh!

sourceant bot Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

sourceant bot Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

nfebe left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nfebe Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

austin047 Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

nfebe Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nfebe Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

austin047 Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

nfebe Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

austin047 Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

austin047 commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nfebe commented Feb 21, 2026

Uh oh!

sourceant bot left a comment

Choose a reason for hiding this comment

Uh oh!

sourceant bot Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

sourceant bot left a comment

Choose a reason for hiding this comment

Uh oh!

sourceant bot Feb 24, 2026

Choose a reason for hiding this comment

sourceant bot commented Feb 18, 2026 •

edited

Loading

nfebe left a comment •

edited

Loading

nfebe Feb 20, 2026 •

edited

Loading

nfebe Feb 20, 2026 •

edited

Loading

austin047 commented Feb 20, 2026 •

edited

Loading