feat(cassandra): move the Cassandra stack off Bitnami to official Apache Cassandra - #328
Conversation
Introduce infra/cassandra, the NVCF Cassandra runtime image built on the official Apache cassandra:5.0.8 image, replacing the archived bitnamilegacy base. Layers a pinned, checksum-verified yq (used by the chart's config init container) and an NCP rack-from-pod cassandra-env.sh. The Prometheus exporter agent jar is not redistributed: files/ ships only .gitkeep and the Dockerfile defaults EXPORTER_JAR to that placeholder, so a bare build runs Cassandra with no metrics agent. Builds that need metrics pass EXPORTER_JAR and EXPORTER_JAVAAGENT together. Verified locally: OSS-mode docker build succeeds (yq checksum verified). NO-REF Signed-off-by: James McNamee <jmcnamee@nvidia.com>
…lSet Drop the bitnamicharts/cassandra subchart dependency and deploy Cassandra directly from an in-house StatefulSet chart targeting the official-base image. Removes Chart.lock and the vendored subchart. Included: - StatefulSet, headless/client services, and a cassandra.yaml overlay ConfigMap merged by the image's yq at init time. - storage_compatibility_mode: NONE by default, to keep official-image nodes stream-compatible with the existing fleet. - cluster.extraSeeds for datacenter-expansion. - Resource-preset map derived from the Bitnami common chart (common.resources.preset, Copyright Broadcom Inc., Apache-2.0). Provenance is marked inline at the helper per OSS review guidance; NOTICE is kept minimal because the upstream chart ships no NOTICE file (per the ASF licensing-howto: do not add to NOTICE what is not legally required). The stack default xlarge stays 3Gi request / 6Gi limit. Validated: helm lint passes; template renders presets (xlarge 3Gi/6Gi), storage_compatibility_mode, and seeds correctly. NO-REF Signed-off-by: James McNamee <jmcnamee@nvidia.com>
…e guide Document the 0.6.1 maintenance release that moves Cassandra off the archived Bitnami runtime to the official Apache Cassandra 5.0.8 image and the in-house Helm chart: - 0.6.1.md release notes (what changed, upgrade pointer). - 0.6.0-to-0.6.1-upgrade.md: fresh-install note plus the three migration methods (datacenter expansion, in-place volume adoption, backup and restore) with trade-offs, and the datacenter-expansion requirements (storage_compatibility_mode NONE, token-allocation fallback). - index and Fern nav entries. img/whitespace lint clean and nav paths resolve. fern check could not run locally (node 16 crash, reproduces on clean main); it runs in CI on a supported node. NO-REF Signed-off-by: James McNamee <jmcnamee@nvidia.com>
…ndra Swap the schema-migration image off the archived bitnami/cassandra:5.0.6 base onto the official cassandra:5.0.8 image, matching the runtime image and the in-house chart. The image still installs kubectl and the golang-migrate binary and runs execute_sqls.sh; the official Debian-based image provides apt and cqlsh, so no other change is needed. README base reference updated. NO-REF Signed-off-by: James McNamee <jmcnamee@nvidia.com>
…fficial base The official Debian-based cassandra image does not ship a CA bundle, so the kubectl download over HTTPS fails with `curl: (77) error setting certificate file: /etc/ssl/certs/ca-certificates.crt`. The archived Bitnami base bundled it. Add ca-certificates to the apt install so the image builds on the official base. Verified: the migration image builds from the official cassandra:5.0.8 base. NO-REF Signed-off-by: James McNamee <jmcnamee@nvidia.com>
… cluster name Local validation of the in-place volume adoption path surfaced two operator requirements the guide did not state: - podSecurityContext.fsGroup: 999. Bitnami wrote the data as UID 1001; the official image runs as UID 999 and cannot read the adopted files without re-grouping the volume at mount. - cluster.name must match the existing cluster, or Cassandra refuses to start on the adopted data. Both were confirmed on k3d: the official image came up on a Bitnami-written volume and read the data with these set. NO-REF Signed-off-by: James McNamee <jmcnamee@nvidia.com>
Local CodeRabbit review flagged several major items in the chart and the operator helper scripts. Fixed here (the two license-header-tooling findings are deferred for separate review): - statefulset.yaml: use the cassandra.imagePullSecrets helper instead of a raw toYaml, so simplified string-form pull secrets render valid YAML. - migrate-from-bitnami.sh: fail fast when only one of --probe-keyspace / --probe-table is set, and verify all ring members are UN post-upgrade rather than just one. - relocate-bitnami-layout.sh: preflight every destination for collisions before moving any data, so a collision aborts with no partial move. - cassandra-env.sh: derive the rack from the pod ordinal mod 3 so the three-rack rotation holds past nine replicas; non-numeric hostnames fall back to r1. Verified: helm lint and render (string and empty pull-secret forms), sh -n on the scripts, and the rack rotation across ordinals. NO-REF Signed-off-by: James McNamee <jmcnamee@nvidia.com>
apply-license-header.sh and check-license-header.sh (and their .license-header.txt template) were byte-identical copies of the NATS chart's scripts, not referenced by the Dockerfile, CI, or anything else in the subtree, and redundant with the repo-wide tools/ci/check-license-headers. Removing them also resolves two CodeRabbit findings about the scripts over-stripping and under-verifying the upstream Apache header on cassandra-env.sh. NO-REF Signed-off-by: James McNamee <jmcnamee@nvidia.com>
A re-review surfaced four more items in the ported helper scripts: - initdb.sh: run the keyspace init through the run_cqlsh helper (stdin) instead of passing -p on the cqlsh argv, so the password is not exposed via the process list. - relocate-bitnami-layout.sh: preflight collisions BEFORE renaming the live data/ dir (correcting the earlier fix, which preflighted after the rename and could strand data in the temp dir and break idempotency on rerun). The data/ child, whose target is the nested parent itself, is excluded from the check. - migrate-from-bitnami.sh: scope pod discovery to the target release (app.kubernetes.io/instance=RELEASE) and die if none are found, instead of selecting every Cassandra pod in the namespace and masking failures. - migrate-from-bitnami.sh: wait (up to ~10m) for all nodes to reach UN rather than probing once, so a node still joining is not reported as a failure. Verified: sh -n, helm lint, and a filesystem test of the relocation (relocates cleanly, aborts idempotently on a real collision with the volume untouched, no false positive on the data/ child). NO-REF Signed-off-by: James McNamee <jmcnamee@nvidia.com>
- relocate-bitnami-layout.sh: guard against an interrupted prior relocation. A leftover .bitnami-relocate.* temp dir means a previous run was cut off mid-move; refuse rather than silently reporting "nothing to do" on a half-relocated volume (the nested parent may already be renamed away). - relocate-bitnami-layout.sh: the dry-run plan now iterates hidden entries too, matching what the execution paths actually move. Not changed: the review also flagged the docs verify one-liner `kubectl get pods -A | grep -vE 'Running|Completed|READY'` for missing a 0/1 Running pod. That is a pre-existing repo-wide docs idiom (also in 0.5.0-to-0.6.0-upgrade.md); changing it only here would diverge from every other upgrade guide, so it is left for a repo-wide docs decision. Verified: sh -n, interrupted-state guard aborts with the volume untouched, normal relocation still completes, and dry-run lists hidden entries. NO-REF Signed-off-by: James McNamee <jmcnamee@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe Cassandra stack now uses official Apache Cassandra 5.0.8 images, an in-house Helm StatefulSet chart, updated initialization and migration tooling, and new 0.6.1 upgrade documentation. ChangesCassandra runtime replacement
In-house Helm chart deployment
Credential and data migration operations
Release and upgrade publication
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant MigrationScript
participant Kubernetes
participant Helm
participant Cassandra
Operator->>MigrationScript: confirm migration
MigrationScript->>Kubernetes: validate release, PVC, and retention
MigrationScript->>Cassandra: snapshot and record optional row count
MigrationScript->>Kubernetes: recreate StatefulSet while retaining PVC
MigrationScript->>Helm: upgrade to in-house chart
Helm->>Cassandra: start pods with retained data
MigrationScript->>Cassandra: verify node status and row count
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-feat-cassandra-remove-bitnami.docs.buildwithfern.com/nvcf |
The license-headers CI job (tools/ci/check-license) verifies the root NOTICE lists every sub-NOTICE. Adding deploy/helm/cassandra/NOTICE requires regenerating the root index; done via tools/scripts/update-license. Signed-off-by: James McNamee <jmcnamee@nvidia.com>
The chart carries no third-party attribution that belongs in a NOTICE: the Bitnami subchart was removed, and the one derived artifact (the resource-preset map) is attributed inline in _helpers.tpl per OSRB guidance. In this repo only charts with real third-party notices carry a NOTICE (nats, openbao, container-cache attribute their bundled upstreams); a contentless NVIDIA-only NOTICE should not exist. This also reverts the root NOTICE index entry added in the prior commit. Signed-off-by: James McNamee <jmcnamee@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/helm/cassandra/helm/templates/statefulset.yaml`:
- Around line 175-179: Update the readinessProbe exec command in the StatefulSet
template to match only the current pod’s nodetool status entry, using the
existing POD_IP environment variable in the grep pattern alongside the UN state.
Preserve the current probe timing settings and shell-based execution.
In `@deploy/helm/cassandra/upgrade/migrate-from-bitnami.sh`:
- Around line 73-75: Validate the --release value parsed in the
argument-handling flow and reject any value other than “cassandra” before
deletion, PVC migration, deployment, or post-upgrade checks proceed. Keep the
chart’s fixed resource naming unchanged and emit a clear error for unsupported
release names.
- Around line 89-93: Extend the preflight validation near the existing VALUES
and CHART_DIR checks to render the Helm chart using the supplied values, then
inspect the Cassandra volume mount configuration. Fail before any deletion
unless the effective mount explicitly resolves to subPath: data, while
preserving the existing required-argument and file/directory validations.
- Around line 164-168: Update the pre-migration probe in the CONFIRM path to
fail immediately unless cqlsh and the subsequent parsing produce a non-empty
numeric pre_count. Remove the error-suppressing behavior that hides
authentication, query, or parsing failures, and ensure the destructive migration
steps do not run when the --probe-* count cannot be obtained.
In `@docs/user/release-notes/0.6.0-to-0.6.1-upgrade.md`:
- Around line 99-109: Finalize the in-place adoption support policy before
publishing the upgrade path: in
docs/user/release-notes/0.6.0-to-0.6.1-upgrade.md:99-109, do not present it as
supported until requirements are finalized; in
deploy/helm/cassandra/docs/upgrade-from-bitnami.md:121-129, document the
complete compatible configuration and runbook or adopt a backup-and-restore-only
policy; and in deploy/helm/cassandra/upgrade/README.md:14-18, align the upgrade
tool status and prerequisites with that same policy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4c7e5dc4-a806-495b-8953-169bba4e30de
⛔ Files ignored due to path filters (1)
deploy/helm/cassandra/helm/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
deploy/helm/cassandra/AGENTS.mddeploy/helm/cassandra/CLAUDE.mddeploy/helm/cassandra/README.mddeploy/helm/cassandra/docs/upgrade-from-bitnami.mddeploy/helm/cassandra/helm/Chart.yamldeploy/helm/cassandra/helm/scripts/initdb.shdeploy/helm/cassandra/helm/templates/_helpers.tpldeploy/helm/cassandra/helm/templates/configmap-cassandra-conf.yamldeploy/helm/cassandra/helm/templates/hook-post-01-initdb.yamldeploy/helm/cassandra/helm/templates/hook-post-02-migrations.yamldeploy/helm/cassandra/helm/templates/hook-pre-01-account-rbac.yamldeploy/helm/cassandra/helm/templates/hook-pre-01-initdb-configmap.yamldeploy/helm/cassandra/helm/templates/service-client.yamldeploy/helm/cassandra/helm/templates/service-headless.yamldeploy/helm/cassandra/helm/templates/statefulset.yamldeploy/helm/cassandra/helm/values.yamldeploy/helm/cassandra/upgrade/README.mddeploy/helm/cassandra/upgrade/migrate-from-bitnami.shdeploy/helm/cassandra/upgrade/relocate-bitnami-layout.shdocs/user/release-notes/0.6.0-to-0.6.1-upgrade.mddocs/user/release-notes/0.6.1.mddocs/user/release-notes/index.mdfern/versions/dev.ymlinfra/cassandra/AGENTS.mdinfra/cassandra/CLAUDE.mdinfra/cassandra/Dockerfileinfra/cassandra/README.mdinfra/cassandra/files/.gitkeepinfra/cassandra/scripts/cassandra-env.shmigrations/cassandra/Dockerfilemigrations/cassandra/README.md
sbaum1994
left a comment
There was a problem hiding this comment.
Follow up notes when comparing with current helmfile stack
- stack itself needs to consume new cassandra image instead of bitnami-cassandra
- stack exposes
cassandra.global.defaultStorageClasswhich is bitnami specific, new chart only readscassandra.persistence.storageClass
nodetool status | grep -E '^UN' matched any Up/Normal member, so a pod could report Ready off another node's (or a seed's) UN line before its own node had joined. Scope the match to the pod's own POD_IP so the readiness gate reflects this node's state. Validated on k3d: both nodes reach Ready and the probe matches only the local node's line. Signed-off-by: James McNamee <jmcnamee@nvidia.com>
Reject any --release other than 'cassandra' (the chart's resource names are fixed, so another value would adopt and verify the wrong resources). Render the chart and refuse unless the data mount resolves to subPath: data before any destructive step. Require a numeric pre-migration probe count so a silent cqlsh failure cannot skip the post-migration row-count comparison. Signed-off-by: James McNamee <jmcnamee@nvidia.com>
…gles Expose nodeSelector, tolerations, and affinity on the StatefulSet pod template (empty by default) for parity with the previous chart. Add cassandra.hooks.initializeCluster.enabled and .migrations.enabled (default true) that gate the two post-install jobs, so a datacenter expansion can skip schema-init and migrations on the new datacenter while the pods still start from the pre-install ConfigMap and RBAC. Validated on k3d: enabled install and upgrade run the jobs (migrations idempotent on re-run); disabled install brings the pod up with no jobs. Signed-off-by: James McNamee <jmcnamee@nvidia.com>
…teps Note that in-place volume adoption is validated for the standard NVCF configuration and that a customized cassandra.yaml must be verified. Point the datacenter-expansion procedure at the concrete mechanism for skipping the bootstrap jobs on the new datacenter (the hooks.*.enabled values) and explain why running the migrations there is unsafe. Signed-off-by: James McNamee <jmcnamee@nvidia.com>
…ageClass fallback The self-managed stack routes global.storageClass to cassandra.global.defaultStorageClass (the Bitnami-subchart key). The in-house chart read only persistence.storageClass, so at cutover that value would be silently dropped and the data PVC would fall back to the cluster default StorageClass. Read global.defaultStorageClass as a fallback behind persistence.storageClass, marked deprecated for removal once the stack emits persistence.storageClass directly. Validated on k3d: persistence.storageClass and the global fallback each bind a named StorageClass; unset falls back to the cluster default. Signed-off-by: James McNamee <jmcnamee@nvidia.com>
|
Thanks @sbaum1994 - following up on the two notes from your review: storageClass ( Stack consuming the new image: agreed - repointing the self-managed stack (and the umbrella) off Separately, the installer-neutral lifecycle-job rendering ( |
|
🎉 This PR is included in version helm-nvcf-cassandra-v0.18.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version nvcf-cassandra-migrations-v0.14.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
TL;DR
Moves the NVCF Cassandra stack off the archived Bitnami base and Helm subchart onto official Apache Cassandra 5.0.8 and an in-house Helm chart. Bitnami withdrew its public catalog and the migration image's
bitnami/cassandra:5.0.6base no longer resolves (#190); this removes the last Bitnami runtime dependency in Cassandra with equivalent behavior and a documented migration path.Additional Details
infra/cassandrabuilt oncassandra:5.0.8, layering the Prometheus exporter agent and a checksum-verifiedyq. The exporter jar is not redistributed (onlyfiles/.gitkeep), so a bare build runs Cassandra without metrics.bitnamicharts/cassandrasubchart with an in-house StatefulSet (StatefulSet, headless/client Services, cassandra.yaml overlay). Defaultsstorage_compatibility_mode: NONEfor streaming compatibility with the existing fleet, and addscluster.extraSeedsfor datacenter expansion. The resource-preset map is derived from the Bitnami common chart (Apache-2.0, Broadcom) and attributed inline;NOTICEstays minimal.bitnami/cassandra:5.0.6ontocassandra:5.0.8, addingca-certificates(the official Debian base ships none, which broke thekubectlHTTPS fetch).storage_compatibility_mode: NONE, token-allocation fallback,fsGroup: 999).helm lint/helm template,sh -non the scripts (plus a filesystem test of the relocation logic), and the end-to-end k3d runs listed under For QA.Review updates
Changes made in response to CodeRabbit and maintainer review, all validated on k3d:
nodetool status | grep '^UN'matched any Up/Normal member, so a pod could report Ready before its own node joined. Now scoped to the pod'sPOD_IP.migrate-from-bitnami.shrejects any--releaseother thancassandra(fixed resource names), renders the chart and refuses unless the data mount resolves tosubPath: databefore any destructive step, and requires a numeric pre-migration probe count.nodeSelector,tolerations, andaffinityexposed on the StatefulSet (empty by default) for parity with the previous chart.cassandra.hooks.initializeCluster.enabledand.migrations.enabled(default true) gate the two post-install jobs, so a datacenter expansion can skip schema-init and migrations on the new datacenter while the pods still start from the pre-install ConfigMap/RBAC. The datacenter-expansion doc points at this mechanism.cassandra.yamlmust be verified.Follow-ups (not in this PR):
hook | resource | disabled) is tracked in Installer-neutral lifecycle-job rendering for Helm charts (hook | resource | disabled) #337; the per-hook toggle here is an interim mechanism it will reconcile.cassandra.global.defaultStorageClassvalue maps to the new chart'scassandra.persistence.storageClass.For the Reviewer
infra/cassandra/Dockerfile,deploy/helm/cassandra/helm/templates/statefulset.yaml, and_helpers.tpl(the in-house chart replacing the subchart)._helpers.tplcarries an inline Broadcom attribution for the derived preset map, andNOTICEis intentionally minimal - reviewed and approved by the OSRB.deploy/helm/cassandra/upgrade/touch data volumes; they have been through three CodeRabbit passes.For QA
Validated end-to-end on a local k3d cluster against the built images and chart:
UN, auth enforced, init + migrations complete (all app keyspaces), exporter serving 123 metric families.subPath: data, reads the data,fsGroup: 999resolving UID-1001 to UID-999 ownership.nodetool rebuildstreams the data.helm lint,helm template, andsh -non the scripts.Review-update validation (k3d):
UN/Ready; the scoped probe matches only the local node's line, not anyUNmember.helm upgradere-runs them with migrations idempotent (golang-migrate reports "no change" for every keyspace); disabled fresh install brings the pod up with no init/migration jobs.Is QA needed? Yes - validate the chosen migration method in staging before production.
Issues
Closes #317
Closes #190
Checklist
Summary by CodeRabbit