-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/swarm #28
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
base: main
Are you sure you want to change the base?
Feature/swarm #28
Conversation
- Introduced DEFAULT_SWARM_DB_GOSSIP_PORT variable with a default value of 7946. - Updated usage function to include --swarm_db_gossip_port option. - Modified parameter parsing to handle the new gossip port argument. - Enhanced network settings to forward UDP and TCP traffic for the Swarm DB Gossip Port.
- Introduced DEFAULT_WG_PORT and DEFAULT_WG_PORT_RANGE variables for WireGuard configuration. - Updated usage function to include --wg_port and --wg_port_range options. - Enhanced parameter parsing to handle WireGuard port and range arguments. - Modified network settings to forward UDP traffic for the specified WireGuard port and range.
- Deleted DEFAULT_WG_PORT_RANGE variable and its associated logic from the script. - Updated usage function to remove --wg_port_range option. - Simplified network settings by eliminating the conditional handling for WireGuard port range.
- Introduced DEFAULT_ROOT_WRITABLE variable to manage root filesystem writability. - Updated usage function to include --root_writable option. - Enhanced parameter parsing to handle the new root writable argument. - Modified kernel command line and image drive settings based on the root writable option.
- Updated logic to create a qcow2 overlay when ROOT_WRITABLE is true, allowing for an immutable base image while enabling writes. - Added checks to create or use an existing overlay image, improving the management of writable root filesystems.
- Removed the check for existing overlay image and added logic to always create a new qcow2 overlay. - Simplified the overlay management by ensuring the previous overlay is removed before creation, enhancing the script's reliability.
- Removed unnecessary conditional statement to ensure the qcow2 overlay is always created after the previous one is removed. - Improved script reliability by streamlining the overlay management process.
|
|
||
| NETWORK_SETTINGS+=",hostfwd=udp:0.0.0.0:$WG_PORT-:51820" | ||
| NETWORK_SETTINGS+=",hostfwd=udp:0.0.0.0:$SWARM_DB_GOSSIP_PORT-:7946" | ||
| NETWORK_SETTINGS+=",hostfwd=tcp:0.0.0.0:$SWARM_DB_GOSSIP_PORT-:7946" |
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.
erroneous duplication?
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.
Different protos (tcp/udp)
scripts/start_super_protocol.sh
Outdated
|
|
||
| IMAGE_DRIVE=" -drive file=${IMAGE_PATH},if=virtio,format=raw" | ||
| if [[ "${ROOT_WRITABLE}" == "true" ]]; then | ||
| # Use qcow2 overlay to keep the base image immutable while allowing writes |
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.
It seems that this is not an overlay, but an addition image file.
WalkthroughAdds CLI options and defaults for WireGuard and Swarm DB Gossip ports, integrates those ports into QEMU hostfwd rules (UDP 51820, UDP/TCP 7946), and introduces a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
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
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
scripts/start_super_protocol.sh(6 hunks)
🔇 Additional comments (6)
scripts/start_super_protocol.sh (6)
31-32: LGTM!The default port values are correct: 51820 is the standard WireGuard port and 7946 is the standard Docker Swarm gossip port.
121-121: LGTM!Variable initialization follows the established pattern and correctly uses the default values.
Also applies to: 124-124
151-151: LGTM!Argument parsing is correct and follows the established pattern.
Also applies to: 154-154
785-786: LGTM!Debug output formatting is consistent with existing debug messages and correctly displays the new port values.
943-943: Clarification: This is NOT a duplicate.Addressing the past review comment: Line 943 (TCP forwarding) is intentional and necessary. Docker Swarm's gossip protocol requires both UDP and TCP on port 7946 for proper operation—UDP for gossip messaging and TCP for failure detection and state synchronization.
941-943: Verify the IP binding choice for the new port forwards.The new port forwards use
0.0.0.0(all interfaces), while HTTP/HTTPS ports use$IP_ADDRESS(lines 935, 938). This difference might be intentional to allow WireGuard VPN and Swarm clustering to listen on all interfaces, but please confirm this is the desired behavior for your security model.
| echo " --wg_port <port> WireGuard port (default: ${DEFAULT_WG_PORT})" | ||
| echo " --http_port <port> HTTP port (default: no port forward)" | ||
| echo " --https_port <port> HTTPS port (default: no port forward)" | ||
| echo " --swarm_db_gossip_port <port> Swarm DB Gossip Port" |
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.
Add default value to help text for consistency.
The --swarm_db_gossip_port option is missing its default value in the help text, unlike other port options (e.g., line 88 for --wg_port).
🔎 Apply this diff to add the default value:
- echo " --swarm_db_gossip_port <port> Swarm DB Gossip Port"
+ echo " --swarm_db_gossip_port <port> Swarm DB Gossip Port (default: ${DEFAULT_SWARM_DB_GOSSIP_PORT})"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo " --swarm_db_gossip_port <port> Swarm DB Gossip Port" | |
| echo " --swarm_db_gossip_port <port> Swarm DB Gossip Port (default: ${DEFAULT_SWARM_DB_GOSSIP_PORT})" |
🤖 Prompt for AI Agents
In scripts/start_super_protocol.sh around line 91, the help text for
"--swarm_db_gossip_port" lacks the default value; update the echo to include the
same default-value format used by other port options (e.g., append " (default:
<port>)") so the help output is consistent—set the default port to the same
value used in the script/config for swarm DB gossip.
Summary by CodeRabbit
New Features
Documentation
Diagnostics
✏️ Tip: You can customize this high-level summary in your review settings.