Merge pull request #12264 from tomponline/tp-revert-listener

lxd: Revert server config if it cannot be set

lxd/api_1.0.go

@@ -22,6 +22,7 @@ import (
 	"github.com/canonical/lxd/lxd/project"
 	"github.com/canonical/lxd/lxd/request"
 	"github.com/canonical/lxd/lxd/response"
+	"github.com/canonical/lxd/lxd/revert"
 	scriptletLoad "github.com/canonical/lxd/lxd/scriptlet/load"
 	"github.com/canonical/lxd/lxd/util"
 	"github.com/canonical/lxd/shared"
@@ -574,13 +575,20 @@ func doApi10Update(d *Daemon, r *http.Request, req api.ServerPut, patch bool) re
 
 	nodeChanged := map[string]string{}
 	var newNodeConfig *node.Config
+	oldNodeConfig := make(map[string]any)
+
 	err = s.DB.Node.Transaction(r.Context(), func(ctx context.Context, tx *db.NodeTx) error {
 		var err error
 		newNodeConfig, err = node.ConfigLoad(ctx, tx)
 		if err != nil {
 			return fmt.Errorf("Failed to load node config: %w", err)
 		}
 
+		// Keep old config around in case something goes wrong. In that case the config will be reverted.
+		for k, v := range newNodeConfig.Dump() {
+			oldNodeConfig[k] = v
+		}
+
 		// We currently don't allow changing the cluster.https_address once it's set.
 		if clustered {
 			curConfig, err := tx.Config(ctx)
@@ -632,6 +640,38 @@ func doApi10Update(d *Daemon, r *http.Request, req api.ServerPut, patch bool) re
 		}
 	}
 
+	revert := revert.New()
+	defer revert.Fail()
+
+	revert.Add(func() {
+		for key := range nodeValues {
+			val, ok := oldNodeConfig[key]
+			if !ok {
+				nodeValues[key] = nil
+			} else {
+				nodeValues[key] = val
+			}
+		}
+
+		err = s.DB.Node.Transaction(r.Context(), func(ctx context.Context, tx *db.NodeTx) error {
+			newNodeConfig, err := node.ConfigLoad(ctx, tx)
+			if err != nil {
+				return fmt.Errorf("Failed to load node config: %w", err)
+			}
+
+			_, err = newNodeConfig.Replace(nodeValues)
+			if err != nil {
+				return fmt.Errorf("Failed updating node config: %w", err)
+			}
+
+			return nil
+		})
+
+		if err != nil {
+			logger.Warn("Failed reverting node config", logger.Ctx{"err": err})
+		}
+	})
+
 	// Validate global configuration
 	hasRBAC := false
 	hasCandid := false
@@ -654,13 +694,20 @@ func doApi10Update(d *Daemon, r *http.Request, req api.ServerPut, patch bool) re
 	// Then deal with cluster wide configuration
 	var clusterChanged map[string]string
 	var newClusterConfig *clusterConfig.Config
+	oldClusterConfig := make(map[string]any)
+
 	err = s.DB.Cluster.Transaction(context.Background(), func(ctx context.Context, tx *db.ClusterTx) error {
 		var err error
 		newClusterConfig, err = clusterConfig.Load(ctx, tx)
 		if err != nil {
 			return fmt.Errorf("Failed to load cluster config: %w", err)
 		}
 
+		// Keep old config around in case something goes wrong. In that case the config will be reverted.
+		for k, v := range newClusterConfig.Dump() {
+			oldClusterConfig[k] = v
+		}
+
 		if patch {
 			clusterChanged, err = newClusterConfig.Patch(req.Config)
 		} else {
@@ -678,6 +725,35 @@ func doApi10Update(d *Daemon, r *http.Request, req api.ServerPut, patch bool) re
 		}
 	}
 
+	revert.Add(func() {
+		for key := range req.Config {
+			val, ok := oldClusterConfig[key]
+			if !ok {
+				req.Config[key] = nil
+			} else {
+				req.Config[key] = val
+			}
+		}
+
+		err = s.DB.Cluster.Transaction(r.Context(), func(ctx context.Context, tx *db.ClusterTx) error {
+			newClusterConfig, err = clusterConfig.Load(ctx, tx)
+			if err != nil {
+				return fmt.Errorf("Failed to load cluster config: %w", err)
+			}
+
+			_, err = newClusterConfig.Replace(req.Config)
+			if err != nil {
+				return fmt.Errorf("Failed updating cluster config: %w", err)
+			}
+
+			return nil
+		})
+
+		if err != nil {
+			logger.Warn("Failed reverting cluster config", logger.Ctx{"err": err})
+		}
+	})
+
 	// Notify the other nodes about changes
 	notifier, err := cluster.NewNotifier(s, s.Endpoints.NetworkCert(), s.ServerCert(), cluster.NotifyAlive)
 	if err != nil {
@@ -716,6 +792,8 @@ func doApi10Update(d *Daemon, r *http.Request, req api.ServerPut, patch bool) re
 		return response.SmartError(err)
 	}
 
+	revert.Success()
+
 	s.Events.SendLifecycle(project.Default, lifecycle.ConfigUpdated.Event(request.CreateRequestor(r), nil))
 
 	return response.EmptySyncResponse