-
Notifications
You must be signed in to change notification settings - Fork 752
Allow an image URL instead of a template reference #3338
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,11 +12,14 @@ import ( | |
| "os" | ||
| "path" | ||
| "path/filepath" | ||
| "regexp" | ||
| "runtime" | ||
| "strings" | ||
| "unicode" | ||
|
|
||
| "github.com/containerd/containerd/identifiers" | ||
| "github.com/lima-vm/lima/pkg/ioutilx" | ||
| "github.com/lima-vm/lima/pkg/limayaml" | ||
| "github.com/lima-vm/lima/pkg/templatestore" | ||
| "github.com/sirupsen/logrus" | ||
| ) | ||
|
|
@@ -30,6 +33,10 @@ func Read(ctx context.Context, name, locator string) (*Template, error) { | |
| Locator: locator, | ||
| } | ||
|
|
||
| if imageTemplate(tmpl, locator) { | ||
| return tmpl, nil | ||
| } | ||
|
|
||
| isTemplateURL, templateURL := SeemsTemplateURL(locator) | ||
| switch { | ||
| case isTemplateURL: | ||
|
|
@@ -121,6 +128,97 @@ func Read(ctx context.Context, name, locator string) (*Template, error) { | |
| return tmpl, nil | ||
| } | ||
|
|
||
| // Locators with an image file format extension, optionally followed by a compression method. | ||
| // This regex is also used to remove the file format suffix from the instance name. | ||
| var imageURLRegex = regexp.MustCompile(`\.(img|qcow2|raw|iso)(\.(gz|xz|bz2|zstd))?$`) | ||
|
|
||
| // Image architecture will be guessed based on the presence of arch keywords. | ||
| var archKeywords = map[string]limayaml.Arch{ | ||
| "aarch64": limayaml.AARCH64, | ||
| "amd64": limayaml.X8664, | ||
| "arm64": limayaml.AARCH64, | ||
| "armhf": limayaml.ARMV7L, | ||
| "armv7l": limayaml.ARMV7L, | ||
| "riscv64": limayaml.RISCV64, | ||
| "x86_64": limayaml.X8664, | ||
| } | ||
|
|
||
| // These generic tags will be stripped from an image name before turning it into an instance name. | ||
| var genericTags = []string{ | ||
| "base", // Fedora, Rocky | ||
| "cloud", // Fedora, openSUSE | ||
| "cloudimg", // Ubuntu, Arch | ||
| "cloudinit", // Alpine | ||
| "daily", // Debian | ||
| "default", // Gentoo | ||
| "generic", // Fedora | ||
| "genericcloud", // CentOS, Debian, Rocky, Alma | ||
| "kvm", // Oracle | ||
| "latest", // Gentoo, CentOS, Rocky, Alma | ||
| "linux", // Arch | ||
| "minimal", // openSUSE | ||
| "openstack", // Gentoo | ||
| "server", // Ubuntu | ||
| "std", // Alpine-Lima | ||
| "stream", // CentOS | ||
| "uefi", // Alpine | ||
| "vm", // openSUSE | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe e.g.,
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I kind of like the more descriptive names. You can always use So I would prefer to keep the current logic, unless you really dislike it. Also don't want to add unit tests if you are going to make me remove it again. 😄
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lima-vm/maintainers Thoughts?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I just noticed that I didn't add
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've selected all image names from all our templates and filtered out just the find templates -name "*.yaml" -exec yq '.images[].location' {} \; | xargs basename | sed s/nocloud_// | sort -fu | grep -E 'amd|x86'I've then manually transformed each of them into the instance name produced by the current algorithm: I think the names look reasonable, and I would prefer them to just picking the first word from the string.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added a special rule to translate |
||
|
|
||
| // imageTemplate checks if the locator specifies an image URL. | ||
| // It will create a minimal template with the image URL and arch derived from the image name | ||
| // and also set the default instance name to the image name, but stripped of generic tags. | ||
| func imageTemplate(tmpl *Template, locator string) bool { | ||
| if !imageURLRegex.MatchString(locator) { | ||
| return false | ||
| } | ||
|
|
||
| var imageArch limayaml.Arch | ||
| for keyword, arch := range archKeywords { | ||
| pattern := fmt.Sprintf(`\b%s\b`, keyword) | ||
| if regexp.MustCompile(pattern).MatchString(locator) { | ||
| imageArch = arch | ||
| break | ||
| } | ||
| } | ||
| if imageArch == "" { | ||
| imageArch = limayaml.NewArch(runtime.GOARCH) | ||
| logrus.Warnf("cannot determine image arch from URL %q; assuming %q", locator, imageArch) | ||
| } | ||
| template := `arch: %q | ||
| images: | ||
| - location: %q | ||
| arch: %q | ||
| ` | ||
| tmpl.Bytes = []byte(fmt.Sprintf(template, imageArch, locator, imageArch)) | ||
| tmpl.Name = InstNameFromImageURL(locator, imageArch) | ||
| return true | ||
| } | ||
|
|
||
| func InstNameFromImageURL(locator, imageArch string) string { | ||
| // We intentionally call both path.Base and filepath.Base in case we are running on Windows. | ||
| name := strings.ToLower(filepath.Base(path.Base(locator))) | ||
| // Remove file format and compression file types | ||
| name = imageURLRegex.ReplaceAllString(name, "") | ||
| // The Alpine "nocloud_" prefix does not fit the genericTags pattern | ||
| name = strings.TrimPrefix(name, "nocloud_") | ||
| for _, tag := range genericTags { | ||
| re := regexp.MustCompile(fmt.Sprintf(`[-_.]%s\b`, tag)) | ||
| name = re.ReplaceAllString(name, "") | ||
| } | ||
| // Remove imageArch as well if it is the native arch | ||
| if limayaml.IsNativeArch(imageArch) { | ||
| re := regexp.MustCompile(fmt.Sprintf(`[-_.]%s\b`, imageArch)) | ||
| name = re.ReplaceAllString(name, "") | ||
| } | ||
| // Remove timestamps from name: 8 digit date, optionally followed by | ||
| // a delimiter and one or more digits before a word boundary | ||
| name = regexp.MustCompile(`[-_.]20\d{6}([-_.]\d+)?\b`).ReplaceAllString(name, "") | ||
| // Normalize archlinux name | ||
| name = regexp.MustCompile(`^arch\b`).ReplaceAllString(name, "archlinux") | ||
| return name | ||
| } | ||
|
|
||
| func SeemsTemplateURL(arg string) (bool, *url.URL) { | ||
| u, err := url.Parse(arg) | ||
| if err != nil { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| // SPDX-FileCopyrightText: Copyright The Lima Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package limatmpl_test | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "runtime" | ||
| "testing" | ||
|
|
||
| "github.com/lima-vm/lima/pkg/limatmpl" | ||
| "github.com/lima-vm/lima/pkg/limayaml" | ||
| "gotest.tools/v3/assert" | ||
| ) | ||
|
|
||
| func TestInstNameFromImageURL(t *testing.T) { | ||
| t.Run("strips image format and compression method", func(t *testing.T) { | ||
| name := limatmpl.InstNameFromImageURL("linux.iso.bz2", "unknown") | ||
| assert.Equal(t, name, "linux") | ||
| }) | ||
| t.Run("removes generic tags", func(t *testing.T) { | ||
| name := limatmpl.InstNameFromImageURL("linux-linux_cloudimg.base-x86_64.raw", "unknown") | ||
| assert.Equal(t, name, "linux-x86_64") | ||
| }) | ||
| t.Run("removes Alpine `nocloud_` prefix", func(t *testing.T) { | ||
| name := limatmpl.InstNameFromImageURL("nocloud_linux-x86_64.raw", "unknown") | ||
| assert.Equal(t, name, "linux-x86_64") | ||
| }) | ||
| t.Run("removes date tag", func(t *testing.T) { | ||
| name := limatmpl.InstNameFromImageURL("linux-20250101.raw", "unknown") | ||
| assert.Equal(t, name, "linux") | ||
| }) | ||
| t.Run("removes date tag including time", func(t *testing.T) { | ||
| name := limatmpl.InstNameFromImageURL("linux-20250101-2000.raw", "unknown") | ||
| assert.Equal(t, name, "linux") | ||
| }) | ||
| t.Run("removes date tag including zero time", func(t *testing.T) { | ||
| name := limatmpl.InstNameFromImageURL("linux-20250101.0.raw", "unknown") | ||
| assert.Equal(t, name, "linux") | ||
| }) | ||
| t.Run("replace arch with archlinux", func(t *testing.T) { | ||
| name := limatmpl.InstNameFromImageURL("arch-aarch64.raw", "unknown") | ||
| assert.Equal(t, name, "archlinux-aarch64") | ||
| }) | ||
| t.Run("don't replace arch in the middle of the name", func(t *testing.T) { | ||
| name := limatmpl.InstNameFromImageURL("my-arch-aarch64.raw", "unknown") | ||
| assert.Equal(t, name, "my-arch-aarch64") | ||
| }) | ||
| t.Run("removes native arch", func(t *testing.T) { | ||
| arch := limayaml.NewArch(runtime.GOARCH) | ||
| image := fmt.Sprintf("linux_cloudimg.base-%s.qcow2.gz", arch) | ||
| name := limatmpl.InstNameFromImageURL(image, arch) | ||
| assert.Equal(t, name, "linux") | ||
| }) | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.