-
Notifications
You must be signed in to change notification settings - Fork 187
fixes helchart to expose gossip protocol port #1250
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fixes helchart to expose gossip protocol port #1250
Conversation
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughVersion bump from 1.5.1 to 1.5.2 introduces conditional gossip protocol ports (TCP/UDP) to Bifrost's deployment and service templates when cluster mode is enabled, with corresponding chart metadata and index updates. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
🧪 Test Suite AvailableThis PR can be tested by a repository admin. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @helm-charts/bifrost/templates/deployment.yaml:
- Around line 48-55: There are duplicate "gossip" definitions in values.yaml;
remove the redundant entry and consolidate into a single gossip block so that
.Values.bifrost.cluster.gossip.port is defined only once (e.g., keep one gossip:
{ port: 7946, ... } and delete the duplicate); ensure the deployment template
references .Values.bifrost.cluster.gossip.port and the service/gossip UDP/TCP
entries remain unchanged and still point to that single gossip definition.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
helm-charts/bifrost/Chart.yamlhelm-charts/bifrost/templates/deployment.yamlhelm-charts/bifrost/templates/service.yamlhelm-charts/index.yaml
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
helm-charts/bifrost/templates/deployment.yamlhelm-charts/index.yamlhelm-charts/bifrost/templates/service.yamlhelm-charts/bifrost/Chart.yaml
🔇 Additional comments (4)
helm-charts/bifrost/Chart.yaml (1)
5-6: LGTM! Version bump is consistent.The version and appVersion are correctly updated to 1.5.2, aligning with the gossip protocol port feature addition in the deployment and service templates.
helm-charts/bifrost/templates/service.yaml (1)
19-28: LGTM! Service ports correctly expose gossip protocol.The conditional gossip ports (TCP and UDP) are properly configured with targetPort names that match the container ports defined in the deployment template. The configuration allows the gossip protocol to function correctly when cluster mode is enabled.
helm-charts/index.yaml (2)
160-160: Generated timestamp correctly updated.The index generation timestamp has been appropriately updated to reflect the new chart entry.
4-25: The digest field will be auto-generated byhelm repo index, but multiple entries in index.yaml currently have empty digests, indicating the workflow may not have run since the index was last updated.The
helm-release.ymlworkflow is properly configured to automatically generate SHA256 digests for all chart entries via thehelm repo indexcommand (line 79). When the workflow runs, it will populate the digest field for version 1.5.2 and other entries.However, the fact that multiple chart versions (1.5.2, 1.5.1, and others) already have empty digests in the committed index.yaml suggests this file may be undergoing manual updates or the workflow hasn't executed since the latest index changes. Ensure the workflow completes successfully after these changes are merged to main so the digests are properly generated and published to GitHub Pages.

Summary
Added support for cluster mode in Bifrost Helm chart by exposing gossip ports and bumped chart version to 1.5.2.
Changes
Type of change
Affected areas
How to test
Deploy Bifrost with cluster mode enabled and verify that gossip ports are properly exposed:
Breaking changes
Related issues
Supports cluster mode functionality for horizontal scaling.
Security considerations
Gossip ports should be properly secured in production environments to prevent unauthorized access to cluster communication.
Checklist