Allow to override default username via command line #15411
Conversation
openshift-ci
bot
added
release-note
do-not-merge/work-in-progress
|
Draft because I first want to double check it with my custom Windows Qemu branch (#13006) as this code around command parsing flags is still too magical for me. |
| @@ -89,6 +88,10 @@ func init() { | |||
| ) | |||
| _ = flags.MarkHidden("reexec") | |||
|
|
|||
| UsernameFlagName := "username" | |||
| flags.StringVar(&initOpts.Username, UsernameFlagName, cfg.Machine.User, "Username used in qcow image") | |||
There was a problem hiding this comment.
Should this be cfg.Config.Machine.User instead of cfg.Machine.User, given that's what the removed bit used?
There was a problem hiding this comment.
I tested this with cherry-pick (w/o additional alterations). And it resulted with
"RemoteUsername": "user", in json config when called w/o explicit value and with "RemoteUsername": "core", giving --username core in the command line. I will investigate the code to understand better how it works. Wrote it this way because lines below references config settings with cfg.Machine.* - w/o additional .Config. And it was the only line with cfg.Config.
I will take a more detailed look before implementing this change. Keeping the draft for now.
There was a problem hiding this comment.
If I read this line correctly:
podman/pkg/domain/entities/engine.go
Line 33 in bec7e8a
Then Config is just expanded/blended into PodmanConfig, so, we can use it bypassing one dereference. My knowledge of advanced Go features is still limited. But at least VS Code code navigation navigates to the same place with .cfg.Machine and .cfg.Config.Machine. And, as I mentioned above, this worked in build the same way I described here, so, I guess this is correct.
@mheon Do you still insist on changing this (and only this) line?
Updates: also rebased PR to latest main.
|
/approve |
openshift-ci
bot
added
the
approved
|
Tests are complaining the flag needs to be added to the manpages: |
|
Added docs in a separate commit. Can squash them if needed/rebase to lasest and squash if needed. |
arixmkii
force-pushed
the
override_default_username
branch
from
34cc7e6
to
7f231af
Compare
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
arixmkii
force-pushed
the
override_default_username
branch
from
7f231af
to
84ce62e
Compare
|
Could you add a test to pkg/machine/e2e/init_test.go Add --username foobar and then inspect the machine to verify "RemoteUsername": "foobar" |
|
Will add test, squash and force push again. |
Signed-off-by: Arthur Sengileyev <arthur.sengileyev@gmail.com>
arixmkii
force-pushed
the
override_default_username
branch
from
84ce62e
to
08a2851
Compare
I believe this one is flaky. IIRC it is exactly the same, which also failed first in #15404 |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: arixmkii, mheon, 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 |
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
Fixes #15402
Registers additional flag to override username during machine init.
Signed-off-by: Arthur Sengileyev arthur.sengileyev@gmail.com
Does this PR introduce a user-facing change?