Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/job-orchestration/job_orchestration/reducer/reducer.py`:
- Around line 71-90: The loop that spawns reducers (building
reducer_instance_cmd, opening log_file and calling subprocess.Popen) must be
wrapped in a try/finally so any exception during open() or Popen() still closes
resources: in the finally iterate reducer_log_files and call .close() for each
opened file and remove/close the optional logging_file_handler, and also
terminate/cleanup any partially-started processes recorded in reducers; update
code around reducer_log_files, reducers, reducer_instance_cmd and
subprocess.Popen to ensure files and handlers are closed regardless of
exceptions.

In
`@components/job-orchestration/job_orchestration/scheduler/compress/compression_scheduler.py`:
- Around line 426-432: The optional file logging code in
compression_scheduler.py should guard against missing/unwritable CLP_LOGS_DIR:
before creating logging.FileHandler for log_file, check if
os.path.isdir(logs_dir) and attempt to create it (os.makedirs(logs_dir,
exist_ok=True)) and validate write permissions; wrap FileHandler creation in a
try/except to catch OSError/PermissionError and on failure skip adding the
FileHandler (or fallback to console) while logging the failure via the existing
logger (created by get_logger()). Ensure references to CLP_LOGS_DIR, log_file,
logging.FileHandler, and logger in compression_scheduler.py are updated
accordingly.

In `@tools/deployment/package-helm/.set-up-common.sh`:
- Around line 10-13: Update the JSDoc-style comment for the
create_clp_directories() function to reflect its narrowed behavior: it now only
creates the "samples" subdirectory under CLP_HOME rather than all "CLP data".
Edit the comment to briefly state that the function creates the CLP samples
directory (e.g., "Creates the CLP samples directory at ${CLP_HOME}/samples") and
remove or replace the misleading "CLP data" phrasing so the comment matches the
mkdir -p "$CLP_HOME/samples" implementation.

In `@tools/deployment/package-helm/templates/query-worker-deployment.yaml`:
- Around line 106-110: The emptyDir for the staged-streams volume currently has
no sizeLimit; update the Helm template that defines the volume (the include
"clp.volumeName" with dict
"component_category":"query-worker","name":"staged-streams") to add a sizeLimit
to the emptyDir (e.g., "5Gi") or wire it to a values key so operators can
configure it, ensuring the emptyDir: block includes sizeLimit to prevent
unbounded node disk usage.

In `@tools/deployment/package-helm/templates/redis-statefulset.yaml`:
- Around line 63-70: The templated volume block triggers YAMLlint brace-spacing
errors; fix by normalizing Helm template whitespace inside the mustaches for the
volume name and configMap name: collapse the multiline dict into a single
expression and ensure single spaces immediately after '{{' and before '}}' for
the include calls (references: include "clp.volumeName" and include
"clp.fullname"), so the volume name and configMap name lines render with
properly spaced braces and no extra line breaks.

In `@tools/deployment/package-helm/templates/results-cache-statefulset.yaml`:
- Around line 65-68: The templates currently set resources.requests.storage to a
smaller hard-coded value which will cause Helm upgrades to fail when an existing
PVC is larger; change the template so volumeClaimTemplates (or the field where
resources.requests.storage is defined) uses a configurable value (e.g.,
.Values.persistence.size) and fall back to the existing PVC size via the Helm
lookup function if present (use lookup to read the current PersistentVolumeClaim
and use its spec.resources.requests.storage), or keep the previous default size
(e.g., 20Gi) to avoid shrinking; update the template to reference that value
instead of the hard-coded "10Gi".

---

Outside diff comments:
In `@components/log-ingestor/src/bin/log_ingestor.rs`:
- Around line 46-82: Extract the duplicated set_up_logging into clp-rust-utils
as a public function (e.g., logging::init_logging(service_name: &str) ->
anyhow::Result<Option<tracing_appender::non_blocking::WorkerGuard>>) that builds
the log filename from service_name, creates the RollingFileAppender and
tracing_appender::non_blocking writer when CLP_LOGS_DIR is set, and returns the
guard; inside clp-rust-utils also add a small helper (e.g., init_subscriber<W:
MakeWriter + 'static>(writer: W)) that configures the subscriber chain
(event_format with level/target/file/line/json, EnvFilter::from_default_env,
with_ansi(false)) and calls .with_writer(writer).init(); replace the local
set_up_logging functions in log_ingestor (and api_server) to call
logging::init_logging("log_ingestor") (or "api_server") so the file-vs-stdout
branching and subscriber setup are centralized.

In `@docs/src/dev-docs/design-deployment-orchestration.md`:
- Around line 242-248: The lead sentence currently reads "Services require
persistent storage for logs, data, archives, and streams." which conflicts with
the Kubernetes note about emitting logs to stdout/stderr; update that sentence
to remove "logs" so it reads something like "Services require persistent storage
for data, archives, and streams." Modify the opening line in the same paragraph
(the one before the "Docker Compose" and "Kubernetes" bullets) to remove the
word "logs" and ensure the rest of the bullets remain unchanged so the
Kubernetes note about stdout/stderr stays accurate.

In `@docs/src/user-docs/guides-k8s-deployment.md`:
- Around line 128-145: Update the "Shared storage for workers" section to
explicitly state that the underlying PVCs must support ReadWriteMany (RWX) for
multi-node mounts to work, and add a short note pointing users to pick an
RWX-capable StorageClass in their Helm values; reference the existing directory
keys (archive_output.storage.directory, stream_output.storage.directory,
logs_input.directory) and say that the corresponding PVC/storageClass used by
the chart must be RWX (and indicate the Helm values field where to set the
storageClass used by persistent volumes so operators know to choose an
RWX-backed StorageClass).

In `@tools/deployment/package-helm/templates/_helpers.tpl`:
- Around line 113-138: The PVC helper clp.createPvc removed
storageClassName/selector causing PVCs to hang when no default or when RWX is
required; re-add an optional override in the template so callers can pass a
storage class or selector (e.g., check for .storageClassName and/or .selector on
the template context and emit a storageClassName: <value> line or selector:
<map> block when present). Update the clp.createPvc definition to conditionally
render storageClassName (using .storageClassName | quote) and/or selector
(render the map from .selector or .storageClassSelector under spec) while
leaving behavior unchanged when those keys are not provided; keep references to
clp.fullname and clp.volumeName unchanged.

In `@tools/deployment/package-helm/templates/compression-worker-deployment.yaml`:
- Around line 41-52: Set the CLP_LOGS_DIR env var to the mounted emptyDir path
so logs persist: change the CLP_LOGS_DIR value from "/tmp" to "/var/tmp" in the
deployment template to match the volumeMount mountPath "/var/tmp" (the volume
defined via include "clp.volumeName" for component_category
"compression-worker"). Ensure this aligns with how compression_task.py writes
stderr logs (referenced via CLP_LOGS_DIR).

In `@tools/deployment/package-helm/templates/queue-statefulset.yaml`:
- Around line 1-49: StatefulSet "queue" currently has no volumeClaimTemplates or
volumeMount for /var/lib/rabbitmq, causing RabbitMQ data to be ephemeral; decide
whether persistence is required and if so add a volumeClaimTemplates block and a
corresponding volumeMount in the "queue" container to mount /var/lib/rabbitmq
(follow the same PVC pattern used by database-statefulset and
results-cache-statefulset), otherwise update documentation to explicitly state
the queue is ephemeral and explain broker restart/ message-loss behavior; adjust
the StatefulSet spec (replicas/selector/template) around the container named
"queue" and include the PVC name used by the volumeClaimTemplates so the mount
references the new claim.

In `@tools/deployment/package-helm/templates/shared-data-archives-pvc.yaml`:
- Around line 1-8: The PVC for archives is being created with accessModes set to
ReadWriteOnce via the clp.createPvc invocation, which prevents multi-node
mounts; update the call in shared-data-archives-pvc.yaml (the include
"clp.createPvc" invocation for component_category "shared-data" and name
"archives") so the accessModes list uses ReadWriteMany (or reads the value from
a new configurable .Values.clpConfig.archive_output.storage.accessMode) to allow
RWX mounts on multi-node filesystem deployments; ensure the change preserves
existing capacity and name parameters and falls back to ReadWriteOnce if RWX is
not requested.

In `@tools/deployment/package-helm/templates/shared-data-streams-pvc.yaml`:
- Around line 1-8: The PVC template currently hardcodes accessModes to
ReadWriteOnce in the clp.createPvc invocation for the "streams" shared-data
component, which prevents multi-node mounts; update the invocation in
templates/shared-data-streams-pvc.yaml to use ReadWriteMany (or expose
.Values.clpConfig.stream_output.storage.accessModes and default to
ReadWriteMany) instead of the fixed (list "ReadWriteOnce") so multiple worker
pods on different nodes can mount the same filesystem; ensure the change is
applied to the clp.createPvc call that supplies "component_category"
"shared-data" and "name" "streams".

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/clp-rust-utils/src/logging.rs`:
- Around line 13-14: The branch that handles CLP_LOGS_DIR currently treats an
empty string as a valid path, causing the appender to be rooted at "."; change
the check that reads std::env::var("CLP_LOGS_DIR") so it only proceeds when the
value is non-empty (e.g., use if let Ok(dir) = std::env::var("CLP_LOGS_DIR") and
guard with !dir.trim().is_empty()), then convert that non-empty string to a Path
with std::path::Path::new(dir.as_str()) and continue creating the file appender;
ensure you reference the CLP_LOGS_DIR env var and the local logs_directory
binding in logging.rs when implementing this guard.
- Around line 18-47: The duplicated tracing_subscriber setup should be extracted
into a shared builder/format variable: create the common event_format (the value
from
tracing_subscriber::fmt::format().with_level(true).with_target(false).with_file(true).with_line_number(true).json())
and the common EnvFilter/ANSI settings once, then call .with_writer(...) with
the differing writers (std::io::stdout.and(non_blocking_writer) in the branch
that returns Some(guard) and std::io::stdout in the other) and call .init() for
each; because Format is owned, clone the shared format/builder (use .clone())
when reusing it for the second call so ownership issues are avoided while
keeping the rest of the configuration identical.

In `@tools/deployment/package-helm/values.yaml`:
- Around line 17-18: Wrap the long comment that mentions filesystem storage and
PVC-backed directories so no line exceeds 100 characters; specifically split the
comment line that references archive_output.storage.type: "fs" and the sentence
about PVC-backed directories and examples (e.g., NFS/CephFS) into two or more
shorter comment lines so each is under the lint limit while preserving the
original wording and examples.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/clp-py-utils/clp_py_utils/clp_logging.py`:
- Around line 31-37: The warning call in set_logging_level eagerly formats the
message; change logger.warning(f"Invalid logging level: {level}, using INFO as
default") to use lazy formatting (logger.warning("Invalid logging level: %s,
using INFO as default", level)) so the string interpolation is deferred when the
warning level is disabled; keep the existing behavior of falling back to
logger.setLevel(logging.INFO) and the checks against get_args(LoggingLevel).

In `@components/job-orchestration/job_orchestration/reducer/reducer.py`:
- Line 66: Replace the list concatenation that builds reducer_instance_cmd with
iterable unpacking to follow RUF005: instead of reducer_instance_cmd =
reducer_cmd + [str(clp_config.reducer.base_port + i)], construct
reducer_instance_cmd using unpacking of reducer_cmd and the new string element
(referencing reducer_instance_cmd, reducer_cmd, clp_config.reducer.base_port,
and i) so the resulting list is created with [*reducer_cmd,
str(clp_config.reducer.base_port + i)] style.

In `@tools/deployment/package-helm/templates/compression-worker-deployment.yaml`:
- Around line 87-91: Add a sizeLimit to the emptyDir volumes for the tmp and
staged-archives volumes to prevent unbounded disk usage; locate the volume
blocks where name is generated with {{ include "clp.volumeName" ... "tmp" }} and
the staged-archives entry and add a sizeLimit (e.g. "1Gi" or another value tuned
to expected workload) under emptyDir: so the emptyDir declaration uses sizeLimit
to cap disk usage for compression-worker intermediate data and S3 staging.

In `@tools/deployment/package-helm/templates/query-worker-deployment.yaml`:
- Around line 41-44: CLP_LOGS_DIR is set to /var/log/query_worker but there is
no corresponding volumeMount/volume, so container writes go to the ephemeral
overlay; either remove CLP_LOGS_DIR if file logs aren't needed or add a volume
and mount it at /var/log/query_worker (e.g., an emptyDir or a PVC) in the
Deployment spec for the container that defines CLP_LOGS_DIR and
CLP_WORKER_LOG_PATH; ensure the volume name used in volumeMounts matches the
volume entry and replicate the same change for the other worker deployment that
also sets CLP_LOGS_DIR.

---

Duplicate comments:
In `@components/clp-py-utils/clp_py_utils/clp_logging.py`:
- Around line 43-63: The FileHandler creation in configure_logging can raise if
CLP_LOGS_DIR is missing or unwritable; wrap the Path/ FileHandler setup in a
try/except and harden it by ensuring the directory exists (use
pathlib.Path(...).parent.mkdir(parents=True, exist_ok=True)) before creating
logging.FileHandler, and on exception fall back to adding a std stream handler
(logging.StreamHandler(sys.stdout)) and emit a warning via logger.warning so
startup doesn't abort; keep calls to get_logging_formatter() and
set_logging_level(logger, os.getenv("CLP_LOGGING_LEVEL")) intact.

In `@components/job-orchestration/job_orchestration/reducer/reducer.py`:
- Around line 65-97: The reducer log file handles are leaked if open() or
subprocess.Popen raises during the spawn loop; wrap the whole spawn-and-wait
section (the loop that builds reducer_instance_cmd, opens files into
reducer_log_files, appends subprocess.Popen into reducers, and the subsequent
for i, reducer in enumerate(reducers): reducer.communicate() block) in a
try/finally so cleanup always runs; in the finally iterate over reducers to
gracefully stop/communicate/kill any started subprocesses (e.g., call
terminate() and then communicate() or kill() if needed) and then iterate over
reducer_log_files to close every open file handle, ensuring both
reducer_log_files and reducers are referenced in the finally block so no file or
process remains leaked.

In `@tools/deployment/package-helm/templates/compression-worker-deployment.yaml`:
- Around line 41-44: The CLP_LOGS_DIR environment variable in the compression
worker deployment points to /var/log/compression_worker but there is no backing
volume; either remove the CLP_LOGS_DIR env entry or add an emptyDir volume and
mount it into the compression worker container at /var/log/compression_worker.
To fix, edit the compression-worker container spec: if keeping CLP_LOGS_DIR, add
a volumeMount with mountPath "/var/log/compression_worker" and a matching
volumes entry (type emptyDir) in the pod spec; otherwise delete the env var
CLP_LOGS_DIR (and any related references) so logs default to CLP_WORKER_LOG_PATH
(/dev/stdout). Ensure the changes target the compression-worker container
definition and the CLP_LOGS_DIR env name.

In `@tools/deployment/package-helm/templates/query-worker-deployment.yaml`:
- Around line 106-110: The emptyDir for the staged-streams volume currently has
no size limit which can exhaust node disk; update the deployment template where
the volume name is generated via include "clp.volumeName" with
dict("component_category":"query-worker","name":"staged-streams") to replace the
bare emptyDir: {} with an emptyDir that specifies a sizeLimit (e.g., "5Gi") or
make it configurable via a Helm value, so the staged-streams emptyDir has an
explicit size cap.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tools/deployment/package-helm/Chart.yaml`:
- Line 3: The Chart.yaml version was only incremented in the pre-release counter
("version: \"0.1.4-dev.4\"") despite this PR being a breaking change (removes
the local-storage StorageClass and many PV/PVC templates and values), so update
the Chart.yaml version to reflect semver-breaking changes—bump at least the
minor segment (e.g., "0.2.0-dev.0") or the major if your release policy requires
it (e.g., "1.0.0-dev.0") so consumers are correctly signaled; ensure the new
version string replaces the existing "0.1.4-dev.4" value in Chart.yaml.
- Line 3: This PR is a breaking change but the Chart.yaml "version" field
remains a pre-release patch; update the Chart.yaml version value (the version
key in Chart.yaml) to bump at least the MINOR for a breaking change—for example
change "version: \"0.1.4-dev.4\"" to "version: \"0.2.0-dev.0\"" (or to your
team's chosen stable-release signaling scheme) so the chart semver reflects the
breaking change.

In
`@tools/deployment/package-helm/templates/compression-scheduler-deployment.yaml`:
- Around line 88-93: The emptyDir for the volume named via include
"clp.volumeName" with component_category "compression-scheduler" and name "log"
is unbounded; update the volume spec (the block that currently shows emptyDir:
{}) to include a sizeLimit (e.g. set emptyDir: { sizeLimit: "<appropriate-size>"
}) to cap disk usage and prevent node disk pressure, choosing a size appropriate
for compression-scheduler logs/artifacts; ensure the change only affects that
specific volume entry and keep the "config" volume unchanged.

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@tools/deployment/package-helm/Chart.yaml`:
- Line 3: The Chart.yaml version was correctly bumped to "0.2.0-dev.0" to
reflect the breaking change; no code changes are required—confirm the version
string in Chart.yaml (the version field) is exactly "0.2.0-dev.0" and keep the
change as-is before merging.