Skip to content

refactor(ctr): relocate root container pause target to /.kukeon/bin/kukepause - #980

Merged
eminwux merged 2 commits into
mainfrom
refactor/pause-target-path
May 31, 2026
Merged

refactor(ctr): relocate root container pause target to /.kukeon/bin/kukepause#980
eminwux merged 2 commits into
mainfrom
refactor/pause-target-path

Conversation

@eminwux

@eminwux eminwux commented May 31, 2026

Copy link
Copy Markdown
Owner

Summary

  • Relocates the in-container root (pause) bind-mount target from the top-level /pause to the kukeon-namespaced /.kukeon/bin/kukepause (issue refactor(ctr): relocate root container pause binary target from /pause to /.kukeon/bin/kukepause #979).
  • Single functional edit: the RootContainerPauseBinaryTarget constant in internal/ctr/container_utils.go. Both the OCI bind-mount Target and the root container's Command derive from it, so they move together; tests assert against the constant (not the literal), so they follow automatically.
  • Remaining diff is doc-comment hygiene updating stale /pause references in cmd/kuke/init/init.go, cmd/kukepause/main.go, internal/controller/runner/kukepause.go, internal/controller/runner/provision.go, internal/ctr/container_utils.go, and internal/ctr/container_utils_test.go (comments only). The bind source basename (RootContainerPauseBinaryName="kukepause", staged at <RunPath>/bin/kukepause) is decoupled and unchanged, per the issue's scope analysis.

Recovery note

Recovered from a prior session that crashed after implementing the change but before committing (left uncommitted on main, branch unpushed). The operator confirmed no other agent was live before recovery. The diff matches the issue's stated scope exactly (6 files, +21/-19).

Test plan

  • go build ./...
  • go vet ./...
  • make test (root-module unit suite incl. internal/ctr, internal/controller/runner, all cmd/*, cmd/kukepause, plus the kukebuild module) — green with a short TMPDIR. The default long /tmp/claude-* path otherwise trips an unrelated SUN_PATH > 107 limit in attachable_test.go (a known host-env constraint, not a regression from this change).
  • Tree-wide sweep for bare /pause literals outside kukepause paths — none remain.
  • make e2e — not run: the e2e suite needs the docker-built local kukeond image + containerd harness, unavailable in this dev session. The e2e cases that exercise cell create/start/attach are the runtime path that confirms the nested mount; deferred to the reviewer's clean host.
  • make dev-init smoke parity + nested-mount runtime checks (AC: default-root cell starts with kukepause as PID 1; nested /.kukeon/bin/kukepause target auto-created on stock busybox and on a ReadOnlyRootFilesystem root container) — not run: make dev-init is a destructive smoke that resets the live kukeond cell, and this session's tool-output observability was degraded; running it blind risked leaving the host daemon indeterminate. runc auto-creating the bind destination's parent dirs is the one path the unit tests don't cover and should be confirmed on a clean host.

Closes #979

@eminwux eminwux added ready-for-review PR is handed off to the reviewer agent in-review Reviewer agent is actively reading this PR and removed ready-for-review PR is handed off to the reviewer agent labels May 31, 2026
@eminwux

eminwux commented May 31, 2026

Copy link
Copy Markdown
Owner Author

Review: refactor(ctr): relocate root container pause target to /.kukeon/bin/kukepause

Verdict: LGTM ✅

Summary

Minimal, correctly scoped change. The single functional edit is the RootContainerPauseBinaryTarget constant in internal/ctr/container_utils.go; all callers consume the constant rather than string literals, so both the OCI bind-mount Target and the root container Command move atomically. No hardcoded /pause literals remain anywhere in the tree (.go, .yaml, .yml, .json, .toml). Tests assert against the constant, so they track the new value without edits.

Correctness

  • Constant migration complete. git grep '"/pause"' returns zero matches on the PR branch. All usages of RootContainerPauseBinaryTarget (container_utils.go:79 bind-mount target, :93 Command, and test assertions at :125, :146, :170, :187) resolve to the new "/.kukeon/bin/kukepause" value.
  • Bind source unchanged. RootContainerPauseBinaryName = "kukepause" and all its callers are untouched — correct per the issue's scope analysis.
  • Runc parent-directory concern addressed by CI. The PR body flags that runc auto-creating /.kukeon/bin/ inside the container rootfs is the one path unit tests don't cover. make e2e passed in CI, and that suite's startKukeondDaemon path exercises actual cell create/start with a real OCI bind mount at the new destination — which confirms runc handles the nested path on the CI host. The make dev-init smoke (ReadOnlyRootFilesystem variant) is in the test plan and can be validated on a clean host post-merge if the parity check shows any divergence.
  • Doc-comment hygiene. All six comment-only hunks update stale /pause references consistently; no substantive logic touched.

Commit signature

7fc6902 — SSH-signed, reason: valid, verified: true.

Non-blocking nit

container_utils_test.go:192: t.Fatalf("no mount for kukepause binary found") — reads slightly awkward ("for … found"). Pre-existing from the original /pause era; no change needed here.


Reviewed by pr-reviewer agent.

@eminwux eminwux added ready-to-merge Reviewed and ready to merge and removed in-review Reviewer agent is actively reading this PR labels May 31, 2026
@eminwux
eminwux merged commit f6f5955 into main May 31, 2026
9 checks passed
@eminwux
eminwux deleted the refactor/pause-target-path branch May 31, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Reviewed and ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(ctr): relocate root container pause binary target from /pause to /.kukeon/bin/kukepause

1 participant