From f6486e702690a324693f7a1b1eb810b69f33930c Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Tue, 7 Aug 2018 11:22:14 -0500 Subject: [PATCH 1/2] channel: support communication channel hotplug Since communication channel (vsock or serial port) is hot plugged by the runtime, newChannel iterates in a loop looking for the serial port or vsock device. The timeout is defined by channelExistMaxTries and channelExistWaitTime and it can be calculated by using the following operation: ``` (channelExistMaxTries * channelExistWaitTime) / 1000 = timeout in seconds ``` If there are neither vsocks nor serial ports, an error is returned. fixes #298 Signed-off-by: Julio Montes --- api.go | 2 +- channel.go | 59 +++++++++++++++++++++++++++++++++++-------------- channel_test.go | 53 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 18 deletions(-) diff --git a/api.go b/api.go index 4e0ae02ef2..91d634a7e9 100644 --- a/api.go +++ b/api.go @@ -11,7 +11,7 @@ import ( ) // Serial channel -const ( +var ( serialChannelName = "agent.channel.0" virtIOPath = "/sys/class/virtio-ports" devRootPath = "/dev" diff --git a/channel.go b/channel.go index 3eb2846d42..194135e2a8 100644 --- a/channel.go +++ b/channel.go @@ -13,6 +13,7 @@ import ( "os" "path/filepath" "strings" + "time" "github.com/hashicorp/yamux" "github.com/mdlayher/vsock" @@ -21,6 +22,12 @@ import ( grpcStatus "google.golang.org/grpc/status" ) +var ( + channelExistMaxTries = 200 + channelExistWaitTime = 50 * time.Millisecond + isAFVSockSupportedFunc = isAFVSockSupported +) + type channel interface { setup() error wait() error @@ -28,28 +35,45 @@ type channel interface { teardown() error } +// Creates a new channel to communicate the agent with the proxy or shim. +// The runtime hot plugs a serial port or a vsock PCI depending of the configuration +// file and if the host has support for vsocks. newChannel iterates in a loop looking +// for the serial port or vsock device. +// The timeout is defined by channelExistMaxTries and channelExistWaitTime and it +// can be calculated by using the following operation: +// (channelExistMaxTries * channelExistWaitTime) / 1000 = timeout in seconds +// If there are neither vsocks nor serial ports, an error is returned. func newChannel() (channel, error) { - // Check for vsock support. - vSockSupported, err := isAFVSockSupported() - if err != nil { - return nil, err - } - - if vSockSupported { - // Check if vsock socket exists. We want to cover the case - // where the guest OS can support vsock, but the runtime is - // still using virtio serial connection. - exist, err := vSockPathExist() - if err != nil { - return nil, err + var serialErr error + var serialPath string + var vsockErr error + var vSockSupported bool + + for i := 0; i < channelExistMaxTries; i++ { + // check vsock path + if _, err := os.Stat(vSockDevPath); err == nil { + if vSockSupported, vsockErr = isAFVSockSupportedFunc(); vSockSupported && vsockErr == nil { + return &vSockChannel{}, nil + } } - if exist { - return &vSockChannel{}, nil + // Check serial port path + if serialPath, serialErr = findVirtualSerialPath(serialChannelName); serialErr == nil { + return &serialChannel{serialPath: serialPath}, nil } + + time.Sleep(channelExistWaitTime) + } + + if serialErr != nil { + agentLog.WithError(serialErr).Error("Serial port not found") + } + + if vsockErr != nil { + agentLog.WithError(vsockErr).Error("VSock not found") } - return &serialChannel{}, nil + return nil, fmt.Errorf("Neither vsocks nor serial ports were found") } type vSockChannel struct { @@ -77,12 +101,13 @@ func (c *vSockChannel) teardown() error { } type serialChannel struct { + serialPath string serialConn *os.File } func (c *serialChannel) setup() error { // Open serial channel. - file, err := openSerialChannel() + file, err := os.OpenFile(c.serialPath, os.O_RDWR, os.ModeDevice) if err != nil { return err } diff --git a/channel_test.go b/channel_test.go index 28aea372da..b20ea7d426 100644 --- a/channel_test.go +++ b/channel_test.go @@ -7,8 +7,10 @@ package main import ( + "errors" "io/ioutil" "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -97,3 +99,54 @@ func TestTeardownSerialChannel(t *testing.T) { err = c.teardown() assert.Nil(t, err, "%v", err) } + +func TestNewChannel(t *testing.T) { + assert := assert.New(t) + + orgChannelExistMaxTries := channelExistMaxTries + orgChannelExistWaitTime := channelExistWaitTime + orgVSockDevPath := vSockDevPath + orgVirtIOPath := virtIOPath + orgIsAFVSockSupportedFunc := isAFVSockSupportedFunc + channelExistMaxTries = 1 + channelExistWaitTime = 0 + vSockDevPath = "/abc/xyz/123" + virtIOPath = "/abc/xyz/123" + isAFVSockSupportedFunc = func() (bool, error) { return false, errors.New("vsock") } + defer func() { + channelExistMaxTries = orgChannelExistMaxTries + channelExistWaitTime = orgChannelExistWaitTime + vSockDevPath = orgVSockDevPath + virtIOPath = orgVirtIOPath + isAFVSockSupportedFunc = orgIsAFVSockSupportedFunc + }() + + c, err := newChannel() + assert.Error(err) + assert.Nil(c) + + vSockDevPath = "/dev/null" + c, err = newChannel() + assert.Error(err) + assert.Nil(c) + + isAFVSockSupportedFunc = func() (bool, error) { return true, nil } + c, err = newChannel() + assert.NoError(err) + _, ok := c.(*vSockChannel) + assert.True(ok) + + vSockDevPath = "/abc/xyz/123" + virtIOPath, err = ioutil.TempDir("", "virtio") + assert.NoError(err) + portPath := filepath.Join(virtIOPath, "port") + err = os.Mkdir(portPath, 0777) + assert.NoError(err) + defer os.Remove(portPath) + err = ioutil.WriteFile(filepath.Join(portPath, "name"), []byte(serialChannelName), 0777) + assert.NoError(err) + c, err = newChannel() + assert.NoError(err) + _, ok = c.(*serialChannel) + assert.True(ok) +} From 4f70b1cf6e11a25b72bc053839d3c31a5612f531 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Tue, 7 Aug 2018 11:26:55 -0500 Subject: [PATCH 2/2] channel: remove unused functions newChannel function was re-implemented, some functions are no more needed. Signed-off-by: Julio Montes --- channel.go | 26 -------------------------- channel_test.go | 32 -------------------------------- 2 files changed, 58 deletions(-) diff --git a/channel.go b/channel.go index 194135e2a8..2c6bfeb50e 100644 --- a/channel.go +++ b/channel.go @@ -227,18 +227,6 @@ func isAFVSockSupported() (bool, error) { return true, nil } -func vSockPathExist() (bool, error) { - if _, err := os.Stat(vSockDevPath); err != nil { - if os.IsNotExist(err) { - return false, nil - } - - return false, err - } - - return true, nil -} - func findVirtualSerialPath(serialName string) (string, error) { dir, err := os.Open(virtIOPath) if err != nil { @@ -270,17 +258,3 @@ func findVirtualSerialPath(serialName string) (string, error) { return "", grpcStatus.Errorf(codes.NotFound, "Could not find virtio port %s", serialName) } - -func openSerialChannel() (*os.File, error) { - serialPath, err := findVirtualSerialPath(serialChannelName) - if err != nil { - return nil, err - } - - file, err := os.OpenFile(serialPath, os.O_RDWR, os.ModeDevice) - if err != nil { - return nil, err - } - - return file, nil -} diff --git a/channel_test.go b/channel_test.go index b20ea7d426..fdecb87f0e 100644 --- a/channel_test.go +++ b/channel_test.go @@ -16,38 +16,6 @@ import ( "github.com/stretchr/testify/assert" ) -func TestVSockPathExistTrue(t *testing.T) { - tmpFile, err := ioutil.TempFile("", "test") - assert.Nil(t, err, "%v", err) - fileName := tmpFile.Name() - defer tmpFile.Close() - defer os.Remove(fileName) - - vSockDevPath = fileName - - result, err := vSockPathExist() - assert.Nil(t, err, "%v", err) - - assert.True(t, result, "VSOCK should be found") -} - -func TestVSockPathExistFalse(t *testing.T) { - tmpFile, err := ioutil.TempFile("", "test") - assert.Nil(t, err, "%v", err) - - fileName := tmpFile.Name() - tmpFile.Close() - err = os.Remove(fileName) - assert.Nil(t, err, "%v", err) - - vSockDevPath = fileName - - result, err := vSockPathExist() - assert.Nil(t, err, "%v", err) - - assert.False(t, result, "VSOCK should not be found") -} - func TestSetupVSockChannel(t *testing.T) { c := &vSockChannel{}