Squash and merge changes from upstrem/rhoai-3.2#14
Conversation
WalkthroughThe changes introduce GitHub Actions automation for releasing Helm charts for the Spark Operator. A new workflow is triggered on pushes to the odh branch affecting Spark Operator chart files and runs the Chart Releaser action. A configuration file specifies Chart Releaser settings for the repository. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/lgtm |
|
/approve |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/helm-release.yaml (2)
7-9: Path filters may be too restrictive.The workflow only triggers when
Chart.yamlorvalues.yamlare modified. Changes to chart templates, helpers, or other files incharts/spark-operator-chart/will not trigger a release.If this is intentional (e.g., releases are controlled only by version bumps in Chart.yaml), consider adding a comment explaining the rationale.
🔎 Alternative: Broaden the path filter to include all chart files
paths: - - charts/spark-operator-chart/Chart.yaml - - charts/spark-operator-chart/values.yaml + - charts/spark-operator-chart/**
11-36: Consider adding failure notifications for operational visibility.The workflow lacks explicit error handling or notifications. While GitHub Actions will mark the workflow as failed, consider adding a notification step to alert maintainers of release failures.
🔎 Example: Add a failure notification step
jobs: release: permissions: contents: write runs-on: ubuntu-latest steps: # ... existing steps ... - name: Notify on failure if: failure() run: | echo "::error::Chart release workflow failed. Check logs for details." # Add additional notification logic (e.g., Slack, email) if needed
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/helm-release.yaml(1 hunks)cr.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: e2e-test (v1.31.4)
- GitHub Check: e2e-test (v1.30.8)
- GitHub Check: e2e-test (v1.32.0)
- GitHub Check: e2e-test (v1.28.15)
- GitHub Check: e2e-test (v1.29.12)
- GitHub Check: e2e-test (v1.27.16)
- GitHub Check: e2e-test (v1.25.16)
- GitHub Check: e2e-test (v1.26.15)
- GitHub Check: e2e-test (v1.24.17)
- GitHub Check: build-helm-chart
- GitHub Check: code-check
- GitHub Check: build-spark-operator
🔇 Additional comments (2)
.github/workflows/helm-release.yaml (1)
14-15: Permissions and authentication configuration looks correct.The workflow properly configures:
contents: writepermission (lines 14-15): Required for chart-releaser to create GitHub releases and push to gh-pages branch- Git identity using
GITHUB_ACTOR(lines 25-26): Standard practice for automated commitsGITHUB_TOKEN(line 36): Provides authentication for the chart-releaser actionAlso applies to: 23-26, 35-36
cr.yaml (1)
1-8: Configuration is correctly set up for Chart Releaser.The cr.yaml file references all required resources:
- The gh-pages branch exists and is ready to receive chart index updates
- The charts directory contains the spark-operator-chart with a valid Chart.yaml file
- The repository owner and name are correctly configured
All prerequisites for the Chart Releaser workflow are in place.
Purpose of this PR
This is same as #9. I just ported them to my local fork.
Proposed changes:
Change Category
Rationale
Checklist
Additional Notes
Summary by CodeRabbit
Chores
✏️ Tip: You can customize this high-level summary in your review settings.