Skip to content

Conversation

@aethernet
Copy link
Contributor

required for preloading

change-type: minor

@aethernet
Copy link
Contributor Author

This is the same as #257 but with the fix (I deleted the branch for the other PR by mistake)

Copy link
Contributor

@dfunckt dfunckt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great stuff. Some general thoughts:

  1. I think a versioning scheme is worth devising early, for both when requesting etchfiles and when applying them. A simple major/minor should be fine, and clients/servers must reject majors larger than they know about and should make reasonable effort to work with older ones. This is only pertinent to this PR because you will likely need to inject the version somewhere in the stream.
  2. Can you please explain (once more :) why the supervisor needs special handling?
  3. Does the .etch format support (or how hard can it be made to support) the base image and injectables being optional? So you can have injectables without a base image, or a base image without injectables (far less useful). Similarly, can the base image be a manifest instead that enables everything etcher currently supports? Eg. from URL.
  4. console.log is fine for the first versions, but we should ultimately use a logger function/object/stream that is passed as an argument to the entrypoint so we can integrate logging with whatever clients use.

@aethernet
Copy link
Contributor Author

aethernet commented Feb 3, 2023

All very valid points :

  1. Absolutely, I'll add back the manifest and add put the version

  2. I know we'd prefer not to have anything related to balena use case in here. Until we support multiapp and hostapps becomes one app beside users app (and even there I'm not sure how we'll start supervisor), we need to tag supervisor specifically in repositories.json otherwise it couldn't be started by the os. I could find a way to make this more generic but I'd prefer to not delay the feature further.

  3. Yes, once the stream is open, you can push anything (or nothing) in it before you close it. But, as it's a tar, the order of operations is important as the order files are delivered matter. So you can omit a base image but you cannot send it after the preloading assets. Same applies for preloading assets. In theory nothing prevent to just ship a manifest.

  4. Noted. I didn't thought much of it as in image-maker they're captured and properly treated, but you're totally right that it need to be more flexible for other users

@aethernet
Copy link
Contributor Author

aethernet commented Feb 3, 2023

I've added the manifest with a version 1.0 (1), it will be trivial to extend it to handle "url" for any asset in the stream (3)

@aethernet aethernet force-pushed the aethernet/dotetchpreloading branch from 5c50d19 to 5c3389f Compare February 7, 2023 17:36
@aethernet aethernet force-pushed the aethernet/dotetchpreloading branch from bd95b71 to eb7ad6b Compare February 17, 2023 16:00
@aethernet aethernet force-pushed the aethernet/dotetchpreloading branch 3 times, most recently from 66fc0cf to e52f1f8 Compare March 8, 2023 15:02
@aethernet aethernet force-pushed the aethernet/dotetchpreloading branch from 6134353 to 8bda19a Compare March 16, 2023 08:51
@jellyfish-bot
Copy link

[aethernet] This has attached https://jel.ly.fish/62ec39b3-f868-46cd-ad7c-3d375b83f8ac

aethernet added 2 commits May 22, 2023 11:55
required for preloading

change-type: minor
@aethernet aethernet force-pushed the aethernet/dotetchpreloading branch 2 times, most recently from c257dbd to 380e7c2 Compare May 22, 2023 12:11
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.

4 participants