From 5ac6e9a897fb1aa80a3bd5b1e70093e851d2fe11 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 13 Sep 2019 13:57:55 +0000 Subject: [PATCH] virtcontainers: make socket generation hypervisor specific Kata support several hypervisor and not all hypervisor support the same type of sockets, for example QEMU support vsock and unix sockets, while firecracker only support hybrid vsocks, hence sockets generations should be hypervisor specific fixes #2027 Signed-off-by: Julio Montes --- virtcontainers/acrn.go | 4 ++ virtcontainers/api_test.go | 1 + virtcontainers/fc.go | 14 +++++ virtcontainers/fc_test.go | 31 +++++++++++ virtcontainers/hypervisor.go | 41 +++++++++++++++ virtcontainers/hypervisor_test.go | 27 ++++++++++ virtcontainers/kata_agent.go | 71 +++++--------------------- virtcontainers/kata_agent_test.go | 30 ++--------- virtcontainers/mock_hypervisor.go | 4 ++ virtcontainers/mock_hypervisor_test.go | 8 +++ virtcontainers/qemu.go | 4 ++ 11 files changed, 153 insertions(+), 82 deletions(-) create mode 100644 virtcontainers/fc_test.go diff --git a/virtcontainers/acrn.go b/virtcontainers/acrn.go index 4ecef48e5e..c308055b41 100644 --- a/virtcontainers/acrn.go +++ b/virtcontainers/acrn.go @@ -657,3 +657,7 @@ func (a *acrn) check() error { return nil } + +func (a *acrn) generateSocket(id string, useVsock bool) (interface{}, error) { + return generateVMSocket(id, useVsock) +} diff --git a/virtcontainers/api_test.go b/virtcontainers/api_test.go index 1c76ec5683..b037b85489 100644 --- a/virtcontainers/api_test.go +++ b/virtcontainers/api_test.go @@ -121,6 +121,7 @@ func newTestSandboxConfigNoop() SandboxConfig { func newTestSandboxConfigKataAgent() SandboxConfig { sandboxConfig := newTestSandboxConfigNoop() sandboxConfig.AgentType = KataContainersAgent + sandboxConfig.AgentConfig = KataAgentConfig{} sandboxConfig.Containers = nil return sandboxConfig diff --git a/virtcontainers/fc.go b/virtcontainers/fc.go index 35c9344e89..83c100c468 100644 --- a/virtcontainers/fc.go +++ b/virtcontainers/fc.go @@ -1027,3 +1027,17 @@ func (fc *firecracker) check() error { return nil } + +func (fc *firecracker) generateSocket(id string, useVsock bool) (interface{}, error) { + if !useVsock { + return nil, fmt.Errorf("Can't start firecracker: vsocks is disabled") + } + + fc.Logger().Debug("Using hybrid-vsock endpoint") + udsPath := filepath.Join(fc.jailerRoot, defaultHybridVSocketName) + + return types.HybridVSock{ + UdsPath: udsPath, + Port: uint32(vSockPort), + }, nil +} diff --git a/virtcontainers/fc_test.go b/virtcontainers/fc_test.go new file mode 100644 index 0000000000..badcf7980a --- /dev/null +++ b/virtcontainers/fc_test.go @@ -0,0 +1,31 @@ +// Copyright (c) 2019 Intel Corporation +// +// SPDX-License-Identifier: Apache-2.0 +// + +package virtcontainers + +import ( + "testing" + + "github.com/kata-containers/runtime/virtcontainers/types" + "github.com/stretchr/testify/assert" +) + +func TestFCGenerateSocket(t *testing.T) { + assert := assert.New(t) + + fc := firecracker{} + i, err := fc.generateSocket("a", false) + assert.Error(err) + assert.Nil(i) + + i, err = fc.generateSocket("a", true) + assert.NoError(err) + assert.NotNil(i) + + hvsock, ok := i.(types.HybridVSock) + assert.True(ok) + assert.NotEmpty(hvsock.UdsPath) + assert.NotZero(hvsock.Port) +} diff --git a/virtcontainers/hypervisor.go b/virtcontainers/hypervisor.go index 7538baa2c8..acae8b82ee 100644 --- a/virtcontainers/hypervisor.go +++ b/virtcontainers/hypervisor.go @@ -10,6 +10,7 @@ import ( "context" "fmt" "os" + "path/filepath" "runtime" "strconv" "strings" @@ -18,6 +19,7 @@ import ( persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" "github.com/kata-containers/runtime/virtcontainers/store" "github.com/kata-containers/runtime/virtcontainers/types" + "github.com/kata-containers/runtime/virtcontainers/utils" ) // HypervisorType describes an hypervisor type. @@ -52,6 +54,15 @@ const ( defaultBridges = 1 defaultBlockDriver = config.VirtioSCSI + + defaultSocketName = "kata.sock" + defaultSocketDeviceID = "channel0" + defaultSocketChannelName = "agent.channel.0" + defaultSocketID = "charch0" + + // port numbers below 1024 are called privileged ports. Only a process with + // CAP_NET_BIND_SERVICE capability may bind to these port numbers. + vSockPort = 1024 ) // In some architectures the maximum number of vCPUs depends on the number of physical cores. @@ -663,6 +674,33 @@ func getHypervisorPid(h hypervisor) int { return pids[0] } +func generateVMSocket(id string, useVsock bool) (interface{}, error) { + if useVsock { + vhostFd, contextID, err := utils.FindContextID() + if err != nil { + return nil, err + } + + return types.VSock{ + VhostFd: vhostFd, + ContextID: contextID, + Port: uint32(vSockPort), + }, nil + } + + path, err := utils.BuildSocketPath(filepath.Join(store.RunVMStoragePath, id), defaultSocketName) + if err != nil { + return nil, err + } + + return types.Socket{ + DeviceID: defaultSocketDeviceID, + ID: defaultSocketID, + HostPath: path, + Name: defaultSocketChannelName, + }, nil +} + // hypervisor is the virtcontainers hypervisor interface. // The default hypervisor implementation is Qemu. type hypervisor interface { @@ -692,4 +730,7 @@ type hypervisor interface { save() persistapi.HypervisorState load(persistapi.HypervisorState) + + // generate the socket to communicate the host and guest + generateSocket(id string, useVsock bool) (interface{}, error) } diff --git a/virtcontainers/hypervisor_test.go b/virtcontainers/hypervisor_test.go index 3953b27fad..7cc97f2bf6 100644 --- a/virtcontainers/hypervisor_test.go +++ b/virtcontainers/hypervisor_test.go @@ -12,6 +12,8 @@ import ( "path/filepath" "testing" + ktu "github.com/kata-containers/runtime/pkg/katatestutils" + "github.com/kata-containers/runtime/virtcontainers/types" "github.com/stretchr/testify/assert" ) @@ -435,3 +437,28 @@ func genericTestRunningOnVMM(t *testing.T, data []testNestedVMMData) { assert.Equal(running, d.expected) } } + +func TestGenerateVMSocket(t *testing.T) { + assert := assert.New(t) + + s, err := generateVMSocket("a", false) + assert.NoError(err) + socket, ok := s.(types.Socket) + assert.True(ok) + assert.NotEmpty(socket.DeviceID) + assert.NotEmpty(socket.ID) + assert.NotEmpty(socket.HostPath) + assert.NotEmpty(socket.Name) + + if tc.NotValid(ktu.NeedRoot()) { + t.Skip(testDisabledAsNonRoot) + } + s, err = generateVMSocket("a", true) + assert.NoError(err) + vsock, ok := s.(types.VSock) + assert.True(ok) + defer assert.NoError(vsock.VhostFd.Close()) + assert.NotZero(vsock.VhostFd) + assert.NotZero(vsock.ContextID) + assert.NotZero(vsock.Port) +} diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index a210e0a19c..26b4ae6726 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -31,7 +31,6 @@ import ( "github.com/kata-containers/runtime/virtcontainers/pkg/uuid" "github.com/kata-containers/runtime/virtcontainers/store" "github.com/kata-containers/runtime/virtcontainers/types" - "github.com/kata-containers/runtime/virtcontainers/utils" "github.com/opencontainers/runtime-spec/specs-go" opentracing "github.com/opentracing/opentracing-go" "github.com/sirupsen/logrus" @@ -53,25 +52,17 @@ const ( ) var ( - checkRequestTimeout = 30 * time.Second - defaultRequestTimeout = 60 * time.Second - defaultKataSocketName = "kata.sock" - defaultKataChannel = "agent.channel.0" - defaultKataDeviceID = "channel0" - defaultKataID = "charch0" - errorMissingProxy = errors.New("Missing proxy pointer") - errorMissingOCISpec = errors.New("Missing OCI specification") - kataHostSharedDir = "/run/kata-containers/shared/sandboxes/" - kataGuestSharedDir = "/run/kata-containers/shared/containers/" - mountGuest9pTag = "kataShared" - kataGuestSandboxDir = "/run/kata-containers/sandbox/" - type9pFs = "9p" - typeVirtioFS = "virtio_fs" - typeVirtioFSNoCache = "none" - vsockSocketScheme = "vsock" - // port numbers below 1024 are called privileged ports. Only a process with - // CAP_NET_BIND_SERVICE capability may bind to these port numbers. - vSockPort = 1024 + checkRequestTimeout = 30 * time.Second + defaultRequestTimeout = 60 * time.Second + errorMissingProxy = errors.New("Missing proxy pointer") + errorMissingOCISpec = errors.New("Missing OCI specification") + kataHostSharedDir = "/run/kata-containers/shared/sandboxes/" + kataGuestSharedDir = "/run/kata-containers/shared/containers/" + mountGuest9pTag = "kataShared" + kataGuestSandboxDir = "/run/kata-containers/sandbox/" + type9pFs = "9p" + typeVirtioFS = "virtio_fs" + typeVirtioFSNoCache = "none" kata9pDevType = "9p" kataMmioBlkDevType = "mmioblk" kataBlkDevType = "blk" @@ -198,31 +189,6 @@ func (k *kataAgent) getSharePath(id string) string { return filepath.Join(kataHostSharedDir, id) } -func (k *kataAgent) generateVMSocket(id string, c KataAgentConfig) error { - if c.UseVSock { - // We want to go through VSOCK. The VM VSOCK endpoint will be our gRPC. - k.Logger().Debug("agent: Using vsock VM socket endpoint") - // We dont know yet the context ID - set empty vsock configuration - k.vmSocket = kataVSOCK{} - } else { - k.Logger().Debug("agent: Using unix socket form VM socket endpoint") - // We need to generate a host UNIX socket path for the emulated serial port. - kataSock, err := utils.BuildSocketPath(k.getVMPath(id), defaultKataSocketName) - if err != nil { - return err - } - - k.vmSocket = types.Socket{ - DeviceID: defaultKataDeviceID, - ID: defaultKataID, - HostPath: kataSock, - Name: defaultKataChannel, - } - } - - return nil -} - // KataAgentSetDefaultTraceConfigOptions validates agent trace options and // sets defaults. func KataAgentSetDefaultTraceConfigOptions(config *KataAgentConfig) error { @@ -293,10 +259,6 @@ func (k *kataAgent) init(ctx context.Context, sandbox *Sandbox, config interface switch c := config.(type) { case KataAgentConfig: - if err := k.generateVMSocket(sandbox.id, c); err != nil { - return false, err - } - disableVMShutdown = k.handleTraceSettings(c) k.keepConn = c.LongLiveConn k.kmodules = c.KernelModules @@ -349,10 +311,11 @@ func (k *kataAgent) capabilities() types.Capabilities { } func (k *kataAgent) internalConfigure(h hypervisor, id, sharePath string, builtin bool, config interface{}) error { + var err error if config != nil { switch c := config.(type) { case KataAgentConfig: - if err := k.generateVMSocket(id, c); err != nil { + if k.vmSocket, err = h.generateSocket(id, c.UseVSock); err != nil { return err } k.keepConn = c.LongLiveConn @@ -381,15 +344,9 @@ func (k *kataAgent) configure(h hypervisor, id, sharePath string, builtin bool, return err } case types.VSock: - s.VhostFd, s.ContextID, err = utils.FindContextID() - if err != nil { - return err - } - s.Port = uint32(vSockPort) if err = h.addDevice(s, vSockPCIDev); err != nil { return err } - k.vmSocket = s case types.HybridVSock: err = h.addDevice(s, hybridVirtioVsockDev) if err != nil { @@ -428,7 +385,7 @@ func (k *kataAgent) createSandbox(sandbox *Sandbox) error { span, _ := k.trace("createSandbox") defer span.Finish() - return k.configure(sandbox.hypervisor, sandbox.id, k.getSharePath(sandbox.id), k.proxyBuiltIn, nil) + return k.configure(sandbox.hypervisor, sandbox.id, k.getSharePath(sandbox.id), k.proxyBuiltIn, sandbox.config.AgentConfig) } func cmdToKataProcess(cmd types.Cmd) (process *grpc.Process, err error) { diff --git a/virtcontainers/kata_agent_test.go b/virtcontainers/kata_agent_test.go index d5ba3c92d0..0376823281 100644 --- a/virtcontainers/kata_agent_test.go +++ b/virtcontainers/kata_agent_test.go @@ -627,25 +627,6 @@ func TestAgentPathAPI(t *testing.T) { path1 = k1.getSharePath(id) path2 = k2.getSharePath(id) assert.Equal(path1, path2) - - // generateVMSocket - c := KataAgentConfig{} - err := k1.generateVMSocket(id, c) - assert.Nil(err) - err = k2.generateVMSocket(id, c) - assert.Nil(err) - assert.Equal(k1, k2) - - err = k1.generateVMSocket(id, c) - assert.Nil(err) - _, ok := k1.vmSocket.(types.Socket) - assert.True(ok) - - c.UseVSock = true - err = k2.generateVMSocket(id, c) - assert.Nil(err) - _, ok = k2.vmSocket.(types.VSock) - assert.True(ok) } func TestAgentConfigure(t *testing.T) { @@ -843,20 +824,19 @@ func TestKataAgentSetProxy(t *testing.T) { func TestKataGetAgentUrl(t *testing.T) { assert := assert.New(t) + var err error - k := &kataAgent{} - err := k.generateVMSocket("foobar", KataAgentConfig{}) - assert.Nil(err) + k := &kataAgent{vmSocket: types.Socket{HostPath: "/abc"}} + assert.NoError(err) url, err := k.getAgentURL() assert.Nil(err) assert.NotEmpty(url) - err = k.generateVMSocket("foobar", KataAgentConfig{UseVSock: true}) - assert.Nil(err) + k.vmSocket = types.VSock{} + assert.NoError(err) url, err = k.getAgentURL() assert.Nil(err) assert.NotEmpty(url) - } func TestKataCopyFile(t *testing.T) { diff --git a/virtcontainers/mock_hypervisor.go b/virtcontainers/mock_hypervisor.go index 86ed118258..30962e87a9 100644 --- a/virtcontainers/mock_hypervisor.go +++ b/virtcontainers/mock_hypervisor.go @@ -125,3 +125,7 @@ func (m *mockHypervisor) load(s persistapi.HypervisorState) {} func (m *mockHypervisor) check() error { return nil } + +func (m *mockHypervisor) generateSocket(id string, useVsock bool) (interface{}, error) { + return types.Socket{HostPath: "/tmp/socket", Name: "socket"}, nil +} diff --git a/virtcontainers/mock_hypervisor_test.go b/virtcontainers/mock_hypervisor_test.go index efcee1c808..799f502758 100644 --- a/virtcontainers/mock_hypervisor_test.go +++ b/virtcontainers/mock_hypervisor_test.go @@ -88,3 +88,11 @@ func TestMockHypervisorCheck(t *testing.T) { assert.NoError(t, m.check()) } + +func TestMockGenerateSocket(t *testing.T) { + var m *mockHypervisor + + i, err := m.generateSocket("a", true) + assert.NoError(t, err) + assert.NotNil(t, i) +} diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 7cab6e34c3..e9b1347b53 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -2061,3 +2061,7 @@ func (q *qemu) check() error { return nil } + +func (q *qemu) generateSocket(id string, useVsock bool) (interface{}, error) { + return generateVMSocket(id, useVsock) +}