feat: Add support for Plane Swarm in self-hosting methods #6406
Conversation
…added redeployStack in script file
WalkthroughThis pull request introduces comprehensive updates to the self-hosted deployment infrastructure for the Plane application. The changes focus on enhancing deployment flexibility and documentation, with key modifications to the GitHub workflow, Docker Compose configuration, installation scripts, and deployment documentation. A new Changes
Possibly related PRs
Suggested Reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
deploy/selfhost/docker-compose.yml (1)
220-223: Enhance port configuration for proxy service.The port configuration for the proxy service uses the new Docker Compose format with explicit mode, protocol, and target/published ports. This is good for Swarm deployments.
Consider adding placement constraints or resource limits in the deploy section for better control over service placement and resource usage in the Swarm cluster.
deploy/selfhost/README.md (2)
58-81: Fix grammatical issues in the documentation.Several grammatical issues need to be addressed:
- Remove duplicate "create a" in the sentence
- Add proper punctuation before conjunctions
- Fix verb tenses in several places
Apply these corrections:
-This will create a create a folder +This will create a folder -will be popped up and this time +will be popped up, and this time🧰 Tools
🪛 Markdownlint (0.37.0)
60-60: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
68-68: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
76-76: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
74-80: Enhance Docker Swarm documentation.The Docker Swarm setup instructions are clear but could benefit from additional details about:
- Swarm initialization requirements
- Node management
- Service scaling considerations
Would you like me to generate expanded documentation covering these aspects?
Also applies to: 214-232, 260-275
🧰 Tools
🪛 Markdownlint (0.37.0)
76-76: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
deploy/selfhost/swarm.sh (2)
41-41: Fix shellcheck warnings about variable declarations.Several instances of variable declarations that mask return values.
Separate declarations and assignments:
-local latest_release=$(curl -s https://api.github.com/repos/$GH_REPO/releases/latest | grep -o '"tag_name": "[^"]*"' | sed 's/"tag_name": "//;s/"//g') +local latest_release +latest_release=$(curl -s https://api.github.com/repos/$GH_REPO/releases/latest | grep -o '"tag_name": "[^"]*"' | sed 's/"tag_name": "//;s/"//g')Apply similar fixes to other instances.
Also applies to: 53-53, 242-242, 252-252, 256-256, 273-273, 275-275, 334-334, 383-383, 410-410, 446-446
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 41-41: Declare and assign separately to avoid masking return values.
(SC2155)
371-463: Consider adding timeout mechanism for log viewing.The log viewing function could benefit from a timeout mechanism to prevent indefinite waiting.
Consider adding a timeout parameter with a default value:
function viewSpecificLogs() { local service=$1 local timeout=${2:-300} # 5 minutes default timeout # Add timeout logic using SECONDS SECONDS=0 while true; do if [ $SECONDS -gt $timeout ]; then echo "Timeout reached after ${timeout} seconds" return 1 fi # ... rest of the function done }🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 383-383: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 400-400: Quote this to prevent word splitting.
(SC2046)
[warning] 410-410: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 446-446: Declare and assign separately to avoid masking return values.
(SC2155)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/build-branch.yml(2 hunks)deploy/selfhost/README.md(12 hunks)deploy/selfhost/docker-compose.yml(6 hunks)deploy/selfhost/install.sh(3 hunks)deploy/selfhost/swarm.sh(1 hunks)
🧰 Additional context used
🪛 Markdownlint (0.37.0)
deploy/selfhost/README.md
60-60: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
68-68: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
76-76: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 LanguageTool
deploy/selfhost/README.md
[grammar] ~109-~109: This phrase is duplicated. You should probably use “create a” only once.
Context: ...p, type "1" as action input. This will create a create a folder plane-app and will download 2 ...
(PHRASE_REPETITION)
[uncategorized] ~114-~114: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...ne.env Again theoptions [1-8]will be popped up and this time hit8` to exit. ####...
(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
[uncategorized] ~114-~114: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...in the options [1-8] will be popped up and this time hit 8 to exit. #### Docker...
(COMMA_COMPOUND_SENTENCE_2)
[grammar] ~133-~133: This phrase is duplicated. You should probably use “create a” only once.
Context: ...p, type "1" as action input. This will create a create a folder plane-app and will download 2 ...
(PHRASE_REPETITION)
[uncategorized] ~138-~138: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...ne.env Again theoptions [1-7]will be popped up and this time hit7` to exit. --- ...
(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
[uncategorized] ~138-~138: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...in the options [1-7] will be popped up and this time hit 7 to exit. --- ### Co...
(COMMA_COMPOUND_SENTENCE_2)
[grammar] ~216-~216: There seems to be a noun/verb agreement error. Did you mean “selects” or “selected”?
Context: ...mpted with the below options. This time select 2 to stop the sevices ```bash Select...
(SINGULAR_NOUN_VERB_AGREEMENT)
[uncategorized] ~237-~237: Use a comma before ‘or’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...` variables, without stopping the server or you noticed some abnormalies in service...
(COMMA_COMPOUND_SENTENCE)
[grammar] ~239-~239: There seems to be a noun/verb agreement error. Did you mean “selects” or “selected”?
Context: ...mpted with the below options. This time select 4 to restart the sevices #### Docker...
(SINGULAR_NOUN_VERB_AGREEMENT)
[grammar] ~282-~282: There seems to be a noun/verb agreement error. Did you mean “selects” or “selected”?
Context: ...mpted with the below options. This time select 5 to upgrade the release. #### Docke...
(SINGULAR_NOUN_VERB_AGREEMENT)
[grammar] ~314-~314: There seems to be a noun/verb agreement error. Did you mean “selects” or “selected”?
Context: ...mpted with the below options. This time select 5 to upgrade the release. ```bash ...
(SINGULAR_NOUN_VERB_AGREEMENT)
[style] ~330-~330: This phrase is redundant. Consider writing “exit”.
Context: ... plane.env. Once done, choose 7 to exit from prompt. > It is very important for you...
(EXIT_FROM)
[uncategorized] ~330-~330: You might be missing the article “the” here.
Context: ...v. Once done, choose 7` to exit from prompt. > It is very important for you to val...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[style] ~332-~332: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...hoose 7 to exit from prompt. > It is very important for you to validate the plane.env for...
(EN_WEAK_ADJECTIVE)
[grammar] ~344-~344: There seems to be a noun/verb agreement error. Did you mean “selects” or “selected”?
Context: ...ted with the below options. This time select 6 to view logs. #### Docker Compose ...
(SINGULAR_NOUN_VERB_AGREEMENT)
🪛 Shellcheck (0.10.0)
deploy/selfhost/swarm.sh
[warning] 41-41: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 53-53: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 151-151: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 233-233: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
[warning] 242-242: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 252-252: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 256-256: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 273-273: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 275-275: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 334-334: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 383-383: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 400-400: Quote this to prevent word splitting.
(SC2046)
[warning] 410-410: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 446-446: Declare and assign separately to avoid masking return values.
(SC2155)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (6)
deploy/selfhost/docker-compose.yml (1)
65-66: LGTM: Consistent deployment configuration across services.The changes correctly implement Docker Swarm deployment configuration by:
- Using environment variables for replica counts
- Setting consistent restart policies
- Maintaining service dependencies
Also applies to: 76-77, 88-89, 101-102, 112-113, 126-129, 143-146, 160-163, 175-178, 186-189, 195-198, 207-210, 220-229
.github/workflows/build-branch.yml (1)
319-319: LGTM: Consistent inclusion of swarm.sh in release artifacts.The script is correctly added to both artifact upload and release creation jobs, ensuring it's available for users.
Also applies to: 362-362
deploy/selfhost/install.sh (4)
460-460: LGTM! Menu item added for RabbitMQ logs.The menu item is properly formatted and aligns well with the existing menu structure.
465-466: LGTM! Input validation updated for RabbitMQ option.The validation range and error message are correctly updated to accommodate the new RabbitMQ option.
485-485: LGTM! Case statements updated for RabbitMQ log viewing.Both numeric (11) and text-based (rabbitmq) selections are properly handled, maintaining consistency with other service log viewing options.
Also applies to: 504-504
460-460: Verify RabbitMQ service configuration.The log viewing implementation looks good. However, let's verify that:
- The RabbitMQ service is defined in the docker-compose file
- The service name "plane-mq" matches the service name in docker-compose
Also applies to: 465-466, 485-485, 504-504
✅ Verification successful
RabbitMQ service configuration verified successfully
The RabbitMQ service is properly defined in docker-compose.yml as "plane-mq" and matches the service name used in the configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify RabbitMQ service configuration in docker-compose file # Test: Check if RabbitMQ service is defined and the service name matches echo "Checking docker-compose.yml for RabbitMQ service configuration..." if [ -f "deploy/selfhost/docker-compose.yml" ]; then grep -A 5 "plane-mq:" "deploy/selfhost/docker-compose.yml" else echo "docker-compose.yml not found in deploy/selfhost/" # Fallback to searching in the entire repository fd -t f "docker-compose.y*ml" -x grep -l "plane-mq:" fiLength of output: 548
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
deploy/selfhost/README.md (1)
133-133: Fix grammatical error: Remove duplicated phrase.-This will create a create a folder `plane-app` and will download 2 files inside that +This will create a folder `plane-app` and will download 2 files inside that🧰 Tools
🪛 LanguageTool
[grammar] ~133-~133: This phrase is duplicated. You should probably use “create a” only once.
Context: ...p, type "1" as action input. This will create a create a folderplane-appand will download 2 ...(PHRASE_REPETITION)
deploy/selfhost/swarm.sh (2)
73-97: Enhance security of environment file operations.Consider adding the following security measures:
- Set restrictive permissions on the environment file
- Validate environment variable values before writing
- Use secure file operations to prevent race conditions
function syncEnvFile(){ echo "Syncing environment variables..." >&2 if [ -f "$PLANE_INSTALL_DIR/plane.env.bak" ]; then + # Set restrictive permissions + chmod 600 "$DOCKER_ENV_PATH" || { + echo "Failed to set permissions on environment file" + exit 1 + } # READ keys of plane.env and update the values from plane.env.bak while IFS= read -r line
331-374: Add semantic version validation.Add validation to ensure version strings are in the correct format before comparison.
+function validate_version() { + local version=$1 + if ! [[ $version =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?$ ]]; then + echo "Invalid version format: $version" + return 1 + fi + return 0 +} function upgrade() { echo "Checking status of ${stack_name} stack..." if [ -z "$stack_name" ]; then echo "Stack name not found" exit 1 fi local latest_release=$(checkLatestRelease) + if ! validate_version "$latest_release"; then + echo "Failed to validate latest release version" + exit 1 + fi🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 339-339: Declare and assign separately to avoid masking return values.
(SC2155)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
deploy/selfhost/README.md(12 hunks)deploy/selfhost/swarm.sh(1 hunks)
🧰 Additional context used
🪛 Markdownlint (0.37.0)
deploy/selfhost/README.md
60-60: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
68-68: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
76-76: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 LanguageTool
deploy/selfhost/README.md
[uncategorized] ~113-~113: Possible missing comma found.
Context: ... - docker-compose.yaml - plane.env Again the options [1-8] will be popped up, ...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~133-~133: This phrase is duplicated. You should probably use “create a” only once.
Context: ...p, type "1" as action input. This will create a create a folder plane-app and will download 2 ...
(PHRASE_REPETITION)
[uncategorized] ~137-~137: Possible missing comma found.
Context: ... - docker-compose.yaml - plane.env Again the options [1-7] will be popped up, ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~215-~215: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...images/stopped.png) #### Docker Swarm Lets again run the ./setup.sh command. You...
(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~216-~216: Possible missing comma found.
Context: ...e prompted with the below options. This time select 2 to stop the sevices ```bash...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~216-~216: There seems to be a noun/verb agreement error. Did you mean “selects” or “selected”?
Context: ...mpted with the below options. This time select 2 to stop the sevices ```bash Select...
(SINGULAR_NOUN_VERB_AGREEMENT)
[uncategorized] ~237-~237: Use a comma before ‘or’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...` variables, without stopping the server or you noticed some abnormalies in service...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~238-~238: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...es with RESTART / REDEPLOY option. Lets again run the ./setup.sh command. You...
(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~239-~239: Possible missing comma found.
Context: ...e prompted with the below options. This time select 4 to restart the sevices ####...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~239-~239: There seems to be a noun/verb agreement error. Did you mean “selects” or “selected”?
Context: ...mpted with the below options. This time select 4 to restart the sevices #### Docker...
(SINGULAR_NOUN_VERB_AGREEMENT)
[uncategorized] ~281-~281: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...ne up to date with the latest release. Lets again run the ./setup.sh command. You...
(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[grammar] ~282-~282: There seems to be a noun/verb agreement error. Did you mean “selects” or “selected”?
Context: ...mpted with the below options. This time select 5 to upgrade the release. #### Docke...
(SINGULAR_NOUN_VERB_AGREEMENT)
[uncategorized] ~313-~313: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...n to Start Server #### Docker Swarm Lets again run the ./setup.sh command. You...
(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[grammar] ~314-~314: There seems to be a noun/verb agreement error. Did you mean “selects” or “selected”?
Context: ...mpted with the below options. This time select 5 to upgrade the release. ```bash ...
(SINGULAR_NOUN_VERB_AGREEMENT)
[style] ~330-~330: This phrase is redundant. Consider writing “exit”.
Context: ... plane.env. Once done, choose 7 to exit from prompt. > It is very important for you...
(EXIT_FROM)
[style] ~332-~332: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...hoose 7 to exit from prompt. > It is very important for you to validate the plane.env for...
(EN_WEAK_ADJECTIVE)
[uncategorized] ~341-~341: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ... API, Worker or any other container. Lets again run the ./setup.sh command. You...
(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~344-~344: Possible missing comma found.
Context: ...prompted with the below options. This time select 6 to view logs. #### Docker C...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~344-~344: There seems to be a noun/verb agreement error. Did you mean “selects” or “selected”?
Context: ...ted with the below options. This time select 6 to view logs. #### Docker Compose ...
(SINGULAR_NOUN_VERB_AGREEMENT)
🪛 Shellcheck (0.10.0)
deploy/selfhost/swarm.sh
[warning] 41-41: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 53-53: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 234-234: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
[warning] 247-247: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 257-257: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 261-261: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 278-278: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 280-280: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 339-339: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 388-388: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 405-405: Quote this to prevent word splitting.
(SC2046)
[warning] 415-415: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 451-451: Declare and assign separately to avoid masking return values.
(SC2155)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (6)
deploy/selfhost/README.md (4)
58-80: LGTM! Clear separation of Docker Compose and Swarm setup instructions.The documentation structure is well-organized, making it easy for users to follow setup instructions for their chosen deployment method.
🧰 Tools
🪛 Markdownlint (0.37.0)
60-60: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
68-68: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
76-76: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Line range hint
140-156: LGTM! Clear environment configuration instructions.The section effectively guides users through environment configuration with well-highlighted important settings.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~158-~158: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ... setup - Start Server (Docker Compose) Lets again run the./setup.shcommand. You...(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~159-~159: Possible missing comma found.
Context: ...e prompted with the below options. This time select2to start the sevices ```bas...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~159-~159: There seems to be a noun/verb agreement error. Did you mean “selects” or “selected”?
Context: ...mpted with the below options. This time select2to start the sevices ```bash Selec...(SINGULAR_NOUN_VERB_AGREEMENT)
Line range hint
188-232: LGTM! Clear instructions for both Docker Compose and Swarm.The section effectively guides users through stopping services in both deployment methods.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~215-~215: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...images/stopped.png) #### Docker Swarm Lets again run the./setup.shcommand. You...(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~216-~216: Possible missing comma found.
Context: ...e prompted with the below options. This time select2to stop the sevices ```bash...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~216-~216: There seems to be a noun/verb agreement error. Did you mean “selects” or “selected”?
Context: ...mpted with the below options. This time select2to stop the sevices ```bash Select...(SINGULAR_NOUN_VERB_AGREEMENT)
[uncategorized] ~237-~237: Use a comma before ‘or’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...` variables, without stopping the server or you noticed some abnormalies in service...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~238-~238: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...es withRESTART/REDEPLOYoption. Lets again run the./setup.shcommand. You...(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~239-~239: Possible missing comma found.
Context: ...e prompted with the below options. This time select4to restart the sevices ####...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~239-~239: There seems to be a noun/verb agreement error. Did you mean “selects” or “selected”?
Context: ...mpted with the below options. This time select4to restart the sevices #### Docker...(SINGULAR_NOUN_VERB_AGREEMENT)
Line range hint
377-394: LGTM! Comprehensive logging instructions.The logging section provides clear instructions for viewing logs of all services, with a well-organized menu structure.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~397-~397: Possible missing comma found.
Context: ... Select any of the service to view the logs e.g.3. Expect something similar to t...(AI_HYDRA_LEO_MISSING_COMMA)
deploy/selfhost/swarm.sh (2)
405-405: Fix word splitting in container count check.-if [ $(echo "$running_containers" | grep -v '^$' | wc -l) -gt 1 ]; then +if [ "$(echo "$running_containers" | grep -v '^$' | wc -l)" -gt 1 ]; then🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 405-405: Quote this to prevent word splitting.
(SC2046)
17-17: 🛠️ Refactor suggestionAdd error handling for directory creation.
Add error handling for the mkdir command to ensure the script fails gracefully if directory creation fails.
-mkdir -p $PLANE_INSTALL_DIR/archive +mkdir -p $PLANE_INSTALL_DIR/archive || { + echo "Failed to create required directories" + exit 1 +}Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
deploy/selfhost/variables.env (1)
8-10: LGTM! Consider documenting replica scaling recommendations.The new replica variables follow the established pattern and provide good defaults. Consider adding comments to document recommended replica counts for different deployment sizes (small/medium/large).
WORKER_REPLICAS=1 BEAT_WORKER_REPLICAS=1 LIVE_REPLICAS=1 +# Recommended replica counts: +# Small deployment (< 100 users): WORKER_REPLICAS=1, BEAT_WORKER_REPLICAS=1, LIVE_REPLICAS=1 +# Medium deployment (100-1000 users): WORKER_REPLICAS=2, BEAT_WORKER_REPLICAS=2, LIVE_REPLICAS=2 +# Large deployment (1000+ users): WORKER_REPLICAS=3+, BEAT_WORKER_REPLICAS=3+, LIVE_REPLICAS=3+deploy/selfhost/docker-compose.yml (2)
121-122: Consider volume management strategy for Swarm mode.While the volume configurations are preserved, consider the following for Swarm deployments:
- Local volumes aren't shared between nodes
- For multi-node deployments, consider using a volume driver that supports distributed storage
Consider adding a comment in the compose file:
volumes: + # For multi-node Swarm deployments: + # 1. Configure a volume driver that supports distributed storage (e.g., rexray, nfs) + # 2. Update volume definitions with the appropriate driver pgdata:Also applies to: 135-138, 152-155, 169-172
184-187: LGTM! Infrastructure services properly configured for Swarm.The single replica configuration for stateful services (db, redis, mq, minio) is correct for maintaining data consistency. Consider adding a warning comment about scaling these services.
plane-db: image: postgres:15.7-alpine command: postgres -c 'max_connections=1000' + # WARNING: Do not scale this service in Swarm mode + # Postgres requires additional configuration for high availability deploy: replicas: 1Also applies to: 195-198, 204-207, 216-219, 235-238
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
deploy/selfhost/docker-compose.yml(6 hunks)deploy/selfhost/variables.env(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (2)
deploy/selfhost/docker-compose.yml (2)
58-66: LGTM! Environment variables section is well-organized.The replica environment variables are properly defined with consistent default values.
229-232: LGTM! Port configuration is properly configured for Swarm mode.The proxy service port configuration correctly uses host mode, which is essential for Swarm deployments.
| restart_policy: | ||
| condition: on-failure |
There was a problem hiding this comment.
💡 Codebase verification
Consider using any restart condition for critical infrastructure services
The on-failure restart policy is too restrictive for infrastructure services that require high availability. While it works in Swarm mode, it won't automatically restart services after clean shutdowns. Consider using condition: any for these critical services:
- plane-db (PostgreSQL)
- plane-redis (Redis)
- plane-mq (RabbitMQ)
- plane-minio (MinIO)
deploy:
replicas: 1
restart_policy:
- condition: on-failure
+ condition: anyThis aligns better with the production configuration in the root docker-compose.yml which uses restart: always.
🔗 Analysis chain
Verify impact of restart policy change.
The restart policy has been changed from unless-stopped to on-failure for all services. While this works in Swarm mode, consider the following:
- Services will not automatically restart after a clean shutdown
- Manual intervention might be needed for non-error shutdowns
Consider if any might be a better condition for critical services:
restart_policy:
- condition: on-failure
+ condition: anyAlso applies to: 85-86, 97-98, 110-111
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Check the complete docker-compose.yml file
cat deploy/selfhost/docker-compose.yml
# Look for README or documentation files in deploy directory
fd -t f "README|\.md$" deploy/
# Search for other docker-compose files
fd -t f "docker-compose\.ya?ml$"
Length of output: 6959
Script:
#!/bin/bash
# Check deployment documentation
cat deploy/selfhost/README.md
cat deploy/kubernetes/README.md
# Check if there's any configuration in the root docker-compose.yml
cat docker-compose.yml
Length of output: 22536
Changes in PR:
Summary by CodeRabbit
Summary by CodeRabbit
Documentation
Deployment
swarm.shscript in the release artifacts for deployment.Tooling