Skip to content

feat(nvca): default self-managed BYOO collector - #720

Open
kristinapathak wants to merge 2 commits into
mainfrom
kpathak/nvca-byoo-default
Open

feat(nvca): default self-managed BYOO collector#720
kristinapathak wants to merge 2 commits into
mainfrom
kpathak/nvca-byoo-default

Conversation

@kristinapathak

@kristinapathak kristinapathak commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

TL;DR

  • Default self-managed function and task workloads to BYOO collector 0.157.11.
  • Include the indirectly injected collector image in the release SBOM inputs.

Additional Details

  • The vendoring overlay accepts BYOO_OTEL_COLLECTOR_IMAGE from local .env or CI.
  • The pinned default keeps the packaged chart usable before release automation provides an override.
  • The release image scanner decodes function and task environment overrides to collect the image.

For the Reviewer

  • Review the vendoring default and the SBOM supplemental-image extraction together.

For QA

  • make lint
  • make template
  • make validate
  • Chart release asset test targets
  • QA needed: No. This changes chart packaging defaults and release metadata only.

Issues

Closes #719

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.

Github

Github commit:
feat(nvca): default self-managed BYOO collector

Default function and task workloads to BYOO 0.157.11 and include the injected
image in release SBOM inputs.

Summary by CodeRabbit

  • New Features

    • Added a default BYOO OpenTelemetry collector image for function and task workloads.
    • Release image manifests now automatically include the configured collector image.
    • Collector image settings can be customized through deployment environment overrides.
  • Bug Fixes

    • Improved image discovery and validation across rendered deployment configurations.
    • Removed duplicate image entries from supplemental release manifests.
    • Invalid collector image references now produce a clear rendering failure.
  • Tests

    • Added coverage verifying consistent collector image references and expected release image contents.

Default function and task workloads to BYOO 0.157.11 and include the injected image in release SBOM inputs.

Closes #719

Signed-off-by: Kristina Pathak <kpathak@nvidia.com>
@kristinapathak
kristinapathak requested a review from a team as a code owner August 6, 2026 22:44
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2204cf86-a21a-445d-bb46-631fa130cf34

📥 Commits

Reviewing files that changed from the base of the PR and between 0c4b8a1 and 6843a0f.

📒 Files selected for processing (2)
  • deploy/helm/nvca-operator/scripts/render_release_supplemental_images.sh
  • deploy/helm/nvca-operator/tests/release_image_manifest_test.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • deploy/helm/nvca-operator/scripts/render_release_supplemental_images.sh

📝 Walkthrough

Walkthrough

The chart sets a default BYOO OTel collector image for function and task workloads. Vendor and release scripts propagate and extract the image. Tests validate matching defaults and supplemental image manifest inclusion.

Changes

BYOO collector image support

Layer / File(s) Summary
Collector image defaults and chart wiring
deploy/helm/nvca-operator/nvca-operator/values.yaml, deploy/helm/nvca-operator/scripts/ci_vendor_nvca_operator_chart
The chart sets the same collector image for function and task overrides. The vendor script supports an environment override and writes the selected image to both workloads.
Supplemental image extraction
deploy/helm/nvca-operator/scripts/render_release_supplemental_images.sh
The renderer decodes function and task overrides, validates collector image references, and deduplicates collected images.
Release image validation
deploy/helm/nvca-operator/tests/release_image_manifest_test.sh, deploy/helm/nvca-operator/tests/self_managed_nvca_image_reference_test.sh
Release tests verify collector image inclusion, the updated unique image count, invalid-reference handling, and matching default references.

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

Suggested reviewers: mikeyrcamp

Sequence Diagram(s)

sequenceDiagram
  participant Chart
  participant VendorScript
  participant RenderedManifest
  participant ImageRenderer
  participant ReleaseTests
  Chart->>VendorScript: provide collector image default
  VendorScript->>RenderedManifest: write function/task overrides
  RenderedManifest->>ImageRenderer: provide encoded overrides
  ImageRenderer->>ImageRenderer: decode and validate image references
  ImageRenderer->>ReleaseTests: generate supplemental image manifest
  ReleaseTests->>ReleaseTests: verify image inclusion and validation errors
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title uses the required Conventional Commits format and accurately describes the default BYOO collector feature.
Linked Issues check ✅ Passed The changes set matching defaults, include the collector image in SBOM inputs, and validate the default image as required by issue #719.
Out of Scope Changes check ✅ Passed All changes support the linked issue objectives for BYOO defaults, SBOM inputs, and release validation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch kpathak/nvca-byoo-default

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

@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: 1

🧹 Nitpick comments (1)
deploy/helm/nvca-operator/tests/release_image_manifest_test.sh (1)

45-47: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Cover the task override path in this release test.

The fixture supplies only --function-env-overrides-b64. Add the matching --task-env-overrides-b64 argument. Keep the exact-six assertion to verify that the same collector image is deduplicated.

As per coding guidelines, code changes must include tests.

🤖 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/nvca-operator/tests/release_image_manifest_test.sh` around lines
45 - 47, Add the matching --task-env-overrides-b64 argument to the fixture
alongside --function-env-overrides-b64, using the same collector image override
so the release test exercises task overrides and deduplication. Preserve the
exact-six assertion and update or add test coverage as needed for this path.

Source: Coding guidelines

🤖 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/nvca-operator/scripts/render_release_supplemental_images.sh`:
- Around line 142-147: Update the BYOO_OTEL_COLLECTOR_CONTAINER handling around
byoo_otel_collector_image to parse the decoded JSON and validate the complete
image reference, rejecting malformed values such as repo/image:. If the key is
present but invalid, emit an error and return non-zero instead of silently
omitting it; retain the existing supplemental-image output for valid references.

---

Nitpick comments:
In `@deploy/helm/nvca-operator/tests/release_image_manifest_test.sh`:
- Around line 45-47: Add the matching --task-env-overrides-b64 argument to the
fixture alongside --function-env-overrides-b64, using the same collector image
override so the release test exercises task overrides and deduplication.
Preserve the exact-six assertion and update or add test coverage as needed for
this path.
🪄 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: 0c10cfaf-cff1-4329-b469-8e0296220c70

📥 Commits

Reviewing files that changed from the base of the PR and between 56862a3 and 0c4b8a1.

📒 Files selected for processing (5)
  • deploy/helm/nvca-operator/nvca-operator/values.yaml
  • deploy/helm/nvca-operator/scripts/ci_vendor_nvca_operator_chart
  • deploy/helm/nvca-operator/scripts/render_release_supplemental_images.sh
  • deploy/helm/nvca-operator/tests/release_image_manifest_test.sh
  • deploy/helm/nvca-operator/tests/self_managed_nvca_image_reference_test.sh

Comment thread deploy/helm/nvca-operator/scripts/render_release_supplemental_images.sh Outdated
Fail release SBOM generation when encoded BYOO image overrides are malformed. Cover task override deduplication.

Relates to #719

Signed-off-by: Kristina Pathak <kpathak@nvidia.com>
Sign up for free to 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.

Default BYOO collector image for self-managed NVCA

1 participant