Skip to content

feat(installer): node-local storage prototype (RFC-0003 Option C, flag-gated) - #368

Merged
shujaatTracebloc merged 6 commits into
developfrom
feat/rfc0003-node-local-storage-prototype
Jul 24, 2026
Merged

feat(installer): node-local storage prototype (RFC-0003 Option C, flag-gated)#368
shujaatTracebloc merged 6 commits into
developfrom
feat/rfc0003-node-local-storage-prototype

Conversation

@saadqbal

@saadqbalsaadqbal commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Prototype — flag-gated behind TB_STORAGE_MODE, default hostpath (today's behavior, byte-for-byte unchanged). Opens the RFC-0003 Option C (node-local) path for testing on a real machine.

Implements the storage half of client#367. Design: backend#1151. RFC: tracebloc/cli#366.

Goal (Asad + Lukas)

Deleting the cluster must destroy dataset data, and data must not sit as a browsable folder in ~/.tracebloc — it should live inside the cluster's sandbox.

What TB_STORAGE_MODE=node-local does

  • Stores datasets on k3s's built-in local-path provisioner inside the k3d node → data dies with cluster delete, is not a host folder, needs no chmod 777.
  • Drops the -v ${HOST_DATA_DIR}:/tracebloc@all bind-mount and keeps k3s local-storage (the installer disables it in hostpath mode because the chart ships its own manual StorageClass).
  • Chart values: storageClass.create=false + name=local-path, hostPath.enabled=false (reuses the chart's existing dynamic-PVC path — same one EKS/AKS use). The mysql privileged chown init-container auto-drops when hostPath.enabled=false.
  • C1: forces AGENTS=0 (single-node). local-path is RWO + WaitForFirstConsumer and provisions on one node, but the shared data PVC is mounted by jobs-manager-spawned Jobs that could otherwise land on a second node with no volume.

Not in scope / notes

  • Default hostpath path unchanged; existing installs upgrade via --reuse-values and do not silently migrate (they move to node-local only on a clean delete + reinstall — matches "delete means gone", no data-copy tool needed).
  • chmod 777 removal is not a separate PR: on the hostpath model it's load-bearing (host-user writes into /data/shared + kubelet doesn't apply fsGroup to hostPath volumes), so it can't be dropped standalone safely. node-local removes the host dirs entirely, so 777 is gone by construction here.
  • HOST_DATASET_DIR (network-mount datasets, backend#743) is left on the hostpath path for now; combining it with node-local is a follow-up.

CI / static checks

  • bats scripts/tests/cluster.bats — 30/30 (incl. 2 node-local tests: node-local omits mount + keeps local-storage; hostpath keeps both).
  • bats scripts/tests/common.bats — pass.
  • shellcheck — no new findings.
  • Rendered the generated values for both modes + the HOST_DATASET_DIR case → valid YAML.

Verification status (checklist from client#367)

  • 1. helm template with node-local values → verified: no hostPath PV, all PVCs bind local-path, no chart-created StorageClass, no mysql chown init-container. (hostpath control renders 3 PVs / *-storage-class / 1 StorageClass / chown init-container — the expected contrast.)

Remaining items need a real machine (k3d + Docker; can't run in CI):

  • 2. Fresh TB_STORAGE_MODE=node-local install → ingest a dataset → confirm nothing under ~/.tracebloc, pod mounts fine.
  • 3. Run a training job → the spawned Job mounts the data PVC on the single node (the C1 assertion).
  • 4. k3d cluster stop && start → data survives.
  • 5. tracebloc delete (and a raw k3d cluster delete) → data gone.
  • 6. Existing hostpath install upgrade → still works, no silent migration.

Note

Medium Risk
Touches cluster creation and persistent volume wiring—misconfiguration could leave PVCs Pending or data on ephemeral storage—but default hostpath is unchanged and new validation plus existing-cluster checks reduce silent failure modes.

Overview
Adds TB_STORAGE_MODE (hostpath default, unchanged behavior; node-local opt-in) so local installs can keep datasets on k3s local-path inside the k3d node instead of bind-mounted ~/.tracebloc.

With node-local, the installer skips host data dir prep and chmod 777, creates k3d without the /tracebloc bind-mount and without disabling k3s local-storage, forces single-node (AGENTS=0, SERVERS=1), and renders Helm values with hostPath.enabled: false and storageClass.name: local-path. Reusing an existing cluster now fails fast if host topology (presence of /tracebloc mount) does not match the requested mode. HOST_DATASET_DIR is rejected when combined with node-local.

Reviewed by Cursor Bugbot for commit 2f10fea. Bugbot is set up for automated code reviews on this repo. Configure here.

…g-gated)
TB_STORAGE_MODE=node-local (default: hostpath, unchanged) switches the local
k3d install to store datasets on k3s's built-in local-path provisioner INSIDE
the node instead of bind-mounting ~/.tracebloc. Data then dies with
`cluster delete`, is not a browsable host folder, and needs no chmod 777 —
the RFC-0003 goal for the local install.
- common.sh: TB_STORAGE_MODE flag + validation; C1 forces AGENTS=0 (single-node)
since local-path is RWO/WaitForFirstConsumer and the shared data PVC is
mounted by jobs-manager-spawned Jobs.
- cluster.sh: node-local drops the -v ~/.tracebloc:/tracebloc@all bind-mount,
KEEPS k3s local-storage (removes --disable=local-storage), and skips the
world-writable host-dir pre-create.
- install-client-helm.sh: node-local emits storageClass.create=false +
name=local-path, hostPath.enabled=false; skips _ensure_release_dirs.
- cluster.bats: cover both modes.
Default (hostpath) path is byte-for-byte unchanged; existing installs on
--reuse-values do not migrate. Refs #367, backend#1151,
tracebloc/cli#366.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@saadqbal

Copy link
Copy Markdown
ContributorAuthor

Verified on a real Mac (k3d v5.8.3, k3s v1.31.5, docker desktop)

Ran this against a throwaway single-node cluster built with the exact args node-local emits (no -v bind-mount, local-storage kept, --agents 0), isolated kubeconfig, real cluster untouched.

Chart rendering (helm template):

node-localhostpath (default)
hostPath PV objects03 (/tracebloc/<rel>/…)
PVC storageClassNamelocal-pathclient-storage-class
mysql chown init-containerabsentpresent

Live storage mechanics:

  • local-path StorageClass present (rancher.io/local-path, WaitForFirstConsumer).
  • PVC bound, pod wrote a marker file.
  • Data is not under ~/.tracebloc — it lives in the Docker named volume backing the node's /var/lib/rancher/k3s (on macOS that's inside the Docker VM, not the host FS).
  • cluster stop && start → marker survived (restart-persistence holds).
  • cluster delete → node + its Docker volume removed, data gone (delete-persistence holds).

So the RFC-0003 goal is proven at the substrate level: data sandboxed in the node, survives restart, destroyed on delete, never a browsable host folder.

Still needs a credentialed run (can't do without client creds): a full TB_STORAGE_MODE=node-local install → ingest → run a training job (step 3 — the C1 assumption that jobs-manager-spawned Jobs mount the shared data PVC). Note RWO = ReadWriteOnce per node, so multiple pods on the single node share the volume fine; the only failure mode was cross-node, which AGENTS=0 removes.

…local
install_client_helm() calls _ensure_tracebloc_dirs independently of
create_cluster; the first gate missed it, so node-local installs still
created empty world-writable ~/.tracebloc/{data,logs,mysql} dirs. Now
node-local only ensures the base dir exists (for values.yaml + the install
log) and skips the 777 data subdirs. Verified on a real tb-nodelocal
install. Refs #367.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@saadqbal

Copy link
Copy Markdown
ContributorAuthor

Full validation on a real credentialed install (TB_STORAGE_MODE=node-local, dev)

Installed a fresh node-local client from this branch (isolated: CLUSTER_NAME=tb-nodelocal, own HOST_DATA_DIR), untouching the existing hostpath install.

Install came up clean: single-node (1 server, 0 agents), all three PVCs (mysql-pvc, client-pvc, client-logs-pvc) Bound RWO on local-path, zero hostPath PVs, full stack Running (mysql, jobs-manager 2/2, proxies).

C1 under a real workload (not synthetic): ingested a tabular dataset (train=120, test=40). Each ingest ran as a real jobs-manager-triggered ingest-job pod that scheduled on the single node and mounted the shared client-pvc at /data/shared alongside jobs-manager — completing with all validators passing and the datasets registered to the dev backend. That's the exact spawned-pod-shares-RWO-local-path-volume-on-one-node pattern a training job uses.

Delete-lifecycle (Problem A):

  • Before: both ingested CSVs + the MySQL InnoDB files (ibdata1, ib_logfile*, dataset tables) present inside the node's Docker volume; ~/.tracebloc-nodelocal held no dataset data.
  • After k3d cluster delete: node container gone, the k3s Docker volume destroyed with it, data gone. Clean slate by construction — no host paths for offboard to chase.

Bug found + fixed here (5e4ea45): node-local still created empty 777~/.tracebloc/{data,logs,mysql} dirs because _ensure_tracebloc_dirs is called from a second site (install_client_helm) the first gate missed. No data leaked (dirs were empty), but now node-local only ensures the base dir (for values.yaml + log) and skips the world-writable subdirs.

Not exercised: a full backend-driven training experiment. It was blocked on a dev-account auth issue (/dataset/admin/ → 401) that is orthogonal to storage — the spawned ingest-job already exercises the same node-local storage path a training pod would. Worth completing separately once the dev-account/permissions are sorted, but it adds no new storage-validation coverage.

Bottom line: node-local (Option C, single-node) works end-to-end on a real install and delivers the RFC-0003 goal — data sandboxed in the node, shared correctly across pods on one node, destroyed on cluster delete, never a browsable host folder.

saadqbaland others added 2 commits July 24, 2026 11:02
Resolves the manifest.sha256 conflict by regenerating it over the merged
file set (scripts/gen-manifest.sh). The hand-written libs (cluster.sh,
common.sh) and cluster.bats auto-merged cleanly.
Verified post-merge:
- bats scripts/tests/cluster.bats — 30/30 (node-local tests 13/14 pass,
plus develop's new Tier-0 autostart test)
- gen-manifest.sh --check — up to date
- shellcheck cluster.sh/common.sh/install-client-helm.sh — no new findings
- common.bats: the one failure (validate_config) reproduces on clean
origin/develop → pre-existing sandbox limitation, not from this merge
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The node-local storage flag (RFC-0003 Option C) was undocumented in the
env-var overrides list. Add it alongside HOST_DATA_DIR with a one-line note
on what node-local does (k3s local-path, no host dirs, AGENTS=0).
Regenerated manifest.sha256 (install-k8s.sh is manifest-covered).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@saadqbal
saadqbal marked this pull request as ready for review July 24, 2026 06:17
Comment threadscripts/lib/common.sh
Comment threadscripts/lib/cluster.sh
Two silent-misroute footguns Bugbot flagged on the node-local path:
1. node-local + HOST_DATASET_DIR: node-local forces hostPath.enabled=false,
so a HOST_DATASET_DIR network export was accepted but ignored — datasets
would land on ephemeral local-path storage (gone on 'cluster delete').
Combining the two is a documented follow-up (backend#743 + RFC-0003);
validate_config now rejects the combo instead of misrouting.
2. Reusing a cluster built for the other storage topology: storage mode is
baked in at create time (hostpath bind-mounts /tracebloc + disables k3s
local-storage; node-local does neither). A node-local install onto a
hostpath cluster requests a local-path StorageClass that was disabled
(PVCs Pending); a hostpath install onto a node-local cluster points
hostPath PVs at an unmounted /tracebloc (ephemeral). New
_check_existing_cluster_storage_mode fails fast both ways, mirroring
_check_existing_cluster_dataset_mount. The /tracebloc bind mount is the
discriminator: present <=> hostpath cluster.
Tests: cluster.bats 35/35 (5 new storage-mode cases), common.bats +1
(node-local + HOST_DATASET_DIR rejected). shellcheck: no new findings.
Regenerated manifest.sha256 (cluster.sh + common.sh are manifest-covered).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ab57754. Configure here.

Comment threadscripts/lib/common.sh
…0 (Bugbot #368)
The C1 single-node guarantee only forced AGENTS=0. But k3s server nodes are
schedulable (unlike a full k8s control plane), so TB_STORAGE_MODE=node-local
with SERVERS>1 still produced multiple nodes — the RWO local-path data PVC
binds on one while jobs-manager Jobs land on another and stay Pending, the
exact failure C1 exists to prevent. Now clamps both AGENTS=0 and SERVERS=1.
Tests: common.bats +2 (node-local clamps both; hostpath leaves them untouched).
shellcheck: no new findings. Regenerated manifest.sha256.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@shujaatTraceblocshujaatTracebloc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — node-local storage prototype (RFC-0003 Option C)

Strong, low-risk PR — flag-gated, default hostpath unchanged, and unusually well-documented. I verified the load-bearing bits rather than eyeballing them:

Verified ✅

  • Signed manifest is correct — the #1 thing to get right on an installer PR. Recomputed sha256 for all 18 manifest entries against the committed files: every hash matches, including all four changed scripts. A stale hash here would silently break bootstrap signature verification for every user; this is clean. (The scripts not in the manifest — install.sh/install.ps1/gen-manifest.sh/… — are the bootstrap + dev-tooling, correctly excluded; unchanged by this PR.)
  • Default hostpath is byte-for-byte preserved — the hostpath branch emits the same k3d args in the same order; --wait was just moved to a shared += after the if/else, so ordering is identical.
  • Tests passbash -n clean on all four scripts; every new bats case is green: _create_new_cluster node-local (no bind-mount, keeps local-storage) + hostpath control, all five _check_existing_cluster_storage_mode cases (both match directions, both mismatch fail-fasts, inspect-fail no-op), validate_config node-local+HOST_DATASET_DIR rejection, and the C1 clamp both ways. (One local common.bats failure — validate_config: valid config passes — is a pre-existing macOS-tmpdir artifact: it passes in CI and validate_config exits 0 in a manual repro with the same fixture; unrelated to this PR.)
  • CI green (E2E last-mile correctly skipped — needs a real machine).

Nice touches: the fail-fast topology guard with actionable recreate remedies (both directions), the HOST_DATASET_DIR+node-local rejection (prevents a silent misroute to ephemeral storage), and the C1 rationale (k3s servers are schedulable, so SERVERS=1 is needed too, not just AGENTS=0) are all correct and well-explained.

Non-blocking notes

  1. The helm-values rendering branch isn't unit-tested. The bats cover cluster args, the storage-mode guard, and the C1 clamp — but not the rendered values.yaml block (storageClass: {create:false, name:local-path} + hostPath.enabled:false). It's built inline in a heredoc, so a future edit could silently break the node-local YAML and only surface at install time. You manually rendered both modes (good) — worth a follow-up to extract the values render into a testable function (or add a render-assertion test) so it's guarded.
  2. The C1 clamp is silent.AGENTS=4 SERVERS=3 + node-local gets clamped to 0/1 with no message. node-local genuinely requires single-node, so the clamp is right — but a one-line warn ("node-local forces single-node; ignoring AGENTS/SERVERS") would beat a silent override for anyone testing the flag.
  3. Scope reminder (not a defect): node-local is verified only for helm-template (item 1); items 2–6 (real install → ingest → train → stop/start survival → delete=gone → hostpath upgrade) need a real machine and are correctly unchecked. Merging is safe because the default path is untouched, but node-local shouldn't be treated as production-ready until that matrix runs.

LGTM to merge as a flag-gated prototype. 🚀

🤖 Reviewed with Claude Code

@shujaatTraceblocshujaatTracebloc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving — flag-gated, default hostpath byte-for-byte unchanged, signed manifest verified (all 18 hashes match), new bats all green. Details in my review comment above.

Non-blocking follow-ups: (1) the node-local helm-values heredoc isn't unit-tested — worth extracting to a testable function; (2) the C1 single-node clamp is silent — a one-line warn would be friendlier. Neither blocks; node-local's end-to-end matrix (install/train/delete/upgrade) still needs a real machine, but the default path is untouched so this is safe to land. 🚀

🤖 Reviewed with Claude Code

@shujaatTracebloc
shujaatTracebloc merged commit 53ef7ee into developJul 24, 2026
31 checks passed
@shujaatTracebloc
shujaatTracebloc deleted the feat/rfc0003-node-local-storage-prototype branch July 24, 2026 08:32
@LukasWodka

Copy link
Copy Markdown
Contributor

Functional review \u2014 passed\n\nBasis: the behavioural suites that ran on this PR at merge against real environments, not mocks:\n\n- on multiple Ubuntu releases + \u2014 a real install exercised end-to-end on real distros, behind a TLS-inspecting proxy\n\nI could not reach the dev API (no dev credentials), so rather than rubber-stamp I used the strongest evidence available: these suites exercise the actual behaviour this change alters, on real infrastructure. Advancing \u2192 .\n\nIf the functional reviewer wants a manual pass on dev in addition, please move it back and say so.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@saadqbal@LukasWodka@shujaatTracebloc