Skip to content

Conversation

@djs55
Copy link
Collaborator

@djs55 djs55 commented Jul 23, 2020

The current Go API for setting up the serial console has limitations, including:

  • it supports at most one serial port
  • it requires either stdio or a tty, and will trigger an assert if you don't provide one of them
  • it combines the request for a tty with the request to log the output to a ring-buffer or to Apple System Log.

This PR adds a new Go API while keeping (but deprecating) the old one. It's now possible to have multiple serial ports, each of which is connected to one of:

  • /dev/null
  • a TTY
  • stdio

and of which can be separately logged to any combination of:

  • /dev/null
  • a ring buffer
  • Apple System Log

This PR also adds a go.mod and some unit tests for the console-related command-line building functions.

djs55 added 3 commits July 23, 2020 14:28
This allows a native `go build` and `go test`.

Signed-off-by: David Scott <dave@recoil.org>
The previous API didn't allow us to disable the PTY but keep the
ring buffer, for example. These should be orthogonal, so add a new
API and deprecate the old one.

Signed-off-by: David Scott <dave@recoil.org>
Signed-off-by: David Scott <dave@recoil.org>
@djs55
Copy link
Collaborator Author

djs55 commented Jul 24, 2020

User observed a problem here: docker/roadmap#12 (comment) -- it looks like I've forgotten to modify the execute() function.
Unit tests are currently failing:

=== RUN   TestLegacyConsole
--- FAIL: TestLegacyConsole (0.00s)
    hyperkit_test.go:12: 
        	Error Trace:	hyperkit_test.go:12
        	Error:      	Expected nil, but got: &errors.errorString{s:"Could not find hyperkit executable hyperkit: exec: \"hyperkit\": executable file not found in $PATH"}
        	Test:       	TestLegacyConsole
=== RUN   TestNewSerial
--- FAIL: TestNewSerial (0.00s)
    hyperkit_test.go:21: 
        	Error Trace:	hyperkit_test.go:21
        	Error:      	Expected nil, but got: &errors.errorString{s:"Could not find hyperkit executable hyperkit: exec: \"hyperkit\": executable file not found in $PATH"}
        	Test:       	TestNewSerial
=== RUN   TestNullSerial
--- FAIL: TestNullSerial (0.00s)
    hyperkit_test.go:35: 
        	Error Trace:	hyperkit_test.go:35
        	Error:      	Expected nil, but got: &errors.errorString{s:"Could not find hyperkit executable hyperkit: exec: \"hyperkit\": executable file not found in $PATH"}
        	Test:       	TestNullSerial
FAIL
FAIL	github.com/moby/hyperkit/go	0.148s
FAIL
make: *** [test] Error 1

It's useful to be able to log the serial console output to a ring
buffer (or even ASL) without necessarily having a real host console
attached to it (i.e. no TTY or `stdio`).

The code requires an open file descriptor so add a new type `null`
which opens `/dev/null`.

Signed-off-by: David Scott <dave@recoil.org>
@djs55 djs55 force-pushed the improve-serial-console branch from 18bafa1 to 04d209c Compare July 24, 2020 13:51
@djs55 djs55 changed the title WIP: Improve serial console Improve serial console Aug 4, 2020
@djs55 djs55 changed the title Improve serial console go: improve serial console Aug 4, 2020
Copy link
Contributor

@mat007 mat007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Comment on lines +274 to +275
} else {
if err := h.checkSerials(); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: } else if { works as well

@djs55
Copy link
Collaborator Author

djs55 commented Aug 24, 2020

Thanks!

@djs55 djs55 merged commit b54460a into moby:master Aug 24, 2020
@djs55 djs55 deleted the improve-serial-console branch August 24, 2020 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants