Skip to content

Rely on waitready service and LXD events to wait for workshop start - #1018

Merged
dmitry-lyfar merged 4 commits into
mainfrom
refactor/waitready
Sep 3, 2026
Merged

Rely on waitready service and LXD events to wait for workshop start#1018
dmitry-lyfar merged 4 commits into
mainfrom
refactor/waitready

Conversation

@jonathan-conder

Copy link
Copy Markdown
Contributor

Description

Splits out the workshop start logic from #911. The only change there is the cloud-config hashes (from rebasing on main).

Also:

  • Fixes the currently-inert attempt to expand spread VM root disks. Recently spread was updated to set no size here, which results in Workshop choosing the minimum of 5GiB. 32GiB makes it more like 6GiB; we can increase this to gigantic numbers if needed because it's lazily allocated.
  • Improves consistency of WorkshopManager.Workshop and WorkshopManager.Workshops. This came from an attempt to call State.Warnf whenever we load an old workshop. I didn't end up taking that route, so the State doesn't currently need to be locked when calling these, but it's useful to keep that requirement since more warnings are likely to appear here.
  • Adds a hard error when trying to manually start an old workshop. We recommend removing the workshop and launching it again, since refresh only works for workshops that are already running. But lxc start should also work fine, it just doesn't wait long enough.

Self-review quick check

  • Make decisions that cost a lot to reverse explicit in the PR description.
  • Avoid nested conditions.
  • Delete dead code and redundant comments.
  • Normalise symmetries by sticking to doing identical things identically.
// one way to handle errors
if err := f(); err != nil {
   ...
}

// one way to handle multiple returns
val, err := f()
if err != nil {
   ...
}
...
  • Check that coupled code elements, files, and directories are adjacent. For example, test data is stored as close as possible to a test.
  • Put variable declaration and initialisation together.
  • Divide large expressions into digestable and self-explanatory ones. Use multiple variables if required.
  • Put a blank line between two logically different chunks of code.
  • Follow the style guide for new error messages.

Docs

Procedure:

  • I have checked and added or updated relevant documentation.
  • I have checked and added or updated relevant release notes.
  • I have included the technical author in the review.

Content:

  • Headings and titles accurately describe the content.
  • New and updated pages include correct metadata.
  • Documentation tests are added or updated where applicable (for tutorial/ and how-to/ sections).
  • Documentation follows the style guide.
  • If needed, docs/.coverage.yaml updated, coverage tags added (.. artefact).

Or:

  • I confirm the PR has no implications for documentation.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The CI storage command targets a profile Workshop instances do not inherit, and the legacy-format rejection lacks coverage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Reworks workshop startup to use the waitready service and LXD lifecycle events.

Changes:

  • Waits for LXD Ready events and cleans up failed starts.
  • Rejects manual starts of legacy-format workshops.
  • Updates integration infrastructure and attempts to enlarge CI disks.
File summaries
File Description
.github/workflows/spread.yaml Changes the CI disk-sizing command.
internal/daemon/api_connections.go Locks state around workshop lookup.
internal/interfaces/lxd_device/tests/integration/backend_test.go Enables waitready in integration tests.
internal/overlord/workshopstate/manager.go Clarifies locking requirements.
internal/overlord/workshopstate/request.go Rejects legacy starts and centralizes lookup.
internal/waitready/waitready.go Makes timeout test-configurable.
internal/workshop/lxd/lxd_backend.go Implements event-driven startup readiness.
internal/workshop/lxd/start_command.sh Removes the obsolete startup script.
internal/workshop/lxd/tests/helper/helper.go Adds test-binary waitready support.
internal/workshop/lxd/tests/integration/project_test.go Configures integration TestMain.
internal/workshop/lxd/tests/integration/snapshot-format.yaml Updates cloud-config hashes.
internal/workshop/lxd/tests/integration/snapshot_test.go Uses the mounted test executable.
internal/workshop/lxd/tests/integration/workshop_exec_test.go Uses shared test devices.
internal/workshop/lxd/tests/integration/workshop_test.go Tests readiness timeout cleanup.
tests/main/start/task.yaml Removes redundant LXD readiness polling.
Review details
  • Files reviewed: 15/15 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/spread.yaml
Comment thread internal/overlord/workshopstate/request.go
@dmitry-lyfar
dmitry-lyfar merged commit ccea6a1 into main Sep 3, 2026
26 checks passed
@dmitry-lyfar
dmitry-lyfar deleted the refactor/waitready branch September 3, 2026 22:47
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.

3 participants