Fix --arch and --os flags to work correctly #9084
Conversation
openshift-ci-robot
added
the
approved
|
@lisongmin PTAL |
cmd/podman/common/create.go
Outdated
| "use `ARCH` instead of the architecture of the machine for choosing images", | ||
| ) | ||
| _ = cmd.RegisterFlagCompletionFunc(overrideArchFlagName, completion.AutocompleteNone) | ||
| _ = cmd.RegisterFlagCompletionFunc(archFlagName, completion.AutocompleteNone) |
There was a problem hiding this comment.
If I remember correctly you added autocomplete functions for arch and os. It would be good if you add them here.
There was a problem hiding this comment.
Thanks for reminding me.
Currently podman implements --override-arch and --overide-os But Podman has made these aliases for --arch and --os. No reason to have to specify --override, since it is clear what the user intends. Currently if the user specifies an --override-arch field but the image was previously pulled for a different Arch, podman run uses the different arch. This PR also fixes this issue. Fixes: containers#8001 Signed-off-by: Daniel J Walsh <[email protected]>
|
@containers/podman-maintainers PTAL |
|
LGTM |
|
LGTM but all I can give it right now is a cursory once-over. FWIW I did run my usual |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhatdan, saschagrunert 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 |
|
/hold I doubt that this change fixes #8001. Will test a bit. |
openshift-ci-robot
added
the
do-not-merge/hold
| cliVals.OverrideOS = split[0] | ||
| if len(split) > 1 { | ||
| cliVals.OverrideArch = split[1] | ||
| if cliVals.Platform != "" || cliVals.Arch != "" || cliVals.OS != "" { |
There was a problem hiding this comment.
@rhatdan can you add a // FIXME that states that all this logic should go into the backend? While this fix works for the clients, it does not work for the server.
I'd prefer something similar to vrothberg@6aa9622 but I never managed to get back to it. I would love to remove the pull entirely (as the backend does that already) but that would lose the progress bars for the remote client.
There was a problem hiding this comment.
When we do the containers/common image, could we take care of this there?
There was a problem hiding this comment.
Yes we could do that. But that would be based on libpod/image which is currently not doing what it should. That's why I think that vrothberg@6aa9622 is closer to tackle the root of the problem.
Looking good 👍 |
rhatdan
added
lgtm
github-actions
bot
added
the
locked - please file new issue/PR
Currently podman implements --override-arch and --overide-os
But Podman has made these aliases for --arch and --os. No
reason to have to specify --override, since it is clear what
the user intends.
Currently if the user specifies an --override-arch field but the
image was previously pulled for a different Arch, podman run uses
the different arch. This PR also fixes this issue.
Fixes: #8001
Signed-off-by: Daniel J Walsh [email protected]