feat(sidecar): support extraVolumes#588
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe Helm telemetry sidecar templates now append ChangesTelemetry sidecar extra volumes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/unit/helm/telemetry-sidecar_test.go`:
- Around line 51-67: The helpers hasVolumeMount and hasVolume should return the
matching VolumeMount/Volume instead of only a boolean, so the telemetry-sidecar
test can assert the rendered fields. Update the test to use these returned
values from hasVolumeMount and hasVolume to verify mountPath, readOnly, and the
volume’s secretName, rather than only checking that the names exist.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7cc0c92e-482b-4a43-866b-10c2e6b80def
⛔ Files ignored due to path filters (7)
helm/fiftyone-teams-app/templates/api-deployment.yamlis excluded by!**/*.yamlhelm/fiftyone-teams-app/templates/app-deployment.yamlis excluded by!**/*.yamlhelm/fiftyone-teams-app/templates/delegated-operator-instance-deployment.yamlis excluded by!**/*.yamlhelm/fiftyone-teams-app/templates/delegated-operator-job-configmap.yamlis excluded by!**/*.yamlhelm/fiftyone-teams-app/templates/plugins-deployment.yamlis excluded by!**/*.yamlhelm/fiftyone-teams-app/values.schema.jsonis excluded by!**/*.jsonhelm/fiftyone-teams-app/values.yamlis excluded by!**/*.yaml
📒 Files selected for processing (3)
helm/fiftyone-teams-app/README.mdhelm/fiftyone-teams-app/templates/_telemetry.tpltests/unit/helm/telemetry-sidecar_test.go
Rationale
Apple requested that we support extraVolumes in the sidecar so they can mount the same certs as the other containers (https://voxel51.slack.com/archives/C0AJ8LW8P6H/p1782404775160409)
https://voxel51.atlassian.net/browse/FOEPD-4139
Review Priority
Changes
Checklist
Testing