Skip to content

Conversation

@taek2222
Copy link
Contributor

@taek2222 taek2222 commented Nov 27, 2025

#️⃣ 이슈 번호

#2


🛠️ 작업 내용

  • CI/CD 전략을 변경했습니다.
  • Dev, Prod CodeDeploy 환경으로 변경했습니다.

🙇🏻 중점 리뷰 요청

  • 각 삭제한 파일이랑 이름 잘 확인해 주세요!
  • 잘못되게 적용된 부분이 있는 지 검토해주세요!

@coderabbitai
Copy link

coderabbitai bot commented Nov 27, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (3)
  • android
  • backend
  • frontend

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized key(s) in object: 'tools'
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ci-cd/2

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request migrates the CI/CD strategy from a self-hosted runner approach to AWS CodeDeploy for both development and production environments.

Key Changes:

  • Replaced self-hosted runner deployment with AWS CodeDeploy using S3 artifact storage
  • Introduced new deployment lifecycle scripts (stop, clean, start, validate) for CodeDeploy hooks
  • Added Slack notification workflows for PR events (opened, review requested, review submitted)

Reviewed changes

Copilot reviewed 10 out of 13 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
src/main/java/com/daedan/festabook/global/config/CloudWatchMetricsConfig.java Removed CloudWatch metrics configuration (no longer needed)
infra/appspec.yml Updated for CodeDeploy with proper lifecycle hooks and script locations
infra/validate.sh New health check script for deployment validation
infra/stop.sh New script to gracefully stop running application
infra/start.sh Refactored to improve error handling and readability
infra/clean.sh New script to clean old JAR files before deployment
.github/workflows/ci.yml Renamed from "Backend CI Test" to "CI Test"
.github/workflows/ci-cd-dev.yml New workflow for dev environment using CodeDeploy
.github/workflows/ci-cd-prod.yml New workflow for prod environment using CodeDeploy
.github/workflows/slack-notify-*.yml New workflows for Slack notifications on PR events
.github/workflows/backend-cd-dev.yml Removed old self-hosted runner deployment workflow

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

infra/stop.sh Outdated
@@ -0,0 +1,11 @@
#!/bin/bash

PID=$(pgrep -f festabook-0.0.1-SNAPSHOT.jar)
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pgrep command searches for festabook-0.0.1-SNAPSHOT.jar, which is a hardcoded version-specific jar name. This will break when the version changes (e.g., to 0.0.2-SNAPSHOT or 1.0.0).

Consider using a more flexible pattern:

PID=$(pgrep -f "festabook.*\.jar")

This will match any version of the festabook jar file.

Suggested change
PID=$(pgrep -f festabook-0.0.1-SNAPSHOT.jar)
PID=$(pgrep -f "festabook.*\.jar")

Copilot uses AI. Check for mistakes.
- name: Upload artifact to S3
run: |
aws s3 cp "${{ steps.bundle.outputs.zip_path }}" \
"s3://${{ env.S3_BUCKET }}/prod/builds/${{ steps.bundle.outputs.zip_name }}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

S3 버킷이 우리 secret에 많아서, 내부용 혹은 배포용 버킷이라고 명명 짓는건 어떤가요?

Copy link
Contributor Author

@taek2222 taek2222 Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋은데요?! 바꾸겠습니다. ARTIFACT_BUCKET 바꾸겠습니다.

@taek2222 taek2222 requested a review from changuii November 27, 2025 13:20
id-token: write

env:
AWS_REGION: ap-northeast-2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

으어 이거 리전도 가려야 하는 값 아닌가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리전도 함께 Secret 처리 하겠습니다. 👍

Copy link
Contributor

@changuii changuii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

브랜치 바꿔주세요 !


on:
push:
branches: [ ci-cd/2 ]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

브랜치가 잘못되었습니다.


on:
push:
branches: [ ci-cd/2 ]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

브랜치 바꿔주세요

@@ -1,4 +1,4 @@
name: Backend CI Test
name: CI Test
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

브랜치 변경해야합니다.

Copy link
Contributor

@eoehd1ek eoehd1ek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코멘트 확인해주세요~


on:
push:
branches: [ ci-cd/2 ]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

브랜치 dev로 변경해야 해요.


on:
push:
branches: [ ci-cd/2 ]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

브랜치 이름 prod로 변경해야 합니다.

with:
aws-region: ${{ env.AWS_REGION }}
role-to-assume: ${{ secrets.AWS_OIDC_ROLE_ARN }}
role-session-name: festabook-ci-cd
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

role-session-name에 dev, prod를 구별할 수 있는 이름을 사용하는 건 어떤가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 이 부분에 대해서 고민을 해봤는데, dev prod 두 방향에서 구분할 필요성이 있다면 하려고 했습니다.

다만, 두 권한이 모두 일치해서 하나의 OIDC만 만들었는데 후유는 어떻게 생각해요?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

어느 배포 작업에서 권한을 부여했는지 별칭을 나타내는 것으로 생각하고 role-session-name에 dev, prod를 구별하자는 코멘트 남겼어요.

어떤 상황에서 role-session-name을 확인할지 떠오르지 않네요.
비타 의견대로 추후 구별을 해야 하는 상황이 온다면 그 때 prod, dev 별칭을 나눠도 될 것 같아요.

Comment on lines 5 to 11
if [ -z "$PID" ]; then
echo "> No running application found"
else
echo "> Killing process $PID"
kill -15 $PID
sleep 5
fi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. java를 sudo 권한으로 실행했으므로 kill 명령어 또한 sudo로 실행해야 합니다.
  2. sleep 5로 5초를 전부 기다리는 것 보다 ps로 해당 프로세스가 종료되었는지 확인하는 게 더 안정적입니다.
# 1. 종료할 프로세스의 PID를 찾기
PID=$(pgrep -f "festabook.*\.jar")  

if [ -z "$PID" ]; then
    echo "> No running application found"
else
    echo "> Killing process $PID"
    # java를 sudo로 실행했기 때문에 kill 명령어도 sudo를 써야 합니다.
    sudo kill -15 $PID
    
    # sleep 5 대신 PID 종료 확인 방식으로 작성한 스크립트
    TIMEOUT=30
    COUNT=0
    
    # 3. 종료될 때까지 대기 루프
    while [ $COUNT -lt $TIMEOUT ]; do
        # 해당 PID를 가진 프로세스가 존재하는지 확인
        # ps -p $PID > /dev/null 2>&1 명령은 PID가 있으면 종료 코드 0, 없으면 0이 아닌 값을 반환
        if ps -p $PID > /dev/null 2>&1; then
            echo "> 종료 대기 중... ($COUNT초 경과)"
            sleep 1
            COUNT=$((COUNT + 1))
        else
            echo "> 프로세스($PID)가 정상적으로 종료되었습니다."
            break 
        fi
    done
    
    # 4. 타임아웃 시 강제 종료 (SIGKILL)
    if ps -p $PID > /dev/null 2>&1; then
        echo "> 경고: $TIMEOUT초 안에 프로세스($PID)가 종료되지 않았습니다."
        echo "> 강제 종료(SIGKILL -9)를 진행합니다."
        sudo kill -9 $PID
    else
        echo "> 이전 애플리케이션 정리 완료."
    fi
fi

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

주신 코드로 변경 완료했습니다! 감사합니다.

@taek2222 taek2222 requested review from changuii and eoehd1ek December 2, 2025 11:56
Copy link
Contributor Author

@taek2222 taek2222 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추가적으로 작업한 내용에 대해서 전달 코멘트 남겼습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

배포를 위해 변경한 파일입니다. 다른 PR 내용과 동일해 겹치지 않을 걸로 예상됩니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

배포를 위해 변경한 파일입니다. 다른 PR 내용과 동일해 겹치지 않을 걸로 예상됩니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

배포를 위해 변경한 파일입니다. 다른 PR 내용과 동일해 겹치지 않을 걸로 예상됩니다.

Copy link
Contributor

@changuii changuii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

with:
aws-region: ${{ env.AWS_REGION }}
role-to-assume: ${{ secrets.AWS_OIDC_ROLE_ARN }}
role-session-name: festabook-ci-cd
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

어느 배포 작업에서 권한을 부여했는지 별칭을 나타내는 것으로 생각하고 role-session-name에 dev, prod를 구별하자는 코멘트 남겼어요.

어떤 상황에서 role-session-name을 확인할지 떠오르지 않네요.
비타 의견대로 추후 구별을 해야 하는 상황이 온다면 그 때 prod, dev 별칭을 나눠도 될 것 같아요.

@taek2222 taek2222 merged commit 9f27f1e into dev Dec 3, 2025
4 checks passed
@taek2222 taek2222 deleted the ci-cd/2 branch December 3, 2025 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants