simplestreams: Fix regression when parsing indexes that contain both combined and non-combined variants #12829
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Don't stop when finding first matching version file because the index is parsed in random order and LXD calls it multiple times when figuring out which image file to download and so stopping early can cause mismatches when trying to match a converted alias to a specific file fingerprint. Introduced with 3e9acc4 Signed-off-by: Thomas Parrott <[email protected]>
Signed-off-by: Thomas Parrott <[email protected]>
tomponline
force-pushed
the
tp-simplestreams
branch
from
857ddb8 to
56364f5
Compare
MusicDin
approved these changes
Feb 7, 2024
mihalicyn
approved these changes
Feb 7, 2024
mr-cal
added a commit
to canonical/craft-providers
that referenced
this pull request
Feb 7, 2024
Testing for canonical/lxd#12829 Signed-off-by: Callahan Kovacs <[email protected]>
3 tasks
mr-cal
added a commit
to canonical/craft-providers
that referenced
this pull request
Feb 7, 2024
Testing for canonical/lxd#12829 Signed-off-by: Callahan Kovacs <[email protected]>
tomponline
added a commit
to tomponline/lxd-pkg-snap
that referenced
this pull request
Feb 7, 2024
Includes fixes from canonical/lxd#12829 Signed-off-by: Thomas Parrott <[email protected]>
tomponline
added a commit
to tomponline/lxd-pkg-snap
that referenced
this pull request
Feb 7, 2024
Includes fixes from canonical/lxd#12829 Signed-off-by: Thomas Parrott <[email protected]>
This was referenced Feb 7, 2024
mr-cal
added a commit
to canonical/craft-providers
that referenced
this pull request
Feb 8, 2024
Testing for canonical/lxd#12829 Signed-off-by: Callahan Kovacs <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When support for Incus images was added in #12260 at the time the LinuxContainers remote contained both LXD and Incus entries and so a fix was added to stop searching for a file when a compatible one had been found during the parsing of the simplestreams index.
However this caused a bug when parsing the remote index for https://cloud-images.ubuntu.com/buildd/daily/streams/v1/com.ubuntu.cloud:daily:download.json because it contains both combined and non-combined image files for LXD.
In principle stopping after the first compatible image was found makes sense and is more efficient.
However LXD parses the simplestreams index (albeit cached) multiple times during a single image resolution process (once to convert alias to fingerprint and again to find the files for the fingerprint) and this causes LXD to call
(s *Products) ToLXD()multiple times.Compounding this issue is that Go iterates maps in random order and so the issue only presented itself intermittently, depending on how Go iterate the simplestreams
versionmap on each call to(s *Products) ToLXD().Since then the LinuxContainers remote has started blocking LXD users anyway, so support for Incus images was removed in #12748 and so we have no need for the original fix.
This PR reverts the fix and improves the logging.