Skip to content

feat(ess) add helm charts - #723

Merged
nvaghela-oss merged 18 commits into
mainfrom
feat/ess-helm-charts
Aug 10, 2026
Merged

feat(ess) add helm charts#723
nvaghela-oss merged 18 commits into
mainfrom
feat/ess-helm-charts

Conversation

@nvaghela-oss

@nvaghela-oss nvaghela-oss commented Aug 7, 2026

Copy link
Copy Markdown
Contributor
  1. Add support for ESS Helm charts
  2. Add support for seeding the initial version, so consumers importing the service/charts can continue their versioning journey from the internal baseline
  3. Important to note that if consumers have already imported the service/chart before this PR anchor will need to happen manually and intial_version will not be honored. Please read this doc section.

Additional Details (optional for docs, build, test, refactor, ci, chore, style, and revert PRs)

For the Reviewer

For QA (optional for docs, build, test, refactor, ci, chore, style, and revert PRs)

Issues

Closes: #722

Checklist

  • I am familiar with the Contributing Guidelines.
  • I have signed off my commits for Developer Certificate of Origin (DCO) compliance.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Summary by CodeRabbit

  • New Features

    • Added Helm-based deployment support for the ESS API.
    • Supports configurable images, services, ports, resources, health checks, security settings, and environment variables.
    • Added optional CPU and memory autoscaling.
    • Added Vault integration for securely supplying application secrets.
    • Added chart packaging and OCI registry publishing workflows.
  • Documentation

    • Added installation, upgrade, configuration, verification, and troubleshooting guidance.
    • Documented configurable initial release versions for services and charts.
  • Chores

    • Added release tracking, version validation, and CI support for the ESS Helm chart.

github-release auto already synthesizes a local floor tag when a service
has no release tag, but the floor was hardcoded to 0.0.0, so a newly
registered subproject always started its release line at 0.1.0.

Add an optional initial_version field to the release metadata and use it
as the synthesized floor, with SemVer validation. This lets an imported
subproject continue an existing upstream version line without a manual
anchor push. Default behavior is unchanged when the field is absent.

Signed-off-by: Nilesh Vaghela <nvaghela@nvidia.com>
Import the ESS API Helm chart from the upstream ess-colocated-deploy
repository into deploy/helm/ess, following the api-keys-colocated peer
layout: the chart lives in the nested ess-api/ directory alongside a
wrapper Makefile, README, and values.local.yaml.

Governance and CI files owned centrally by the monorepo are dropped
(.gitlab-ci.yml, .oss-allowlist, CODEOWNERS, SECURITY.md, license-header
scripts). The Makefile license-header targets are removed with them.

Register ess-helm as a release subproject with initial_version 1.7.0 to
continue the upstream chart version line, and add a CI-only values file
so the chart, which requires image.registry and image.repository, can be
linted and templated in CI.

Signed-off-by: Nilesh Vaghela <nvaghela@nvidia.com>
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • ✅ Review completed - (🔄 Check again to review again)
📝 Walkthrough

Walkthrough

The PR imports the ESS API Helm chart into the monorepo. It adds Kubernetes templates, Vault integration, deployment tooling, documentation, CI validation values, and release automation metadata with initial-version handling.

Changes

ESS Helm chart release

Layer / File(s) Summary
Chart contracts and secret configuration
deploy/helm/ess/ess-api/Chart.yaml, deploy/helm/ess/ess-api/values.yaml, deploy/helm/ess/ess-api/templates/_helpers.tpl, deploy/helm/ess/ess-api/templates/configmap-*.yaml, deploy/helm/ess/ess-api/vault-agent-templates/secrets.json.tmpl
Defines chart metadata, configurable values, naming and image helpers, environment configuration, Vault annotations, and rendered ESS secrets.
Kubernetes workload resources
deploy/helm/ess/ess-api/templates/deployment.yaml, deploy/helm/ess/ess-api/templates/service.yaml, deploy/helm/ess/ess-api/templates/serviceaccount.yaml, deploy/helm/ess/ess-api/templates/hpa.yaml
Renders the ESS API Deployment, ClusterIP Service, ServiceAccount, optional HPA, Vault volumes, runtime settings, ports, resources, and health probes.
Helm operations and documentation
deploy/helm/ess/Makefile, deploy/helm/ess/README.md, deploy/helm/ess/AGENTS.md, deploy/helm/ess/CLAUDE.md, deploy/helm/ess/values.local.yaml, deploy/helm/ess/.gitignore, deploy/helm/ess/ess-api/.helmignore
Adds Helm lifecycle, validation, packaging, and OCI publishing targets. Documents deployment and HPA configuration. Adds local values, subtree guidance, and ignore rules.
Release registration and validation
tools/ci/github-release, tools/ci/github-release-subprojects.json, tools/ci/helm-validate-values/ess.yaml, tools/ci/test-github-release.py, docs/dev/github-release-process.md
Registers the chart as ess-helm at version 1.7.0, adds CI image values, supports configured initial release floors, and validates and documents default, configured, and invalid versions.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Helm
  participant Kubernetes
  participant VaultAgent
  participant ESSAPI
  Helm->>Kubernetes: Apply ESS API Deployment and Service
  Kubernetes->>VaultAgent: Request injected secrets
  VaultAgent->>ESSAPI: Mount rendered secrets
  Kubernetes->>ESSAPI: Start configured container
Loading

Suggested reviewers: balajinvda

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ⚠️ Warning The title correctly uses the customer-impact type and scope, but it omits the required colon after the scope. Change the title to feat(ess): add helm charts.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR imports the ESS chart, adds CI values, registers the release subproject, and continues versioning from 1.7.0 as required by issue #722.
Out of Scope Changes check ✅ Passed The release-versioning updates and documentation support registering and versioning the imported ESS Helm chart, so no unrelated changes are evident.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/ess-helm-charts

Comment @coderabbitai help to get the list of available commands.

@nvaghela-oss nvaghela-oss changed the title Feat/ess helm charts feat/ess helm charts Aug 8, 2026
@nvaghela-oss
nvaghela-oss marked this pull request as ready for review August 8, 2026 00:04
@nvaghela-oss
nvaghela-oss requested review from a team as code owners August 8, 2026 00:04

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 7

🧹 Nitpick comments (1)
tools/ci/helm-validate-values/ess.yaml (1)

1-5: 🎯 Functional Correctness | 🔵 Trivial

Validate the chart with the CI values file.

The values provide the required ess.image.registry and ess.image.repository keys. Run both checks before merge:

(cd deploy/helm/ess && helm lint ess-api -f ../../../tools/ci/helm-validate-values/ess.yaml)
(cd deploy/helm/ess && helm template ess-api ess-api -f ../../../tools/ci/helm-validate-values/ess.yaml)

Based on learnings, run both Helm validations with the ESS CI values file.

🤖 Prompt for 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.

In `@tools/ci/helm-validate-values/ess.yaml` around lines 1 - 5, Run both Helm
validations for the ESS chart using tools/ci/helm-validate-values/ess.yaml: helm
lint and helm template from the deploy/helm/ess directory, ensuring both checks
pass before merge.

Source: Learnings

🤖 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/ess/ess-api/templates/_helpers.tpl`:
- Line 5: Update the chart naming helpers around the `nameOverride` and
`fullnameOverride` expressions to read `.Values.ess.nameOverride` and
`.Values.ess.fullnameOverride` instead of root-level values, while preserving
the existing truncation and suffix handling.

In `@deploy/helm/ess/ess-api/vault-agent-templates/secrets.json.tmpl`:
- Around line 4-10: Update the Vault template entries for cassandraUsername,
cassandraPassword, audit-hmac-keys, and audit-hmac-kid to apply the Helm toJSON
function directly to each Vault value, removing the surrounding template quotes
so special characters and control characters produce valid JSON.

In `@deploy/helm/ess/Makefile`:
- Around line 84-85: Remove the unconditional packaged-charts deletion from the
clean target and the post-helm-push flow in deploy/helm/ess/Makefile at lines
84-85 and 102-103. Preserve artifacts by default, or gate deletion behind an
explicit user confirmation variable.
- Around line 62-80: Update the Makefile by defining validation_values with the
CI validation file default and using it in the lint and template targets, while
retaining values for deployment targets; ensure validate receives
validation_values through template. Add explicit confirmation before clean and
push-oci remove ./packaged-charts, and keep lint/template invocations aligned
with the documented Helm commands.

In `@deploy/helm/ess/README.md`:
- Around line 66-70: Update the README configuration section to match the
chart’s actual supported values: replace the incorrect ess.config.* reference
with ess.configuration.* and remove ess.vault.* unless corresponding values and
template consumers are added; do not document keys the chart does not read.
- Around line 76-123: Update the autoscaling documentation to remove all
Markdown bold markers, including emphasis around disabled by default, request,
and the memory caveat text. Replace the em dash in the JVM explanation with
plain ASCII punctuation or wording, while preserving the existing content and
formatting otherwise.

In `@tools/ci/github-release`:
- Around line 314-318: The initial_floor_version function must default only when
initial_version is absent or None, not for false-y values such as an empty
string; preserve explicit values for SemVer validation so invalid ones raise
SystemExit. In tools/ci/github-release lines 314-318, update the fallback
condition accordingly. In tools/ci/test-github-release.py lines 508-517, add
coverage asserting that an empty-string initial_version raises SystemExit.

---

Nitpick comments:
In `@tools/ci/helm-validate-values/ess.yaml`:
- Around line 1-5: Run both Helm validations for the ESS chart using
tools/ci/helm-validate-values/ess.yaml: helm lint and helm template from the
deploy/helm/ess directory, ensuring both checks pass before merge.
🪄 Autofix

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: 7acedae8-54f2-4664-9fcf-dea218ce4958

📥 Commits

Reviewing files that changed from the base of the PR and between 9dbf12f and bbcb7f9.

📒 Files selected for processing (21)
  • deploy/helm/ess/.gitignore
  • deploy/helm/ess/AGENTS.md
  • deploy/helm/ess/CLAUDE.md
  • deploy/helm/ess/Makefile
  • deploy/helm/ess/README.md
  • deploy/helm/ess/ess-api/.helmignore
  • deploy/helm/ess/ess-api/Chart.yaml
  • deploy/helm/ess/ess-api/templates/_helpers.tpl
  • deploy/helm/ess/ess-api/templates/configmap-env.yaml
  • deploy/helm/ess/ess-api/templates/configmap-vault-agent-template.yaml
  • deploy/helm/ess/ess-api/templates/deployment.yaml
  • deploy/helm/ess/ess-api/templates/hpa.yaml
  • deploy/helm/ess/ess-api/templates/service.yaml
  • deploy/helm/ess/ess-api/templates/serviceaccount.yaml
  • deploy/helm/ess/ess-api/values.yaml
  • deploy/helm/ess/ess-api/vault-agent-templates/secrets.json.tmpl
  • deploy/helm/ess/values.local.yaml
  • tools/ci/github-release
  • tools/ci/github-release-subprojects.json
  • tools/ci/helm-validate-values/ess.yaml
  • tools/ci/test-github-release.py

Comment thread deploy/helm/ess/ess-api/templates/_helpers.tpl Outdated
Comment thread deploy/helm/ess/ess-api/vault-agent-templates/secrets.json.tmpl Outdated
Comment thread deploy/helm/ess/Makefile
Comment thread deploy/helm/ess/Makefile
Comment thread deploy/helm/ess/README.md Outdated
Comment thread deploy/helm/ess/README.md Outdated
Comment thread tools/ci/github-release
The vault-agent template interpolated cassandraUsername, cassandraPassword,
audit-hmac-keys, and audit-hmac-kid inside literal JSON quotes, so a secret
containing a quote, backslash, newline, or other control character produced
invalid JSON. Pipe each value through toJSON, which emits a properly quoted
and escaped JSON string, and drop the surrounding quotes.

Rendered by Vault Agent at runtime; the chart embeds the template verbatim
via Files.Get, so Helm does not evaluate these expressions.

Signed-off-by: Nilesh Vaghela <nvaghela@nvidia.com>
The name and fullname helpers read root-level .Values.nameOverride and
.Values.fullnameOverride, but values.yaml nests both under the ess key, so
setting either override had no effect. Point the helpers at
.Values.ess.nameOverride and .Values.ess.fullnameOverride, keeping the
existing truncation and suffix handling.

Signed-off-by: Nilesh Vaghela <nvaghela@nvidia.com>
The chart requires image.registry/repository, which the default
values.yaml leaves empty, so `make template` and `make validate`
failed to render. Add a validation_values variable defaulting to the
CI validation values and use it in the lint and template targets, so
local rendering matches the documented helm commands in AGENTS.md.
Deployment targets keep using values.
Stop silently deleting ./packaged-charts after a successful push.
Preserve the pushed artifact by default and print its path plus how to
remove it (make clean), matching the llm-api-gateway peer chart. The
clean target still removes ./packaged-charts on explicit invocation.
Replace the incorrect ess.config.* reference with ess.configuration.*
(springProfile, additionalJavaOpts), which is what deployment.yaml
reads, and drop the ess.vault.* bullet since the chart has no such
values (Vault Agent settings are hardcoded in _helpers.tpl).
Align the autoscaling section with the repo documentation style: remove
Markdown bold emphasis and replace the em dash in the JVM note with
plain ASCII punctuation. Content and formatting are otherwise preserved.
initial_floor_version now falls back to the release floor only when
initial_version is absent or None. An explicit empty string (or other
non-None value) is preserved and SemVer-validated, so invalid values
raise SystemExit instead of silently defaulting. Add a test covering
the empty-string case.
@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
deploy/helm/ess/README.md (1)

128-128: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use the rendered HPA name in the verification command.

<release>-helm-nvcf-ess-api is valid only with default naming values and a release name that does not already contain the chart name. Use kubectl get hpa -n <namespace> first, or document these naming constraints.

🤖 Prompt for 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.

In `@deploy/helm/ess/README.md` at line 128, Update the HPA verification
instructions near the kubectl describe command to avoid assuming the default
rendered name: instruct users to discover the rendered HPA name with kubectl get
hpa -n <namespace> first, then use that actual name in kubectl describe hpa.
🤖 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/ess/README.md`:
- Around line 109-110: Update the two scaling bullet phrases in the README to
begin with “Scale-up” and “Scale-down” instead of “Scale up” and “Scale down,”
preserving the remaining wording unchanged.
- Around line 107-110: Update the scaling-timings description in the HPA
behavior section to qualify ~15s and 5 minutes as typical Kubernetes defaults
rather than guaranteed timings. Mention that actual response may vary with
controller sync settings, metrics collection, and readiness delays.

---

Outside diff comments:
In `@deploy/helm/ess/README.md`:
- Line 128: Update the HPA verification instructions near the kubectl describe
command to avoid assuming the default rendered name: instruct users to discover
the rendered HPA name with kubectl get hpa -n <namespace> first, then use that
actual name in kubectl describe hpa.
🪄 Autofix

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: c25948b1-7f5c-4d09-aa0d-07ff674a479d

📥 Commits

Reviewing files that changed from the base of the PR and between bbcb7f9 and dabc779.

📒 Files selected for processing (6)
  • deploy/helm/ess/Makefile
  • deploy/helm/ess/README.md
  • deploy/helm/ess/ess-api/templates/_helpers.tpl
  • deploy/helm/ess/ess-api/vault-agent-templates/secrets.json.tmpl
  • tools/ci/github-release
  • tools/ci/test-github-release.py
🚧 Files skipped from review as they are similar to previous changes (4)
  • tools/ci/github-release
  • deploy/helm/ess/ess-api/vault-agent-templates/secrets.json.tmpl
  • deploy/helm/ess/ess-api/templates/_helpers.tpl
  • tools/ci/test-github-release.py

Comment thread deploy/helm/ess/README.md Outdated
Comment thread deploy/helm/ess/README.md Outdated

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 4

♻️ Duplicate comments (2)
deploy/helm/ess/Makefile (1)

90-91: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Require confirmation before deleting packaged charts.

Line [91] still runs rm -rf without confirmation. Because package depends on clean, a normal package operation deletes existing chart artifacts. Gate deletion with an explicit confirmation variable, or remove the destructive prerequisite and use a fresh output directory.

As per coding guidelines, never delete artifact directories without explicit user confirmation.

Proposed fix
 clean:
-	rm -rf ./packaged-charts
+	`@test` "$(CONFIRM_CLEAN)" = "1" || { \
+		echo "Set CONFIRM_CLEAN=1 to delete ./packaged-charts."; \
+		exit 1; \
+	}
+	rm -rf ./packaged-charts
🤖 Prompt for 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.

In `@deploy/helm/ess/Makefile` around lines 90 - 91, Update the clean target in
the Makefile so removing packaged-charts requires explicit user confirmation,
and ensure the package dependency path cannot delete existing artifacts without
that confirmation. Preserve packaging behavior by either gating rm -rf with a
confirmation variable or removing the destructive clean prerequisite and using a
fresh output directory.

Source: Coding guidelines

deploy/helm/ess/README.md (1)

107-110: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Describe HPA timings as typical defaults.

The text presents ~15s and 5-minute timing as operational behavior. The HPA sync period can change, and metrics collection or pod readiness can delay scaling. State that these are typical defaults and that actual timing varies with cluster settings.

Verify the wording against the Kubernetes version and controller configuration used by deployments.

Proposed wording
-The chart does not set a custom `behavior` block, so Kubernetes defaults apply:
+The chart does not set a custom `behavior` block, so typical Kubernetes defaults apply. Actual timing depends on controller sync settings, metrics collection, and pod readiness:
 
-- Scale up is fast: the controller reacts within one sync interval (~15s) with no stabilization delay.
-- Scale down is conservative: it requires sustained low utilization across the default 5-minute stabilization window before reducing replicas.
+- Scale-up is fast by default: the controller can react after roughly one sync interval when metrics are available.
+- Scale-down is conservative by default: the controller uses a 5-minute stabilization window before reducing replicas.
🤖 Prompt for 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.

In `@deploy/helm/ess/README.md` around lines 107 - 110, Update the HPA timing
description in the chart README to identify the ~15-second sync interval and
5-minute scale-down stabilization as typical defaults, not guaranteed
operational timings. Add that actual scaling may vary with Kubernetes version,
controller or cluster configuration, metrics collection, and pod readiness, and
verify the wording against the deployment configuration.
🧹 Nitpick comments (1)
tools/ci/test-github-release.py (1)

492-506: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Exercise the registered ESS metadata in this test.

This test hard-codes an ad hoc service and 0.58.5. It does not load tools/ci/github-release-subprojects.json, so an incorrect ESS path, service name, or initial_version could pass the test. Load the ess-helm entry and assert the registered deploy/helm/ess/v1.7.0 anchor.

Proposed test update
-            service = {
-                "id": "ess-helm",
-                "path": "deploy/helm/ess",
-                "service_name": "helm-nvcf-ess-api",
-                "initial_version": "0.58.5",
-            }
+            metadata = json.loads(
+                SCRIPT_PATH.with_name("github-release-subprojects.json").read_text()
+            )
+            service = next(item for item in metadata["services"] if item["id"] == "ess-helm")
 
...
-            self.assertIn("deploy/helm/ess/v0.58.5", tags)
+            self.assertIn("deploy/helm/ess/v1.7.0", tags)
🤖 Prompt for 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.

In `@tools/ci/test-github-release.py` around lines 492 - 506, Update
test_initial_version_anchor_honors_metadata to load the registered ess-helm
entry from tools/ci/github-release-subprojects.json instead of constructing an
ad hoc service, then pass that metadata to synthesize_initial_version_anchor and
assert deploy/helm/ess/v1.7.0 is created while the v0.0.0 anchor is absent.
🤖 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/ess/ess-api/templates/deployment.yaml`:
- Around line 93-96: Update the JAVA_TOOL_OPTIONS value in the deployment
template to read additionalJavaOpts from
.Values.ess.configuration.additionalJavaOpts, preserving the existing
default-empty behavior and app.version option.

In `@deploy/helm/ess/Makefile`:
- Line 20: Remove the non-empty default assignment for values so the existing
ifndef values validation rejects make install when no override is provided.
Preserve the current Helm invocation behavior when callers explicitly supply
values.
- Around line 42-43: Update the .PHONY declaration to list the Make targets
actually defined in this file: install, uninstall, clean, validate, package, and
push-oci, replacing the unrelated deploy, delete, status, lint, and template
entries.
- Line 18: Update the standalone ESS Makefile’s namespace default from ess to
the documented ess-api value. Leave the self-managed Helmfile namespace
explicitly set to ess.

---

Duplicate comments:
In `@deploy/helm/ess/Makefile`:
- Around line 90-91: Update the clean target in the Makefile so removing
packaged-charts requires explicit user confirmation, and ensure the package
dependency path cannot delete existing artifacts without that confirmation.
Preserve packaging behavior by either gating rm -rf with a confirmation variable
or removing the destructive clean prerequisite and using a fresh output
directory.

In `@deploy/helm/ess/README.md`:
- Around line 107-110: Update the HPA timing description in the chart README to
identify the ~15-second sync interval and 5-minute scale-down stabilization as
typical defaults, not guaranteed operational timings. Add that actual scaling
may vary with Kubernetes version, controller or cluster configuration, metrics
collection, and pod readiness, and verify the wording against the deployment
configuration.

---

Nitpick comments:
In `@tools/ci/test-github-release.py`:
- Around line 492-506: Update test_initial_version_anchor_honors_metadata to
load the registered ess-helm entry from tools/ci/github-release-subprojects.json
instead of constructing an ad hoc service, then pass that metadata to
synthesize_initial_version_anchor and assert deploy/helm/ess/v1.7.0 is created
while the v0.0.0 anchor is absent.
🪄 Autofix

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: 6a00cf1b-2664-4d3e-9d88-804f517144f0

📥 Commits

Reviewing files that changed from the base of the PR and between 9dbf12f and dabc779.

📒 Files selected for processing (21)
  • deploy/helm/ess/.gitignore
  • deploy/helm/ess/AGENTS.md
  • deploy/helm/ess/CLAUDE.md
  • deploy/helm/ess/Makefile
  • deploy/helm/ess/README.md
  • deploy/helm/ess/ess-api/.helmignore
  • deploy/helm/ess/ess-api/Chart.yaml
  • deploy/helm/ess/ess-api/templates/_helpers.tpl
  • deploy/helm/ess/ess-api/templates/configmap-env.yaml
  • deploy/helm/ess/ess-api/templates/configmap-vault-agent-template.yaml
  • deploy/helm/ess/ess-api/templates/deployment.yaml
  • deploy/helm/ess/ess-api/templates/hpa.yaml
  • deploy/helm/ess/ess-api/templates/service.yaml
  • deploy/helm/ess/ess-api/templates/serviceaccount.yaml
  • deploy/helm/ess/ess-api/values.yaml
  • deploy/helm/ess/ess-api/vault-agent-templates/secrets.json.tmpl
  • deploy/helm/ess/values.local.yaml
  • tools/ci/github-release
  • tools/ci/github-release-subprojects.json
  • tools/ci/helm-validate-values/ess.yaml
  • tools/ci/test-github-release.py

Comment thread deploy/helm/ess/ess-api/templates/deployment.yaml Outdated
Comment thread deploy/helm/ess/Makefile
Comment thread deploy/helm/ess/Makefile
Comment thread deploy/helm/ess/Makefile Outdated
Hyphenate Scale-up/Scale-down, qualify the ~15s and 5-minute figures
as typical Kubernetes defaults that vary with controller sync, metrics
collection, and readiness delays, and tell users to discover the
rendered HPA name with kubectl get hpa before describing it.
Document the initial_version metadata field and add a section on seeding
the first release tag for a new service or chart: auto floor synthesis,
explicit anchoring, the already-released plain-tag fallback, and worked
ess and ess-helm examples.
@nvaghela-oss
nvaghela-oss requested a review from a team as a code owner August 8, 2026 01:32
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 3

🤖 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 `@docs/dev/github-release-process.md`:
- Around line 413-417: Update the tag-workflow trigger statement in the
release-process documentation to qualify that tag pushes start the workflow only
when performed with NV_GITHUB_TOKEN or another workflow-capable token; preserve
the existing dry-run gate behavior and clarify that default GITHUB_TOKEN pushes
do not trigger the follow-up workflow.
- Around line 323-325: Update the initial_version documentation to state that
omitting it starts from a 0.0.0 floor, with the next version determined by
commit type: feat yields 0.1.0, fix yields 0.0.1, and release-neutral commits
produce no release; present 0.1.0 only as an example while preserving the
empty-string validation guidance.
- Around line 410-411: Update the tag-workflow documentation around the git
ls-remote command to retain the existing command and clarify that Git matches
slash-boundary suffixes; explicitly state that tags pushed with NV_GITHUB_TOKEN
trigger the follow-up workflow, while tags pushed with GITHUB_TOKEN do not.
🪄 Autofix

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: 8367489c-bc46-4533-b8f6-9b3b3f74e7fe

📥 Commits

Reviewing files that changed from the base of the PR and between dabc779 and 2f2d637.

📒 Files selected for processing (2)
  • deploy/helm/ess/README.md
  • docs/dev/github-release-process.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • deploy/helm/ess/README.md

Comment thread docs/dev/github-release-process.md Outdated
Comment thread docs/dev/github-release-process.md
Comment thread docs/dev/github-release-process.md Outdated
- deployment: read additionalJavaOpts from ess.configuration so the
  configured value is actually applied
- Makefile: correct .PHONY to the targets defined in this file
- README: document namespace ess consistently and note HPA timings are
  typical defaults that vary with cluster/controller configuration
- test: load the registered ess-helm entry and derive expected tags
  instead of hardcoding versions
Clarify that omitting initial_version starts from a 0.0.0 floor with the
next version set by commit type (feat 0.1.0, fix 0.0.1, release-neutral
none), note that git ls-remote matches the path pattern on slash
boundaries, and qualify that only tags pushed with a workflow-capable
token (NV_GITHUB_TOKEN) trigger the follow-up workflow.
@nvaghela-oss nvaghela-oss changed the title feat/ess helm charts feat(ess) add helm charts Aug 8, 2026
Reframe the section as seeding and pinning service or chart versions and
split it into three cases: new line via initial_version (automatic), new
line seeded manually with anchor, and pinning a new floor on a line that
already has tags. Add the ess-helm registration example and drop the
local auto preview, which creates a local tag as a side effect.
Reword the intro so it plainly states automation bumps the highest
existing tag and a new line has nothing to bump from until seeded.
@NVIDIA NVIDIA deleted a comment from coderabbitai Bot Aug 8, 2026
@NVIDIA NVIDIA deleted a comment from coderabbitai Bot Aug 8, 2026
@nvaghela-oss nvaghela-oss self-assigned this Aug 10, 2026
@nvaghela-oss
nvaghela-oss added this pull request to the merge queue Aug 10, 2026
Merged via the queue into main with commit 5bd73b3 Aug 10, 2026
18 checks passed
@nvaghela-oss
nvaghela-oss deleted the feat/ess-helm-charts branch August 10, 2026 14:44
@balajinvda

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version helm-nvcf-ess-api-v1.7.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Import ESS API Helm chart into the monorepo

3 participants