Skip to content

feat: add imperative --image source to kuke create cell (step 2) - #1305

Merged
eminwux merged 2 commits into
mainfrom
feat/create-cell-image
Jun 14, 2026
Merged

feat: add imperative --image source to kuke create cell (step 2)#1305
eminwux merged 2 commits into
mainfrom
feat/create-cell-image

Conversation

@eminwux

@eminwux eminwux commented Jun 14, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds --image <ref> (plus --command <cmd>) as a fourth source to kuke create cell, making the install onboarding command kuke create cell my-first --image docker.io/library/alpine:3 work (it previously errored unknown flag: --image).
  • Reuses the step-1 synthesis helper cell.SynthesizeFromImage (cmd/kuke/create/cell/image.go, cli: add imperative --image flag to kuke run (step 1) #1244/feat: add imperative --image source to kuke run (step 1) #1287) — no re-implementation. The create-side path persists the cell stopped via MaterializeCell (the create verb's materialise-without-start contract, epic:create planning: kuke create — kind coverage symmetry + scaffolding for complex kinds #814), where kuke run --image create+start+attaches via CreateCell.
  • --image is mutually exclusive with --from-blueprint/--from-config/--clone (cobra MarkFlagsMutuallyExclusive); the binding render-time/override knobs --param/--param-file/--env and --command-without---image are rejected with clear errors (parity with kuke run's flag-compat checks).
  • Rewrites the onboarding success block in scripts/install.sh + its docs/site/install.sh mirror to a verified, copy-pasteable sequence (create → list → start, plus the kuke run --image one-shot alternative).
  • Updates docs that enumerate the cell sources: docs/site/cli/kuke-create.md (sources/name-rule/flag-table/example) and docs/cli-use-cases.md. The no-source error message now names --image; its test assertion and the docs/cli-use-cases.md reference were swept to match.

Bundled daemon fix (#1306)

Testing surfaced that a freshly materialised cell, followed by kuke run <cell>, failed with cell "x" is recorded Ready but its containers are gone from containerd. Root cause: provisionNewCell (internal/controller/runner/provision.go) unconditionally calls markCellReady, so every MaterializeCell-materialised cell was persisted Ready despite never being started — kuke run then read that bogus Ready, found no live root task, and refused with a spurious divergence error pointing at kuke delete cell. Pre-existing and source-independent (affected --from-blueprint/--from-config/--clone too); the --image flag merely exposed it, and it directly undermined this PR's AC#1 ("leaves it Stopped").

Fix (internal/controller/create_cell.go): on the MaterializeCell (don't-start) path for a freshly-created cell, persist Status.State = Stopped (mirroring runner.StopCell, leaving ReadyObserved latched) so kuke run <cell> reaches the Stopped→start branch and kuke get cells reports it Stopped. Scoped to wasCreated so an existing cell's live state is never clobbered. The CreateCell (start) path is untouched.

Notes for reviewer

  • --command flag added alongside --image (not strictly enumerated by the AC): the shared helper SynthesizeFromImage(image, command) already takes a command and kuke run --image exposes --command, so adding it here keeps the two verbs symmetric. Low blast radius; rejected when --image is absent.
  • No-source error message changed from requires --from-blueprint, --from-config, or --clone to requires --image, --from-blueprint, --from-config, or --clone for accuracy now that --image is a valid source. This touched one unit-test assertion and one docs/cli-use-cases.md line (both updated in this PR; git grep confirms no other references).
  • Bug fix bundled deliberately (at the user's direction): bug: kuke create cell materialises the cell as Ready (not Stopped), breaking kuke run <cell> #1306 is pre-existing and in the controller subsystem, but it directly breaks this feature's intended create→run workflow and AC#1, so it ships here rather than as a separate PR.

Test plan

  • make test (full CI target, GOWORK=off) — passes (exit 0), including the new controller test.
  • Unit tests in cmd/kuke/create/cell/cell_test.go: TestCreateCell_FromImage_HappyPath (asserts MaterializeCell called with the single synthesized attachable container + "containers: not started" → AC#1 stopped-persist), _GeneratedName, _SourceMutex (AC#2), _RejectsBindingKnobs, and TestCreateCell_CommandRejectedWithoutImage. Synthesis itself is covered by the existing image_test.go (AC#3/AC#5).
  • Controller test internal/controller/materialize_cell_test.go: TestMaterializeCell_NewCell_SkipsStart now asserts the materialised cell persists Stopped (not Ready) via UpdateCellMetadata; _ExistingCell_SkipsStart asserts an existing cell's state is left untouched (bug: kuke create cell materialises the cell as Ready (not Stopped), breaking kuke run <cell> #1306 regression guards).
  • Real kuke binary (make kuke) daemon-free checks: --help shows --image/--command; source mutex rejects --image+--from-blueprint and --image+--clone; --command without --image rejected; no-source error names --image.
  • make dev-init smoke / daemon-parity tail — not run here: the kuke build (BuildKit) phase fails with no space left on device (/var/lib/containerd is a 4 GB tmpfs at 97% on this host). CLI + controller behavior is covered by the unit/controller tests above; recommend the reviewer run the live smoke on a host with adequate containerd storage.

Closes #1245
Closes #1306

@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 Jun 14, 2026
@eminwux

eminwux commented Jun 14, 2026

Copy link
Copy Markdown
Owner Author

PR #1305 Review — feat: add imperative --image source to kuke create cell (step 2)

LGTM — adds --image/--command as a fourth kuke create cell source by reusing the existing SynthesizeFromImage helper and persisting stopped via MaterializeCell (the create-verb contract), correctly mirroring the kuke run --image sibling without re-implementing or fragmenting the flag plumbing. Scope resolution, name finalization, source mutex, and the --command/binding-knob rejections all match precedent; new symbols are wired, old error string fully swept, install.sh mirror stays byte-identical, single signed commit, CI green.

Deferred for you to run pre-merge: make dev-init smoke / daemon-parity tail — the author documented it as host-blocked (containerd tmpfs at 97%), and it's a CLI-only change with no daemon/build-path//opt/kukeon surface, so it's non-blocking here.

@eminwux eminwux added ready-to-merge Reviewed and ready to merge and removed in-review Reviewer agent is actively reading this PR labels Jun 14, 2026
@eminwux

eminwux commented Jun 14, 2026

Copy link
Copy Markdown
Owner Author

Heads-up for review: testing surfaced that a freshly kuke create cell --image'd cell, when followed by kuke run <cell>, fails with cell "x" is recorded Ready but its containers are gone from containerd.

I traced this to a pre-existing daemon bug independent of this PR: provisionNewCell (internal/controller/runner/provision.go:1141) unconditionally calls markCellReady, so every MaterializeCell-materialised cell (--image/--from-blueprint/--from-config/--clone) is persisted Ready despite never being started. This PR correctly routes --image through MaterializeCell exactly like the existing sources — it just exposes the bug via a new path. kuke start <cell> (the documented pairing, and what the onboarding block here uses) works fine; only kuke run <existing-cell> trips the Ready-divergence guard.

Filed as #1306 (bug, priority:B) with the full root-cause trace and fix proposal. This PR is unaffected — the daemon fix lands separately.

@eminwux

eminwux commented Jun 14, 2026

Copy link
Copy Markdown
Owner Author

Update: per maintainer direction, the #1306 daemon fix is now bundled into this PR (commit 6894027) rather than landing separately — it directly breaks this feature's create→run workflow and AC#1 ("leaves it Stopped"). #1306 is closed as folded-in. See the updated PR body's Bundled daemon fix section.

@eminwux eminwux added ready-for-review PR is handed off to the reviewer agent and removed ready-to-merge Reviewed and ready to merge labels Jun 14, 2026
@eminwux

eminwux commented Jun 14, 2026

Copy link
Copy Markdown
Owner Author

⚠️ Re-review needed: this PR was already marked ready-to-merge, but I've since pushed a new substantive commit (6894027) bundling the #1306 daemon fix (materialise path now persists Stopped in the controller, with regression tests) at the maintainer's direction. The prior approval predates this commit, so I've moved it back to ready-for-review. The new commit touches internal/controller/create_cell.go (behavioral) + materialize_cell_test.go — please re-review those. make test is green.

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

eminwux commented Jun 14, 2026

Copy link
Copy Markdown
Owner Author

PR #1305 Review — feat: add imperative --image source to kuke create cell (step 2)

LGTM — adds --image/--command to kuke create cell by reusing the shared SynthesizeFromImage helper (no re-implementation), with source-mutex and binding-knob rejection mirroring kuke run, and bundles a sound fix for #1306 (provisionNewCell unconditionally marks new cells Ready at provision.go:1141; the new else if wasCreated branch in create_cell.go overrides to Stopped via UpdateCellMetadata, mirroring StopCell's State-without-ReadyObserved-clear and correctly scoped to wasCreated). Tests cover the CLI paths and both #1306 controller branches; no dead code, sentinel reused from errdefs, docs/install.sh kept in sync (CI sync check green).

The unchecked make dev-init smoke is host-gated (BuildKit no space left on device on the sandbox's tmpfs containerd) — non-blocking per the documented carve-out; CI's make e2e + full-install + --check smokes passed. Worth running the live make dev-init daemon-parity tail on a host with adequate containerd storage before merge, since this touches the controller create path.

@eminwux eminwux added ready-to-merge Reviewed and ready to merge and removed in-review Reviewer agent is actively reading this PR labels Jun 14, 2026
@eminwux
eminwux merged commit 838e600 into main Jun 14, 2026
9 checks passed
@eminwux
eminwux deleted the feat/create-cell-image branch June 14, 2026 21:26
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

1 participant