diff --git a/internal/agentruntime/signer.go b/internal/agentruntime/signer.go new file mode 100644 index 00000000..f026df62 --- /dev/null +++ b/internal/agentruntime/signer.go @@ -0,0 +1,59 @@ +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 new file mode 100644 index 00000000..313533a7 --- /dev/null +++ b/internal/agentruntime/signer_test.go @@ -0,0 +1,58 @@ +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/embed/embed_pdb_test.go b/internal/embed/embed_pdb_test.go new file mode 100644 index 00000000..41180c1e --- /dev/null +++ b/internal/embed/embed_pdb_test.go @@ -0,0 +1,103 @@ +package embed + +import ( + "strings" + "testing" +) + +// TestPodDisruptionBudgets pins the PDB stance for the singleton services on +// user-facing paths. On multi-node clusters a `kubectl drain` would otherwise +// silently evict the only pod of the payment gate (x402-verifier → every +// /services/* route 5xxs) or the RPC front door (eRPC). minAvailable: 1 on a +// single replica deliberately blocks voluntary eviction until the operator +// deletes the pod explicitly — the same stance llm.yaml takes for litellm. +// +// Rollouts are NOT affected by PDBs; both deployments surge gaplessly via +// RollingUpdate maxUnavailable 0 (default rounding at 1 replica). +func TestPodDisruptionBudgets(t *testing.T) { + tests := []struct { + name string + file string + pdbName string + namespace string + wantSelector map[string]string + }{ + { + name: "x402-verifier payment gate", + file: "base/templates/x402.yaml", + pdbName: "x402-verifier", + namespace: "x402", + wantSelector: map[string]string{ + "app": "x402-verifier", + }, + }, + { + name: "erpc RPC front door", + file: "base/templates/erpc.yaml", + pdbName: "erpc", + namespace: "erpc", + // Must match the ethereum/erpc chart's selectorLabels, which the + // base chart does not control — verified against the live + // deployment's spec.selector.matchLabels. + wantSelector: map[string]string{ + "app.kubernetes.io/name": "erpc", + "app.kubernetes.io/instance": "erpc", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + data, err := ReadInfrastructureFile(tt.file) + if err != nil { + t.Fatalf("ReadInfrastructureFile(%s): %v", tt.file, err) + } + + pdb := findDocByName(multiDoc(data), "PodDisruptionBudget", tt.pdbName) + if pdb == nil { + t.Fatalf("PodDisruptionBudget %q missing from %s", tt.pdbName, tt.file) + } + if ns := nested(pdb, "metadata", "namespace"); ns != tt.namespace { + t.Errorf("namespace = %v, want %s", ns, tt.namespace) + } + if ma := nested(pdb, "spec", "minAvailable"); ma != 1 { + t.Errorf("minAvailable = %v, want 1", ma) + } + sel, ok := nested(pdb, "spec", "selector", "matchLabels").(map[string]any) + if !ok { + t.Fatal("PDB missing spec.selector.matchLabels") + } + for k, v := range tt.wantSelector { + if sel[k] != v { + t.Errorf("selector[%s] = %v, want %s", k, sel[k], v) + } + } + if len(sel) != len(tt.wantSelector) { + t.Errorf("selector has %d labels %v, want exactly %v — extra labels risk matching no pods", len(sel), sel, tt.wantSelector) + } + }) + } +} + +// TestCloudflaredPDBTemplate pins the pre-existing tunnel-connector PDB: it +// must stay gated on an active tunnel (local/remote mode) with >1 replica, +// where minAvailable: 1 lets drains roll one connector at a time without +// dropping the public path. +func TestCloudflaredPDBTemplate(t *testing.T) { + data, err := ReadInfrastructureFile("cloudflared/templates/pdb.yaml") + if err != nil { + t.Fatalf("ReadInfrastructureFile: %v", err) + } + raw := string(data) + + for _, want := range []string{ + "kind: PodDisruptionBudget", + "minAvailable: 1", + "app.kubernetes.io/name: cloudflared", + "gt $persistentReplicas 1", + } { + if !strings.Contains(raw, want) { + t.Errorf("cloudflared pdb.yaml missing %q", want) + } + } +} diff --git a/internal/embed/infrastructure/base/templates/erpc.yaml b/internal/embed/infrastructure/base/templates/erpc.yaml index 635665d3..e80f92ac 100644 --- a/internal/embed/infrastructure/base/templates/erpc.yaml +++ b/internal/embed/infrastructure/base/templates/erpc.yaml @@ -98,3 +98,24 @@ data: } } } + +--- +# PodDisruptionBudget: eRPC is the single RPC front door for agents and the +# frontend. The upstream ethereum/erpc chart (0.0.4) has no PDB template of +# its own (its `podDisruptionBudget` value is unwired), so it lives here with +# the other erpc-namespace resources. With one replica, minAvailable: 1 makes +# voluntary evictions (multi-node drains, upgrades) require explicit operator +# intent instead of silently gapping local RPC. Rollouts are unaffected — the +# chart's RollingUpdate at 1 replica surges 0-unavailable behind its +# readiness probe (`network add/remove` restarts stay gapless). +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: erpc + namespace: erpc +spec: + minAvailable: 1 + selector: + matchLabels: + app.kubernetes.io/name: erpc + app.kubernetes.io/instance: erpc diff --git a/internal/embed/infrastructure/base/templates/x402.yaml b/internal/embed/infrastructure/base/templates/x402.yaml index a1504973..6a053213 100644 --- a/internal/embed/infrastructure/base/templates/x402.yaml +++ b/internal/embed/infrastructure/base/templates/x402.yaml @@ -411,6 +411,26 @@ spec: targetPort: http protocol: TCP +--- +# PodDisruptionBudget: the verifier is the ForwardAuth gate for every paid +# route — evicting its only pod takes all /services/* traffic down. With the +# deliberate single replica (see the Deployment comment on per-pod metric +# registries), minAvailable: 1 makes voluntary evictions (multi-node drains, +# upgrades) require explicit operator intent instead of silently gapping the +# payment path. Same stance as the litellm PDB in llm.yaml. Rollouts are +# unaffected — the default RollingUpdate surge at 1 replica is 0-unavailable, +# gated by /readyz (config + routes loaded). +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: x402-verifier + namespace: x402 +spec: + minAvailable: 1 + selector: + matchLabels: + app: x402-verifier + --- # ServiceMonitor for x402-verifier — scrapes the stable Service endpoint # rather than per-pod IPs (which is what a PodMonitor would do). Lives diff --git a/internal/hermes/hermes.go b/internal/hermes/hermes.go index 479ca5a5..27d6db76 100644 --- a/internal/hermes/hermes.go +++ b/internal/hermes/hermes.go @@ -254,6 +254,13 @@ 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 3fba946c..442735b4 100644 --- a/internal/openclaw/openclaw.go +++ b/internal/openclaw/openclaw.go @@ -454,6 +454,13 @@ 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,