-
Notifications
You must be signed in to change notification settings - Fork 1
Fix: improve Docker startup and compose validation #78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
2ad91e8
6a3e0e6
0ee7627
0c3283e
dd4200b
cf82f53
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,6 +5,7 @@ import ( | |||||||||||||||||||||||||||||
| "fmt" | ||||||||||||||||||||||||||||||
| "log" | ||||||||||||||||||||||||||||||
| "os" | ||||||||||||||||||||||||||||||
| "os/exec" | ||||||||||||||||||||||||||||||
| "os/signal" | ||||||||||||||||||||||||||||||
| "syscall" | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
@@ -42,6 +43,15 @@ func main() { | |||||||||||||||||||||||||||||
| log.Fatalf("Failed to load config from %s: %v", resolvedConfigPath, err) | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| os.Setenv("DOCKER_HOST", cfg.DockerSocket) | ||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Setting
Suggested change
|
||||||||||||||||||||||||||||||
| log.Printf("Using Docker socket from config: %s", cfg.DockerSocket) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| ensureDockerReachable(cfg.DockerSocket) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if err := os.MkdirAll(cfg.DeploymentsPath, 0755); err != nil { | ||||||||||||||||||||||||||||||
| log.Fatalf("Failed to create deployments directory '%s': %v", cfg.DeploymentsPath, err) | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| log.Printf("Starting Flatrun Agent v%s", version.Version) | ||||||||||||||||||||||||||||||
| log.Printf("Config loaded from: %s", resolvedConfigPath) | ||||||||||||||||||||||||||||||
| log.Printf("Deployments path: %s", cfg.DeploymentsPath) | ||||||||||||||||||||||||||||||
|
|
@@ -70,6 +80,15 @@ func main() { | |||||||||||||||||||||||||||||
| _ = apiServer.Stop() | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| func ensureDockerReachable(_ string) { | ||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Let's use the Docker Go SDK instead:
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The previous review suggested using the Moby client library's
Suggested change
|
||||||||||||||||||||||||||||||
| log.Println("Checking if Docker is reachable...") | ||||||||||||||||||||||||||||||
| cmd := exec.Command("docker", "info") | ||||||||||||||||||||||||||||||
| if _, err := cmd.CombinedOutput(); err != nil { | ||||||||||||||||||||||||||||||
| log.Fatalf("Docker is not reachable: Ensure the Docker daemon is running and docker socket in config is correct (e.g. unix:///var/run/docker.sock).") | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| log.Println("Docker is reachable") | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| func printVersion() { | ||||||||||||||||||||||||||||||
| info := version.Get() | ||||||||||||||||||||||||||||||
| fmt.Printf("Flatrun Agent\n") | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| package api | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/flatrun/agent/pkg/config" | ||
| ) | ||
|
|
||
| // Tests use compose-go for validation - no Docker required. | ||
|
|
||
| func TestValidateComposeWithComposeGo_ValidCompose(t *testing.T) { | ||
| validCompose := `name: test | ||
| services: | ||
| app: | ||
| image: nginx:alpine | ||
| ` | ||
| err := validateComposeWithComposeGo(validCompose) | ||
| if err != nil { | ||
| t.Errorf("validateComposeWithComposeGo(valid compose) = %v, want nil", err) | ||
| } | ||
| } | ||
|
|
||
| func TestValidateComposeWithComposeGo_InvalidCompose(t *testing.T) { | ||
| // Invalid: image must be string, not number | ||
| invalidCompose := `name: test | ||
| services: | ||
| app: | ||
| image: 12345 | ||
| ` | ||
| err := validateComposeWithComposeGo(invalidCompose) | ||
| if err == nil { | ||
| t.Error("validateComposeWithComposeGo(invalid compose) = nil, want error") | ||
| } | ||
| if err != nil && !strings.Contains(err.Error(), "invalid compose") { | ||
| t.Errorf("validateComposeWithComposeGo error should mention 'invalid compose', got: %v", err) | ||
| } | ||
| } | ||
|
|
||
| func TestValidateComposeWithComposeGo_InvalidYAML(t *testing.T) { | ||
| invalidCompose := `name: test | ||
| services: | ||
| app: | ||
| image: [broken yaml | ||
| ` | ||
| err := validateComposeWithComposeGo(invalidCompose) | ||
| if err == nil { | ||
| t.Error("validateComposeWithComposeGo(invalid YAML) = nil, want error") | ||
| } | ||
| } | ||
|
|
||
| func TestValidateComposeContent_ValidCompose_Integration(t *testing.T) { | ||
| cfg := &config.Config{ | ||
| Infrastructure: config.InfrastructureConfig{ | ||
| DefaultProxyNetwork: "proxy", | ||
| }, | ||
| } | ||
| s := &Server{config: cfg} | ||
|
|
||
| validCompose := `name: test | ||
| services: | ||
| app: | ||
| image: nginx:alpine | ||
| networks: | ||
| - proxy | ||
| networks: | ||
| proxy: | ||
| external: true | ||
| ` | ||
| err := s.validateComposeContent(validCompose, "test") | ||
| if err != nil { | ||
| t.Errorf("validateComposeContent(valid) = %v, want nil", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
exec.Command("docker", "info")relies on thedockerCLI being installed in the PATH. Since you've added themoby/mobyclient dependency, it is more efficient and reliable to check connectivity using the library'sPing()orInfo()methods instead of spawning a shell process.