Skip to content

fix orion ci from vms#1978

Merged
benjamin-747 merged 1 commit into
gitmono-dev:mainfrom
Ivanbeethoven:main
Mar 2, 2026
Merged

fix orion ci from vms#1978
benjamin-747 merged 1 commit into
gitmono-dev:mainfrom
Ivanbeethoven:main

Conversation

@Ivanbeethoven

Copy link
Copy Markdown
Collaborator

No description provided.

Signed-off-by: Luxian <lux1an@qq.com>
@github-actions

github-actions Bot commented Mar 2, 2026

Copy link
Copy Markdown

Claude finished @Ivanbeethoven's task in 1m 28s —— View job


Code Review

  • Gather context and understand the PR
  • Read changed files
  • Analyze CI workflow changes
  • Analyze documentation changes
  • Provide comprehensive code review

Summary

This PR re-enables and improves the legacy Orion CI deployment to VMs, and translates the deployment documentation from Chinese to English. The changes are generally positive, though there are a few issues worth noting.


.github/workflows/orion-client-deploy.yml

Positive Changes

  • Re-enabled deploy-legacy job with proper repository guard (github.repository == 'web3infra-foundation/mega') instead of hard-coded if: false.
  • Added FUSE unmount to legacy deploy stop step — previously this was only in the GCP job; now both jobs clean up FUSE mounts before stopping.
  • Added command_timeout: 30s to SSH steps to prevent hanging jobs.
  • Async service start via nohup avoids SSH timeout on slow starts.
  • Systemd service file update logic is now consistent between legacy and GCP jobs.

Issues

1. Potential deployment failure: Missing orion user on legacy VM (Medium risk)

The remote path was changed from /root/orion-runner/ to /home/orion/orion-runner/ while still connecting as root. Since the legacy VM was originally provisioned for root-only access, the orion user and /home/orion/orion-runner/ directory may not exist on it. If rsync tries to write to a nonexistent path, deployment will fail silently or error.

# .github/workflows/orion-client-deploy.yml
remote_path: /home/orion/orion-runner/   # Was /root/orion-runner/
remote_user: root                         # Still root

Suggest adding a pre-deployment step to ensure the path exists:

mkdir -p /home/orion/orion-runner/

Or use the Stop service on orion_vm step to create it.

2. Security: strict_hostkeys_checking: no (Low-Medium risk)

Both rsync deployments now disable strict host key checking:

strict_hostkeys_checking: no

This disables MITM protection. For a CI environment where hosts are known, a better practice would be to store the host key fingerprint as a GitHub secret and validate it. This is a commonly accepted trade-off in CI though, so this may be intentional.

3. No verification of async service start (Low risk)

The legacy job switched from synchronous start with status verification to fire-and-forget nohup:

# Before (verified)
systemctl start orion-runner.service
systemctl status orion-runner.service --no-pager

# After (no verification)
nohup systemctl start orion-runner.service > /tmp/orion-start.log 2>&1 &
echo "✓ Service start command issued (async)"

Consider adding a brief sleep 5 && systemctl is-active orion-runner.service || cat /tmp/orion-start.log to detect immediate failures. Fix this →

4. pkill -9 orion without sudo on legacy VM (Very low risk)

In the stop step for the legacy VM, pkill -9 orion runs as root, which is fine. However, this is different from the GCP job (sudo pkill -9 orion). The inconsistency is harmless but worth noting for clarity.

5. Trailing whitespace on line 193

The GCP prepare step retains a trailing space on the echo separator line. Minor but may cause linting noise.


orion/docs/deployment.md

Positive Changes

  • Full translation to English — makes the documentation accessible to a wider audience and consistent with the codebase.
  • Added "VM Initialization" section with clear, step-by-step setup instructions including FUSE config, passwordless sudo, and SSH limits.
  • Added "Troubleshooting" section with common issues and fixes.
  • Added deployment secrets reference table for clarity.

Issues

1. Minor documentation inaccuracy

The CI/CD pipeline description says "SCP transfer" but the workflow actually uses rsync:

4. Uploads files using `scp` to the `/home/orion/orion-runner/` directory.

Should be rsync. Fix this →

2. Missing tmp_build directory in VM initialization

The directory structure section references /data/scorpio/tmp_build/ but the VM initialization mkdir command doesn't create it:

sudo mkdir -p /data/scorpio/{store,antares/{upper,cl,mnt}}
# Missing: /data/scorpio/tmp_build

Overall Assessment

The core CI improvements are solid: FUSE cleanup is now consistent between both jobs, timeout handling is improved, and the async service start is a reasonable fix for SSH timeout issues. The primary concern is whether the /home/orion/orion-runner/ path exists on the legacy VM — this could cause a regression. I'd recommend verifying this against the actual legacy VM state before merging.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cb39db243e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".


echo "✓ Service start command completed"
systemctl status orion-runner.service --no-pager
nohup systemctl start orion-runner.service > /tmp/orion-start.log 2>&1 &

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Wait for systemd start result before finishing deployment

Both deploy jobs now start orion-runner.service with nohup ... &, which returns success as soon as the command is queued, not when the service has actually started; this means the workflow can report a green deploy even if systemctl start immediately fails (e.g., bad unit file, permission issue, missing config), so production failures become silent.

Useful? React with 👍 / 👎.

Comment on lines 135 to 139
switches: -avz --progress
path: artifacts/
remote_path: /root/orion-runner/
remote_path: /home/orion/orion-runner/
remote_host: ${{ secrets.ORION_DEPLOY_HOST }}
remote_user: root

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Prevent root rsync from breaking orion-owned runtime files

In deploy-legacy, rsync runs as root with -a (--archive), and rsync --help states archive mode includes -o (--owner, super-user only), so uploaded files in /home/orion/orion-runner/ can end up owned by a non-orion UID; since the service runs as orion and run.sh does chmod +x ./orion, startup can fail with permission errors when ownership does not match.

Useful? React with 👍 / 👎.

@benjamin-747 benjamin-747 added this pull request to the merge queue Mar 2, 2026
Merged via the queue into gitmono-dev:main with commit 1ee3d9d Mar 2, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants