Re-organize some reload logic

go/daemon/network/garage.go

@@ -25,19 +25,21 @@ const (
 	garageGlobalBucketBootstrapHostsDirPath = "bootstrap/hosts"
 )
 
-func (n *network) getGarageClientParams(
+func getGarageClientParams(
-	ctx context.Context, currBootstrap bootstrap.Bootstrap,
+	ctx context.Context,
+	secretsStore secrets.Store,
+	currBootstrap bootstrap.Bootstrap,
 ) (
 	GarageClientParams, error,
 ) {
 	creds, err := daecommon.GetGarageS3APIGlobalBucketCredentials(
-		ctx, n.secretsStore,
+		ctx, secretsStore,
 	)
 	if err != nil {
 		return GarageClientParams{}, fmt.Errorf("getting garage global bucket creds: %w", err)
 	}
 
-	rpcSecret, err := daecommon.GetGarageRPCSecret(ctx, n.secretsStore)
+	rpcSecret, err := daecommon.GetGarageRPCSecret(ctx, secretsStore)
 	if err != nil && !errors.Is(err, secrets.ErrNotFound) {
 		return GarageClientParams{}, fmt.Errorf("getting garage rpc secret: %w", err)
 	}
@@ -155,12 +157,17 @@ func garageInitializeGlobalBucket(
 	return creds, nil
 }
 
-func (n *network) getGarageBootstrapHosts(
+func getGarageBootstrapHosts(
-	ctx context.Context, currBootstrap bootstrap.Bootstrap,
+	ctx context.Context,
+	logger *mlog.Logger,
+	secretsStore secrets.Store,
+	currBootstrap bootstrap.Bootstrap,
 ) (
 	map[nebula.HostName]bootstrap.Host, error,
 ) {
-	garageClientParams, err := n.getGarageClientParams(ctx, currBootstrap)
+	garageClientParams, err := getGarageClientParams(
+		ctx, secretsStore, currBootstrap,
+	)
 	if err != nil {
 		return nil, fmt.Errorf("getting garage client params: %w", err)
 	}
@@ -202,13 +209,13 @@ func (n *network) getGarageBootstrapHosts(
 		obj.Close()
 
 		if err != nil {
-			n.logger.Warn(ctx, "Object contains invalid json", err)
+			logger.Warn(ctx, "Object contains invalid json", err)
 			continue
 		}
 
 		host, err := authedHost.Unwrap(currBootstrap.CAPublicCredentials)
 		if err != nil {
-			n.logger.Warn(ctx, "Host could not be authenticated", err)
+			logger.Warn(ctx, "Host could not be authenticated", err)
 		}
 
 		hosts[host.Name] = host
@@ -220,10 +227,14 @@ func (n *network) getGarageBootstrapHosts(
 // putGarageBoostrapHost places the <hostname>.json.signed file for this host
 // into garage so that other hosts are able to see relevant configuration for
 // it.
-func (n *network) putGarageBoostrapHost(
+func putGarageBoostrapHost(
-	ctx context.Context, currBootstrap bootstrap.Bootstrap,
+	ctx context.Context,
+	secretsStore secrets.Store,
+	currBootstrap bootstrap.Bootstrap,
 ) error {
-	garageClientParams, err := n.getGarageClientParams(ctx, currBootstrap)
+	garageClientParams, err := getGarageClientParams(
+		ctx, secretsStore, currBootstrap,
+	)
 	if err != nil {
 		return fmt.Errorf("getting garage client params: %w", err)
 	}

go/daemon/network/network.go

@@ -166,8 +166,7 @@ func (o *Opts) withDefaults() *Opts {
 }
 
 type network struct {
-	logger        *mlog.Logger
+	logger *mlog.Logger
-	networkConfig daecommon.NetworkConfig
 
 	envBinDirPath string
 	stateDir      toolkit.Dir
@@ -179,6 +178,7 @@ type network struct {
 
 	l             sync.RWMutex
 	children      *children.Children
+	networkConfig daecommon.NetworkConfig
 	currBootstrap bootstrap.Bootstrap
 
 	shutdownCh chan struct{}
@@ -455,8 +455,8 @@ func (n *network) initialize(
 	// Do this now so that everything is stable before returning. This also
 	// serves a dual-purpose, as it makes sure that the PUT from the postInit
 	// above has propagated from the local garage instance, if there is one.
-	n.logger.Info(ctx, "Reloading bootstrap from network")
+	n.logger.Info(ctx, "Reloading hosts from network storage")
-	if err = n.reload(ctx); err != nil {
+	if err = n.reloadHosts(ctx); err != nil {
 		return fmt.Errorf("Reloading network bootstrap: %w", err)
 	}
 
@@ -523,20 +523,24 @@ func (n *network) postInit(ctx context.Context) error {
 	}
 
 	n.logger.Info(ctx, "Updating host info in garage")
-	if err = n.putGarageBoostrapHost(ctx, n.currBootstrap); err != nil {
+	err = putGarageBoostrapHost(ctx, n.secretsStore, n.currBootstrap)
+	if err != nil {
 		return fmt.Errorf("updating host info in garage: %w", err)
 	}
 
 	return nil
 }
 
-func (n *network) reload(ctx context.Context) error {
+func (n *network) reloadHosts(ctx context.Context) error {
 	n.l.RLock()
+	networkConfig := n.networkConfig
 	currBootstrap := n.currBootstrap
 	n.l.RUnlock()
 
 	n.logger.Info(ctx, "Checking for bootstrap changes")
-	newHosts, err := n.getGarageBootstrapHosts(ctx, currBootstrap)
+	newHosts, err := getGarageBootstrapHosts(
+		ctx, n.logger, n.secretsStore, currBootstrap,
+	)
 	if err != nil {
 		return fmt.Errorf("getting hosts from garage: %w", err)
 	}
@@ -546,7 +550,11 @@ func (n *network) reload(ctx context.Context) error {
 	// garage and the bootstrap, but then this reload call removes the
 	// host from this bootstrap/children until the next reload.
 
-	if err := n.reloadWithHosts(ctx, currBootstrap, newHosts); err != nil {
+	newBootstrap := currBootstrap
+	newBootstrap.Hosts = newHosts
+
+	err = n.reload(ctx, networkConfig, newBootstrap)
+	if err != nil {
 		return fmt.Errorf("reloading with new host data: %w", err)
 	}
 
@@ -554,7 +562,8 @@ func (n *network) reload(ctx context.Context) error {
 }
 
 func (n *network) reloadLoop(ctx context.Context) {
-	ticker := time.NewTicker(3 * time.Minute)
+	const period = 3 * time.Minute
+	ticker := time.NewTicker(period)
 	defer ticker.Stop()
 
 	for {
@@ -563,7 +572,7 @@ func (n *network) reloadLoop(ctx context.Context) {
 			return
 
 		case <-ticker.C:
-			if err := n.reload(ctx); err != nil {
+			if err := n.reloadHosts(ctx); err != nil {
 				n.logger.Error(ctx, "Attempting to reload", err)
 				continue
 			}
@@ -571,27 +580,31 @@ func (n *network) reloadLoop(ctx context.Context) {
 	}
 }
 
-// reloadWithHosts will check the existing hosts data from currBootstrap against
+// reload will check the existing hosts data from currBootstrap against
 // a potentially updated set of hosts data, and if there are any differences
 // will perform whatever changes are necessary.
-func (n *network) reloadWithHosts(
+func (n *network) reload(
 	ctx context.Context,
-	currBootstrap bootstrap.Bootstrap,
+	newNetworkConfig daecommon.NetworkConfig,
-	newHosts map[nebula.HostName]bootstrap.Host,
+	newBootstrap bootstrap.Bootstrap,
 ) error {
-	var (
+	n.l.Lock()
-		newBootstrap = currBootstrap
+	defer n.l.Unlock()
-		thisHost     = currBootstrap.ThisHost()
-	)
-
-	newBootstrap.Hosts = newHosts
 
 	// the daemon's view of this host's bootstrap info takes precedence over
-	// whatever is in garage
+	// whatever is in garage. The garage version lacks the private credentials
+	// which must be stored locally.
+	thisHost := n.currBootstrap.ThisHost()
 	newBootstrap.Hosts[thisHost.Name] = thisHost
 
+	n.logger.Info(ctx, "Writing updated bootstrap to state dir")
+	err := writeBootstrapToStateDir(n.stateDir.Path, newBootstrap)
+	if err != nil {
+		return fmt.Errorf("writing bootstrap to state dir: %w", err)
+	}
+
 	diff, err := children.CalculateReloadDiff(
-		n.networkConfig, currBootstrap, newBootstrap,
+		n.networkConfig, n.currBootstrap, newBootstrap,
 	)
 	if err != nil {
 		return fmt.Errorf("calculating diff between bootstraps: %w", err)
@@ -600,13 +613,11 @@ func (n *network) reloadWithHosts(
 		return nil
 	}
 
-	n.logger.Info(ctx, "Bootstrap has changed, storing new bootstrap")
+	n.networkConfig = newNetworkConfig
-	n.l.Lock()
-	networkConfig := n.networkConfig
 	n.currBootstrap = newBootstrap
-	n.l.Unlock()
 
-	err = n.children.Reload(ctx, networkConfig, newBootstrap, diff)
+	n.logger.Info(ctx, "Reloading child processes")
+	err = n.children.Reload(ctx, newNetworkConfig, newBootstrap, diff)
 	if err != nil {
 		return fmt.Errorf("reloading child processes (diff:%+v): %w", diff, err)
 	}
@@ -637,18 +648,17 @@ func (n *network) getBootstrap() (
 }
 
 func (n *network) GetHosts(ctx context.Context) ([]bootstrap.Host, error) {
-	// TODO use withCurrBootstrap in here
+	return withCurrBootstrap(n, func(
-	b, err := n.getBootstrap()
+		currBootstrap bootstrap.Bootstrap,
-	if err != nil {
+	) (
-		return nil, fmt.Errorf("retrieving bootstrap: %w", err)
+		[]bootstrap.Host, error,
-	}
+	) {
-
+		hosts := maps.Values(currBootstrap.Hosts)
-	hosts := maps.Values(b.Hosts)
+		slices.SortFunc(hosts, func(a, b bootstrap.Host) int {
-	slices.SortFunc(hosts, func(a, b bootstrap.Host) int {
+			return cmp.Compare(a.Name, b.Name)
-		return cmp.Compare(a.Name, b.Name)
+		})
+		return hosts, nil
 	})
-
-	return hosts, nil
 }
 
 func (n *network) GetGarageClientParams(
@@ -661,7 +671,7 @@ func (n *network) GetGarageClientParams(
 	) (
 		GarageClientParams, error,
 	) {
-		return n.getGarageClientParams(ctx, currBootstrap)
+		return getGarageClientParams(ctx, n.secretsStore, currBootstrap)
 	})
 }
 
@@ -688,7 +698,9 @@ func (n *network) RemoveHost(ctx context.Context, hostName nebula.HostName) erro
 	) (
 		struct{}, error,
 	) {
-		garageClientParams, err := n.getGarageClientParams(ctx, currBootstrap)
+		garageClientParams, err := getGarageClientParams(
+			ctx, n.secretsStore, currBootstrap,
+		)
 		if err != nil {
 			return struct{}{}, fmt.Errorf("get garage client params: %w", err)
 		}
@@ -798,6 +810,7 @@ func (n *network) CreateHost(
 	JoiningBootstrap, error,
 ) {
 	n.l.RLock()
+	networkConfig := n.networkConfig
 	currBootstrap := n.currBootstrap
 	n.l.RUnlock()
 
@@ -851,17 +864,19 @@ func (n *network) CreateHost(
 	}
 
 	n.logger.Info(ctx, "Putting new host in garage")
-	err = n.putGarageBoostrapHost(ctx, joiningBootstrap.Bootstrap)
+	err = putGarageBoostrapHost(ctx, n.secretsStore, joiningBootstrap.Bootstrap)
 	if err != nil {
 		return JoiningBootstrap{}, fmt.Errorf("putting new host in garage: %w", err)
 	}
 
 	// the new bootstrap will have been initialized with both all existing hosts
 	// (based on currBootstrap) and the host being created.
-	newHosts := joiningBootstrap.Bootstrap.Hosts
+	newBootstrap := currBootstrap
+	newBootstrap.Hosts = joiningBootstrap.Bootstrap.Hosts
 
 	n.logger.Info(ctx, "Reloading local state with new host")
-	if err := n.reloadWithHosts(ctx, currBootstrap, newHosts); err != nil {
+	err = n.reload(ctx, networkConfig, newBootstrap)
+	if err != nil {
 		return JoiningBootstrap{}, fmt.Errorf("reloading child processes: %w", err)
 	}
 
@@ -900,6 +915,8 @@ func (n *network) CreateNebulaCertificate(
 }
 
 func (n *network) GetConfig(context.Context) (daecommon.NetworkConfig, error) {
+	n.l.RLock()
+	defer n.l.RUnlock()
 	return n.networkConfig, nil
 }
 
@@ -957,7 +974,6 @@ func (n *network) GetNetworkCreationParams(
 ) (
 	bootstrap.CreationParams, error,
 ) {
-
 	return withCurrBootstrap(n, func(
 		currBootstrap bootstrap.Bootstrap,
 	) (