Incorporate base image in refresh plan

[jonathan-conder]

Description

Refresh now queries the image server at the same time as the Store. If the base image is outdated, the workshop is rebuilt from scratch based on the new image. This is the same behaviour as switching to a different base. If the current base is up to date, the workshop will likely be restored from a snapshot.

There's no cleanup logic for images at present. They will start to pile up over time, but the snap's remove hook should keep this under control for now.

Implementation

I added a new method GetBase to BaseImageManager. It looks up the latest version of a given base. For the LXD backend, the version is a fingerprint, which is likely to be globally unique, but the API only assumes the version string is unique within a given base.

---

I also renamed Download to DownloadBase. It now accepts a version instead of returning one.

Originally I wanted DownloadBase to always download the latest image, for two reasons:

1. The fingerprint might expire between planning a refresh and downloading the image.
2. LXD includes an UpdateSource field in the local copy when downloading the latest image.

I don't think the first one is a major issue. Even the daily image servers keep at least 2 fingerprints alive at all times. So the fingerprint used in a refresh plan should be valid for at least 1 day.

The second one is not an issue for LXD, because we disable auto-updates for our images. But the UpdateSource field would have been convenient when cleaning up old images, and is currently useful for the image download integration tests.

I didn't want to continue using aliases, since the previous implementation had some concurrency issues and I don't know if they've been fixed in LXD. Also, we can't give different images the same alias.

Instead, we now add a workshop-base property to our images, similar to the existing user.workshop.* instance config options. I avoided the user.workshop.* format because LXD's filtering syntax treats dots like path separators. I don't think there's a high chance of name collisions, unless the image servers start supporting Workshop explicitly.

In order to apply the property, I had to use CreateImage instead of CopyImage. Note that CopyImage is already a client-side wrapper around CreateImage, so there's no difference on the server side. However, for LXD image servers with multiple addresses, CopyImage tries all the addresses, whereas CreateImage only uses the main address. As far as I can tell, servers only have multiple addresses when they listen on multiple network interfaces (on the same machine), so I don't think this makes much difference in practice.

The upside of getting DownloadBase to download a specific fingerprint is that it simplifies the API, and detaches it from the LXD implementation details. Given that both downsides are quite minor, I now think this is the right call.

---

I removed the Base field from the Workshop struct, and added BaseVersion. Note that Name and Base are already available in the File field. I opted not to remove Name since it is used much more heavily than Base. But we could consider embedding File into Workshop to preserve the current convenience without duplicating data.

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.

[dmitry-lyfar]

I think the approach is fine, but am not sure about the interface and naming. Specifically, version.

[github-actions]

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/daemon/api_workshops.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
• internal/workshop/workshop.go

Automatic-tests / code-coverage / TICS GitHub Action