-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Allow podman machine to download from oci registry #21602
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
artifactVersion := getVersion() | ||
switch runtime.GOARCH { | ||
case "amd64": | ||
arch = "x86_64" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you switching arch and not just using runtime.GOARCH?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because fcos "arch" and go "arch" are different... nothing there is normalized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few questions.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: baude, giuseppe, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@nalind PTAL |
Should the location of the podman machine be listed in containers.conf, or are we going to rely on Mirroring for disconnected environments? |
I have a number of behaviors that need to be discussed as a team; this is one. I think this is good enough as is and a couple of clean up PRs will solidify things. |
Code LGTM aside from existing comments |
fixed up |
1a98a67
to
42e2691
Compare
Ephemeral COPR build failed. @containers/packit-build please check. |
this pr represents a shift in how we download podman machine images. these images will now be stored in oci registry and will replace the default method of downloading an image. you can still use a reference to a disk image as a path or url too with the --image-path switch. the final registry and location of the images has not been determined; and will need to be updated in podman as well. i dont think we need to allow --image-path to accept a registry/image for the podman 5.0 release. i do think there will be demand for this. upgrades also need to be plumbed. for example, updating from an oci registry. once we make decisions on final image locations/registrties as well as some behaviors of init and the oci pull, we must update the machine-init documentation. Signed-off-by: Brent Baude <bbaude@redhat.com>
/lgtm |
/hold cancel |
this pr represents a shift in how we download podman machine images. these images will now be stored in oci registry and will replace the default method of downloading an image. you can still use a reference to a disk image as a path or url too with the --image-path switch.
the final registry and location of the images has not been determined; and will need to be updated in podman as well.
i dont think we need to allow --image-path to accept a registry/image for the podman 5.0 release. i do think there will be demand for this.
upgrades also need to be plumbed. for example, updating from an oci registry.
once we make decisions on final image locations/registrties as well as some behaviors of init and the oci pull, we must update the machine-init documentation.
Does this PR introduce a user-facing change?