Add Websocket endpoint to access VM consoles #162
Conversation
cmd/virt-handler/virt-handler.go
Outdated
| "kubevirt.io/kubevirt/pkg/virt-handler/virtwrap" | ||
| virtcache "kubevirt.io/kubevirt/pkg/virt-handler/virtwrap/cache" | ||
| "net/http" |
There was a problem hiding this comment.
Builtin libs should be in first section
cmd/virt-handler/virt-handler.go
Outdated
| @@ -28,6 +31,8 @@ func main() { | |||
| libvirtUri := flag.String("libvirt-uri", "qemu:///system", "Libvirt connection string.") | |||
| libvirtUser := flag.String("user", "", "Libvirt user") | |||
| libvirtPass := flag.String("pass", "", "Libvirt password") | |||
| listen := flag.String("listen", "0.0.0.0", "Address and port where to listen on") | |||
There was a problem hiding this comment.
This is just the address, right? Help text says "and port"
| "github.com/emicklei/go-restful" | ||
| "github.com/gorilla/websocket" | ||
| "github.com/libvirt/libvirt-go" | ||
| "io" |
There was a problem hiding this comment.
Put a space between the go standard ones like io and the github ones, and the tooling will sort them within the groups. That does not have to be in this patch.
pkg/virt-handler/rest/console.go
Outdated
| domain, err := t.connection.LookupDomainByName(vm) | ||
| if err != nil { | ||
| if err.(libvirt.Error).Code == libvirt.ERR_NO_DOMAIN { | ||
| logging.DefaultLogger().Error().Reason(err).Msg("Domain not found.") |
There was a problem hiding this comment.
Do we have easy access to the domain to communicate in the logs? (also goes for line 39)
pkg/virt-handler/rest/console.go
Outdated
| err = <-errorChan | ||
|
|
||
| if err != nil { | ||
| logging.DefaultLogger().Error().Reason(err).Msg("I") |
There was a problem hiding this comment.
Error message is "I"... Is this a placeholder?
rmohr
changed the title
|
retest this please |
virt-handler now has an '/api/v1/console/myvm' endpoint, which also takes a 'device' header, to specify the console to connect to.
Like with all other libvirt interactions, allow mocking out contact points with libvirt.
When using CNI hostPort, alone does not allow binding the container to all interfaces. if that works, depends on the CNI provider. When using the host network int works.
| "github.com/emicklei/go-restful" | ||
| "github.com/gorilla/websocket" | ||
| "github.com/libvirt/libvirt-go" | ||
| "io" |
There was a problem hiding this comment.
Put a space between the go standard ones like io and the github ones, and the tooling will sort them within the groups. That does not have to be in this patch.
| t, body, err := con.ReadMessage() | ||
| Expect(t).To(Equal(websocket.TextMessage)) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| Expect(body).To(Equal([]byte("hello client!"))) |
There was a problem hiding this comment.
Why is this "client" and not "console" as was written on line 134?
There was a problem hiding this comment.
The client sends "hello console" and the console writes "hello client!". Line 124
|
@admiyo thx for the review |
Signed-off-by: Artyom Lukianov <alukiano@redhat.com>
virt-handler now has an '/api/v1/console/myvm' endpoint, which also takes
a 'device' header, to specify the console to connect to.