๐ซ feat: ๋ฐฐํฌ ๋ฆฌ์์ค ์ ๋ฆฌ ์๋ํ ๋ฐ ์ด์ ์์ ์ฑ ๋ณด๊ฐ#8
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. ๐ โน๏ธ Recent review infoโ๏ธ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ๐ Files selected for processing (2)
๐ง Files skipped from review as they are similar to previous changes (1)
๐ WalkthroughWalkthroughDeployment cleanup standards are documented, and the deployment script now removes outdated local images from the same ECR repository and prunes dangling images after a successful Compose deployment. ChangesDeployment operations
Estimated code review effort: 2 (Simple) | ~10 minutes ๐ฅ Pre-merge checks | โ 4 | โ 1โ Failed checks (1 warning)
โ Passed checks (4 passed)
โจ Finishing Touches๐ Generate docstrings
๐งช Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
๐ค 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 @.github/workflows/deploy-dev.yml:
- Line 132: Replace the direct sudo invocation of the user-writable deploy.sh
path with a root-owned trusted deployment wrapper in
.github/workflows/deploy-dev.yml at line 132 and
.github/workflows/deploy-prod.yml at line 132, preserving the existing
environment and deployment arguments.
- Line 132: Update the remote deployment command invoking deploy.sh so ECR
authentication and docker compose pull execute under the same user context:
either perform the ECR login as root or remove sudo from the deploy.sh
invocation. Preserve the existing deployment arguments and ensure the
authenticated Docker context matches the context used by deploy.sh.
In @.github/workflows/deploy-prod.yml:
- Line 132: Update the deployment flow around scripts/deploy.sh so Docker
authentication and the subsequent docker compose pull run under the same user
context. Either perform the registry login as root before the sudo-managed
deploy, or remove sudo from the compose operation, while preserving the existing
production deployment arguments.
In `@scripts/deploy.sh`:
- Line 56: Update the cleanup step around docker image prune -f to explicitly
remove stale dev-${github.sha} and prod-${github.sha} deployment images while
retaining the current image, or revise related documentation so it no longer
claims those tagged images are reclaimed.
- Around line 55-57: Scope container cleanup in scripts/deploy.sh around docker
container prune so it removes only containers associated with this deployment,
rather than every stopped container on the host; retain the existing safe image
cleanup. Update docs/deployment.md lines 6-7 to describe the resulting scoped
behavior, or explicitly document the host-wide cleanup and require a dedicated
EC2 host if that behavior remains.
- Around line 55-57: Make the post-deployment cleanup commands in
scripts/deploy.sh best-effort under set -e: ensure failures from docker image
prune and docker container prune are logged as cleanup warnings while preserving
the successful deployment result and SSH exit status.
- Around line 52-53: Ensure Docker authentication and Compose use the same user:
update scripts/deploy.sh lines 52-53 to run docker compose as ${EC2_USER} rather
than root, while preserving the existing pull and up arguments. Update
.github/workflows/deploy-dev.yml line 132 and .github/workflows/deploy-prod.yml
line 132 as needed so deployment invokes deploy.sh without switching the Compose
commands to a different user.
๐ช 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3eb82c48-2e7e-4bef-bd72-9d57f5229c95
๐ Files selected for processing (4)
.github/workflows/deploy-dev.yml.github/workflows/deploy-prod.ymldocs/deployment.mdscripts/deploy.sh
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#๏ธโฃ Issue Number
๐ ์์ฝ(Summary)
deploy.sh๋ฅผsudo๋ก ์คํํ๋๋ก ์์ ํด Nginx ์ค์ ๋ฐ์ ๋ฐ ์๋น์ค reload ๊ถํ ๋ฌธ์ ๋ฅผ ํด๊ฒฐํ์ต๋๋ค.docker image prune -f,docker container prune -f๋ฅผ ์ํํ๋๋ก ๊ตฌ์ฑํด EC2 ๋ด๋ถ Docker ๋ฆฌ์์ค ๋์ ์ ์ค์ด๋๋ก ํ์ต๋๋ค.๐ ๋ฆฌ๋ทฐ ์์ฒญ์ฌํญ
sudo์คํ ๊ธฐ์ค์ผ๋ก ์ ๋ฆฌํ์ต๋๋ค.๐ป ํ ์คํธ ๊ฒฐ๊ณผ
/etc/nginx/conf.d๋ฐ์ ๊ถํ ๋ฌธ์ ์์ธ์ ํ์ธํ์ต๋๋ค.deploy.sh๋ฅผsudo๋ก ์คํํ๋๋ก ๋ณ๊ฒฝํด ๊ถํ ๋ฌธ์ ํด๊ฒฐ ๋ฐฉํฅ์ ๋ฐ์ํ์ต๋๋ค.๐ ํฌ์ธํธ
โ๏ธ ํ๊ณ
Summary by CodeRabbit
New Features
Documentation