Fix release bundle check to detect CLI-only installs#759
Merged
phinze merged 2 commits intoApr 14, 2026
Merged
Conversation
The bash installer (api.miren.cloud/install) drops a CLI-only miren binary at /var/lib/miren/release/miren. Both server install and runner install used that path as their "is the release bundle present?" sentinel, so they'd see the binary and skip the download. The runner would then crash-loop on "containerd binary not found" because the full bundle (containerd, runc, shims) was never fetched. Switch the sentinel to the containerd binary, which only exists in the full release tarball. Also consolidate the duplicated check-download-selinux block into a shared ensureReleaseBundlePresent helper and pull the scattered path literals into package-level consts.
📝 WalkthroughWalkthroughThe changes refactor release-bundle provisioning into a new helper 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/commands/server_install.go`:
- Around line 226-231: The call to PerformDownloadRelease passes
DownloadReleaseOptions with Force: false which prevents replacing an existing
partial CLI-only release directory, causing full bundle downloads to fail;
change the options at the PerformDownloadRelease call site to set Force: true
(i.e., update the DownloadReleaseOptions used by PerformDownloadRelease in
server_install.go) so the existing releaseDir can be overwritten after detecting
the missing containerd sentinel, allowing the full bundle to be downloaded and
installed.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 64662d4f-5649-400f-b60a-ed412110c084
📒 Files selected for processing (2)
cli/commands/runner_install.gocli/commands/server_install.go
When the CLI-only installer has already run, the release directory exists (containing just the miren binary). PerformDownloadRelease with Force: false would bail with "release directory already exists", so the full bundle never gets fetched. Force: true is safe here because we've already confirmed the bundle is incomplete via the containerd sentinel check.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/commands/server_install.go`:
- Around line 220-237: The SELinux fix is only applied when the release is
downloaded; ensure fixSELinuxContext(releaseBinPath) runs for existing bundles
too by moving or adding the call outside the download-only branch so it executes
regardless of whether PerformDownloadRelease was invoked; update the block that
checks sentinelPath (referencing sentinelPath, PerformDownloadRelease, and
releaseDir) to always call fixSELinuxContext(releaseBinPath) after the existence
check (and after any download path completes) so systemd can exec the binary on
enforcing SELinux hosts.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c699e652-856d-4b6b-b93a-dff4bff6746c
📒 Files selected for processing (1)
cli/commands/server_install.go
Comment on lines
+220
to
+237
| if _, err := os.Stat(sentinelPath); err != nil { | ||
| if !os.IsNotExist(err) { | ||
| return fmt.Errorf("failed to inspect %s: %w", sentinelPath, err) | ||
| } | ||
| ctx.Info("Release bundle not found at %s, downloading...", releaseDir) | ||
|
|
||
| if err := PerformDownloadRelease(ctx, DownloadReleaseOptions{ | ||
| Branch: branch, | ||
| Global: true, | ||
| Force: true, | ||
| Output: releaseDir, | ||
| }); err != nil { | ||
| return fmt.Errorf("failed to download release: %w", err) | ||
| } | ||
|
|
||
| ctx.Completed("Release downloaded successfully") | ||
| fixSELinuxContext(ctx, releaseBinPath) | ||
| } |
There was a problem hiding this comment.
Always re-apply the SELinux context for an existing release bundle.
Line 236 only runs fixSELinuxContext after a fresh download. If the full bundle is already present before miren server install runs, the containerd sentinel passes and this path now skips the context fix entirely, so systemd can still fail to exec /var/lib/miren/release/miren on enforcing SELinux hosts.
🔧 Proposed fix
func ensureReleaseBundlePresent(ctx *Context, branch string) error {
sentinelPath := filepath.Join(releaseDir, "containerd")
if _, err := os.Stat(sentinelPath); err != nil {
if !os.IsNotExist(err) {
return fmt.Errorf("failed to inspect %s: %w", sentinelPath, err)
}
ctx.Info("Release bundle not found at %s, downloading...", releaseDir)
if err := PerformDownloadRelease(ctx, DownloadReleaseOptions{
Branch: branch,
Global: true,
Force: true,
Output: releaseDir,
}); err != nil {
return fmt.Errorf("failed to download release: %w", err)
}
ctx.Completed("Release downloaded successfully")
- fixSELinuxContext(ctx, releaseBinPath)
}
+ fixSELinuxContext(ctx, releaseBinPath)
return nil
}Based on learnings: full release artifacts are handled through cli/commands/download_release.go, separate from binary-only installs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cli/commands/server_install.go` around lines 220 - 237, The SELinux fix is
only applied when the release is downloaded; ensure
fixSELinuxContext(releaseBinPath) runs for existing bundles too by moving or
adding the call outside the download-only branch so it executes regardless of
whether PerformDownloadRelease was invoked; update the block that checks
sentinelPath (referencing sentinelPath, PerformDownloadRelease, and releaseDir)
to always call fixSELinuxContext(releaseBinPath) after the existence check (and
after any download path completes) so systemd can exec the binary on enforcing
SELinux hosts.
evanphx
approved these changes
Apr 14, 2026
phinze
deleted the
phinze/mir-1008-runner-install-skips-release-bundle-download-when-cli-only
branch
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
2 participants
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.
Found this on
miren-toys-runner-2during MIR-965 validation. When the startup script runs the bash installer first (to get the CLI on the box) and thenmiren runner install, the install sees/var/lib/miren/release/mirenand thinks the full release bundle is already there. It's not, containerd and friends are missing, so the runner crash-loops.The fix swaps the sentinel from the
mirenbinary (which the CLI-only installer also drops) to thecontainerdbinary (which only comes from the full release tarball). While I was in there,runner_install.goandserver_install.gohad near-identical copies of this check-download-selinux block, so I pulled it into a sharedensureReleaseBundlePresent()helper and promoted the hardcoded path strings to package-level consts.Closes MIR-1008