fix: Caddy config regeneration due to non-deterministic container serialisation (fixes #111)

internal/machine/caddyconfig/caddyfile.go

@@ -19,7 +19,7 @@ import (
 )
 
 const (
-	caddyfileHeaderFmt = `# Caddyfile autogenerated by Uncloud (DO NOT EDIT): %s
+	caddyfileHeaderFmt = `# Caddyfile autogenerated by Uncloud on machine '%s' (DO NOT EDIT): %s
 # Automatically updated on service or health status changes.
 # Docs: https://uncloud.run/docs/concepts/ingress/overview
 `
@@ -68,8 +68,10 @@ https://{{$hostname}} {
 type CaddyfileGenerator struct {
 	// machineID is the unique identifier of the machine where the controller is running.
 	machineID string
-	validator CaddyfileValidator
-	log       *slog.Logger
+	// machineName is the human-friendly name of the machine.
+	machineName string
+	validator   CaddyfileValidator
+	log         *slog.Logger
 }
 
 // CaddyfileValidator is an interface for validating Caddyfile configurations.
@@ -77,14 +79,17 @@ type CaddyfileValidator interface {
 	Validate(ctx context.Context, caddyfile string) error
 }
 
-func NewCaddyfileGenerator(machineID string, validator CaddyfileValidator, log *slog.Logger) *CaddyfileGenerator {
+func NewCaddyfileGenerator(
+	machineID, machineName string, validator CaddyfileValidator, log *slog.Logger,
+) *CaddyfileGenerator {
 	if log == nil {
 		log = slog.Default()
 	}
 	return &CaddyfileGenerator{
-		machineID: machineID,
-		validator: validator,
-		log:       log,
+		machineID:   machineID,
+		machineName: machineName,
+		validator:   validator,
+		log:         log,
 	}
 }
 
@@ -124,7 +129,7 @@ func (g *CaddyfileGenerator) Generate(
 		return "", fmt.Errorf("generate base Caddyfile from service ports: %w", err)
 	}
 
-	caddyfileHeader := fmt.Sprintf(caddyfileHeaderFmt, time.Now().UTC().Format(time.RFC3339))
+	caddyfileHeader := fmt.Sprintf(caddyfileHeaderFmt, g.machineName, time.Now().UTC().Format(time.RFC3339))
 	if !includeCustom {
 		return fmt.Sprintf("%s\n%s\n%s", caddyfileHeader, caddyfile, caddyfileUnavailabeFooter), nil
 	}

internal/machine/caddyconfig/caddyfile_test.go

@@ -26,7 +26,7 @@ func normaliseGeneratedTimestamp(caddyfile string) string {
 	return generatedTimestampRegex.ReplaceAllString(caddyfile, "(DO NOT EDIT): TIMESTAMP_PLACEHOLDER")
 }
 
-const testCaddyfileHeader = `# Caddyfile autogenerated by Uncloud (DO NOT EDIT): TIMESTAMP_PLACEHOLDER
+const testCaddyfileHeader = `# Caddyfile autogenerated by Uncloud on machine 'test-machine' (DO NOT EDIT): TIMESTAMP_PLACEHOLDER
 # Automatically updated on service or health status changes.
 # Docs: https://uncloud.run/docs/concepts/ingress/overview
 
@@ -190,7 +190,7 @@ https://secure.example.com {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			// Validator is not expected to be called in these tests.
-			generator := NewCaddyfileGenerator("test-machine-id", nil, nil)
+			generator := NewCaddyfileGenerator("test-machine-id", "test-machine", nil, nil)
 
 			config, err := generator.Generate(ctx, tt.containers, true)
 
@@ -226,7 +226,7 @@ func TestCaddyfileGeneratorWithCustomConfigs(t *testing.T) {
 					time.Now(),
 				),
 			},
-			want: `# Caddyfile autogenerated by Uncloud (DO NOT EDIT): TIMESTAMP_PLACEHOLDER
+			want: `# Caddyfile autogenerated by Uncloud on machine 'test-machine' (DO NOT EDIT): TIMESTAMP_PLACEHOLDER
 # Automatically updated on service or health status changes.
 # Docs: https://uncloud.run/docs/concepts/ingress/overview
 
@@ -423,7 +423,7 @@ web.example.com {
 					time.Now(),
 				),
 			},
-			want: `# Caddyfile autogenerated by Uncloud (DO NOT EDIT): TIMESTAMP_PLACEHOLDER
+			want: `# Caddyfile autogenerated by Uncloud on machine 'test-machine' (DO NOT EDIT): TIMESTAMP_PLACEHOLDER
 # Automatically updated on service or health status changes.
 # Docs: https://uncloud.run/docs/concepts/ingress/overview
 
@@ -661,7 +661,7 @@ badconfig.com {
 					time.Now(),
 				),
 			},
-			want: `# Caddyfile autogenerated by Uncloud (DO NOT EDIT): TIMESTAMP_PLACEHOLDER
+			want: `# Caddyfile autogenerated by Uncloud on machine 'test-machine' (DO NOT EDIT): TIMESTAMP_PLACEHOLDER
 # Automatically updated on service or health status changes.
 # Docs: https://uncloud.run/docs/concepts/ingress/overview
 
@@ -834,7 +834,7 @@ valid.example.com {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			generator := NewCaddyfileGenerator("test-machine-id", validator, nil)
+			generator := NewCaddyfileGenerator("test-machine-id", "test-machine", validator, nil)
 
 			config, err := generator.Generate(ctx, tt.containers, true)
 
@@ -976,7 +976,7 @@ http://api.example.com {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			// Validator is not expected to be called in these tests.
-			generator := NewCaddyfileGenerator("test-machine-id", nil, nil)
+			generator := NewCaddyfileGenerator("test-machine-id", "test-machine", nil, nil)
 
 			config, err := generator.Generate(ctx, tt.containers, false)
 			require.NoError(t, err)

internal/machine/caddyconfig/client.go

@@ -34,21 +34,16 @@ func NewCaddyAdminClient(socketPath string) *CaddyAdminClient {
 	}
 }
 
-// IsAvailable checks if the local Caddy instance is running and responding to admin API requests.
-func (c *CaddyAdminClient) IsAvailable(ctx context.Context) bool {
-	// Caddy doesn't serve a /ping endpoint. It's a random endpoint we can use to check if Caddy is running.
-	req, err := http.NewRequestWithContext(ctx, "GET", "http://localhost/ping", nil)
+// IsAvailable checks if the local Caddy instance is listening on the admin socket.
+func (c *CaddyAdminClient) IsAvailable() bool {
+	conn, err := net.DialTimeout("unix", c.socketPath, 1*time.Second)
+	// A stale socket file left over from a crashed Caddy container returns ECONNREFUSED so this is correctly handled
+	// as unavailable.
 	if err != nil {
 		return false
 	}
+	conn.Close()
 
-	resp, err := c.client.Do(req)
-	if err != nil {
-		return false
-	}
-	defer resp.Body.Close()
-
-	// Any HTTP response means Caddy is running and accessible.
 	return true
 }
 

internal/machine/caddyconfig/controller.go

@@ -5,8 +5,11 @@ import (
 	"encoding/json"
 	"fmt"
 	"log/slog"
+	"net/netip"
 	"os"
 	"path/filepath"
+	"slices"
+	"strings"
 
 	"github.com/psviderski/uncloud/internal/fs"
 	"github.com/psviderski/uncloud/internal/machine/store"
@@ -29,6 +32,28 @@ type Controller struct {
 	client        *CaddyAdminClient
 	store         *store.Store
 	log           *slog.Logger
+	// lastFingerprint caches the fingerprint of the containers used to generate the latest successfully loaded
+	// Caddyfile. nil means it hasn't been loaded yet or the last load failed.
+	lastFingerprint []containerFingerprint
+	// lastCaddyfile caches the last generated Caddyfile.
+	lastCaddyfile string
+}
+
+// containerFingerprint is the subset of container data that the Caddyfile generator depends on.
+// Comparing fingerprints lets the controller skip no-op regenerations.
+type containerFingerprint struct {
+	ID          string
+	IP          netip.Addr
+	Ports       []api.PortSpec
+	CaddyConfig string
+}
+
+// Equal returns whether two fingerprints describe the same container input to the Caddyfile generator.
+func (f containerFingerprint) Equal(other containerFingerprint) bool {
+	return f.ID == other.ID &&
+		f.IP == other.IP &&
+		api.PortsEqual(f.Ports, other.Ports) &&
+		f.CaddyConfig == other.CaddyConfig
 }
 
 func NewController(machineID, configDir, adminSock string, store *store.Store) (*Controller, error) {
@@ -41,12 +66,11 @@ func NewController(machineID, configDir, adminSock string, store *store.Store) (
 
 	log := slog.With("component", "caddy-controller")
 	client := NewCaddyAdminClient(adminSock)
-	generator := NewCaddyfileGenerator(machineID, client, log)
 
+	// generator is initialised by Run() once the machine name is resolved from the store.
 	return &Controller{
 		machineID:     machineID,
 		caddyfilePath: filepath.Join(configDir, "Caddyfile"),
-		generator:     generator,
 		client:        client,
 		store:         store,
 		log:           log,
@@ -54,6 +78,17 @@ func NewController(machineID, configDir, adminSock string, store *store.Store) (
 }
 
 func (c *Controller) Run(ctx context.Context) error {
+	// Default the machine name to the machine ID so the Caddyfile header still carries a stable identifier if
+	// the store lookup fails.
+	machineName := c.machineID
+	if m, err := c.store.GetMachine(ctx, c.machineID); err != nil {
+		c.log.Error("Failed to get machine from store, Caddy configuration will use machine ID as the name.",
+			"machine_id", c.machineID, "err", err)
+	} else {
+		machineName = m.Name
+	}
+	c.generator = NewCaddyfileGenerator(c.machineID, machineName, c.client, c.log)
+
 	containers, changes, err := c.store.SubscribeContainers(ctx)
 	if err != nil {
 		return fmt.Errorf("subscribe to container changes: %w", err)
@@ -74,7 +109,7 @@ func (c *Controller) Run(ctx context.Context) error {
 			if !ok {
 				return fmt.Errorf("containers subscription failed")
 			}
-			c.log.Info("Cluster containers changed, updating Caddy configuration.")
+			c.log.Debug("Cluster containers changed, regenerating Caddy configuration.")
 
 			containers, err = c.store.ListContainers(ctx, store.ListOptions{})
 			if err != nil {
@@ -111,9 +146,20 @@ func filterHealthyContainers(containers []store.ContainerRecord) []store.Contain
 	return healthy
 }
 
+// generateAndLoadCaddyfile regenerates the Caddyfile from the given containers and loads it into the local Caddy
+// if available.
 func (c *Controller) generateAndLoadCaddyfile(ctx context.Context, containers []store.ContainerRecord) {
 	// Check if Caddy is available before attempting to generate and load config.
-	caddyAvailable := c.client.IsAvailable(ctx)
+	caddyAvailable := c.client.IsAvailable()
+
+	// Skip regeneration when Caddy is available and the containers since the last successful load haven't changed.
+	// When Caddy is unavailable we still regenerate to keep the Caddyfile on disk updated.
+	fingerprint := fingerprintContainers(containers)
+	if caddyAvailable && slices.EqualFunc(fingerprint, c.lastFingerprint, containerFingerprint.Equal) {
+		c.log.Debug("Caddy configuration is unchanged.", "path", c.caddyfilePath)
+		return
+	}
+
 	caddyfile, err := c.generator.Generate(ctx, containers, caddyAvailable)
 	if err != nil {
 		c.log.Error("Failed to generate Caddyfile configuration.", "err", err)
@@ -123,7 +169,7 @@ func (c *Controller) generateAndLoadCaddyfile(ctx context.Context, containers []
 	if !caddyAvailable {
 		// Caddy is not running so the generated Caddyfile should not include user-defined configs thus must be valid.
 		// It's safe to write the config to disk so that when Caddy is deployed on this machine, it can pick it up.
-		if err = c.writeCaddyfile(caddyfile); err != nil {
+		if err = c.writeCaddyfileIfChanged(caddyfile); err != nil {
 			c.log.Error("Failed to write Caddyfile to disk.", "err", err)
 			return
 		}
@@ -137,30 +183,71 @@ func (c *Controller) generateAndLoadCaddyfile(ctx context.Context, containers []
 	if err = c.client.Load(ctx, caddyfile); err != nil {
 		c.log.Error("Failed to load new Caddy configuration into local Caddy instance.",
 			"err", err, "path", c.caddyfilePath)
+		// Mark the cache stale so the next container change retries the load even if the container set is unchanged.
+		c.lastFingerprint = nil
 		// Don't write invalid config to disk.
 		return
 	}
+	c.lastFingerprint = fingerprint
 
 	// Config loaded successfully, now write it to disk.
-	if err = c.writeCaddyfile(caddyfile); err != nil {
+	if err = c.writeCaddyfileIfChanged(caddyfile); err != nil {
 		c.log.Error("Failed to write Caddyfile to disk after successful load.", "err", err)
-		// Config is already loaded in Caddy, so this is not critical.
+		// Config is already loaded in Caddy, so this is not critical. The next regeneration retries the disk write.
+		return
 	}
 
 	c.log.Info("New Caddy configuration loaded into local Caddy instance.", "path", c.caddyfilePath)
 }
 
-// writeCaddyfile writes the Caddyfile content to disk with proper permissions.
-func (c *Controller) writeCaddyfile(caddyfile string) error {
+// fingerprintContainers returns a fingerprint of containers that the Caddyfile generator depends on.
+func fingerprintContainers(containers []store.ContainerRecord) []containerFingerprint {
+	fingerprints := make([]containerFingerprint, len(containers))
+	for i, cr := range containers {
+		// Ignore ports parsing error as not much we can do about it. The generator just logs them and continues.
+		ports, _ := cr.Container.ServicePorts()
+		fingerprints[i] = containerFingerprint{
+			ID:          cr.Container.ID,
+			IP:          cr.Container.UncloudNetworkIP(),
+			Ports:       ports,
+			CaddyConfig: cr.Container.ServiceSpec.CaddyConfig(),
+		}
+	}
+	slices.SortFunc(fingerprints, func(a, b containerFingerprint) int {
+		return strings.Compare(a.ID, b.ID)
+	})
+
+	return fingerprints
+}
+
+// writeCaddyfileIfChanged writes the Caddyfile content to disk with proper permissions only if its body differs
+// from the last successfully written content. The first line of the Caddyfile carries a generation timestamp that
+// changes on every regeneration, so it's excluded from the comparison to avoid redundant writes.
+func (c *Controller) writeCaddyfileIfChanged(caddyfile string) error {
+	if caddyfileBody(caddyfile) == caddyfileBody(c.lastCaddyfile) {
+		return nil
+	}
+
 	if err := os.WriteFile(c.caddyfilePath, []byte(caddyfile), 0o640); err != nil {
 		return fmt.Errorf("write Caddyfile to file '%s': %w", c.caddyfilePath, err)
 	}
 	if err := fs.Chown(c.caddyfilePath, "", CaddyGroup); err != nil {
 		return fmt.Errorf("change owner of Caddyfile '%s': %w", c.caddyfilePath, err)
 	}
+	c.lastCaddyfile = caddyfile
+
 	return nil
 }
 
+// caddyfileBody returns the Caddyfile content without its first line, which carries a generation timestamp that
+// rotates on every regeneration.
+func caddyfileBody(caddyfile string) string {
+	if i := strings.IndexByte(caddyfile, '\n'); i >= 0 {
+		return caddyfile[i+1:]
+	}
+	return caddyfile
+}
+
 func (c *Controller) generateJSONConfig(containers []store.ContainerRecord) error {
 	serviceContainers := make([]api.ServiceContainer, len(containers))
 	for i, cr := range containers {

internal/machine/caddyconfig/controller_test.go

@@ -0,0 +1,64 @@
+package caddyconfig
+
+import (
+	"net/netip"
+	"reflect"
+	"testing"
+
+	"github.com/psviderski/uncloud/pkg/api"
+	"github.com/stretchr/testify/assert"
+)
+
+// TestContainerFingerprint_EqualCoversAllFields is a guard: when a field is added to containerFingerprint,
+// Equal must also compare it. A mutation of any single field should flip equality to false. If this test fails
+// after adding a field, update Equal to include it.
+func TestContainerFingerprint_EqualCoversAllFields(t *testing.T) {
+	t.Parallel()
+
+	base := containerFingerprint{
+		ID: "container-1",
+		IP: netip.MustParseAddr("10.210.0.2"),
+		Ports: []api.PortSpec{{
+			Hostname:      "app.example.com",
+			ContainerPort: 8080,
+			Protocol:      api.ProtocolHTTP,
+			Mode:          api.PortModeIngress,
+		}},
+		CaddyConfig: "caddy-config",
+	}
+
+	assert.True(t, base.Equal(base), "base fingerprint must be equal to itself")
+
+	rt := reflect.TypeOf(base)
+	for i := 0; i < rt.NumField(); i++ {
+		field := rt.Field(i)
+		t.Run(field.Name, func(t *testing.T) {
+			t.Parallel()
+
+			mutated := base
+			mutated.Ports = append([]api.PortSpec(nil), base.Ports...)
+			v := reflect.ValueOf(&mutated).Elem().FieldByName(field.Name)
+
+			switch field.Name {
+			case "ID", "CaddyConfig":
+				v.SetString(v.String() + "-changed")
+			case "IP":
+				v.Set(reflect.ValueOf(netip.MustParseAddr("10.210.0.99")))
+			case "Ports":
+				mutated.Ports = []api.PortSpec{{
+					Hostname:      "different.example.com",
+					ContainerPort: 9090,
+					Protocol:      api.ProtocolHTTP,
+					Mode:          api.PortModeIngress,
+				}}
+			default:
+				t.Fatalf("containerFingerprint has a new field %q without a mutation case in this test. "+
+					"Add a case here and make sure Equal() compares it.", field.Name)
+			}
+
+			assert.False(t, base.Equal(mutated),
+				"changing %q must flip Equal to false. Update containerFingerprint.Equal to compare it.",
+				field.Name)
+		})
+	}
+}