-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
machine: Save machine serial console by default #21150
Conversation
I've tested this on Linux/amd64 and MacOS/aarch64 (qemu)...but the main goal was to debug applehv, so going to try implementing it there next. |
9dc9889
to
99bf2bc
Compare
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: baude, cgwalters 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 |
pkg/machine/applehv/machine.go
Outdated
@@ -463,6 +463,10 @@ func (m *MacMachine) Set(name string, opts machine.SetOptions) ([]error, error) | |||
} | |||
} | |||
|
|||
func (m *MacMachine) GetConsole() error { | |||
return fmt.Errorf("not implemented") |
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'm not sure what is better here .. this or just using build tags to conditionally add the command. we have done both in the past. hopefully folks weigh in their preference.
@cgwalters lgtm generally ... i think our various linters and the like will require to make some more changes to the docs, etc because you added a command. i can help you through that if the feedback from the tooling is not clear. i'll check this PR in the morning. |
@baude (and others interested) please xref coreos/fedora-coreos-config#2785 - I think to make this really work for early kernel on MacOS we're going to need to change the kernel config and the default kargs. Also see a lot of similar discussion in lima-vm/lima#1659 |
pkg/machine/qemu/machine.go
Outdated
if _, err := io.Copy(os.Stdout, f); err != nil { | ||
return err | ||
} |
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.
should we copy stdin to it as well to make it interactive or does this not work? Although I guess there in no point because we are unable to login as a user without a static password.
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.
Right, the other console PR will help enable that.
cmd/podman/machine/console.go
Outdated
|
||
var ( | ||
getConsoleCmd = &cobra.Command{ | ||
Use: "get-console [NAME]", |
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 would just name it console
.
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.
For what it's worth, there is e.g. aws ec2 get-console-output
to be clear that it just fetches. Hmm. Thinking about this a bit more if we were to make a non-SSH way to login, I think it'd be podman machine login
.
(There's a whole sub-thread here of whether we should actually enable podman machine exec
...)
nice! |
Motivated by making it easier to debug problems like the applehv issues we're seeing today. This is also easy to do with qemu. However note that in practice this really wants coreos/fedora-coreos-config#2785 so that we can get the full console from the first boot on applehv. For now, this is just a debugging aid for us. Signed-off-by: Colin Walters <[email protected]>
99bf2bc
to
087c107
Compare
LGTM once CI is happy |
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@cgwalters still working on this one? |
Not recently, probably going to focus more on https://gitlab.com/germag/osc/ then we'll figure out how to epically merge some code with that in here. |
We'll circle back and do more here like this later. |
Motivated by making it easier to debug problems; I specifically want to make this change for MacOS in the future with applehv too. But I just know how to do it with qemu today.
There's a new
podman machine get-console
verb that is hidden until:virsh console
which is interactive.For now, this is just a debugging aid for us.