fix(commands): apply batch — multi-node, node-file patch merge, error handling

README.md

@@ -140,6 +140,23 @@ Re-template and update generated file in place (this will overwrite it):
 talm template -f nodes/node1.yaml -I
 ```
 
+> **Per-node patches inside node files.** A node file can carry Talos config
+> below its modeline (for example, a custom `hostname`, secondary
+> interfaces with `deviceSelector`, VIP placement, or extra etcd args).
+> When `talm apply -f node.yaml` runs the template-rendering branch, that
+> body is applied as a strategic merge patch on top of the rendered
+> template before the result is sent to the node — so per-node fields
+> survive even when the template auto-generates conflicting values
+> (e.g. `hostname: talos-XXXXX`).
+>
+> `talm template -f node.yaml` (with or without `-I`) does **not** apply
+> the same overlay: its output is the rendered template plus the modeline
+> and the auto-generated warning, byte-identical to what the template
+> alone would produce. Routing it through the patcher would drop every
+> YAML comment (including the modeline) and re-sort keys, breaking
+> downstream commands that read the file back. Use `apply --dry-run` if
+> you want to preview the exact bytes that will be sent to the node.
+
 ## Using talosctl commands
 
 Talm offers a similar set of commands to those provided by talosctl.

pkg/commands/apply.go

@@ -113,32 +113,40 @@ func apply(args []string) error {
 		withSecretsPath := ResolveSecretsPath(applyCmdFlags.withSecrets)
 
 		if len(modelineTemplates) > 0 {
-			// Template rendering path: connect to the node first, render templates
-			// online (so lookup() functions resolve real discovery data), then apply.
+			// Template rendering path: render templates online per node and
+			// apply the rendered config plus the node file overlay. See
+			// applyTemplatesPerNode for why the loop is mandatory.
 			opts := buildApplyRenderOptions(modelineTemplates, withSecretsPath)
+			nodes := append([]string(nil), GlobalArgs.Nodes...)
+			fmt.Printf("- talm: file=%s, nodes=%s, endpoints=%s\n", configFile, GlobalArgs.Nodes, GlobalArgs.Endpoints)
 
-			err = withApplyClient(func(ctx context.Context, c *client.Client) error {
-				fmt.Printf("- talm: file=%s, nodes=%s, endpoints=%s\n", configFile, GlobalArgs.Nodes, GlobalArgs.Endpoints)
-
-				result, err := engine.Render(ctx, c, opts)
-				if err != nil {
-					return fmt.Errorf("template rendering error: %w", err)
-				}
-
+			applyClosure := func(ctx context.Context, c *client.Client, data []byte) error {
 				resp, err := c.ApplyConfiguration(ctx, &machineapi.ApplyConfigurationRequest{
-					Data:           result,
+					Data:           data,
 					Mode:           applyCmdFlags.Mode.Mode,
 					DryRun:         applyCmdFlags.dryRun,
 					TryModeTimeout: durationpb.New(applyCmdFlags.configTryTimeout),
 				})
 				if err != nil {
 					return fmt.Errorf("error applying new configuration: %w", err)
 				}
-
 				helpers.PrintApplyResults(resp)
-
 				return nil
-			})
+			}
+
+			if applyCmdFlags.insecure {
+				openClient := openClientPerNodeMaintenance(applyCmdFlags.certFingerprints, WithClientMaintenance)
+				if err := applyTemplatesPerNode(opts, configFile, nodes, openClient, engine.Render, applyClosure); err != nil {
+					return err
+				}
+			} else {
+				if err := withApplyClientBare(func(parentCtx context.Context, c *client.Client) error {
+					openClient := openClientPerNodeAuth(parentCtx, c)
+					return applyTemplatesPerNode(opts, configFile, nodes, openClient, engine.Render, applyClosure)
+				}); err != nil {
+					return err
+				}
+			}
 		} else {
 			// Direct patch path: apply config file as patch against empty bundle
 			opts := buildApplyPatchOptions(withSecretsPath)
@@ -173,9 +181,6 @@ func apply(args []string) error {
 				return err
 			}
 		}
-		if err != nil {
-			return err
-		}
 
 		// Reset args
 		if !applyCmdFlags.nodesFromArgs {
@@ -188,23 +193,131 @@ func apply(args []string) error {
 	return nil
 }
 
-// withApplyClient creates a Talos client appropriate for the current apply mode
-// and invokes the given action with it.
+// withApplyClient creates a Talos client appropriate for the current apply
+// mode and invokes the given action with it. The action receives a context
+// in which gRPC node metadata is set to the resolved node list — either
+// GlobalArgs.Nodes (when set) or the talosconfig context's Nodes (when not).
+// Used by the direct-patch branch where multi-node fan-out happens at the
+// gRPC layer inside ApplyConfiguration.
 func withApplyClient(f func(ctx context.Context, c *client.Client) error) error {
+	return withApplyClientBare(wrapWithNodeContext(f))
+}
+
+// withApplyClientBare connects to Talos for the current apply mode but does
+// NOT inject node metadata into the context — leaving that decision to the
+// caller. Used by the template-rendering path (see applyTemplatesPerNode for
+// the rationale).
+func withApplyClientBare(f func(ctx context.Context, c *client.Client) error) error {
 	if applyCmdFlags.insecure {
-		// Maintenance mode connects directly to the node IP without talosconfig;
-		// node context injection is not needed — the maintenance client handles
-		// node targeting internally via GlobalArgs.Nodes.
+		// Maintenance mode reads its endpoints directly from
+		// GlobalArgs.Nodes — gRPC node metadata is not consulted.
 		return WithClientMaintenance(applyCmdFlags.certFingerprints, f)
 	}
 
-	wrappedF := wrapWithNodeContext(f)
-
 	if GlobalArgs.SkipVerify {
-		return WithClientSkipVerify(wrappedF)
+		return WithClientSkipVerify(f)
+	}
+
+	return WithClientNoNodes(f)
+}
+
+// renderFunc, applyFunc and openClientFunc are injection points for
+// applyTemplatesPerNode so unit tests can drive the loop with fakes instead
+// of a real Talos client.
+type renderFunc func(ctx context.Context, c *client.Client, opts engine.Options) ([]byte, error)
+type applyFunc func(ctx context.Context, c *client.Client, data []byte) error
+
+// openClientFunc opens a Talos client suitable for a single node and runs
+// action with it. Authenticated mode reuses one parent client and rotates
+// the node via single-target gRPC metadata (client.WithNode); insecure
+// (maintenance) mode opens a fresh single-endpoint client per node because
+// Talos's maintenance client ignores node metadata in the context and
+// round-robins between its configured endpoints.
+type openClientFunc func(node string, action func(ctx context.Context, c *client.Client) error) error
+
+// applyTemplatesPerNode runs render → MergeFileAsPatch → apply once per
+// node. Two reasons it has to iterate:
+//
+//   - engine.Render's FailIfMultiNodes guard rejects a context that carries
+//     more than one node, so the auth-mode caller has to attach a single
+//     node per iteration — and discovery via lookup() should resolve each
+//     node's own topology in any case.
+//   - In insecure (maintenance) mode the client connects directly to a
+//     Talos node and ignores nodes-in-context entirely, so each node needs
+//     its own client; otherwise gRPC round-robins ApplyConfiguration
+//     across the endpoint list and most nodes never see the config.
+//
+// Both modes share this loop via openClient.
+func applyTemplatesPerNode(
+	opts engine.Options,
+	configFile string,
+	nodes []string,
+	openClient openClientFunc,
+	render renderFunc,
+	apply applyFunc,
+) error {
+	if len(nodes) == 0 {
+		return fmt.Errorf("no nodes specified for template-rendering apply")
+	}
+	for _, node := range nodes {
+		if err := openClient(node, func(ctx context.Context, c *client.Client) error {
+			return renderMergeAndApply(ctx, c, opts, configFile, render, apply)
+		}); err != nil {
+			return fmt.Errorf("node %s: %w", node, err)
+		}
+	}
+	return nil
+}
+
+// maintenanceClientFunc is the contract WithClientMaintenance satisfies in
+// production and a fake satisfies in tests. Injection lets the unit tests
+// run the real openClientPerNodeMaintenance body without dialing a Talos
+// node.
+type maintenanceClientFunc func(fingerprints []string, action func(ctx context.Context, c *client.Client) error) error
+
+// openClientPerNodeMaintenance returns an openClientFunc that opens a
+// fresh single-endpoint maintenance client per node. Multi-node insecure
+// apply (first bootstrap of a multi-node cluster) needs this because
+// WithClientMaintenance creates a client with all endpoints and gRPC then
+// round-robins ApplyConfiguration across them — most nodes never see the
+// config. Narrowing GlobalArgs.Nodes to the current iteration's node and
+// restoring it via defer keeps the wrapper's signature unchanged.
+//
+// mkClient is normally WithClientMaintenance; tests pass a fake that
+// captures the GlobalArgs.Nodes value at the moment WithClientMaintenance
+// would have read it.
+func openClientPerNodeMaintenance(fingerprints []string, mkClient maintenanceClientFunc) openClientFunc {
+	return func(node string, action func(ctx context.Context, c *client.Client) error) error {
+		savedNodes := append([]string(nil), GlobalArgs.Nodes...)
+		GlobalArgs.Nodes = []string{node}
+		defer func() { GlobalArgs.Nodes = savedNodes }()
+		return mkClient(fingerprints, action)
+	}
+}
+
+// openClientPerNodeAuth returns an openClientFunc that reuses one
+// authenticated client (the one withApplyClientBare opened above this
+// callback) and rotates the addressed node via client.WithNode on the
+// per-iteration context. WithNode (rather than WithNodes) sets the
+// "node" metadata key for single-target proxying, which engine.Render's
+// FailIfMultiNodes guard treats as one node.
+func openClientPerNodeAuth(parentCtx context.Context, c *client.Client) openClientFunc {
+	return func(node string, action func(ctx context.Context, c *client.Client) error) error {
+		return action(client.WithNode(parentCtx, node), c)
 	}
+}
 
-	return WithClientNoNodes(wrappedF)
+// renderMergeAndApply is the per-node body shared by every apply mode.
+func renderMergeAndApply(ctx context.Context, c *client.Client, opts engine.Options, configFile string, render renderFunc, apply applyFunc) error {
+	rendered, err := render(ctx, c, opts)
+	if err != nil {
+		return fmt.Errorf("template rendering: %w", err)
+	}
+	merged, err := engine.MergeFileAsPatch(rendered, configFile)
+	if err != nil {
+		return fmt.Errorf("merging node file as patch: %w", err)
+	}
+	return apply(ctx, c, merged)
 }
 
 // buildApplyRenderOptions constructs engine.Options for the template rendering path.
@@ -214,14 +327,14 @@ func withApplyClient(f func(ctx context.Context, c *client.Client) error) error
 func buildApplyRenderOptions(modelineTemplates []string, withSecretsPath string) engine.Options {
 	resolvedTemplates := resolveTemplatePaths(modelineTemplates, Config.RootDir)
 	return engine.Options{
-		Insecure:          applyCmdFlags.insecure,
 		TalosVersion:      applyCmdFlags.talosVersion,
 		WithSecrets:       withSecretsPath,
 		KubernetesVersion: applyCmdFlags.kubernetesVersion,
 		Debug:             applyCmdFlags.debug,
 		Full:              true,
 		Root:              Config.RootDir,
 		TemplateFiles:     resolvedTemplates,
+		CommandName:       "talm apply",
 	}
 }