Merge pull request #160 from flatrun/feat/albacore-quick-wins

feat(agent): Improve nginx vhost generation, CLI help, and service discovery

Author: nfebe

cmd/agent/main.go

@@ -7,6 +7,7 @@ import (
 	"log"
 	"os"
 	"os/signal"
+	"strings"
 	"syscall"
 	"time"
 
@@ -16,42 +17,112 @@ import (
 	"github.com/flatrun/agent/pkg/updater"
 	"github.com/flatrun/agent/pkg/version"
 	"github.com/moby/moby/client"
+	"github.com/spf13/cobra"
 )
 
 func main() {
-	if len(os.Args) > 1 {
-		switch os.Args[1] {
-		case "update":
-			handleUpdate(os.Args[2:])
-			return
-		case "setup":
-			handleSetup(os.Args[2:])
-			return
-		case "version":
-			printVersion()
-			return
+	root := newRootCmd()
+	root.SetArgs(normalizeLegacyFlags(os.Args[1:]))
+	if err := root.Execute(); err != nil {
+		os.Exit(1)
+	}
+}
+
+// normalizeLegacyFlags keeps the single-dash long forms the previous flag-based
+// CLI accepted (-config, -version) working under pflag, which otherwise reads a
+// single dash as a cluster of shorthand flags. This preserves existing launch
+// commands such as `flatrun-agent -config /etc/flatrun/config.yml`.
+func normalizeLegacyFlags(args []string) []string {
+	out := make([]string, 0, len(args))
+	for _, a := range args {
+		if a == "-config" || a == "-version" ||
+			strings.HasPrefix(a, "-config=") || strings.HasPrefix(a, "-version=") {
+			a = "-" + a
 		}
+		out = append(out, a)
 	}
+	return out
+}
 
-	configPath := flag.String("config", "", "Path to configuration file")
-	showVersion := flag.Bool("version", false, "Print version information")
-	flag.Parse()
+func newRootCmd() *cobra.Command {
+	var configPath string
+	var showVersion bool
 
-	if *showVersion {
-		printVersion()
-		return
+	root := &cobra.Command{
+		Use:          "flatrun-agent",
+		Short:        "Flatrun Agent - flat-file container orchestration",
+		SilenceUsage: true,
+		RunE: func(cmd *cobra.Command, args []string) error {
+			if showVersion {
+				printVersion()
+				return nil
+			}
+			// A bare invocation is most likely someone looking for guidance,
+			// not an attempt to start the daemon: the service is always launched
+			// with --config. Show help rather than failing on a missing default
+			// config. Pass --config (or use `serve`) to actually start.
+			if !cmd.Flags().Changed("config") {
+				return cmd.Help()
+			}
+			return runServer(configPath)
+		},
+	}
+	root.CompletionOptions.DisableDefaultCmd = true
+	root.PersistentFlags().StringVar(&configPath, "config", "", "Path to configuration file")
+	root.Flags().BoolVar(&showVersion, "version", false, "Print version information")
+
+	serve := &cobra.Command{
+		Use:          "serve",
+		Short:        "Start the agent server",
+		SilenceUsage: true,
+		RunE: func(cmd *cobra.Command, args []string) error {
+			return runServer(configPath)
+		},
 	}
 
-	resolvedConfigPath := config.FindConfigPath(*configPath)
+	setup := &cobra.Command{
+		Use:                "setup <target> <service> [options]",
+		Short:              "Deploy an infrastructure service from embedded templates",
+		DisableFlagParsing: true,
+		Run: func(cmd *cobra.Command, args []string) {
+			handleSetup(args)
+		},
+	}
+
+	update := &cobra.Command{
+		Use:                "update [options]",
+		Short:              "Update the agent to the latest version",
+		DisableFlagParsing: true,
+		Run: func(cmd *cobra.Command, args []string) {
+			handleUpdate(args)
+		},
+	}
+
+	versionCmd := &cobra.Command{
+		Use:   "version",
+		Short: "Print version information",
+		Run: func(cmd *cobra.Command, args []string) {
+			printVersion()
+		},
+	}
+
+	root.AddCommand(serve, setup, update, versionCmd)
+	return root
+}
+
+func runServer(configPath string) error {
+	resolvedConfigPath := config.FindConfigPath(configPath)
 	cfg, err := config.Load(resolvedConfigPath)
 	if err != nil {
-		log.Fatalf("Failed to load config from %s: %v", resolvedConfigPath, err)
+		return fmt.Errorf("failed to load config from %s: %w", resolvedConfigPath, err)
 	}
 
-	ensureDockerReachable(cfg.DockerSocket)
+	if err := ensureDockerReachable(cfg.DockerSocket); err != nil {
+		return err
+	}
 
 	if err := os.MkdirAll(cfg.DeploymentsPath, 0755); err != nil {
-		log.Fatalf("Failed to create deployments directory '%s': %v", cfg.DeploymentsPath, err)
+		return fmt.Errorf("failed to create deployments directory %q: %w", cfg.DeploymentsPath, err)
 	}
 
 	log.Printf("Starting Flatrun Agent v%s", version.Version)
@@ -61,7 +132,7 @@ func main() {
 
 	fileWatcher, err := watcher.New(cfg.DeploymentsPath)
 	if err != nil {
-		log.Fatalf("Failed to create file watcher: %v", err)
+		return fmt.Errorf("failed to create file watcher: %w", err)
 	}
 	defer fileWatcher.Close()
 
@@ -80,9 +151,10 @@ func main() {
 
 	log.Println("Shutting down Flatrun Agent...")
 	_ = apiServer.Stop()
+	return nil
 }
 
-func ensureDockerReachable(dockerHost string) {
+func ensureDockerReachable(dockerHost string) error {
 	log.Println("Checking if Docker is reachable...")
 
 	opts := []client.Opt{client.FromEnv}
@@ -92,19 +164,18 @@ func ensureDockerReachable(dockerHost string) {
 
 	cli, err := client.New(opts...)
 	if err != nil {
-		log.Fatalf("Failed to create Docker client: %v", err)
+		return fmt.Errorf("failed to create Docker client: %w", err)
 	}
 	defer cli.Close()
 
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
 
 	if _, err := cli.Ping(ctx, client.PingOptions{}); err != nil {
-		log.Fatalf("Docker is not reachable: %v. "+
-			"Ensure the Docker daemon is running and the socket "+
-			"in config is correct.", err)
+		return fmt.Errorf("docker is not reachable: %w. Ensure the Docker daemon is running and the socket in config is correct", err)
 	}
 	log.Println("Docker is reachable")
+	return nil
 }
 
 func printVersion() {

cmd/agent/main_test.go

@@ -0,0 +1,60 @@
+package main
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+)
+
+func TestRootCmd_BareShowsHelp(t *testing.T) {
+	cmd := newRootCmd()
+	var out bytes.Buffer
+	cmd.SetOut(&out)
+	cmd.SetErr(&out)
+	cmd.SetArgs([]string{})
+
+	if err := cmd.Execute(); err != nil {
+		t.Fatalf("bare invocation should not error, got: %v", err)
+	}
+
+	got := out.String()
+	for _, want := range []string{"serve", "setup", "update", "version"} {
+		if !strings.Contains(got, want) {
+			t.Errorf("help output missing %q command, got:\n%s", want, got)
+		}
+	}
+}
+
+func TestNormalizeLegacyFlags(t *testing.T) {
+	tests := []struct {
+		name string
+		in   []string
+		want []string
+	}{
+		{"single-dash config", []string{"-config", "/etc/x.yml"}, []string{"--config", "/etc/x.yml"}},
+		{"single-dash config equals", []string{"-config=/etc/x.yml"}, []string{"--config=/etc/x.yml"}},
+		{"single-dash version", []string{"-version"}, []string{"--version"}},
+		{"double-dash untouched", []string{"--config", "/etc/x.yml"}, []string{"--config", "/etc/x.yml"}},
+		{"subcommand untouched", []string{"setup", "infra", "nginx"}, []string{"setup", "infra", "nginx"}},
+		{"unrelated short flag untouched", []string{"update", "-check"}, []string{"update", "-check"}},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := normalizeLegacyFlags(tt.in)
+			if strings.Join(got, " ") != strings.Join(tt.want, " ") {
+				t.Errorf("normalizeLegacyFlags(%v) = %v, want %v", tt.in, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestRootCmd_RegistersSubcommands(t *testing.T) {
+	cmd := newRootCmd()
+	for _, name := range []string{"serve", "setup", "update", "version"} {
+		sub, _, err := cmd.Find([]string{name})
+		if err != nil || sub.Name() != name {
+			t.Errorf("expected subcommand %q to be registered, got %v (err %v)", name, sub, err)
+		}
+	}
+}

go.mod

@@ -22,6 +22,7 @@ require (
 	github.com/lib/pq v1.10.9
 	github.com/moby/moby/client v0.2.2
 	github.com/robfig/cron/v3 v3.0.1
+	github.com/spf13/cobra v1.10.2
 	github.com/testcontainers/testcontainers-go v0.41.0
 	github.com/testcontainers/testcontainers-go/modules/compose v0.41.0
 	golang.org/x/crypto v0.48.0
@@ -152,7 +153,6 @@ require (
 	github.com/sigstore/sigstore-go v1.1.4-0.20251124094504-b5fe07a5a7d7 // indirect
 	github.com/sirupsen/logrus v1.9.4 // indirect
 	github.com/skratchdot/open-golang v0.0.0-20200116055534-eef842397966 // indirect
-	github.com/spf13/cobra v1.10.2 // indirect
 	github.com/spf13/pflag v1.0.10 // indirect
 	github.com/stretchr/testify v1.11.1 // indirect
 	github.com/tilt-dev/fsnotify v1.4.8-0.20220602155310-fff9c274a375 // indirect

internal/docker/discovery.go

@@ -582,7 +582,9 @@ func (d *Discovery) UpdateComposeFile(name string, content string) error {
 				if newMeta.Networking.Service != "" {
 					existing.Networking.Service = newMeta.Networking.Service
 				}
-				d.SaveMetadata(name, existing)
+				if err := d.SaveMetadata(name, existing); err != nil {
+					return fmt.Errorf("failed to sync service metadata: %w", err)
+				}
 			}
 		}
 	}
@@ -665,13 +667,20 @@ func (d *Discovery) generateMetadataFromCompose(composePath, name string) *model
 
 // pickPrimaryService selects the service to use for networking metadata.
 // For single-service composes, returns that service. For multi-service,
-// returns the first service with exposed ports.
+// it prefers a service named "app" or "web" that exposes ports, then falls
+// back to the first service with exposed ports. The preference keeps the
+// selection deterministic, since Go map iteration order is random.
 func (d *Discovery) pickPrimaryService(services map[string]composeService) (string, composeService) {
 	if len(services) == 1 {
 		for name, svc := range services {
 			return name, svc
 		}
 	}
+	for name, svc := range services {
+		if (name == "app" || name == "web") && (len(svc.Ports) > 0 || len(svc.Expose) > 0) {
+			return name, svc
+		}
+	}
 	for name, svc := range services {
 		if len(svc.Ports) > 0 || len(svc.Expose) > 0 {
 			return name, svc

internal/docker/discovery_test.go

@@ -498,6 +498,26 @@ func TestGenerateMetadataFromCompose_ServiceName(t *testing.T) {
 	}
 }
 
+func TestPickPrimaryService_PrefersAppOrWeb(t *testing.T) {
+	d := NewDiscovery(t.TempDir())
+
+	withPorts := composeService{Ports: []interface{}{"80:80"}}
+	services := map[string]composeService{
+		"queue": withPorts,
+		"app":   withPorts,
+		"redis": withPorts,
+	}
+
+	// Map iteration order is random, so a service with ports could be returned
+	// in any order. Repeat to confirm "app" is selected deterministically.
+	for i := 0; i < 100; i++ {
+		name, _ := d.pickPrimaryService(services)
+		if name != "app" {
+			t.Fatalf("pickPrimaryService = %q, want \"app\"", name)
+		}
+	}
+}
+
 func TestGenerateMetadataFromCompose_Expose(t *testing.T) {
 	tests := []struct {
 		name     string