Skip to content

nvsnap: replace environment-variable configuration with typed flags and chart values #1023

Description

@balajinvda

Problem

nvsnap behaviour is configured through roughly 40 distinct NVSNAP_* environment
variables read across the agent, webhook, restore entrypoint and helper binaries.
Thirty of them appear nowhere in the Helm chart, so the only way to learn what a
running cluster is set to is to exec into a DaemonSet and read the pod spec.

This is not theoretical. During the recent restore-reliability investigation
(#925) time was lost suspecting NVSNAP_CUDA_PARALLELISM precisely because its
effective value was not visible anywhere. A configuration surface you cannot read
is one you end up guessing about.

The most significant example is NVSNAP_CRIU_V2
(internal/agent/checkpoint_v2.go:85): which capture engine runs is decided by an
environment variable with no chart surface, no schema and no validation.

A recent change of ours added a 41st and it has already been removed
(NVSNAP_DUMP_PIDNS_ROOT, commit 625f556) rather than merged, on the grounds
that an env switch which changes what a capture contains is invisible, untyped
and outlives the experiment that introduced it. That reasoning applies more
broadly.

Proposed categories

Not every one of these is wrong. They should be sorted before anything is
changed:

  1. Process-to-process wiring, legitimate as env. The agent or webhook sets these
    for a child it launches; they are not user configuration.
    NVSNAP_ORIG_COMMAND, NVSNAP_ORIG_CWD, NVSNAP_CHECKPOINT_HOST_PATH,
    NVSNAP_CAPTURE_NODE, NVSNAP_AGENT_TOKEN, NVSNAP_VERSION.

  2. Behaviour switches that change what the system does. These should be agent
    flags plumbed through chart values, with the value visible in the rendered
    spec.
    NVSNAP_CRIU_V2, NVSNAP_DEFAULT_CAPTURE_PATH, NVSNAP_CAPTURE_PLAN,
    NVSNAP_NO_OVERLAY, NVSNAP_LAZY_PAGES, NVSNAP_COMPRESS_CHECKPOINT,
    NVSNAP_CRIU_V2_COMPRESS, NVSNAP_REPLAY_MOUNTS, NVSNAP_ALLOW_WHOLE_ROOTFS.

  3. Tuning knobs. Chart values with documented defaults and range validation.
    NVSNAP_CUDA_PARALLELISM, NVSNAP_PEER_FETCH_CONCURRENCY,
    NVSNAP_PREP_DEADLINE, NVSNAP_PREP_POLL, NVSNAP_KEEPALIVE_SECONDS.

  4. Debug-only. Keep as env, but name them consistently and confirm they are inert
    by default.
    NVSNAP_STRACE_ENABLED, NVSNAP_DEBUG_IO_URING, NVSNAP_DISABLE_IO_URING_REINIT.

  5. Deployment wiring that belongs in values.
    NVSNAP_L2_*, NVSNAP_REPLICATION_*, NVSNAP_BLOB_STORE_URL,
    NVSNAP_CATALOG_URL, NVSNAP_WEBHOOK_AGENT_BASE_URL, NVSNAP_FSSTORE_PATH,
    NVSNAP_POD_CACHE_DIR, NVSNAP_WORKLOADS_DIR.

  6. Dead. Some of these are likely unused; delete rather than migrate.

Tasks

  • Complete the inventory, including any non-NVSNAP_ prefixed reads.
  • Sort every variable into the categories above and confirm which are dead.
  • Migrate category 2 and 3 to agent flags plus chart values, keeping the env
    var as a deprecated fallback for one release.
  • Log effective configuration once at agent startup, so a running cluster can
    be inspected from logs rather than by exec.
  • Fail startup on an unrecognised NVSNAP_* variable, so a typo is loud
    rather than silently ignored.
  • Document the surviving set.

Notes

Reproduce the inventory with:

grep -rnoE 'os\.Getenv\("NVSNAP_[A-Z0-9_]+"\)' --include=*.go internal/ cmd/

The startup-logging and unknown-variable tasks are worth doing first even if the
migration is slow: both make the current state visible without changing any
behaviour.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions