Skip to content

Update base image before launching or rebuilding a workshop#491

Merged
dmitry-lyfar merged 4 commits into
mainfrom
feature/update-base
Sep 22, 2025
Merged

Update base image before launching or rebuilding a workshop#491
dmitry-lyfar merged 4 commits into
mainfrom
feature/update-base

Conversation

@jonathan-conder
Copy link
Copy Markdown
Contributor

@jonathan-conder jonathan-conder commented Sep 18, 2025

Description

When launching a new workshop, or rebuilding one from an image (i.e. when refreshing after changing the base), Workshop will now attempt to download the latest image for the given base. This PR does not affect refreshes which restore from SDK snapshots (but this is planned as the next step).

To be safe I would probably clean up all local images before migrating to this version of Workshop. It will probably break wait-on-error launches and some refreshes.

There are some known issues with managing LXD images, most notably canonical/lxd#16515. I don't know of a viable workaround, but we haven't observed it in the wild yet, and I don't think this PR does anything to make it worse. It only triggers when another LXD project uses the same images as us. On my machine the other projects are snapcraft and sdkcraft, and these both use a different image server (buildd).

The way image downloads work is:

  1. WorkshopManager creates a download-base task if there's no SDK snapshot to restore from.
  2. WorkshopManager passes the ID of that task to create-workshop.
  3. The download-base task downloads the latest image and saves its fingerprint in the task data.
  4. The create-workshop task obtains the fingerprint from the download-base task data and uses it to launch or rebuild a workshop.

In download-base, we query the image server before starting the download. This query is mostly redundant, we only use it to obtain the image size (and, for LXD servers, whether the image is public). If the image is updated before starting the download, the size might be inaccurate. We should fix this by adding numbers to LXD's progress reporting.

A workaround would be to use the fingerprint from the query for the download. This seems to be safe, since LXD's image servers seem to keep at least 2 images per alias at all times. However, the resulting local copy of the image won't have any provenance information. I don't think it's worth losing this in order to remove a tiny chance of inaccurate progress reporting.

Last thing I'll mention is that we now use aliases like 24.04 rather than 24.04/amd64. This should be more reliable, see comments in code for more details.

See #492 for the included bug fixes.

Fixes some minor style issues, and adds support for more image download progress string formats.

From what I've seen in the documentation, I think it already describes the state of affairs we're aiming for with this series of PRs.

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

  • 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.

Or:

  • I confirm the PR has no implications for documentation.

@jonathan-conder jonathan-conder self-assigned this Sep 18, 2025
@jonathan-conder jonathan-conder marked this pull request as ready for review September 19, 2025 05:04
@jonathan-conder jonathan-conder marked this pull request as draft September 19, 2025 05:04
@jonathan-conder jonathan-conder marked this pull request as ready for review September 19, 2025 05:45
Comment thread internal/workshop/backend.go
Comment thread internal/workshop/backend.go
Comment thread internal/overlord/workshopstate/handlers.go
@github-actions
Copy link
Copy Markdown

TICS Quality Gate

✔️ Passed

workshop

All conditions passed

See the results in the TICS Viewer

The following files have been checked for this project
  • internal/overlord/sdkstate/handlers.go
  • internal/overlord/workshopstate/handlers.go
  • internal/overlord/workshopstate/request.go
  • internal/workshop/backend.go
  • internal/workshop/fakebackend/backend.go
  • internal/workshop/lxd/lxd_backend.go
  • internal/workshop/lxd/lxd_base_manager.go

Automatic-tests / code-coverage / TICS GitHub Action

@dmitry-lyfar dmitry-lyfar merged commit b4d633d into main Sep 22, 2025
11 checks passed
@dmitry-lyfar dmitry-lyfar deleted the feature/update-base branch September 22, 2025 04:54
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