Don't restart garage on config change

go/daemon/children/children.go

@@ -205,7 +205,7 @@ func (c *Children) reloadGarage(
 		return nil
 	}
 
-	var anyChanged bool
+	var anyCreated bool
 	for _, alloc := range allocs {
 		var (
 			procName = garagePmuxProcName(alloc)
@@ -216,7 +216,7 @@ func (c *Children) reloadGarage(
 			)
 		)
 
-		childConfigPath, changed, err := garageWriteChildConfig(
+		childConfigPath, err := garageWriteChildConfig(
 			ctx,
 			c.logger,
 			c.garageRPCSecret,
@@ -227,26 +227,22 @@ func (c *Children) reloadGarage(
 		)
 		if err != nil {
 			return fmt.Errorf("writing child config file for alloc %+v: %w", alloc, err)
-		} else if !changed {
-			c.logger.Info(ctx, "No changes to garage config file")
+		}
+
+		if _, ok := c.garageProcs[procName]; ok {
+			c.logger.Info(ctx, "Garage instance already exists")
 			continue
 		}
 
-		anyChanged = true
+		anyCreated = true
 
-		if proc, ok := c.garageProcs[procName]; ok {
-			c.logger.Info(ctx, "garage config has changed, restarting process")
-			proc.Restart()
-			continue
-		}
-
-		c.logger.Info(ctx, "garage config has been added, creating process")
+		c.logger.Info(ctx, "Garage config has been added, creating process")
 		c.garageProcs[procName] = garagePmuxProc(
 			ctx, c.logger, c.binDirPath, procName, childConfigPath,
 		)
 	}
 
-	if anyChanged {
+	if anyCreated {
 		if err := waitForGarage(
 			ctx,
 			c.logger,

go/daemon/children/garage.go

@@ -71,7 +71,7 @@ func garageWriteChildConfig(
 	hostBootstrap bootstrap.Bootstrap,
 	alloc daecommon.ConfigStorageAllocation,
 ) (
-	string, bool, error,
+	string, error,
 ) {
 	var (
 		thisHost = hostBootstrap.ThisHost()
@@ -94,10 +94,10 @@ func garageWriteChildConfig(
 
 	dbEngine, err := garagesrv.GetDBEngine(alloc.MetaPath)
 	if err != nil {
-		return "", false, fmt.Errorf("getting alloc db engine: %w", err)
+		return "", fmt.Errorf("getting alloc db engine: %w", err)
 	}
 
-	changed, err := garagesrv.WriteGarageTomlFile(
+	err = garagesrv.WriteGarageTomlFile(
 		ctx,
 		logger,
 		garageTomlPath,
@@ -115,12 +115,12 @@ func garageWriteChildConfig(
 	)
 
 	if err != nil {
-		return "", false, fmt.Errorf(
+		return "", fmt.Errorf(
 			"creating garage.toml file at %q: %w", garageTomlPath, err,
 		)
 	}
 
-	return garageTomlPath, changed, nil
+	return garageTomlPath, nil
 }
 
 func garagePmuxProcName(alloc daecommon.ConfigStorageAllocation) string {
@@ -164,7 +164,7 @@ func garagePmuxProcs(
 	}
 
 	for _, alloc := range allocs {
-		childConfigPath, _, err := garageWriteChildConfig(
+		childConfigPath, err := garageWriteChildConfig(
 			ctx,
 			logger,
 			rpcSecret, runtimeDirPath, adminToken,

go/garage/garagesrv/tpl.go

@@ -1,15 +1,15 @@
 package garagesrv
 
 import (
-	"cmp"
+	"bytes"
 	"context"
+	"fmt"
 	"io"
-	"slices"
+	"os"
 	"strconv"
 	"text/template"
 
 	"isle/garage"
-	"isle/toolkit"
 
 	"dev.mediocregopher.com/mediocre-go-lib.git/mlog"
 )
@@ -58,26 +58,18 @@ func RenderGarageToml(into io.Writer, data GarageTomlData) error {
 }
 
 // WriteGarageTomlFile renders a garage.toml using the given data to a new file
-// at the given path, returning true if the file changed or didn't
-// previously exist.
+// at the given path.
 func WriteGarageTomlFile(
-	ctx context.Context,
-	logger *mlog.Logger,
-	path string,
-	data GarageTomlData,
-) (
-	bool, error,
-) {
-	slices.SortFunc(data.BootstrapPeers, func(i, j garage.RemoteNode) int {
-		return cmp.Or(
-			cmp.Compare(i.IP, j.IP),
-			cmp.Compare(i.RPCPort, j.RPCPort),
-		)
-	})
+	ctx context.Context, logger *mlog.Logger, path string, data GarageTomlData,
+) error {
+	buf := new(bytes.Buffer)
+	if err := garageTomlTpl.Execute(buf, data); err != nil {
+		return fmt.Errorf("executing template: %w", err)
+	}
 
-	return toolkit.WriteFileCheckChanged(
-		ctx, logger, path, 0600, func(w io.Writer) error {
-			return garageTomlTpl.Execute(w, data)
-		},
-	)
+	if err := os.WriteFile(path, buf.Bytes(), 0600); err != nil {
+		return fmt.Errorf("writing file: %w", err)
+	}
+
+	return nil
 }

tasks/bugs/garage-dont-restart-on-peer-change.md

@@ -1,11 +0,0 @@
----
-type: task
----
-
-# Don't Restart Garage on Peer Change
-
-When the garage configuration has changed, it is not necessary to issue a
-restart to the garage processes if only the bootstrap peers have changed. Those
-are only necessary on startup of garage.
-
-Implementing this should speed up integration tests a bit.

tasks/bugs/garage-remove-then-re-add-alloc.md

@@ -0,0 +1,14 @@
+---
+type: task
+after:
+  - ./garage-apply-layout-before-stopping-instance.md
+---
+
+I think there is currently a bug related to re-adding a storage allocation on an
+RPC port of a garage instance which was previously used:
+
+- Step 1) Remove a storage allocation using RPC port N
+- Step 2) Add a new allocation using RPC port N, but with a new data/meta dir
+
+I believe in this case garage will go back to using the old data/meta dir, and
+possibly even re-use the old pubkey.