diff --git a/internal/agentruntime/charts.go b/internal/agentruntime/charts.go index 0cb03414..ae774862 100644 --- a/internal/agentruntime/charts.go +++ b/internal/agentruntime/charts.go @@ -5,7 +5,7 @@ package agentruntime // deployments. It MUST be updated as a single edit; bumping it here // updates every consumer in lockstep. // -// Chart 0.3.3 ships remote-signer image `v0.4.0`, which honours +// Chart 0.4.0 ships remote-signer image `v0.4.0`, which honours // `SIGNER__AUTH__TOKEN` (the bearer token the controller mints into the // keystore Secret and injects via env). Canonical Ethereum recovery-id // signatures (`v=27/28`) from `/sign/.../message`, `/sign/.../typed-data`, @@ -14,5 +14,11 @@ package agentruntime // the buy.py caller to renormalize for EIP-712 / ERC-3009 verifiers like // USDC `transferWithAuthorization`. // +// 0.4.0 also renders `spec.strategy` from `.Values.strategy` (defaulting to +// `Recreate`), so the RWO-keystore singleton no longer needs an imperative +// post-sync `kubectl patch` to stay off RollingUpdate. `values-remote-signer.yaml` +// pins `strategy.type: Recreate` explicitly so the intent is visible at the +// obol-stack layer and survives any future change to the chart default. +// // renovate: datasource=helm depName=remote-signer registryUrl=https://obolnetwork.github.io/helm-charts/ -const RemoteSignerChartVersion = "0.3.3" +const RemoteSignerChartVersion = "0.4.0" diff --git a/internal/agentruntime/signer.go b/internal/agentruntime/signer.go deleted file mode 100644 index f026df62..00000000 --- a/internal/agentruntime/signer.go +++ /dev/null @@ -1,59 +0,0 @@ -package agentruntime - -import ( - "fmt" - "os" - "os/exec" - "path/filepath" - "strings" - - "github.com/ObolNetwork/obol-stack/internal/config" -) - -// RemoteSignerStrategyPatchArgs builds the kubectl args that pin -// strategy.type=Recreate on the remote-signer deployment while clearing the -// k8s-defaulted strategy.rollingUpdate (the explicit null in a merge patch is -// what removes the field — the API rejects type=Recreate while rollingUpdate -// is still set). -// -// Pure / testable — no side effects. -func RemoteSignerStrategyPatchArgs(namespace string) []string { - return []string{ - "patch", "deployment/remote-signer", - "-n", namespace, - "--type=merge", - "-p", `{"spec":{"strategy":{"type":"Recreate","rollingUpdate":null}}}`, - } -} - -// EnforceRemoteSignerRecreate pins the remote-signer deployment to the -// Recreate strategy after a helmfile sync. -// -// The obol/remote-signer chart (0.3.3) does not expose spec.strategy, so the -// deployment is created with the default RollingUpdate — but the signer is a -// singleton over a ReadWriteOnce keystore PVC. On a multi-node cluster a -// RollingUpdate surge pod can land on a different node and wedge forever on -// the volume attach; on any cluster it briefly runs two signers over the -// same keystore. Recreate is the same stance hermes and x402-buyer already -// take for RWO-backed singletons. Patching spec.strategy does not touch the -// pod template, so this never triggers a rollout by itself, and helm leaves -// it alone on later upgrades because the chart never renders the field. -// -// Runs after (not before) helmfile sync so a fresh install is patched in the -// same run that created the deployment. Best-effort: a missing deployment -// (wallet-less instance) returns nil; other failures are returned for the -// caller to warn on — sync must not fail over a strategy patch. -func EnforceRemoteSignerRecreate(cfg *config.Config, namespace string) error { - kubectlBinary := filepath.Join(cfg.BinDir, "kubectl") - kubeconfigPath := filepath.Join(cfg.ConfigDir, "kubeconfig.yaml") - - cmd := exec.Command(kubectlBinary, RemoteSignerStrategyPatchArgs(namespace)...) - cmd.Env = append(os.Environ(), "KUBECONFIG="+kubeconfigPath) - if out, err := cmd.CombinedOutput(); err != nil { - if strings.Contains(string(out), "NotFound") || strings.Contains(string(out), "not found") { - return nil - } - return fmt.Errorf("patch remote-signer strategy in %s: %w\n%s", namespace, err, strings.TrimSpace(string(out))) - } - return nil -} diff --git a/internal/agentruntime/signer_test.go b/internal/agentruntime/signer_test.go deleted file mode 100644 index 313533a7..00000000 --- a/internal/agentruntime/signer_test.go +++ /dev/null @@ -1,58 +0,0 @@ -package agentruntime - -import ( - "encoding/json" - "testing" -) - -// TestRemoteSignerStrategyPatchArgs guards the post-sync pin of -// strategy.type=Recreate on the remote-signer deployment. The chart (0.3.3) -// cannot render spec.strategy, so this imperative patch is the only thing -// keeping a RWO-keystore singleton off RollingUpdate — where a surge pod on -// another node wedges on the volume attach. The patch must be a merge patch -// that pins type=Recreate AND carries an explicit rollingUpdate:null — the -// null is what removes the k8s-defaulted field the API would otherwise -// reject the type flip over. -func TestRemoteSignerStrategyPatchArgs(t *testing.T) { - args := RemoteSignerStrategyPatchArgs("hermes-obol-agent") - - want := map[string]bool{ - "patch": false, - "deployment/remote-signer": false, - "hermes-obol-agent": false, - "--type=merge": false, - } - var patchJSON string - for i, a := range args { - if _, ok := want[a]; ok { - want[a] = true - } - if a == "-p" && i+1 < len(args) { - patchJSON = args[i+1] - } - } - for arg, seen := range want { - if !seen { - t.Errorf("RemoteSignerStrategyPatchArgs missing expected arg %q in %v", arg, args) - } - } - if patchJSON == "" { - t.Fatal("RemoteSignerStrategyPatchArgs missing -p ") - } - - var patch struct { - Spec struct { - Strategy map[string]any `json:"strategy"` - } `json:"spec"` - } - if err := json.Unmarshal([]byte(patchJSON), &patch); err != nil { - t.Fatalf("patch is not valid JSON: %v\n%s", err, patchJSON) - } - if got := patch.Spec.Strategy["type"]; got != "Recreate" { - t.Errorf("strategy.type = %v, want Recreate", got) - } - ru, present := patch.Spec.Strategy["rollingUpdate"] - if !present || ru != nil { - t.Errorf("strategy.rollingUpdate = %v (present=%v), want explicit null — it is what clears the k8s-defaulted block", ru, present) - } -} diff --git a/internal/hermes/hermes.go b/internal/hermes/hermes.go index 27d6db76..479ca5a5 100644 --- a/internal/hermes/hermes.go +++ b/internal/hermes/hermes.go @@ -254,13 +254,6 @@ func Sync(cfg *config.Config, id string, u *ui.UI) error { return fmt.Errorf("helmfile sync failed: %w", err) } - // The remote-signer chart cannot express strategy: Recreate itself; pin - // it post-sync so the RWO keystore PVC never faces a RollingUpdate surge - // (deadlocks on multi-node volume attach). - if err := agentruntime.EnforceRemoteSignerRecreate(cfg, agentruntime.Namespace(agentruntime.Hermes, id)); err != nil { - u.Warnf("Could not pin remote-signer Recreate strategy (continuing): %v", err) - } - // Publish wallet-metadata ConfigMap for the frontend (namespace now exists). applyWalletMetadataConfigMap(cfg, id, deploymentDir) diff --git a/internal/openclaw/openclaw.go b/internal/openclaw/openclaw.go index 442735b4..3fba946c 100644 --- a/internal/openclaw/openclaw.go +++ b/internal/openclaw/openclaw.go @@ -454,13 +454,6 @@ func doSync(cfg *config.Config, id string, u *ui.UI) error { return fmt.Errorf("helmfile sync failed: %w", err) } - // The remote-signer chart cannot express strategy: Recreate itself; pin - // it post-sync so the RWO keystore PVC never faces a RollingUpdate surge - // (deadlocks on multi-node volume attach). - if err := agentruntime.EnforceRemoteSignerRecreate(cfg, namespace); err != nil { - u.Warnf("Could not pin remote-signer Recreate strategy (continuing): %v", err) - } - // Patch ConfigMap to inject heartbeat config that the chart template // does not render. The chart's _helpers.tpl only outputs // agents.defaults.model and agents.defaults.workspace into openclaw.json, diff --git a/internal/openclaw/wallet.go b/internal/openclaw/wallet.go index af096a73..0320bb2f 100644 --- a/internal/openclaw/wallet.go +++ b/internal/openclaw/wallet.go @@ -463,6 +463,14 @@ func generateRemoteSignerValues(wallet *WalletInfo) string { keystorePassword: value: %q +# The signer is a singleton over a ReadWriteOnce keystore PVC: a RollingUpdate +# surge pod can wedge on the volume attach on multi-node clusters and briefly +# double-runs the signer over the same keystore on any cluster. Recreate is the +# chart default as of remote-signer 0.4.0; pinned explicitly here so the intent +# is visible and robust to a future chart-default change. +strategy: + type: Recreate + persistence: enabled: true size: 100Mi