add "additionalArchives" config option #1123

pendo324 · 2022-10-19T19:37:40Z

Adds a new config option that allows users to include an "extra" archive in the CIDATA that gets mounted on OS boot.

This allows users to supply their own local packages or package repositories for use with provisioning scripts that run before the host filesystem is available (such as the dependency provisioning scripts proposed in #1105).

With 9p becoming the default at some point in the future, users wouldn't have to use an extra archive for this purpose since the filesystem should be available without any additional dependency installation (like fuse/sshfs), but it would allow parity between 9p and sshfs users.

There's an argument to be made as to whether this should allow users to upload n extra archives or just 1. I'm currently in favor of just 1 because it can contain arbitrary data and users can decide what to do with it.

More details in #1093

AkihiroSuda · 2022-10-20T08:26:50Z

pkg/limayaml/limayaml.go

@@ -31,6 +31,7 @@ type LimaYAML struct {
 	UseHostResolver   *bool             `yaml:"useHostResolver,omitempty" json:"useHostResolver,omitempty"` // DEPRECATED, use `HostResolver.Enabled` instead
 	PropagateProxyEnv *bool             `yaml:"propagateProxyEnv,omitempty" json:"propagateProxyEnv,omitempty"`
 	CACertificates    CACertificates    `yaml:"caCerts,omitempty" json:"caCerts,omitempty"`
+	ExtraArchive      *File             `yaml:"extraArchive,omitempty" json:"extraArchive,omitempty"`


Suggested change

ExtraArchive *File `yaml:"extraArchive,omitempty" json:"extraArchive,omitempty"`

ExtraArchives []File `yaml:"extraArchives,omitempty" json:"extraArchives,omitempty"`

for supporting multi-arch

AkihiroSuda · 2022-10-20T08:27:32Z

pkg/start/start.go

+			if err := y.ExtraArchive.Digest.Validate(); err != nil {
+				return err
+			}
+			eaBuf, err := os.ReadFile(y.ExtraArchive.Location)


Needs to support URL too (see how nerdctl-full-* archive are downloaded)

AkihiroSuda · 2022-10-20T08:28:21Z

examples/default.yaml

@@ -196,6 +196,13 @@ containerd:
 #     vim was not installed in the guest. Make sure the package system is working correctly.
 #     Also see "/var/log/cloud-init-output.log" in the guest.

+# Adds an additional archive to the CIDATA image which is mounted on boot


Needs more explanation (how to access the archive from the provisioning scripts?)

pendo324 · 2022-10-25T17:12:34Z

Thanks for the review @AkihiroSuda. I just pushed a new revision which adds more information to the defaults.yaml example, and supports multi-arch and remote downloads by refactoring and reusing the ensureNerdctlArchiveCache code from start.go.

jandubois · 2022-10-25T17:24:01Z

I'm sorry, I'm too busy this week to do a proper review, but I just noticed that this PR uses extraArchives, but the volume one uses additionalDisks. I would prefer to use the same prefix, as the concept is basically the same.

I would change this PR to use additionalArchives; @AkihiroSuda WDYT?

pendo324 · 2022-10-25T19:49:55Z

I'm sorry, I'm too busy this week to do a proper review, but I just noticed that this PR uses extraArchives, but the volume one uses additionalDisks. I would prefer to use the same prefix, as the concept is basically the same.

I would change this PR to use additionalArchives; @AkihiroSuda WDYT?

additionalArchives makes sense to me to keep it consistent. Will wait to hear back from Akihiro before changing.

AkihiroSuda · 2022-10-25T19:59:46Z

Yes, additional seems better for consistency

pendo324 · 2022-10-25T23:06:50Z

Renamed in latest revision. Also updated some comments to be clearer.

AkihiroSuda · 2022-11-02T02:06:36Z

Please squash commits and update the PR title

AkihiroSuda

Thanks

ningziwen · 2022-11-05T09:49:23Z

There is a typo in PR title: "additionalArchives" instead of "additonalArchives"

jandubois · 2022-11-12T04:43:03Z

There's an argument to be made as to whether this should allow users to upload n extra archives or just 1. I'm currently in favor of just 1 because it can contain arbitrary data and users can decide what to do with it.

I think it would be preferable to support multiple additional archives:

For example I may want to create an instance template that pulls various other archives via URLs from e.g. Github releases pages. This way I can take advantage of the Lima downloader and caching mechanism to fetch the files just once, and then bundling them automatically on instance start.

If I have to combine all the releases manually, I have to download the files myself, and repackage them. And when any one is being updated, I have to do this all over again.

And when I want to share the template with somebody else, I also need to provide a script to download and repackage the individual releases.

So I think it would be better to design the lima.yaml options for this to support multiple additional archives (in addition to supporting different architectures for each). Changing is later would just become a compatibility hassle.

AkihiroSuda · 2022-11-12T14:23:34Z

So I think it would be better to design the lima.yaml options for this to support multiple additional archives (in addition to supporting different architectures for each).

Will this be [][]File?
Seems kinda complicated, but I don't see other options.

jandubois · 2022-11-13T23:29:45Z

Will this be [][]File?
Seems kinda complicated, but I don't see other options.

I'm afraid so, but I also don't see any alternative. I agree it is ugly when you only need a single file.

Part of the complication comes from the fact that we support multiple sources for the same arch, so we can't use a map keyed by the archname instead.

BTW, I would like to be able to specify a "target name" for the additional archives, so it can be made available under a different name than the basename of the download URL.

additionalArchives:
- name: helm.tgz
  - location: https://get.helm.sh/helm-v3.10.2-linux-amd64.tar.gz
    arch: x86_64
  - location: https://get.helm.sh/helm-v3.10.2-linux-arm64.tar.gz
    arch: aarch64

That way I can write a provisioning script that just refers to /mnt/lima-cidata/helm.tgz without having to bother about the arch¹. And the script doesn't have to be changed if the archive is updated to point to a newer version of the package.

So I guess we could make this a map[string][]File:

additionalArchives:
  helm.tgz:
  - location: https://get.helm.sh/helm-v3.10.2-linux-amd64.tar.gz
    arch: x86_64
  - location: https://get.helm.sh/helm-v3.10.2-linux-arm64.tar.gz
    arch: aarch64

That would prevent us from adding additional keys to the Archive, but I think that is fine. The only thing I could think of adding would be some automated unpacking or filtering options, and I don't think we want to go there.

To avoid conflicts with filenames used by Lima it would probably be best to copy them into a subdirectory of the cidata volume, e.g. /mnt/lima-cidata/archives/helm.tgz.

Yes, I know that helm releases use the arch name in the directory name inside the tarball, e.g. linux-arm64/helm, but this can still be handled with wildcarding. ↩

AkihiroSuda · 2022-11-13T23:34:24Z

Yes, map[string][]File seems better

afbjorklund · 2022-12-27T14:59:41Z

Seems to be missing "digest" ? Better use the same setup as the containerd archives ?

        "containerd": {
            "system": false,
            "user": true,
            "archives": [
                {
                    "location": "https://github.com/containerd/nerdctl/releases/download/v1.1.0/nerdctl-full-1.1.0-linux-amd64.tar.gz",
                    "arch": "x86_64",
                    "digest": "sha256:5440c7b3af63df2ad2c98e185e06a27b4a21eea334b05408e84f8502251d9459"
                },
                {
                    "location": "https://github.com/containerd/nerdctl/releases/download/v1.1.0/nerdctl-full-1.1.0-linux-arm64.tar.gz",
                    "arch": "aarch64",
                    "digest": "sha256:3b613a1be5a24460c44bb93a3609b790ada94e06efd1a86467d45bec7da8b449"
                }
            ]
        },

But it is there in the code, since it is using File

pendo324 · 2023-01-14T00:04:25Z

Thanks for all the feedback. I just rebased and switched additionalArchives to use a map[string]string instead of just a list. I didn't add a new struct to hold the values, since its just a simple KVP for now (name => location), can definitely change that though

AkihiroSuda

Thanks

AkihiroSuda · 2023-01-14T10:08:49Z

examples/default.yaml

+# from a provisioning script.
+# additionalArchives:
+#   archive.tgz:
+#   - location: "/path/to/additional.amd64.tar.gz"


Suggested change

# - location: "/path/to/additional.amd64.tar.gz"

# - location: "/path/to/additional.x86_64.tar.gz"

or s/aarch64.tar.gz/arm64.tar.gz/ below

AkihiroSuda · 2023-01-23T00:14:30Z

@jandubois LGTY?

AkihiroSuda · 2023-01-30T00:55:58Z

@lima-vm/maintainers LGTY?

balajiv113 · 2023-01-30T04:08:40Z

cmd/limactl/hostagent.go

@@ -28,6 +28,7 @@ func newHostagentCommand() *cobra.Command {
 	hostagentCommand.Flags().StringP("pidfile", "p", "", "write pid to file")
 	hostagentCommand.Flags().String("socket", "", "hostagent socket")
 	hostagentCommand.Flags().String("nerdctl-archive", "", "local file path (not URL) of nerdctl-full-VERSION-linux-GOARCH.tar.gz")
+	hostagentCommand.Flags().StringToString("additional-archive", map[string]string{}, "local file path (not URL) of an arbitrary archive to add to CIDATA")


Just a thought, why not move the downloading of these archive to hostagent ?
This way, we don't need these specific args here.

One advantage i see in this way is, hostagent can just rely on the yaml config and do necessary stuffs.
Also this model provides flexibility if we want to ignore failures.

From start.go, we can only do the necessary things to bootstrap the hostagent itself (creating & downloading disks).

AkihiroSuda · 2023-04-07T10:16:15Z

Needs rebase

Signed-off-by: Justin Alvarez <alvajus@amazon.com>

AkihiroSuda reviewed Oct 20, 2022

View reviewed changes

pendo324 force-pushed the extra-archive branch from 18320f9 to d78bd2b Compare October 25, 2022 16:36

AkihiroSuda added this to the v0.14 (tentative) milestone Nov 2, 2022

pendo324 changed the title ~~add "extraArchive" config option~~ add "additonalArchive" config option Nov 2, 2022

pendo324 force-pushed the extra-archive branch from 73ba30e to cbd691d Compare November 2, 2022 16:33

pendo324 changed the title ~~add "additonalArchive" config option~~ add "additonalArchives" config option Nov 2, 2022

AkihiroSuda previously approved these changes Nov 2, 2022

View reviewed changes

AkihiroSuda changed the title ~~add "additonalArchives" config option~~ add "additionalArchives" config option Nov 5, 2022

AkihiroSuda requested a review from jandubois November 7, 2022 07:48

AkihiroSuda modified the milestones: v0.14, v0.15 (tentative) Nov 25, 2022

AkihiroSuda mentioned this pull request Dec 23, 2022

Allow local repo directories in containerd archives #1263

Open

pendo324 dismissed AkihiroSuda’s stale review via eaf0ea7 January 12, 2023 23:00

pendo324 force-pushed the extra-archive branch 2 times, most recently from eaf0ea7 to e4eee53 Compare January 14, 2023 00:01

AkihiroSuda previously approved these changes Jan 14, 2023

View reviewed changes

AkihiroSuda reviewed Jan 14, 2023

View reviewed changes

balajiv113 reviewed Jan 30, 2023

View reviewed changes

AkihiroSuda modified the milestones: v0.15.0, v0.16 (tentative) Feb 21, 2023

AkihiroSuda removed this from the v0.15.1 milestone Apr 5, 2023

pendo324 dismissed AkihiroSuda’s stale review via 7bfadb7 April 13, 2023 20:49

pendo324 force-pushed the extra-archive branch 2 times, most recently from 7bfadb7 to d859a9d Compare April 13, 2023 20:52

pendo324 requested a review from AkihiroSuda April 13, 2023 20:52

AkihiroSuda added this to the v0.16.0 (tentative) milestone Apr 14, 2023

add "additionalArchives" config option

e007439

Signed-off-by: Justin Alvarez <alvajus@amazon.com>

pendo324 force-pushed the extra-archive branch from d859a9d to e007439 Compare April 17, 2023 15:52

AkihiroSuda modified the milestones: v0.16.0, v0.17.0 (tentative) May 19, 2023

AkihiroSuda modified the milestones: v0.17.0, v0.18.0 Jul 30, 2023

AkihiroSuda modified the milestones: v0.17.1, v0.18.0, v0.17.2 Aug 11, 2023

AkihiroSuda removed this from the v0.18.0 milestone Sep 30, 2023

afbjorklund mentioned this pull request Oct 2, 2023

Cut down on the amount of logging when nerdctl-full cache already exist #1875

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add "additionalArchives" config option #1123

add "additionalArchives" config option #1123

pendo324 commented Oct 19, 2022

AkihiroSuda Oct 20, 2022

AkihiroSuda Oct 20, 2022

AkihiroSuda Oct 20, 2022

pendo324 commented Oct 25, 2022

jandubois commented Oct 25, 2022

pendo324 commented Oct 25, 2022

AkihiroSuda commented Oct 25, 2022

pendo324 commented Oct 25, 2022

AkihiroSuda commented Nov 2, 2022

AkihiroSuda left a comment

ningziwen commented Nov 5, 2022

jandubois commented Nov 12, 2022

AkihiroSuda commented Nov 12, 2022 •

edited

jandubois commented Nov 13, 2022 •

edited

AkihiroSuda commented Nov 13, 2022

afbjorklund commented Dec 27, 2022 •

edited

pendo324 commented Jan 14, 2023 •

edited

AkihiroSuda left a comment

AkihiroSuda Jan 14, 2023

AkihiroSuda commented Jan 23, 2023

AkihiroSuda commented Jan 30, 2023

balajiv113 Jan 30, 2023

AkihiroSuda commented Apr 7, 2023

	ExtraArchive *File `yaml:"extraArchive,omitempty" json:"extraArchive,omitempty"`
	ExtraArchives []File `yaml:"extraArchives,omitempty" json:"extraArchives,omitempty"`

	# - location: "/path/to/additional.amd64.tar.gz"
	# - location: "/path/to/additional.x86_64.tar.gz"

add "additionalArchives" config option #1123

Are you sure you want to change the base?

add "additionalArchives" config option #1123

Conversation

pendo324 commented Oct 19, 2022

AkihiroSuda Oct 20, 2022

Choose a reason for hiding this comment

AkihiroSuda Oct 20, 2022

Choose a reason for hiding this comment

AkihiroSuda Oct 20, 2022

Choose a reason for hiding this comment

pendo324 commented Oct 25, 2022

jandubois commented Oct 25, 2022

pendo324 commented Oct 25, 2022

AkihiroSuda commented Oct 25, 2022

pendo324 commented Oct 25, 2022

AkihiroSuda commented Nov 2, 2022

AkihiroSuda left a comment

Choose a reason for hiding this comment

ningziwen commented Nov 5, 2022

jandubois commented Nov 12, 2022

AkihiroSuda commented Nov 12, 2022 • edited

jandubois commented Nov 13, 2022 • edited

Footnotes

AkihiroSuda commented Nov 13, 2022

afbjorklund commented Dec 27, 2022 • edited

pendo324 commented Jan 14, 2023 • edited

AkihiroSuda left a comment

Choose a reason for hiding this comment

AkihiroSuda Jan 14, 2023

Choose a reason for hiding this comment

AkihiroSuda commented Jan 23, 2023

AkihiroSuda commented Jan 30, 2023

balajiv113 Jan 30, 2023

Choose a reason for hiding this comment

AkihiroSuda commented Apr 7, 2023

AkihiroSuda commented Nov 12, 2022 •

edited

jandubois commented Nov 13, 2022 •

edited

afbjorklund commented Dec 27, 2022 •

edited

pendo324 commented Jan 14, 2023 •

edited