Feat/mc detect hookscript - #148
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new Proxmox hook script (mc-detect-hook.sh) to automatically detect Minecraft servers running inside a VM during the post-start phase. The review feedback highlights several critical improvements for robustness: initializing the paths variable and adding a guard to prevent an unbounded find execution, using string comparison and suppressing stderr for jq parsing to avoid shell errors, continuing the retry loop instead of exiting prematurely on transient failures, and explicitly returning non-zero exit codes (exit 1) on failures so that Proxmox can correctly detect errors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| result=$(qm guest exec "$VMID" -- sh -c ' | ||
| for dir in /home /opt /srv /root; do # vm에 folder 있는지 확인 | ||
| if [ -d "$dir" ]; then | ||
| paths="$paths $dir" | ||
| fi | ||
| done | ||
| # find의 에러는 out-data가 아닌 err-data로 분리되므로 JSON 파싱에 영향 없음. 단, exitcode는 0이 아니게 되므로 없는 경로는 위 -d 검사로 미리 걸러냄 | ||
| find $paths -name "server.properties" -type f # vm에 있는 folder만 조회') |
There was a problem hiding this comment.
개선 사항
paths변수 초기화: 환경 변수 상속을 방지하기 위해paths=""로 명시적 초기화를 수행합니다.- 빈 경로 가드: 만약 대상 디렉토리(
/home,/opt,/srv,/root)가 하나도 존재하지 않는 경우,paths가 빈 상태로find가 실행됩니다. 이 경우find는 기본값으로 현재 디렉토리(보통/또는/root) 전체를 재귀 탐색하게 되어 VM에 심각한 CPU 및 디스크 I/O 부하를 유발할 수 있습니다. 따라서paths가 비어있지 않을 때만find를 실행하도록 가드를 추가합니다. - 정상 종료 보장: 디렉토리가 없을 때 스크립트가 마지막 비교문의 실패 코드(1)를 반환하지 않고 안전하게
0을 반환하도록echo ""를 실행합니다.
| result=$(qm guest exec "$VMID" -- sh -c ' | |
| for dir in /home /opt /srv /root; do # vm에 folder 있는지 확인 | |
| if [ -d "$dir" ]; then | |
| paths="$paths $dir" | |
| fi | |
| done | |
| # find의 에러는 out-data가 아닌 err-data로 분리되므로 JSON 파싱에 영향 없음. 단, exitcode는 0이 아니게 되므로 없는 경로는 위 -d 검사로 미리 걸러냄 | |
| find $paths -name "server.properties" -type f # vm에 있는 folder만 조회') | |
| result=$(qm guest exec "$VMID" -- sh -c ' | |
| paths="" | |
| for dir in /home /opt /srv /root; do | |
| if [ -d "$dir" ]; then | |
| paths="$paths $dir" | |
| fi | |
| done | |
| if [ -n "$paths" ]; then | |
| find $paths -name "server.properties" -type f | |
| else | |
| echo "" | |
| fi') |
There was a problem hiding this comment.
반영했습니다 (85bd871). paths="" 초기화 + [ -n "$paths" ] 가드 추가, 루프 밖 무가드 find 제거했습니다.
| exitcode=$(echo "$result" | jq -r '.exitcode') # exitcode 검사 후 0 이면, path 검사 진행하는 로직 | ||
|
|
||
| if [ "$exitcode" -ne 0 ]; then | ||
| echo "검사 실패 (exitcode: $exitcode)" | ||
| exit | ||
| fi |
There was a problem hiding this comment.
개선 사항
- JSON 파싱 에러 방지:
qm guest exec가 실패하거나 비정상적인 출력을 반환할 때jq파싱 에러가 stderr로 출력되는 것을 방지하기 위해2>/dev/null을 추가합니다. - 문자열 비교 사용:
exitcode가 비어있거나"null"인 경우[ "$exitcode" -ne 0 ]비교 시integer expression expected와 같은 Bash 문법 에러가 발생합니다.[ "$exitcode" != "0" ]문자열 비교를 사용하면 이러한 에러를 완벽히 방지할 수 있습니다. - 재시도 루프 유지:
exitcode가 0이 아닐 때exit로 스크립트를 즉시 종료하면, 일시적인 에러(예: 에이전트 기동 중 일시적 지연) 시 남은 재시도 횟수(MAX_RETRIES)를 활용하지 못하고 즉시 실패하게 됩니다.sleep후continue를 사용하여 다음 루프에서 재시도할 수 있도록 개선합니다.
| exitcode=$(echo "$result" | jq -r '.exitcode') # exitcode 검사 후 0 이면, path 검사 진행하는 로직 | |
| if [ "$exitcode" -ne 0 ]; then | |
| echo "검사 실패 (exitcode: $exitcode)" | |
| exit | |
| fi | |
| exitcode=$(echo "$result" | jq -r '.exitcode' 2>/dev/null) | |
| if [ "$exitcode" != "0" ]; then | |
| echo "검사 실패 (exitcode: ${exitcode:-unknown})" | |
| sleep "$SLEEP_SEC" | |
| continue | |
| fi |
There was a problem hiding this comment.
문자열 비교([ "$exitcode" != "0" ])로 반영했습니다 (85bd871). 다만 검사 실패 시 continue 재시도 대신 exit 0으로 처리했습니다 — post-start 훅 실패가 VM 기동에 영향 주지 않도록 하는 의도된 설계입니다.
| if [ "$success" = false ]; then | ||
| echo "agent 부팅 실패" | ||
| exit | ||
| fi |
There was a problem hiding this comment.
There was a problem hiding this comment.
의도적으로 exit 0을 유지합니다. post-start 훅이 non-zero로 종료하면 Proxmox 태스크가 실패로 표시되지만 VM은 정상 동작하므로, "탐지 실패"가 "VM 기동 실패"처럼 보이지 않게 exit 0으로 통일했습니다. 주석에도 근거를 남겼습니다.
Closes #149
a67be28 to
196ec69
Compare
개요
Proxmox VM에서 minecraft server를 탐지하는 hookscript.
post-start 시 자동 실행되어 server.properties 존재를 검사.
동작 방식
의존성
남은 작업 (draft 사유)
Closes #149