-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Introduce podman machine os commands #16648
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ashley-cui 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 |
Leaving release note as none for now and will update when this is no longer experimental. @baude PTAL |
cmd/podman/machine/os/apply.go
Outdated
var ( | ||
applyCmd = &cobra.Command{ | ||
Use: "apply", | ||
Short: "Apply OS image to existing VM", |
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.
s/OS image/OCI image
wdyt ?
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.
I think OCI image is technically more correct but I'm concerned about it being confusing to someone who isn't familiar with OSTree layering. But we also have the long desc that should clear it up? Not sure on the right answer, but definitely open to re-wording. :)
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.
I'll change it to OCI image for now though. I think the long desc is enough to clear it up.
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.
This is going to cause havoc with man page checker once experimental is turned on. Is this just so you and @baude can work together on the os commands?
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.
I think by the time we have manpages (probably the next commit) we'll have removed the experimental build tag and have it just as a regular command. I think we're just using the experimental tag to hide it from other builds ATM if I'm not mistaken.
Introduce machine os and machine os apply. Note that these are both stubs at the current moment, and do not introduce functionality. In order to build them, you must use the `experimental` build tag, or use `make podman-remote-experimental` [NO NEW TESTS NEEDED] as there is no actual functionality and this is a WIP. Signed-off-by: Ashley Cui <[email protected]>
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
@containers/podman-maintainers PTAL |
/lgtm |
Introduce machine os and machine os apply. Note that these are both stubs at the current moment, and do not introduce functionality. In order to build them, you must use the
experimental
build tag, or usemake podman-remote-experimental
[NO NEW TESTS NEEDED]
as there is no actual functionality and this is a WIP.
Signed-off-by: Ashley Cui [email protected]
Does this PR introduce a user-facing change?