Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions .github/workflows/Dev_CD.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,25 @@ jobs:
uses: actions/download-artifact@v4
with:
name: app-artifact
path: ~/app
path: ~/app/staging

- name: Download deploy scripts
uses: actions/download-artifact@v4
with:
name: deploy-scripts
path: ~/app/scripts/

- name: Replace application to latest
run: sudo sh ~/app/scripts/replace-new-version.sh
- name: Setup log directory
run: |
sudo mkdir -p /home/ubuntu/logs
sudo chown -R ubuntu:ubuntu /home/ubuntu/logs
chmod 755 /home/ubuntu/logs

- name: Make deploy script executable
run: chmod +x ~/app/scripts/zero-downtime-deploy.sh

- name: Health Check
run: sh ~/app/scripts/health-check.sh
- name: Zero Downtime Deployment
run: sh ~/app/scripts/zero-downtime-deploy.sh

- name: Send Discord Alert on Failure
if: failure()
Expand Down
16 changes: 11 additions & 5 deletions .github/workflows/Prod_CD.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,25 @@ jobs:
uses: actions/download-artifact@v4
with:
name: app-artifact
path: ~/app
path: ~/app/staging

- name: Download deploy scripts
uses: actions/download-artifact@v4
with:
name: deploy-scripts
path: ~/app/scripts/

- name: Replace application to latest
run: sudo sh ~/app/scripts/replace-new-version.sh
- name: Setup log directory
run: |
sudo mkdir -p /home/ubuntu/logs
sudo chown -R ubuntu:ubuntu /home/ubuntu/logs
chmod 755 /home/ubuntu/logs

- name: Make deploy script executable
run: chmod +x ~/app/scripts/zero-downtime-deploy.sh

- name: Health Check
run: sh ~/app/scripts/health-check.sh
- name: Zero Downtime Deployment
run: sh ~/app/scripts/zero-downtime-deploy.sh

- name: Send Discord Alert on Failure
if: failure()
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,4 @@ out/

### application-local.yml
/src/main/resources/application-local.yml
.serena
34 changes: 34 additions & 0 deletions nginx/api.dev.debate-timer.com
Copy link
Member

Choose a reason for hiding this comment

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

nginx 파일을 전부 여기에서 관리하는 이유가 있을까요?
(일부는 Certbot 측에서 관리하다 보니, Certbot에서 해당 파일을 바꿔버리면 여기 적용 안될 것 같은데;;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이건 단지 코드 관리 측면에서 업로드 한 것일 뿐 실제로 깃허브에 있는 이 파일을 사용하지는 않습니다. 제가 직접 ec2에 접속해 수정한 뒤 어떻게 수정한지를 기록한 것입니다.

Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
upstream debate_timer_backend {
server 127.0.0.1:8080;
keepalive 32;
}

server {
server_name api.dev.debate-timer.com;

location / {
proxy_pass http://debate_timer_backend;
proxy_set_header Host $host;
proxy_set_header X-Real-IP $remote_addr;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Proto $scheme;
}

listen [::]:443 ssl ipv6only=on; # managed by Certbot
listen 443 ssl; # managed by Certbot
ssl_certificate /etc/letsencrypt/live/api.dev.debate-timer.com/fullchain.pem; # managed by Certbot
ssl_certificate_key /etc/letsencrypt/live/api.dev.debate-timer.com/privkey.pem; # managed by Certbot
include /etc/letsencrypt/options-ssl-nginx.conf; # managed by Certbot
ssl_dhparam /etc/letsencrypt/ssl-dhparams.pem; # managed by Certbot
}

server {
if ($host = api.dev.debate-timer.com) {
return 308 https://$host$request_uri;
} # managed by Certbot

listen 80;
listen [::]:80;
server_name api.dev.debate-timer.com;
return 404; # managed by Certbot
Comment on lines +26 to +33

Choose a reason for hiding this comment

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

medium

Nginx server 블록 내에서 if 지시어 사용은 비효율적일 수 있습니다. server_name 지시어가 이미 api.dev.debate-timer.com으로 들어오는 요청만 이 블록에서 처리하도록 보장하므로, if ($host = ...) 조건문은 불필요합니다. 또한 이 조건문 때문에 33번 줄의 return 404;는 사실상 도달할 수 없는 코드가 됩니다. 아래와 같이 if문 없이 return을 직접 사용하여 코드를 간결하게 개선하는 것을 제안합니다. 이 변경 사항은 nginx/api.prod.debate-timer.com 파일에도 동일하게 적용하는 것이 좋습니다.

    listen 80;
    listen [::]:80;
    server_name api.dev.debate-timer.com;
    return 308 https://$host$request_uri; # managed by Certbot

}
34 changes: 34 additions & 0 deletions nginx/api.prod.debate-timer.com
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
upstream debate_timer_backend {
server 127.0.0.1:8080;
keepalive 32;
}

server {
server_name api.prod.debate-timer.com;

location / {
proxy_pass http://debate_timer_backend;
proxy_set_header Host $host;
proxy_set_header X-Real-IP $remote_addr;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Proto $scheme;
}

listen [::]:443 ssl ipv6only=on; # managed by Certbot
listen 443 ssl; # managed by Certbot
ssl_certificate /etc/letsencrypt/live/api.prod.debate-timer.com/fullchain.pem; # managed by Certbot
ssl_certificate_key /etc/letsencrypt/live/api.prod.debate-timer.com/privkey.pem; # managed by Certbot
include /etc/letsencrypt/options-ssl-nginx.conf; # managed by Certbot
ssl_dhparam /etc/letsencrypt/ssl-dhparams.pem; # managed by Certbot
}

server {
if ($host = api.prod.debate-timer.com) {
return 308 https://$host$request_uri;
} # managed by Certbot

listen 80;
listen [::]:80;
server_name api.prod.debate-timer.com;
return 404; # managed by Certbot
}
238 changes: 238 additions & 0 deletions scripts/dev/zero-downtime-deploy.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,238 @@
#!/bin/bash

set -e

APP_DIR="/home/ubuntu/app"
PORT_FILE="$APP_DIR/current_port.txt"
LOG_FILE="$APP_DIR/deploy.log"
BLUE_PORT=8080
GREEN_PORT=8081
BLUE_MONITOR_PORT=8083
GREEN_MONITOR_PORT=8084
MAX_HEALTH_CHECK_RETRIES=60
HEALTH_CHECK_INTERVAL=2
PROFILE="dev"
TIMEZONE="Asia/Seoul"

log() {
local timestamp=$(date '+%Y-%m-%d %H:%M:%S')
echo "${timestamp} $@" | tee -a "$LOG_FILE"
}

error_exit() {
log "$1"
exit 1
}

get_current_port() {
if [ ! -f "$PORT_FILE" ]; then
log "Port file not found. Initializing with default port $BLUE_PORT"
echo "$BLUE_PORT" > "$PORT_FILE"
echo "$BLUE_PORT"
else
cat "$PORT_FILE"
fi
}

get_inactive_port() {
local current_port=$1
if [ "$current_port" -eq "$BLUE_PORT" ]; then
echo "$GREEN_PORT"
else
echo "$BLUE_PORT"
fi
}

get_monitor_port() {
local app_port=$1
if [ "$app_port" -eq "$BLUE_PORT" ]; then
echo "$BLUE_MONITOR_PORT"
else
echo "$GREEN_MONITOR_PORT"
fi
}

is_port_in_use() {
local port=$1
sudo lsof -t -i:$port > /dev/null 2>&1
return $?
}

kill_process_on_port() {
local port=$1
local pid=$(sudo lsof -t -i:$port 2>/dev/null)

if [ -z "$pid" ]; then
log "No process running on port $port"
return 0
fi

log "Sending graceful shutdown signal to process $pid on port $port"
sudo kill -15 "$pid"

local wait_count=0
while [ $wait_count -lt 65 ] && is_port_in_use "$port"; do
Copy link
Contributor

Choose a reason for hiding this comment

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

wait_count 65초 정도로 대기 시간을 설정한 이유가 있나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

큰 이유는 없습니다. shutdown의 시간보다 5초정도 더 여유를 줬습니다

sleep 1
wait_count=$((wait_count + 1))
done

if is_port_in_use "$port"; then
log "Process didn't stop gracefully, forcing shutdown"
sudo kill -9 "$pid" 2>/dev/null || true
sleep 2
fi

log "Process on port $port stopped successfully"
}

health_check() {
local port=$1
local monitor_port=$2
local health_url="http://localhost:$monitor_port/monitoring/health"

log "Starting health check for port $port (monitor: $monitor_port)"

local retry=1
while [ $retry -le $MAX_HEALTH_CHECK_RETRIES ]; do
local status=$(curl -s -o /dev/null -w "%{http_code}" "$health_url" 2>/dev/null || echo "000")

log "Health check attempt $retry/$MAX_HEALTH_CHECK_RETRIES - Status: $status"

if [ "$status" = "200" ]; then
log "Health check passed!"
return 0
fi

sleep $HEALTH_CHECK_INTERVAL
retry=$((retry + 1))
done

log "Health check failed after $MAX_HEALTH_CHECK_RETRIES attempts"
return 1
}

start_application() {
local port=$1
local monitor_port=$2
local staging_jar="$APP_DIR/staging/app.jar"
local jar_file="$APP_DIR/app-$port.jar"

if [ ! -f "$staging_jar" ]; then
error_exit "No JAR file found in staging directory: $staging_jar"
fi

log "Copying JAR from staging to $jar_file"
cp "$staging_jar" "$jar_file"

log "Starting application on port $port with JAR: $jar_file"

if is_port_in_use "$port"; then
log "Port $port is in use, cleaning up..."
kill_process_on_port "$port"
fi

sudo nohup java \
-Dspring.profiles.active=$PROFILE,monitor \
-Duser.timezone=$TIMEZONE \
-Dserver.port=$port \
-Dmanagement.server.port=$monitor_port \
-Ddd.service=debate-timer \
-Ddd.env=$PROFILE \
-jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 &
Comment on lines +134 to +141
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# Check if file exists and read lines 134-141
if [ -f "scripts/dev/zero-downtime-deploy.sh" ]; then
    echo "=== Lines 130-145 of zero-downtime-deploy.sh ==="
    sed -n '130,145p' scripts/dev/zero-downtime-deploy.sh | cat -n
else
    echo "File not found"
fi

Repository: debate-timer/debate-timer-be

Length of output: 704


🌐 Web query:

sudo output redirection shell behavior does redirection happen inside or outside sudo command

💡 Result:

The shell (your unprivileged shell) performs the redirection before sudo runs the command — so sudo applies to the command being run, not to the > or >> open that writes the file. [1][2][3]

Common fixes:

  • Run the whole thing under a root shell: sudo bash -c 'echo "text" > /path/file'. [1]
  • Use tee so the writing is done by a sudo'd process: echo "text" | sudo tee /path/file >/dev/null (use -a to append). [2][3]

Sources: [1] Stack Overflow, [2] Baeldung, [3] ShellHacks.


🏁 Script executed:

# Also check for application logging configuration
echo "=== Check for logging in application configuration files ==="
find . -type f \( -name "application*.yml" -o -name "application*.properties" \) 2>/dev/null | head -5

echo -e "\n=== Check surrounding context of the deploy script ==="
if [ -f "scripts/dev/zero-downtime-deploy.sh" ]; then
    wc -l scripts/dev/zero-downtime-deploy.sh
fi

Repository: debate-timer/debate-timer-be

Length of output: 437


Critical: sudo does not apply to output redirection (SC2024 logic issue)

The redirection > "$APP_DIR/app-$port.log" 2>&1 is evaluated by the shell before sudo executes, so the log file will be written with unprivileged permissions and likely fail. Wrap the command in a shell invocation to apply redirection inside the sudo context:

-    sudo nohup java \
+    sudo sh -c 'nohup java \
         -Dspring.profiles.active=$PROFILE,monitor \
         -Duser.timezone=$TIMEZONE \
         -Dserver.port=$port \
         -Dmanagement.server.port=$monitor_port \
         -Ddd.service=debate-timer \
         -Ddd.env=$PROFILE \
-        -jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 &
+        -jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 &'

Alternatively, verify that Java logging is properly configured via logging.file.name in your application.yml.

📝 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.

Suggested change
sudo nohup java \
-Dspring.profiles.active=$PROFILE,monitor \
-Duser.timezone=$TIMEZONE \
-Dserver.port=$port \
-Dmanagement.server.port=$monitor_port \
-Ddd.service=debate-timer \
-Ddd.env=$PROFILE \
-jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 &
sudo sh -c 'nohup java \
-Dspring.profiles.active=$PROFILE,monitor \
-Duser.timezone=$TIMEZONE \
-Dserver.port=$port \
-Dmanagement.server.port=$monitor_port \
-Ddd.service=debate-timer \
-Ddd.env=$PROFILE \
-jar "$jar_file" > "$APP_DIR/app-$port.log" 2>&1 &'
🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 141-141: sudo doesn't affect redirects. Use ..| sudo tee file

(SC2024)

🤖 Prompt for AI Agents
In scripts/dev/zero-downtime-deploy.sh around lines 134-141 the shell
redirection is evaluated before sudo runs so the log file is created with
unprivileged permissions; fix by running the entire java invocation (including
its redirection) inside a root shell invoked by sudo so the redirection is
performed as root, or alternatively pipe the process output through a root-owned
writer (e.g., sudo tee) or configure Java logging via logging.file.name in
application.yml so the app writes logs itself.


local pid=$!
log "Application started with PID: $pid"

sleep 3

if ! kill -0 $pid 2>/dev/null; then
error_exit "Application process died immediately after start. Check logs at $APP_DIR/app-$port.log"
fi
}

switch_nginx_upstream() {
local new_port=$1
local nginx_conf="/etc/nginx/sites-available/api.dev.debate-timer.com"
local temp_conf="/tmp/api.dev.debate-timer.com.tmp"

Choose a reason for hiding this comment

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

medium

임시 파일 경로로 /tmp/api.dev.debate-timer.com.tmp를 하드코딩하여 사용하고 있습니다. 이 방식은 동시에 여러 배포 프로세스가 실행될 경우 파일 충돌을 일으킬 수 있는 잠재적인 위험이 있습니다.

mktemp 명령어를 사용하여 유니크한 임시 파일을 생성하는 것이 더 안전하고 견고한 방법입니다. mktemp는 예측 불가능한 파일명을 만들어주어 race condition과 같은 보안 문제를 예방합니다. 또한, trap과 함께 사용하면 스크립트 종료 시 임시 파일을 자동으로 정리할 수 있습니다.

# 스크립트 상단에 추가
temp_conf=$(mktemp)
trap 'rm -f "$temp_conf"' EXIT INT TERM

이 변경 사항은 prod 배포 스크립트에도 동일하게 적용해야 합니다.

Suggested change
local temp_conf="/tmp/api.dev.debate-timer.com.tmp"
local temp_conf=$(mktemp)

local backup_conf="${nginx_conf}.bak"

if [ ! -f "$nginx_conf" ]; then
error_exit "nginx configuration not found at $nginx_conf"
fi

log "Switching nginx upstream to port $new_port"
sudo cp "$nginx_conf" "$backup_conf"

sed "s/server 127\.0\.0\.1:[0-9]\+;/server 127.0.0.1:$new_port;/" "$nginx_conf" > "$temp_conf"
sudo cp "$temp_conf" "$nginx_conf"

if ! sudo nginx -t 2>/dev/null; then

Choose a reason for hiding this comment

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

medium

nginx -t 명령어의 표준 에러(stderr)를 /dev/null로 리다이렉션하고 있습니다. 이렇게 하면 Nginx 설정 파일에 문법 오류가 있을 경우, 어떤 오류가 발생했는지 알 수 없어 디버깅이 어려워집니다.

배포 실패 시 원인을 빠르게 파악하기 위해 에러 메시지를 로그 파일에 기록하는 것이 좋습니다. 2>/dev/null 부분을 제거하여 에러가 로그에 남도록 수정하는 것을 제안합니다.

이 변경 사항은 prod 배포 스크립트에도 동일하게 적용해야 합니다.

Suggested change
if ! sudo nginx -t 2>/dev/null; then
if ! sudo nginx -t; then

log "nginx configuration test failed, rolling back."
sudo cp "$backup_conf" "$nginx_conf"
sudo rm "$backup_conf"
return 1
fi

sudo nginx -s reload
log "nginx reloaded successfully"

sleep 2
local response=$(curl -s -o /dev/null -w "%{http_code}" "http://localhost/" 2>/dev/null || echo "000")
if [ "$response" = "000" ] || [ "$response" = "502" ] || [ "$response" = "503" ]; then
Comment on lines +180 to +181

Choose a reason for hiding this comment

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

high

switch_nginx_upstream 함수에서 Nginx 리로드 후 상태 확인을 위해 http://localhost/를 호출하고 있습니다. 만약 루트 경로(/)가 항상 200 OK를 반환하지 않는다면, 이 확인 방법은 신뢰할 수 없습니다. 예를 들어, 애플리케이션이 정상적으로 동작하더라도 루트 경로가 404를 반환하면 배포가 성공했다고 잘못 판단할 수 있습니다.

scripts/nginx-switch-port.sh 스크립트의 55번 줄에서처럼 명시적인 health check 엔드포인트(http://localhost/monitoring/health)를 사용하고, 응답 코드가 200이 아닌 모든 경우를 실패로 처리하는 것이 더 안전하고 정확합니다. 이 변경 사항은 prod 배포 스크립트에도 동일하게 적용해야 합니다.

Suggested change
local response=$(curl -s -o /dev/null -w "%{http_code}" "http://localhost/" 2>/dev/null || echo "000")
if [ "$response" = "000" ] || [ "$response" = "502" ] || [ "$response" = "503" ]; then
local response=$(curl -s -o /dev/null -w "%{http_code}" "http://localhost/monitoring/health" 2>/dev/null || echo "000")
if [ "$response" != "200" ]; then

log "nginx health check failed after reload (status: $response). Rolling back nginx config."
sudo cp "$backup_conf" "$nginx_conf"
sudo nginx -s reload
sudo rm "$backup_conf"
return 1
fi

log "nginx is now routing traffic to port $new_port"
sudo rm "$backup_conf"
return 0
}

main() {
local current_port=$(get_current_port)
local new_port=$(get_inactive_port "$current_port")
local new_monitor_port=$(get_monitor_port "$new_port")

log "Current active port: $current_port"
log "Deploying to port: $new_port"
log "Monitor port: $new_monitor_port"

log "Step 1/4: Starting new version on port $new_port"
start_application "$new_port" "$new_monitor_port"

log "Step 2/4: Performing health check"
if ! health_check "$new_port" "$new_monitor_port"; then
log "Deployment failed: Health check did not pass"
log "Rolling back: Stopping new version on port $new_port"
kill_process_on_port "$new_port"
error_exit "Deployment aborted due to health check failure"
fi

log "Step 3/4: Switching nginx to new version"
if ! switch_nginx_upstream "$new_port"; then
log "nginx switch failed, rolling back"
kill_process_on_port "$new_port"
error_exit "Deployment aborted due to nginx switch failure"
fi

log "Step 4/4: Stopping old version on port $current_port"
kill_process_on_port "$current_port"

local old_jar="$APP_DIR/app-$current_port.jar"
if [ -f "$old_jar" ]; then
log "Removing old JAR file: $old_jar"
rm -f "$old_jar"
fi

echo "$new_port" > "$PORT_FILE"
log "Updated active port file to $new_port"

log "Deployment completed successfully!"
log "Active port: $new_port"
log "Inactive port: $current_port"
}

main "$@"
Comment on lines +1 to +238

Choose a reason for hiding this comment

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

high

scripts/dev/zero-downtime-deploy.shscripts/prod/zero-downtime-deploy.sh 스크립트의 내용이 거의 동일합니다. 이렇게 중복된 코드는 유지보수를 어렵게 만듭니다. 예를 들어, 로직 수정이 필요할 때 두 파일을 모두 수정해야 하는 번거로움이 있습니다.

두 스크립트를 하나로 통합하고, 환경(dev/prod)을 인자로 받아 동적으로 설정을 변경하도록 리팩토링하는 것을 강력히 권장합니다.

예시:
./zero-downtime-deploy.sh dev

이렇게 하면 PROFILE 변수와 nginx_conf 경로를 인자에 따라 설정할 수 있어 코드 중복을 제거하고 유지보수성을 크게 향상시킬 수 있습니다.

Loading