Skip to content
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

unified image loading #2038

Open
BenTheElder opened this issue Jan 27, 2021 · 6 comments
Open

unified image loading #2038

BenTheElder opened this issue Jan 27, 2021 · 6 comments
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.
Milestone

Comments

@BenTheElder
Copy link
Member

BenTheElder commented Jan 27, 2021

As discussed when podman has come up previously I don't think we should continue making unique image loading commands, instead we should:

  • support podman + docker + tarball in a single kind load image
  • for each image argument, we should:
    • check for a file first if it looks like a path / check if the file exists
    • check docker
    • check podman

For checking docker vs podman, we can potentially enhance it to cache if docker or podman are even in PATH and skip the ones that aren't installed.

We could also perhaps match the node provider in preferring these, but a couple things to consider:

  • what about once we support another provider that may not have the ability to export images?
  • what about predictable consistency across hosts?

I think it may be worth sticking to the simplest: a fixed order of these, and ensuring this is fast.
If a script/user really want to load from a particular source, they can always do like podman save hello-world:2 | kind load image -

EDIT: once we agree on the high level approach, I have a few more specific points to add about implementing this correctly.

cc @aojea @amwat @bostrt

@BenTheElder BenTheElder added the kind/feature Categorizes issue or PR as related to a new feature. label Jan 27, 2021
@BenTheElder BenTheElder added this to the v0.11.0 milestone Jan 27, 2021
@aojea
Copy link
Contributor

aojea commented Jan 27, 2021

For checking docker vs podman, we can potentially enhance it to cache if docker or podman are even in PATH and skip the ones that aren't installed.

We could also perhaps match the node provider in preferring these, but a couple things to consider:

  • what about once we support another provider that may not have the ability to export images?
  • what about predictable consistency across hosts?

and adding the methods to the provider interface?
If a new provider doesn´t support it, it just throw an error

type Image interface {
   Save(image, dest string) error
   ImageID(image string) (string,error)
}

@BenTheElder
Copy link
Member Author

BenTheElder commented Jan 28, 2021

and adding the methods to the provider interface?

No, because we shouldn't need a provider instance. Even if I use docker to build some image (maybe I'm using a project that uses buildkit / buildx) I should be able to load it into my podman cluster smoothly, and vice versa.

Not to mention file based images that are not even remotely a node provider.

@aojea
Copy link
Contributor

aojea commented Jan 28, 2021

If a script/user really want to load from a particular source, they can always do like podman save hello-world:2 | kind load image -

That is great, and is easy to implement #2041

I like the piping approach and leverage other tools like https://github.com/containers/skopeo to deal with the whole image format thing

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 28, 2021
@kubernetes-sigs kubernetes-sigs deleted a comment from fejta-bot Apr 28, 2021
@BenTheElder BenTheElder removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 28, 2021
@BenTheElder BenTheElder modified the milestones: v0.11.0, 1.0 Oct 14, 2021
@BenTheElder BenTheElder added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Apr 26, 2022
@BenTheElder
Copy link
Member Author

We could also perhaps match the node provider in preferring these, but a couple things to consider:

Thinking about this some more, I think it would be reasonable to bump the current node provider to the 2nd preference (after checking for a file matching the argument) for each provider, just moving them and keeping the rest of the list in a fixed order.

Then the results are predictable with future providers, but with each provider we know to have images, we can give preference for the matching image source.

I feel like as a user I would expect podman to have priority when using podman, and docker when using docker.
The order is going to be fairly arbitrary for ignite or any other future provider without this, but should be fixed and predictable / known.

@BenTheElder
Copy link
Member Author

probably also consider OCI images from disk #2626

@corinz
Copy link

corinz commented Nov 2, 2022

+1 on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants