From c4658c3a30f69d09f09c67a11bbc7ea19b8e5967 Mon Sep 17 00:00:00 2001 From: fupan Date: Fri, 22 Jun 2018 16:07:34 +0800 Subject: [PATCH 01/39] api: To watch the vm console in FetchSandbox api When do sandbox release, the kataBuiltInProxy will be closed, and it will stop the watch of vm's console; Thus it needs to restart the proxy to monitor the vm console once to restore the sandbox. Fixes: #441 Signed-off-by: fupan --- virtcontainers/agent.go | 3 +++ virtcontainers/api.go | 16 +++++++++++++++- virtcontainers/cc_proxy.go | 5 +++++ virtcontainers/hyperstart_agent.go | 18 ++++++++++++++++-- virtcontainers/kata_agent.go | 15 ++++++++++++++- virtcontainers/kata_builtin_proxy.go | 7 ++++++- virtcontainers/kata_proxy.go | 5 +++++ virtcontainers/no_proxy.go | 5 +++++ virtcontainers/noop_agent.go | 5 +++++ virtcontainers/noop_proxy.go | 5 +++++ virtcontainers/proxy.go | 3 +++ virtcontainers/sandbox.go | 11 +++++++++++ 12 files changed, 93 insertions(+), 5 deletions(-) diff --git a/virtcontainers/agent.go b/virtcontainers/agent.go index 0f32e9d03c..c65406e455 100644 --- a/virtcontainers/agent.go +++ b/virtcontainers/agent.go @@ -140,6 +140,9 @@ type agent interface { // disconnect will disconnect the connection to the agent disconnect() error + // start the proxy + startProxy(sandbox *Sandbox) error + // createSandbox will tell the agent to perform necessary setup for a Sandbox. createSandbox(sandbox *Sandbox) error diff --git a/virtcontainers/api.go b/virtcontainers/api.go index b91c8812f2..dfaf0c881a 100644 --- a/virtcontainers/api.go +++ b/virtcontainers/api.go @@ -110,7 +110,21 @@ func FetchSandbox(sandboxID string) (VCSandbox, error) { defer unlockSandbox(lockFile) // Fetch the sandbox from storage and create it. - return fetchSandbox(sandboxID) + sandbox, err := fetchSandbox(sandboxID) + if err != nil { + return nil, err + } + + // If the proxy is KataBuiltInProxyType type, it needs to restart the proxy to watch the + // guest console if it hadn't been watched. + if isProxyBuiltIn(sandbox.config.ProxyType) { + err = sandbox.startProxy() + if err != nil { + return nil, err + } + } + + return sandbox, nil } // StartSandbox is the virtcontainers sandbox starting entry point. diff --git a/virtcontainers/cc_proxy.go b/virtcontainers/cc_proxy.go index 7cdedd776b..ef6cce5eb0 100644 --- a/virtcontainers/cc_proxy.go +++ b/virtcontainers/cc_proxy.go @@ -41,3 +41,8 @@ func (p *ccProxy) start(sandbox *Sandbox, params proxyParams) (int, string, erro func (p *ccProxy) stop(sandbox *Sandbox, pid int) error { return nil } + +// The ccproxy doesn't need to watch the vm console, thus return false always. +func (p *ccProxy) consoleWatched() bool { + return false +} diff --git a/virtcontainers/hyperstart_agent.go b/virtcontainers/hyperstart_agent.go index 4669930a2c..6c08b706a4 100644 --- a/virtcontainers/hyperstart_agent.go +++ b/virtcontainers/hyperstart_agent.go @@ -340,8 +340,11 @@ func (h *hyper) exec(sandbox *Sandbox, c Container, cmd Cmd) (*Process, error) { return process, nil } -// startSandbox is the agent Sandbox starting implementation for hyperstart. -func (h *hyper) startSandbox(sandbox *Sandbox) error { +func (h *hyper) startProxy(sandbox *Sandbox) error { + if h.proxy.consoleWatched() { + return nil + } + // Start the proxy here pid, uri, err := h.proxy.start(sandbox, proxyParams{}) if err != nil { @@ -357,6 +360,17 @@ func (h *hyper) startSandbox(sandbox *Sandbox) error { h.Logger().WithField("proxy-pid", pid).Info("proxy started") + return nil +} + +// startSandbox is the agent Sandbox starting implementation for hyperstart. +func (h *hyper) startSandbox(sandbox *Sandbox) error { + + err := h.startProxy(sandbox) + if err != nil { + return err + } + if err := h.register(); err != nil { return err } diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 3167bc5ab3..9bb5c796dd 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -419,11 +419,15 @@ func (k *kataAgent) generateInterfacesAndRoutes(networkNS NetworkNamespace) ([]* return ifaces, routes, nil } -func (k *kataAgent) startSandbox(sandbox *Sandbox) error { +func (k *kataAgent) startProxy(sandbox *Sandbox) error { if k.proxy == nil { return errorMissingProxy } + if k.proxy.consoleWatched() { + return nil + } + // Get agent socket path to provide it to the proxy. agentURL, err := k.agentURL() if err != nil { @@ -454,6 +458,15 @@ func (k *kataAgent) startSandbox(sandbox *Sandbox) error { "proxy-url": uri, }).Info("proxy started") + return nil +} + +func (k *kataAgent) startSandbox(sandbox *Sandbox) error { + err := k.startProxy(sandbox) + if err != nil { + return err + } + hostname := sandbox.config.Hostname if len(hostname) > maxHostnameLen { hostname = hostname[:maxHostnameLen] diff --git a/virtcontainers/kata_builtin_proxy.go b/virtcontainers/kata_builtin_proxy.go index f5f67743b0..3ab80c934c 100644 --- a/virtcontainers/kata_builtin_proxy.go +++ b/virtcontainers/kata_builtin_proxy.go @@ -21,11 +21,16 @@ type kataBuiltInProxy struct { conn net.Conn } +// check if the proxy has watched the vm console. +func (p *kataBuiltInProxy) consoleWatched() bool { + return p.conn != nil +} + // start is the proxy start implementation for kata builtin proxy. // It starts the console watcher for the guest. // It returns agentURL to let agent connect directly. func (p *kataBuiltInProxy) start(sandbox *Sandbox, params proxyParams) (int, string, error) { - if p.conn != nil { + if p.consoleWatched() { return -1, "", fmt.Errorf("kata builtin proxy running for sandbox %s", p.sandboxID) } diff --git a/virtcontainers/kata_proxy.go b/virtcontainers/kata_proxy.go index 608b1d6f0d..5d996e7ead 100644 --- a/virtcontainers/kata_proxy.go +++ b/virtcontainers/kata_proxy.go @@ -17,6 +17,11 @@ import ( type kataProxy struct { } +// The kata proxy doesn't need to watch the vm console, thus return false always. +func (p *kataProxy) consoleWatched() bool { + return false +} + // start is kataProxy start implementation for proxy interface. func (p *kataProxy) start(sandbox *Sandbox, params proxyParams) (int, string, error) { if sandbox.agent == nil { diff --git a/virtcontainers/no_proxy.go b/virtcontainers/no_proxy.go index 520afbbafe..a3e9d0b78a 100644 --- a/virtcontainers/no_proxy.go +++ b/virtcontainers/no_proxy.go @@ -35,3 +35,8 @@ func (p *noProxy) start(sandbox *Sandbox, params proxyParams) (int, string, erro func (p *noProxy) stop(sandbox *Sandbox, pid int) error { return nil } + +// The noproxy doesn't need to watch the vm console, thus return false always. +func (p *noProxy) consoleWatched() bool { + return false +} diff --git a/virtcontainers/noop_agent.go b/virtcontainers/noop_agent.go index 57dc0e6da1..ed8abdbc75 100644 --- a/virtcontainers/noop_agent.go +++ b/virtcontainers/noop_agent.go @@ -16,6 +16,11 @@ import ( type noopAgent struct { } +//start the proxy to watch the vm console. It does nothing. +func (n *noopAgent) startProxy(sandbox *Sandbox) error { + return nil +} + // init initializes the Noop agent, i.e. it does nothing. func (n *noopAgent) init(sandbox *Sandbox, config interface{}) error { return nil diff --git a/virtcontainers/noop_proxy.go b/virtcontainers/noop_proxy.go index 1142875dd1..473d8a63ae 100644 --- a/virtcontainers/noop_proxy.go +++ b/virtcontainers/noop_proxy.go @@ -22,3 +22,8 @@ func (p *noopProxy) start(sandbox *Sandbox, params proxyParams) (int, string, er func (p *noopProxy) stop(sandbox *Sandbox, pid int) error { return nil } + +// The noopproxy doesn't need to watch the vm console, thus return false always. +func (p *noopProxy) consoleWatched() bool { + return false +} diff --git a/virtcontainers/proxy.go b/virtcontainers/proxy.go index 0d5886b830..4af852254f 100644 --- a/virtcontainers/proxy.go +++ b/virtcontainers/proxy.go @@ -170,4 +170,7 @@ type proxy interface { // stop terminates a proxy instance after all communications with the // agent inside the VM have been properly stopped. stop(sandbox *Sandbox, pid int) error + + //check if the proxy has watched the vm console. + consoleWatched() bool } diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 776c0c102e..87f8eda529 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -352,6 +352,17 @@ type SandboxConfig struct { SharePidNs bool } +func (s *Sandbox) startProxy() error { + + // If the proxy is KataBuiltInProxyType type, it needs to restart the proxy + // to watch the guest console if it hadn't been watched. + if s.agent == nil { + return fmt.Errorf("sandbox %s missed agent pointer", s.ID()) + } + + return s.agent.startProxy(s) +} + // valid checks that the sandbox configuration is valid. func (sandboxConfig *SandboxConfig) valid() bool { if sandboxConfig.ID == "" { From fdf131210d01fc5159df3d9c88d588b4017c0808 Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Wed, 4 Jul 2018 14:45:43 +0100 Subject: [PATCH 02/39] create: Remove redundant logging code Don't add the container ID as a log fields as it is already a field (added on #453). Signed-off-by: James O. D. Hunt --- cli/create.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/cli/create.go b/cli/create.go index 165bf6b698..e7b4fda14e 100644 --- a/cli/create.go +++ b/cli/create.go @@ -16,7 +16,6 @@ import ( vc "github.com/kata-containers/runtime/virtcontainers" "github.com/kata-containers/runtime/virtcontainers/pkg/oci" - "github.com/sirupsen/logrus" "github.com/urfave/cli" ) @@ -283,11 +282,7 @@ func createContainer(ociSpec oci.CompatOCISpec, containerID, bundlePath, func createCgroupsFiles(containerID string, cgroupsDirPath string, cgroupsPathList []string, pid int) error { if len(cgroupsPathList) == 0 { - fields := logrus.Fields{ - "container": containerID, - "pid": pid, - } - kataLog.WithFields(fields).Info("Cgroups files not created because cgroupsPath was empty") + kataLog.WithField("pid", pid).Info("Cgroups files not created because cgroupsPath was empty") return nil } From 371dc14b1018daa1c540b953dc40824d5a09283c Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Wed, 4 Jul 2018 14:48:43 +0100 Subject: [PATCH 03/39] logging: Add function to handle external loggers Created a new `setExternalLogger()` which sets (or resets) the logger used by the external packages which allow a logger to be specified. Signed-off-by: James O. D. Hunt --- cli/main.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/cli/main.go b/cli/main.go index e5b7f61e25..99a4a890eb 100644 --- a/cli/main.go +++ b/cli/main.go @@ -187,6 +187,16 @@ func setupSignalHandler() { }() } +// setExternalLoggers registers the specified logger with the external +// packages which accept a logger to handle their own logging. +func setExternalLoggers(logger *logrus.Entry) { + // Set virtcontainers logger. + vci.SetLogger(logger) + + // Set the OCI package logger. + oci.SetLogger(logger) +} + // beforeSubcommands is the function to perform preliminary checks // before command-line parsing occurs. func beforeSubcommands(context *cli.Context) error { @@ -225,11 +235,7 @@ func beforeSubcommands(context *cli.Context) error { return fmt.Errorf("unknown log-format %q", context.GlobalString("log-format")) } - // Set virtcontainers logger. - vci.SetLogger(kataLog) - - // Set the OCI package logger. - oci.SetLogger(kataLog) + setExternalLoggers(kataLog) ignoreLogging := false From 9774c2ab1aa0cd67904c791727d0e8a582d83f78 Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Wed, 4 Jul 2018 14:49:58 +0100 Subject: [PATCH 04/39] logging: Reset external loggers when cid+sid available Once `containerID` and `sandboxID` fields are available, re-register the logger with the external packages to ensure they too display these important fields. Fixes #467. Signed-off-by: James O. D. Hunt --- cli/create.go | 3 +++ cli/delete.go | 2 ++ cli/events.go | 2 ++ cli/exec.go | 2 ++ cli/kill.go | 2 ++ cli/pause.go | 2 ++ cli/ps.go | 2 ++ cli/start.go | 2 ++ cli/state.go | 2 ++ 9 files changed, 19 insertions(+) diff --git a/cli/create.go b/cli/create.go index e7b4fda14e..858d4233fc 100644 --- a/cli/create.go +++ b/cli/create.go @@ -89,6 +89,7 @@ func create(containerID, bundlePath, console, pidFilePath string, detach bool, var err error kataLog = kataLog.WithField("container", containerID) + setExternalLoggers(kataLog) // Checks the MUST and MUST NOT from OCI runtime specification if bundlePath, err = validCreateParams(containerID, bundlePath); err != nil { @@ -240,6 +241,7 @@ func createSandbox(ociSpec oci.CompatOCISpec, runtimeConfig oci.RuntimeConfig, } kataLog = kataLog.WithField("sandbox", sandbox.ID()) + setExternalLoggers(kataLog) containers := sandbox.GetAllContainers() if len(containers) != 1 { @@ -267,6 +269,7 @@ func createContainer(ociSpec oci.CompatOCISpec, containerID, bundlePath, } kataLog = kataLog.WithField("sandbox", sandboxID) + setExternalLoggers(kataLog) _, c, err := vci.CreateContainer(sandboxID, contConfig) if err != nil { diff --git a/cli/delete.go b/cli/delete.go index 0d83314e93..31ee87227f 100644 --- a/cli/delete.go +++ b/cli/delete.go @@ -64,6 +64,8 @@ func delete(containerID string, force bool) error { "sandbox": sandboxID, }) + setExternalLoggers(kataLog) + containerID = status.ID containerType, err := oci.GetContainerType(status.Annotations) diff --git a/cli/events.go b/cli/events.go index 7da633cc99..67dfc56aac 100644 --- a/cli/events.go +++ b/cli/events.go @@ -156,6 +156,8 @@ information is displayed once every 5 seconds.`, "sandbox": sandboxID, }) + setExternalLoggers(kataLog) + if status.State.State == vc.StateStopped { return fmt.Errorf("container with id %s is not running", status.ID) } diff --git a/cli/exec.go b/cli/exec.go index 1e71c90d27..2d5d1f28ad 100644 --- a/cli/exec.go +++ b/cli/exec.go @@ -194,6 +194,8 @@ func execute(context *cli.Context) error { "sandbox": sandboxID, }) + setExternalLoggers(kataLog) + // Retrieve OCI spec configuration. ociSpec, err := oci.GetOCIConfig(status) if err != nil { diff --git a/cli/kill.go b/cli/kill.go index 5d93df0527..f66de9be72 100644 --- a/cli/kill.go +++ b/cli/kill.go @@ -104,6 +104,8 @@ func kill(containerID, signal string, all bool) error { "sandbox": sandboxID, }) + setExternalLoggers(kataLog) + signum, err := processSignal(signal) if err != nil { return err diff --git a/cli/pause.go b/cli/pause.go index 4b9a49087e..2cec0beaec 100644 --- a/cli/pause.go +++ b/cli/pause.go @@ -55,6 +55,8 @@ func toggleContainerPause(containerID string, pause bool) (err error) { "sandbox": sandboxID, }) + setExternalLoggers(kataLog) + if pause { err = vci.PauseContainer(sandboxID, containerID) } else { diff --git a/cli/ps.go b/cli/ps.go index 4dfab940f8..2d0817270c 100644 --- a/cli/ps.go +++ b/cli/ps.go @@ -61,6 +61,8 @@ func ps(containerID, format string, args []string) error { "sandbox": sandboxID, }) + setExternalLoggers(kataLog) + // container MUST be running if status.State.State != vc.StateRunning { return fmt.Errorf("Container %s is not running", containerID) diff --git a/cli/start.go b/cli/start.go index 7da8a5a28b..24723209a4 100644 --- a/cli/start.go +++ b/cli/start.go @@ -52,6 +52,8 @@ func start(containerID string) (vc.VCSandbox, error) { "sandbox": sandboxID, }) + setExternalLoggers(kataLog) + containerID = status.ID containerType, err := oci.GetContainerType(status.Annotations) diff --git a/cli/state.go b/cli/state.go index 57412e6182..f100105842 100644 --- a/cli/state.go +++ b/cli/state.go @@ -36,6 +36,8 @@ instance of a container.`, func state(containerID string) error { kataLog = kataLog.WithField("container", containerID) + setExternalLoggers(kataLog) + // Checks the MUST and MUST NOT from OCI runtime specification status, _, err := getExistingContainerInfo(containerID) if err != nil { From 61607e784ba0dc641ca51c886c1904221c0b64fc Mon Sep 17 00:00:00 2001 From: Penny Zheng Date: Tue, 3 Jul 2018 07:24:25 +0000 Subject: [PATCH 05/39] ci: reconstructure image-type and initrd for architecture-independant as default image-type and initrd weren't for non-x86_64 arch, reconstructuring them to be architecture-specific. Fixes: #461 Signed-off-by: Penny Zheng Signed-off-by: Jianyong Wu Signed-off-by: James O. D. Hunt --- versions.yaml | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/versions.yaml b/versions.yaml index d616dc0b25..78d88b6197 100644 --- a/versions.yaml +++ b/versions.yaml @@ -76,18 +76,34 @@ assets: Root filesystem disk image used to boot the guest virtual machine. url: "https://github.com/kata-containers/osbuilder" - release: "20640" + architecture: + aarch64: + name: "fedora" + version: "latest" + ppc64le: + name: "centos" + version: "latest" + x86_64: + name: &default-image-name "clearlinux" + version: "20640" meta: - image-type: "clearlinux" + image-type: *default-image-name initrd: description: | Root filesystem initrd used to boot the guest virtual machine. url: "https://github.com/kata-containers/osbuilder" - meta: - base-os: "alpine" - os-version: "3.7" + architecture: + aarch64: + name: &default-initrd-name "alpine" + version: &default-initrd-version "3.7" + ppc64le: + name: *default-initrd-name + version: *default-initrd-version + x86_64: + name: *default-initrd-name + version: *default-initrd-version kernel: description: "Linux kernel optimised for virtual machines" From 3dda2601eb8e4c84b95e3de84e70f77df4b78b81 Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Fri, 6 Jul 2018 11:50:14 +0100 Subject: [PATCH 06/39] tests: Refactor CC proxy test for Kata Reworked `TestCCProxyStart` to create a generic `testProxyStart()` that is now used for testing both CC and Kata proxies. Signed-off-by: James O. D. Hunt --- virtcontainers/cc_proxy_test.go | 75 +------------------------- virtcontainers/kata_proxy_test.go | 17 ++++++ virtcontainers/proxy_test.go | 90 +++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 74 deletions(-) create mode 100644 virtcontainers/kata_proxy_test.go diff --git a/virtcontainers/cc_proxy_test.go b/virtcontainers/cc_proxy_test.go index 641e7f1a1f..ed19855271 100644 --- a/virtcontainers/cc_proxy_test.go +++ b/virtcontainers/cc_proxy_test.go @@ -6,84 +6,11 @@ package virtcontainers import ( - "fmt" - "io/ioutil" - "os" - "path/filepath" "testing" - - "github.com/stretchr/testify/assert" ) func TestCCProxyStart(t *testing.T) { - assert := assert.New(t) - - tmpdir, err := ioutil.TempDir("", "") - assert.NoError(err) - defer os.RemoveAll(tmpdir) - proxy := &ccProxy{} - type testData struct { - sandbox *Sandbox - expectedURI string - expectError bool - } - - invalidPath := filepath.Join(tmpdir, "enoent") - expectedSocketPath := filepath.Join(runStoragePath, testSandboxID, "proxy.sock") - expectedURI := fmt.Sprintf("unix://%s", expectedSocketPath) - - data := []testData{ - {&Sandbox{}, "", true}, - { - &Sandbox{ - config: &SandboxConfig{ - ProxyType: "invalid", - }, - }, "", true, - }, - { - &Sandbox{ - config: &SandboxConfig{ - ProxyType: CCProxyType, - // invalid - no path - ProxyConfig: ProxyConfig{}, - }, - }, "", true, - }, - { - &Sandbox{ - config: &SandboxConfig{ - ProxyType: CCProxyType, - ProxyConfig: ProxyConfig{ - Path: invalidPath, - }, - }, - }, "", true, - }, - { - &Sandbox{ - id: testSandboxID, - config: &SandboxConfig{ - ProxyType: CCProxyType, - ProxyConfig: ProxyConfig{ - Path: "echo", - }, - }, - }, expectedURI, false, - }, - } - - for _, d := range data { - pid, uri, err := proxy.start(d.sandbox, proxyParams{}) - if d.expectError { - assert.Error(err) - continue - } - - assert.NoError(err) - assert.True(pid > 0) - assert.Equal(d.expectedURI, uri) - } + testProxyStart(t, nil, proxy, CCProxyType) } diff --git a/virtcontainers/kata_proxy_test.go b/virtcontainers/kata_proxy_test.go new file mode 100644 index 0000000000..0e202d7a57 --- /dev/null +++ b/virtcontainers/kata_proxy_test.go @@ -0,0 +1,17 @@ +// Copyright (c) 2018 Intel Corporation +// +// SPDX-License-Identifier: Apache-2.0 +// + +package virtcontainers + +import ( + "testing" +) + +func TestKataProxyStart(t *testing.T) { + agent := &kataAgent{} + proxy := &kataProxy{} + + testProxyStart(t, agent, proxy, KataProxyType) +} diff --git a/virtcontainers/proxy_test.go b/virtcontainers/proxy_test.go index 753dfaa48a..b796d30780 100644 --- a/virtcontainers/proxy_test.go +++ b/virtcontainers/proxy_test.go @@ -7,9 +7,13 @@ package virtcontainers import ( "fmt" + "io/ioutil" + "os" "path/filepath" "reflect" "testing" + + "github.com/stretchr/testify/assert" ) func testSetProxyType(t *testing.T, value string, expected ProxyType) { @@ -248,3 +252,89 @@ func TestDefaultProxyURLUnknown(t *testing.T) { t.Fatal() } } + +func testProxyStart(t *testing.T, agent agent, proxy proxy, proxyType ProxyType) { + assert := assert.New(t) + + assert.NotNil(proxy) + + tmpdir, err := ioutil.TempDir("", "") + assert.NoError(err) + defer os.RemoveAll(tmpdir) + + type testData struct { + sandbox *Sandbox + params proxyParams + expectedURI string + expectError bool + } + + invalidPath := filepath.Join(tmpdir, "enoent") + expectedSocketPath := filepath.Join(runStoragePath, testSandboxID, "proxy.sock") + expectedURI := fmt.Sprintf("unix://%s", expectedSocketPath) + + data := []testData{ + {&Sandbox{}, proxyParams{}, "", true}, + { + &Sandbox{ + config: &SandboxConfig{ + ProxyType: "invalid", + }, + }, + proxyParams{}, + "", true, + }, + { + &Sandbox{ + config: &SandboxConfig{ + ProxyType: proxyType, + // invalid - no path + ProxyConfig: ProxyConfig{}, + }, + }, + proxyParams{}, + "", true, + }, + { + &Sandbox{ + config: &SandboxConfig{ + ProxyType: proxyType, + ProxyConfig: ProxyConfig{ + Path: invalidPath, + }, + }, + }, + proxyParams{}, + "", true, + }, + + { + &Sandbox{ + id: testSandboxID, + agent: agent, + config: &SandboxConfig{ + ProxyType: proxyType, + ProxyConfig: ProxyConfig{ + Path: "echo", + }, + }, + }, + proxyParams{ + agentURL: "agentURL", + }, + expectedURI, false, + }, + } + + for _, d := range data { + pid, uri, err := proxy.start(d.sandbox, d.params) + if d.expectError { + assert.Error(err) + continue + } + + assert.NoError(err) + assert.True(pid > 0) + assert.Equal(d.expectedURI, uri) + } +} From d10b1d86ffbef282228f9054a833801119c2b94c Mon Sep 17 00:00:00 2001 From: Nitesh Konkar Date: Tue, 19 Jun 2018 19:48:18 +0530 Subject: [PATCH 07/39] virtcontainers: Set ppc64le maxmem depending on qemu version The "Failed to allocate HTAB of requested size, try with smaller maxmem" error in ppc64le occurs when maxmem allocated is very high. This got fixed in qemu 2.10 and kernel 4.11. Hence put a maxmem restriction of 32GB per kata-container if qemu version less than 2.10 Fixes: #415 Signed-off-by: Nitesh Konkar --- virtcontainers/qemu.go | 5 +++++ virtcontainers/qemu_ppc64le.go | 16 ++++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 882294ad39..262928e7ad 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -64,6 +64,9 @@ const qmpSocket = "qmp.sock" const defaultConsole = "console.sock" +var qemuMajorVersion int +var qemuMinorVersion int + // agnostic list of kernel parameters var defaultKernelParameters = []Param{ {"panic", "1"}, @@ -456,6 +459,8 @@ func (q *qemu) waitSandbox(timeout int) error { } q.qmpMonitorCh.qmp = qmp + qemuMajorVersion = ver.Major + qemuMinorVersion = ver.Minor q.Logger().WithFields(logrus.Fields{ "qmp-major-version": ver.Major, diff --git a/virtcontainers/qemu_ppc64le.go b/virtcontainers/qemu_ppc64le.go index 930cc3fe95..ba259389eb 100644 --- a/virtcontainers/qemu_ppc64le.go +++ b/virtcontainers/qemu_ppc64le.go @@ -12,6 +12,7 @@ import ( govmmQemu "github.com/intel/govmm/qemu" "github.com/kata-containers/runtime/virtcontainers/device/drivers" "github.com/kata-containers/runtime/virtcontainers/utils" + "github.com/sirupsen/logrus" ) type qemuPPC64le struct { @@ -54,6 +55,11 @@ var supportedQemuMachines = []govmmQemu.Machine{ }, } +// Logger returns a logrus logger appropriate for logging qemu messages +func (q *qemuPPC64le) Logger() *logrus.Entry { + return virtLog.WithField("subsystem", "qemu") +} + // MaxQemuVCPUs returns the maximum number of vCPUs supported func MaxQemuVCPUs() uint32 { return uint32(128) @@ -105,12 +111,14 @@ func (q *qemuPPC64le) cpuModel() string { func (q *qemuPPC64le) memoryTopology(memoryMb, hostMemoryMb uint64) govmmQemu.Memory { - if hostMemoryMb > defaultMemMaxPPC64le { - hostMemoryMb = defaultMemMaxPPC64le - } else { - // align hostMemoryMb to 256MB multiples + if qemuMajorVersion >= 2 && qemuMinorVersion >= 10 { + q.Logger().Debug("Aligning maxmem to multiples of 256MB. Assumption: Kernel Version >= 4.11") hostMemoryMb -= (hostMemoryMb % 256) + } else { + q.Logger().Debug("Restricting maxmem to 32GB as Qemu Version < 2.10, Assumption: Kernel Version >= 4.11") + hostMemoryMb = defaultMemMaxPPC64le } + return genericMemoryTopology(memoryMb, hostMemoryMb) } From 9868cf249889a966c922d95753141eac3a33acee Mon Sep 17 00:00:00 2001 From: fupan Date: Fri, 6 Jul 2018 18:56:05 +0800 Subject: [PATCH 08/39] api: To stop its monitor after a sandbox paused After the sandbox is paused, it's needed to stop its monitor, Otherwise, its monitors will receive timeout errors if it is paused for a long time, thus its monitor will not tell it's a crash caused timeout or just a paused timeout. Fixes: #472 Signed-off-by: fupan --- virtcontainers/sandbox.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 87f8eda529..b289db5769 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -1203,6 +1203,14 @@ func (s *Sandbox) Pause() error { return err } + //After the sandbox is paused, it's needed to stop its monitor, + //Otherwise, its monitors will receive timeout errors if it is + //paused for a long time, thus its monitor will not tell it's a + //crash caused timeout or just a paused timeout. + if s.monitor != nil { + s.monitor.stop() + } + return s.pauseSetStates() } From e64065526375e2a7333bad9a5ae7a13674b7bf14 Mon Sep 17 00:00:00 2001 From: Graham Whaley Date: Tue, 10 Jul 2018 14:57:32 +0100 Subject: [PATCH 09/39] ci: no-exit: Document and echo what the check is Make it clearer why we run the check. Make it announce itself. Signed-off-by: Graham Whaley --- .ci/go-no-os-exit.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.ci/go-no-os-exit.sh b/.ci/go-no-os-exit.sh index 51eac257ab..fe3bbf64ab 100755 --- a/.ci/go-no-os-exit.sh +++ b/.ci/go-no-os-exit.sh @@ -2,9 +2,14 @@ # Copyright (c) 2018 Intel Corporation # # SPDX-License-Identifier: Apache-2.0 +# +# Check there are no os.Exit() calls creeping into the code +# We don't use that exit path in the Kata codebase. go_packages=. +echo "Checking for no os.Exit() calls for package [${go_packages}]" + candidates=`go list -f '{{.Dir}}/*.go' $go_packages` for f in $candidates; do filename=`basename $f` From 1b23e150e5dfc0d09572f823aa95a22bc11dae3e Mon Sep 17 00:00:00 2001 From: Graham Whaley Date: Tue, 10 Jul 2018 15:02:06 +0100 Subject: [PATCH 10/39] ci: no-exit: Skip check if no files to check If we find no files to check, gracefully quit the test. Formerly, if the list was empty we ended up trying to read from stdin, and thus hung. Signed-off-by: Graham Whaley --- .ci/go-no-os-exit.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.ci/go-no-os-exit.sh b/.ci/go-no-os-exit.sh index fe3bbf64ab..efac2835ad 100755 --- a/.ci/go-no-os-exit.sh +++ b/.ci/go-no-os-exit.sh @@ -22,6 +22,8 @@ for f in $candidates; do files="$f $files" done +[ -z "$files" ] && echo "No files to check, skipping" && exit 0 + if egrep -n '\' $files; then echo "Direct calls to os.Exit() are forbidden, please use exit() so atexit() works" exit 1 From b71cae0b733716d5b9b13147f6bd220ef9fe41d6 Mon Sep 17 00:00:00 2001 From: Graham Whaley Date: Tue, 10 Jul 2018 15:14:24 +0100 Subject: [PATCH 11/39] ci: no-exit: Allow path override for os.Exit check Allow the path being checked by the os-no-exit script to be passed in, and update the Makefile to use that to check the current code paths of the cli and virtcontainers. Fixes: #477 Signed-off-by: Graham Whaley --- .ci/go-no-os-exit.sh | 4 +++- Makefile | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.ci/go-no-os-exit.sh b/.ci/go-no-os-exit.sh index efac2835ad..b429ed5a46 100755 --- a/.ci/go-no-os-exit.sh +++ b/.ci/go-no-os-exit.sh @@ -6,7 +6,9 @@ # Check there are no os.Exit() calls creeping into the code # We don't use that exit path in the Kata codebase. -go_packages=. +# Allow the path to check to be over-ridden. +# Default to the current directory. +go_packages=${1:-.} echo "Checking for no os.Exit() calls for package [${go_packages}]" diff --git a/Makefile b/Makefile index 10a7f851d2..cf68c09bc1 100644 --- a/Makefile +++ b/Makefile @@ -395,7 +395,8 @@ go-test: $(GENERATED_FILES) check-go-static: $(QUIET_CHECK).ci/static-checks.sh - $(QUIET_CHECK).ci/go-no-os-exit.sh + $(QUIET_CHECK).ci/go-no-os-exit.sh ./cli + $(QUIET_CHECK).ci/go-no-os-exit.sh ./virtcontainers coverage: $(QUIET_TEST).ci/go-test.sh html-coverage From b4595ae3f154da8dac867174f2d01f0a8face6d5 Mon Sep 17 00:00:00 2001 From: Graham Whaley Date: Tue, 10 Jul 2018 17:04:27 +0100 Subject: [PATCH 12/39] ci: no-exit: Do not run no-exit check on test files The test files do not have access to our app level exit() function, and are thus OK to call os.Exit() if they need. Skip them from the check. Signed-off-by: Graham Whaley --- .ci/go-no-os-exit.sh | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.ci/go-no-os-exit.sh b/.ci/go-no-os-exit.sh index b429ed5a46..5f0f98436a 100755 --- a/.ci/go-no-os-exit.sh +++ b/.ci/go-no-os-exit.sh @@ -15,12 +15,10 @@ echo "Checking for no os.Exit() calls for package [${go_packages}]" candidates=`go list -f '{{.Dir}}/*.go' $go_packages` for f in $candidates; do filename=`basename $f` + # skip all go test files + [[ $filename == *_test.go ]] && continue # skip exit.go where, the only file we should call os.Exit() from. [[ $filename == "exit.go" ]] && continue - # skip exit_test.go - [[ $filename == "exit_test.go" ]] && continue - # skip main_test.go - [[ $filename == "main_test.go" ]] && continue files="$f $files" done From b21646a42c4a8f2c89bb16916b3b0b74e6a8309c Mon Sep 17 00:00:00 2001 From: Graham Whaley Date: Fri, 6 Jul 2018 17:27:55 +0100 Subject: [PATCH 13/39] kata-env: Add ability to output as JSON Having a direct JSON output for kata-env will help record results in our CIs in some instances. Add that ability with a kata-env command line extension. Fixes: #474 Signed-off-by: Graham Whaley --- cli/kata-env.go | 59 +++++++++++++++++++++++++++++++------------- cli/kata-env_test.go | 10 +++++++- 2 files changed, 51 insertions(+), 18 deletions(-) diff --git a/cli/kata-env.go b/cli/kata-env.go index d0daf3490f..150b51ca7f 100644 --- a/cli/kata-env.go +++ b/cli/kata-env.go @@ -6,6 +6,7 @@ package main import ( + "encoding/json" "errors" "os" "strings" @@ -328,28 +329,17 @@ func getEnvInfo(configFile string, config oci.RuntimeConfig) (env EnvInfo, err e return env, nil } -func showSettings(env EnvInfo, file *os.File) error { - encoder := toml.NewEncoder(file) - - err := encoder.Encode(env) - if err != nil { - return err - } - - return nil -} - -func handleSettings(file *os.File, metadata map[string]interface{}) error { +func handleSettings(file *os.File, c *cli.Context) error { if file == nil { return errors.New("Invalid output file specified") } - configFile, ok := metadata["configFile"].(string) + configFile, ok := c.App.Metadata["configFile"].(string) if !ok { return errors.New("cannot determine config file") } - runtimeConfig, ok := metadata["runtimeConfig"].(oci.RuntimeConfig) + runtimeConfig, ok := c.App.Metadata["runtimeConfig"].(oci.RuntimeConfig) if !ok { return errors.New("cannot determine runtime config") } @@ -359,13 +349,48 @@ func handleSettings(file *os.File, metadata map[string]interface{}) error { return err } - return showSettings(env, file) + if c.Bool("json") { + return writeJSONSettings(env, file) + } + + return writeTOMLSettings(env, file) +} + +func writeTOMLSettings(env EnvInfo, file *os.File) error { + encoder := toml.NewEncoder(file) + + err := encoder.Encode(env) + if err != nil { + return err + } + + return nil +} + +func writeJSONSettings(env EnvInfo, file *os.File) error { + encoder := json.NewEncoder(file) + + // Make it more human readable + encoder.SetIndent("", " ") + + err := encoder.Encode(env) + if err != nil { + return err + } + + return nil } var kataEnvCLICommand = cli.Command{ Name: envCmd, - Usage: "display settings", + Usage: "display settings. Default to TOML", + Flags: []cli.Flag{ + cli.BoolFlag{ + Name: "json", + Usage: "Format output as JSON", + }, + }, Action: func(context *cli.Context) error { - return handleSettings(defaultOutputFile, context.App.Metadata) + return handleSettings(defaultOutputFile, context) }, } diff --git a/cli/kata-env_test.go b/cli/kata-env_test.go index d8a9100cd8..84971fd111 100644 --- a/cli/kata-env_test.go +++ b/cli/kata-env_test.go @@ -7,6 +7,7 @@ package main import ( "bytes" + "flag" "fmt" "io/ioutil" "os" @@ -717,7 +718,7 @@ func testEnvShowSettings(t *testing.T, tmpdir string, tmpfile *os.File) error { Host: expectedHostDetails, } - err = showSettings(env, tmpfile) + err = writeTOMLSettings(env, tmpfile) if err != nil { return err } @@ -906,6 +907,13 @@ func TestEnvCLIFunction(t *testing.T) { err = fn(ctx) assert.NoError(t, err) + + set := flag.NewFlagSet("", 0) + set.Bool("json", true, "") + ctx = cli.NewContext(app, set, nil) + + err = fn(ctx) + assert.NoError(t, err) } func TestEnvCLIFunctionFail(t *testing.T) { From 89e22e5e8227cc3b49b496fff278d696fe01abcb Mon Sep 17 00:00:00 2001 From: Graham Whaley Date: Tue, 10 Jul 2018 20:04:27 +0100 Subject: [PATCH 14/39] kata-env: Fix test cases for kata-env JSON With the addition of the JSON kata-env output, we need to fix up the tests: - add a test for the JSON flag - fix the format/layout of the other tests to take into account the change in function API and the additon of a flagset to the cmdline ctx. Signed-off-by: Graham Whaley --- cli/kata-env_test.go | 65 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 50 insertions(+), 15 deletions(-) diff --git a/cli/kata-env_test.go b/cli/kata-env_test.go index 84971fd111..6cf1fb0f65 100644 --- a/cli/kata-env_test.go +++ b/cli/kata-env_test.go @@ -783,7 +783,11 @@ func TestEnvHandleSettings(t *testing.T) { _, err = getExpectedSettings(config, tmpdir, configFile) assert.NoError(t, err) - m := map[string]interface{}{ + app := cli.NewApp() + set := flag.NewFlagSet("test", flag.ContinueOnError) + ctx := cli.NewContext(app, set, nil) + app.Name = "foo" + ctx.App.Metadata = map[string]interface{}{ "configFile": configFile, "runtimeConfig": config, } @@ -792,7 +796,7 @@ func TestEnvHandleSettings(t *testing.T) { assert.NoError(t, err) defer os.Remove(tmpfile.Name()) - err = handleSettings(tmpfile, m) + err = handleSettings(tmpfile, ctx) assert.NoError(t, err) var env EnvInfo @@ -816,7 +820,10 @@ func TestEnvHandleSettingsInvalidShimConfig(t *testing.T) { config.ShimConfig = "invalid shim config" - m := map[string]interface{}{ + app := cli.NewApp() + ctx := cli.NewContext(app, nil, nil) + app.Name = "foo" + ctx.App.Metadata = map[string]interface{}{ "configFile": configFile, "runtimeConfig": config, } @@ -825,47 +832,75 @@ func TestEnvHandleSettingsInvalidShimConfig(t *testing.T) { assert.NoError(err) defer os.Remove(tmpfile.Name()) - err = handleSettings(tmpfile, m) + err = handleSettings(tmpfile, ctx) assert.Error(err) } func TestEnvHandleSettingsInvalidParams(t *testing.T) { - err := handleSettings(nil, map[string]interface{}{}) - assert.Error(t, err) + assert := assert.New(t) + + tmpdir, err := ioutil.TempDir("", "") + assert.NoError(err) + defer os.RemoveAll(tmpdir) + + configFile, _, err := makeRuntimeConfig(tmpdir) + assert.NoError(err) + + app := cli.NewApp() + ctx := cli.NewContext(app, nil, nil) + app.Name = "foo" + ctx.App.Metadata = map[string]interface{}{ + "configFile": configFile, + } + err = handleSettings(nil, ctx) + assert.Error(err) } func TestEnvHandleSettingsEmptyMap(t *testing.T) { - err := handleSettings(os.Stdout, map[string]interface{}{}) + app := cli.NewApp() + ctx := cli.NewContext(app, nil, nil) + app.Name = "foo" + ctx.App.Metadata = map[string]interface{}{} + err := handleSettings(os.Stdout, ctx) assert.Error(t, err) } func TestEnvHandleSettingsInvalidFile(t *testing.T) { - m := map[string]interface{}{ + app := cli.NewApp() + ctx := cli.NewContext(app, nil, nil) + app.Name = "foo" + ctx.App.Metadata = map[string]interface{}{ "configFile": "foo", "runtimeConfig": oci.RuntimeConfig{}, } - err := handleSettings(nil, m) + err := handleSettings(nil, ctx) assert.Error(t, err) } func TestEnvHandleSettingsInvalidConfigFileType(t *testing.T) { - m := map[string]interface{}{ + app := cli.NewApp() + ctx := cli.NewContext(app, nil, nil) + app.Name = "foo" + ctx.App.Metadata = map[string]interface{}{ "configFile": 123, "runtimeConfig": oci.RuntimeConfig{}, } - err := handleSettings(os.Stderr, m) + err := handleSettings(os.Stderr, ctx) assert.Error(t, err) } func TestEnvHandleSettingsInvalidRuntimeConfigType(t *testing.T) { - m := map[string]interface{}{ + app := cli.NewApp() + ctx := cli.NewContext(app, nil, nil) + app.Name = "foo" + ctx.App.Metadata = map[string]interface{}{ "configFile": "/some/where", "runtimeConfig": true, } - err := handleSettings(os.Stderr, m) + err := handleSettings(os.Stderr, ctx) assert.Error(t, err) } @@ -883,7 +918,8 @@ func TestEnvCLIFunction(t *testing.T) { assert.NoError(t, err) app := cli.NewApp() - ctx := cli.NewContext(app, nil, nil) + set := flag.NewFlagSet("test", flag.ContinueOnError) + ctx := cli.NewContext(app, set, nil) app.Name = "foo" ctx.App.Metadata = map[string]interface{}{ @@ -908,7 +944,6 @@ func TestEnvCLIFunction(t *testing.T) { err = fn(ctx) assert.NoError(t, err) - set := flag.NewFlagSet("", 0) set.Bool("json", true, "") ctx = cli.NewContext(app, set, nil) From ceac6fdc96ac874d7a0cc208c64e21b9f0876051 Mon Sep 17 00:00:00 2001 From: Graham Whaley Date: Wed, 11 Jul 2018 17:10:06 +0100 Subject: [PATCH 15/39] kata-env: Do not leave temp files on test One of the test cases was not defer removing the tmpfile it uses. Add that defer. Signed-off-by: Graham Whaley --- cli/kata-env_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cli/kata-env_test.go b/cli/kata-env_test.go index 6cf1fb0f65..417e6240e8 100644 --- a/cli/kata-env_test.go +++ b/cli/kata-env_test.go @@ -762,6 +762,7 @@ func TestEnvShowSettingsInvalidFile(t *testing.T) { tmpfile, err := ioutil.TempFile("", "envShowSettings-") assert.NoError(t, err) + defer os.Remove(tmpfile.Name()) // close the file tmpfile.Close() From 8a1469bc37a410582d72af1bdbfd7bf907343fc5 Mon Sep 17 00:00:00 2001 From: Graham Whaley Date: Wed, 11 Jul 2018 17:21:40 +0100 Subject: [PATCH 16/39] kata-env: tests: add JSON out/in verify test Add a test to ensure the JSON output passes the same parameter check and write/re-read test as the TOML one. Signed-off-by: Graham Whaley --- cli/kata-env_test.go | 90 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 87 insertions(+), 3 deletions(-) diff --git a/cli/kata-env_test.go b/cli/kata-env_test.go index 417e6240e8..cf285c7c40 100644 --- a/cli/kata-env_test.go +++ b/cli/kata-env_test.go @@ -7,6 +7,7 @@ package main import ( "bytes" + "encoding/json" "flag" "fmt" "io/ioutil" @@ -669,7 +670,7 @@ func TestEnvGetAgentInfo(t *testing.T) { assert.Equal(t, expectedAgent, agent) } -func testEnvShowSettings(t *testing.T, tmpdir string, tmpfile *os.File) error { +func testEnvShowTOMLSettings(t *testing.T, tmpdir string, tmpfile *os.File) error { runtime := RuntimeInfo{} @@ -738,6 +739,77 @@ func testEnvShowSettings(t *testing.T, tmpdir string, tmpfile *os.File) error { return nil } +func testEnvShowJSONSettings(t *testing.T, tmpdir string, tmpfile *os.File) error { + + runtime := RuntimeInfo{} + + hypervisor := HypervisorInfo{ + Path: "/resolved/hypervisor/path", + MachineType: "hypervisor-machine-type", + } + + image := ImageInfo{ + Path: "/resolved/image/path", + } + + kernel := KernelInfo{ + Path: "/kernel/path", + Parameters: "foo=bar xyz", + } + + proxy := ProxyInfo{ + Type: "proxy-type", + Version: "proxy-version", + Path: "file:///proxy-url", + Debug: false, + } + + shim := ShimInfo{ + Type: "shim-type", + Version: "shim-version", + Path: "/resolved/shim/path", + } + + agent := AgentInfo{ + Type: "agent-type", + } + + expectedHostDetails, err := getExpectedHostDetails(tmpdir) + assert.NoError(t, err) + + env := EnvInfo{ + Runtime: runtime, + Hypervisor: hypervisor, + Image: image, + Kernel: kernel, + Proxy: proxy, + Shim: shim, + Agent: agent, + Host: expectedHostDetails, + } + + err = writeJSONSettings(env, tmpfile) + if err != nil { + return err + } + + contents, err := getFileContents(tmpfile.Name()) + assert.NoError(t, err) + + buf := new(bytes.Buffer) + encoder := json.NewEncoder(buf) + // Ensure we have the same human readable layout + encoder.SetIndent("", " ") + err = encoder.Encode(env) + assert.NoError(t, err) + + expectedContents := buf.String() + + assert.Equal(t, expectedContents, contents) + + return nil +} + func TestEnvShowSettings(t *testing.T) { tmpdir, err := ioutil.TempDir("", "") if err != nil { @@ -749,7 +821,13 @@ func TestEnvShowSettings(t *testing.T) { assert.NoError(t, err) defer os.Remove(tmpfile.Name()) - err = testEnvShowSettings(t, tmpdir, tmpfile) + err = testEnvShowTOMLSettings(t, tmpdir, tmpfile) + assert.NoError(t, err) + + // Reset the file to empty for next test + tmpfile.Truncate(0) + tmpfile.Seek(0, 0) + err = testEnvShowJSONSettings(t, tmpdir, tmpfile) assert.NoError(t, err) } @@ -767,7 +845,13 @@ func TestEnvShowSettingsInvalidFile(t *testing.T) { // close the file tmpfile.Close() - err = testEnvShowSettings(t, tmpdir, tmpfile) + err = testEnvShowTOMLSettings(t, tmpdir, tmpfile) + assert.Error(t, err) + + // Reset the file to empty for next test + tmpfile.Truncate(0) + tmpfile.Seek(0, 0) + err = testEnvShowJSONSettings(t, tmpdir, tmpfile) assert.Error(t, err) } From 8c5be5146a21830f85a0710373d87aabc0712a5a Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Mon, 16 Jul 2018 10:58:38 -0700 Subject: [PATCH 17/39] makefile: update PREFIX to remove redundant slash Fixes: #488 Signed-off-by: Eric Ernst --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index cf68c09bc1..c77ddf3657 100644 --- a/Makefile +++ b/Makefile @@ -54,7 +54,7 @@ DESTDIR := / installing = $(findstring install,$(MAKECMDGOALS)) ifeq ($(PREFIX),) -PREFIX := /usr/ +PREFIX := /usr EXEC_PREFIX := $(PREFIX)/local else EXEC_PREFIX := $(PREFIX) From a712155bb937d2fe23d0daefa1e81e33ec8ab4a4 Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Wed, 18 Jul 2018 00:45:21 +0800 Subject: [PATCH 18/39] kata_agent: print request details It helps tracking each request that is sent and we can match with the one printed by kata-agent on the guest side to find out any stack requests in the middle. Fixes: #494 Signed-off-by: Peng Tao --- virtcontainers/kata_agent.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 9bb5c796dd..896cd63277 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -1269,6 +1269,8 @@ func (k *kataAgent) sendReq(request interface{}) (interface{}, error) { if msgName == "" || handler == nil { return nil, errors.New("Invalid request type") } + message := request.(proto.Message) + k.Logger().WithField("name", msgName).WithField("req", message.String()).Debug("sending request") return handler(context.Background(), request) } From b0e5614a399abada62e6b13ffacd6eeaa6420294 Mon Sep 17 00:00:00 2001 From: Graham Whaley Date: Fri, 13 Jul 2018 17:44:23 +0100 Subject: [PATCH 19/39] cli: update_test: defer remove tmpfile Ensure we remove the tmpfile used for testing. Signed-off-by: Graham Whaley --- cli/update_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cli/update_test.go b/cli/update_test.go index e2c2e38ce1..713382f8b2 100644 --- a/cli/update_test.go +++ b/cli/update_test.go @@ -111,6 +111,7 @@ func TestUpdateCLIFailure(t *testing.T) { // json decode error f, err := ioutil.TempFile("", "resources") + defer os.Remove(f.Name()) assert.NoError(err) assert.NotNil(f) f.WriteString("no json") From 9fac082d2459bfad31e91b3c0f5f0f1a89d7255a Mon Sep 17 00:00:00 2001 From: Graham Whaley Date: Wed, 18 Jul 2018 10:55:30 +0100 Subject: [PATCH 20/39] cli: tests: remove the tmpdir to the config.json We were defer removing the temporary config.json files but not the tmpdir path we had created to store them in. Expose that path out so we can defer removeall it. Fixes: #480 Signed-off-by: Graham Whaley --- cli/delete_test.go | 24 +++++++++++++++--------- cli/exec_test.go | 24 ++++++++++++++++-------- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/cli/delete_test.go b/cli/delete_test.go index 20fea73c8c..2a927bcc4e 100644 --- a/cli/delete_test.go +++ b/cli/delete_test.go @@ -121,7 +121,7 @@ func TestDeleteInvalidConfig(t *testing.T) { assert.False(vcmock.IsMockError(err)) } -func testConfigSetup(t *testing.T) string { +func testConfigSetup(t *testing.T) (rootPath string, configPath string) { assert := assert.New(t) tmpdir, err := ioutil.TempDir("", "") @@ -135,8 +135,8 @@ func testConfigSetup(t *testing.T) string { assert.NoError(err) // config json path - configPath := filepath.Join(bundlePath, "config.json") - return configPath + configPath = filepath.Join(bundlePath, "config.json") + return tmpdir, configPath } func TestDeleteSandbox(t *testing.T) { @@ -146,7 +146,8 @@ func TestDeleteSandbox(t *testing.T) { MockID: testSandboxID, } - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -223,7 +224,8 @@ func TestDeleteInvalidContainerType(t *testing.T) { MockID: testSandboxID, } - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -261,7 +263,8 @@ func TestDeleteSandboxRunning(t *testing.T) { MockID: testSandboxID, } - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -340,7 +343,8 @@ func TestDeleteRunningContainer(t *testing.T) { }, } - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -422,7 +426,8 @@ func TestDeleteContainer(t *testing.T) { }, } - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -523,7 +528,8 @@ func TestDeleteCLIFunctionSuccess(t *testing.T) { }, } - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) diff --git a/cli/exec_test.go b/cli/exec_test.go index d45bd3091b..3fdf8b8f03 100644 --- a/cli/exec_test.go +++ b/cli/exec_test.go @@ -89,7 +89,8 @@ func TestExecuteErrors(t *testing.T) { assert.False(vcmock.IsMockError(err)) // Container state undefined - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -147,7 +148,8 @@ func TestExecuteErrorReadingProcessJson(t *testing.T) { flagSet.Parse([]string{testContainerID}) ctx := cli.NewContext(cli.NewApp(), flagSet, nil) - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -195,7 +197,8 @@ func TestExecuteErrorOpeningConsole(t *testing.T) { flagSet.Parse([]string{testContainerID}) ctx := cli.NewContext(cli.NewApp(), flagSet, nil) - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -261,7 +264,8 @@ func TestExecuteWithFlags(t *testing.T) { flagSet.Parse([]string{testContainerID, "/tmp/foo"}) ctx := cli.NewContext(cli.NewApp(), flagSet, nil) - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -349,7 +353,8 @@ func TestExecuteWithFlagsDetached(t *testing.T) { flagSet.Parse([]string{testContainerID, "/tmp/foo"}) ctx := cli.NewContext(cli.NewApp(), flagSet, nil) - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -427,7 +432,8 @@ func TestExecuteWithInvalidProcessJson(t *testing.T) { flagSet.Parse([]string{testContainerID}) ctx := cli.NewContext(cli.NewApp(), flagSet, nil) - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -478,7 +484,8 @@ func TestExecuteWithValidProcessJson(t *testing.T) { flagSet.Parse([]string{testContainerID, "/tmp/foo"}) ctx := cli.NewContext(cli.NewApp(), flagSet, nil) - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) @@ -578,7 +585,8 @@ func TestExecuteWithEmptyEnvironmentValue(t *testing.T) { flagSet.Parse([]string{testContainerID}) ctx := cli.NewContext(cli.NewApp(), flagSet, nil) - configPath := testConfigSetup(t) + rootPath, configPath := testConfigSetup(t) + defer os.RemoveAll(rootPath) configJSON, err := readOCIConfigJSON(configPath) assert.NoError(err) From a3b11dfed9831e123f01cf74068e9f0d00686072 Mon Sep 17 00:00:00 2001 From: Graham Whaley Date: Wed, 18 Jul 2018 11:05:02 +0100 Subject: [PATCH 21/39] cli: tests: Clarify who cleans up tmpdir Add a comment to clarify that the caller of testRunContainerSetup() cleans up the tmpdir. Signed-off-by: Graham Whaley --- cli/run_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cli/run_test.go b/cli/run_test.go index 9377a4900b..2d051b5342 100644 --- a/cli/run_test.go +++ b/cli/run_test.go @@ -168,6 +168,7 @@ func testRunContainerSetup(t *testing.T) runContainerData { assert.NoError(err, "unable to start fake container workload %+v: %s", workload, err) // temporal dir to place container files + // Note - it is returned to the caller, who does the defer remove to clean up. tmpdir, err := ioutil.TempDir("", "") assert.NoError(err) From b2d9d95ace6879a4d4170c18be8735293de98b31 Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Mon, 23 Jul 2018 08:32:11 +0800 Subject: [PATCH 22/39] sandbox: change container slice to a map ContainerID is supposed to be unique within a sandbox. It is better to use a map to describe containers of a sandbox. Fixes: #502 Signed-off-by: Peng Tao --- virtcontainers/filesystem_test.go | 5 ++-- virtcontainers/sandbox.go | 41 +++++++++++++++++-------------- virtcontainers/sandbox_test.go | 41 ++++++++++++++----------------- 3 files changed, 45 insertions(+), 42 deletions(-) diff --git a/virtcontainers/filesystem_test.go b/virtcontainers/filesystem_test.go index 7341b61ca5..7687acb917 100644 --- a/virtcontainers/filesystem_test.go +++ b/virtcontainers/filesystem_test.go @@ -34,6 +34,7 @@ func TestFilesystemCreateAllResourcesSuccessful(t *testing.T) { storage: fs, config: sandboxConfig, devManager: manager.NewDeviceManager(manager.VirtioBlock), + containers: map[string]*Container{}, } if err := sandbox.newContainers(); err != nil { @@ -110,8 +111,8 @@ func TestFilesystemCreateAllResourcesFailingSandboxIDEmpty(t *testing.T) { func TestFilesystemCreateAllResourcesFailingContainerIDEmpty(t *testing.T) { fs := &filesystem{} - containers := []*Container{ - {id: ""}, + containers := map[string]*Container{ + testContainerID: {}, } sandbox := &Sandbox{ diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index b289db5769..9422599821 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -452,7 +452,7 @@ type Sandbox struct { volumes []Volume - containers []*Container + containers map[string]*Container runPath string configPath string @@ -521,8 +521,10 @@ func (s *Sandbox) GetAnnotations() map[string]string { func (s *Sandbox) GetAllContainers() []VCContainer { ifa := make([]VCContainer, len(s.containers)) - for i, v := range s.containers { + i := 0 + for _, v := range s.containers { ifa[i] = v + i++ } return ifa @@ -530,8 +532,8 @@ func (s *Sandbox) GetAllContainers() []VCContainer { // GetContainer returns the container named by the containerID. func (s *Sandbox) GetContainer(containerID string) VCContainer { - for _, c := range s.containers { - if c.id == containerID { + for id, c := range s.containers { + if id == containerID { return c } } @@ -741,6 +743,7 @@ func newSandbox(sandboxConfig SandboxConfig) (*Sandbox, error) { config: &sandboxConfig, devManager: deviceManager.NewDeviceManager(sandboxConfig.HypervisorConfig.BlockDeviceDriver), volumes: sandboxConfig.Volumes, + containers: map[string]*Container{}, runPath: filepath.Join(runStoragePath, sandboxConfig.ID), configPath: filepath.Join(configStoragePath, sandboxConfig.ID), state: State{}, @@ -794,8 +797,8 @@ func (s *Sandbox) storeSandbox() error { return err } - for _, container := range s.containers { - err = s.storage.storeContainerResource(s.id, container.id, configFileType, *(container.config)) + for id, container := range s.containers { + err = s.storage.storeContainerResource(s.id, id, configFileType, *(container.config)) if err != nil { return err } @@ -846,8 +849,8 @@ func (s *Sandbox) findContainer(containerID string) (*Container, error) { return nil, errNeedContainerID } - for _, c := range s.containers { - if containerID == c.id { + for id, c := range s.containers { + if containerID == id { return c, nil } } @@ -867,15 +870,14 @@ func (s *Sandbox) removeContainer(containerID string) error { return errNeedContainerID } - for idx, c := range s.containers { - if containerID == c.id { - s.containers = append(s.containers[:idx], s.containers[idx+1:]...) - return nil - } + if _, ok := s.containers[containerID]; !ok { + return fmt.Errorf("Could not remove the container %q from the sandbox %q containers list", + containerID, s.id) } - return fmt.Errorf("Could not remove the container %q from the sandbox %q containers list", - containerID, s.id) + delete(s.containers, containerID) + + return nil } // Delete deletes an already created sandbox. @@ -954,7 +956,10 @@ func (s *Sandbox) startVM() error { } func (s *Sandbox) addContainer(c *Container) error { - s.containers = append(s.containers, c) + if _, ok := s.containers[c.id]; ok { + return fmt.Errorf("Duplicated container: %s", c.id) + } + s.containers[c.id] = c return nil } @@ -1066,8 +1071,8 @@ func (s *Sandbox) StatusContainer(containerID string) (ContainerStatus, error) { return ContainerStatus{}, errNeedContainerID } - for _, c := range s.containers { - if c.id == containerID { + for id, c := range s.containers { + if id == containerID { return ContainerStatus{ ID: c.id, State: c.state, diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index eec5a0d2aa..958e9e054b 100644 --- a/virtcontainers/sandbox_test.go +++ b/virtcontainers/sandbox_test.go @@ -634,8 +634,8 @@ func TestSandboxSetContainersStateFailingEmptySandboxID(t *testing.T) { storage: &filesystem{}, } - containers := []*Container{ - { + containers := map[string]*Container{ + "100": { id: "100", sandbox: sandbox, }, @@ -787,11 +787,11 @@ func TestSandboxDeleteContainersStateFailingEmptySandboxID(t *testing.T) { func TestGetContainer(t *testing.T) { containerIDs := []string{"abc", "123", "xyz", "rgb"} - containers := []*Container{} + containers := map[string]*Container{} for _, id := range containerIDs { c := Container{id: id} - containers = append(containers, &c) + containers[id] = &c } sandbox := Sandbox{ @@ -813,11 +813,11 @@ func TestGetContainer(t *testing.T) { func TestGetAllContainers(t *testing.T) { containerIDs := []string{"abc", "123", "xyz", "rgb"} - containers := []*Container{} + containers := map[string]*Container{} for _, id := range containerIDs { - c := Container{id: id} - containers = append(containers, &c) + c := &Container{id: id} + containers[id] = c } sandbox := Sandbox{ @@ -826,8 +826,8 @@ func TestGetAllContainers(t *testing.T) { list := sandbox.GetAllContainers() - for i, c := range list { - if c.ID() != containerIDs[i] { + for _, c := range list { + if containers[c.ID()] == nil { t.Fatal() } } @@ -1168,19 +1168,20 @@ func TestSandboxAttachDevicesVFIO(t *testing.T) { }, } - containers := []*Container{c} + containers := map[string]*Container{} + containers[c.id] = c sandbox := Sandbox{ containers: containers, hypervisor: &mockHypervisor{}, } - containers[0].sandbox = &sandbox + containers[c.id].sandbox = &sandbox - err = containers[0].attachDevices() + err = containers[c.id].attachDevices() assert.Nil(t, err, "Error while attaching devices %s", err) - err = containers[0].detachDevices() + err = containers[c.id].detachDevices() assert.Nil(t, err, "Error while detaching devices %s", err) } @@ -1256,17 +1257,15 @@ func TestFindContainerNoContainerMatchFailure(t *testing.T) { func TestFindContainerSuccess(t *testing.T) { sandbox := &Sandbox{ - containers: []*Container{ - { - id: testContainerID, - }, + containers: map[string]*Container{ + testContainerID: {id: testContainerID}, }, } c, err := sandbox.findContainer(testContainerID) assert.NotNil(t, c, "Container pointer should not be nil") assert.Nil(t, err, "Should not have returned an error: %v", err) - assert.True(t, c == sandbox.containers[0], "Container pointers should point to the same address") + assert.True(t, c == sandbox.containers[testContainerID], "Container pointers should point to the same address") } func TestRemoveContainerSandboxNilFailure(t *testing.T) { @@ -1285,10 +1284,8 @@ func TestRemoveContainerNoContainerMatchFailure(t *testing.T) { func TestRemoveContainerSuccess(t *testing.T) { sandbox := &Sandbox{ - containers: []*Container{ - { - id: testContainerID, - }, + containers: map[string]*Container{ + testContainerID: {id: testContainerID}, }, } err := sandbox.removeContainer(testContainerID) From 3438a116bbd911519a8ada5631a8026e29a0837e Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Tue, 24 Jul 2018 11:11:17 -0700 Subject: [PATCH 23/39] codecov: Explicitly disable codecov/patch coverage Because codecov coverage regarding the patch is very inconsistent, this commit introduces codecov.yml config file in order to disable this check. Fixes #511 Signed-off-by: Sebastien Boeuf --- codecov.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 codecov.yml diff --git a/codecov.yml b/codecov.yml new file mode 100644 index 0000000000..a41de4b6a7 --- /dev/null +++ b/codecov.yml @@ -0,0 +1,11 @@ +# +# Copyright (c) 2018 Intel Corporation +# +# SPDX-License-Identifier: Apache-2.0 +# + +coverage: + status: + patch: + default: + enabled: no From 7ca24d692febe352cda48b15e806c3006f7e4c84 Mon Sep 17 00:00:00 2001 From: George Kennedy Date: Wed, 25 Jul 2018 12:05:47 -0400 Subject: [PATCH 24/39] cli: add AMD support to kata-check Added support for identifying AMD CPUs in the `kata-check` CLI command. Signed-off-by: George Kennedy Fixes #476. --- cli/kata-check.go | 2 + cli/kata-check_amd64.go | 130 +++++++++++++++++++++++++++-------- cli/kata-check_amd64_test.go | 75 ++++++++++++++------ cli/kata-check_arm64.go | 3 + cli/kata-check_ppc64le.go | 3 + 5 files changed, 163 insertions(+), 50 deletions(-) diff --git a/cli/kata-check.go b/cli/kata-check.go index 89c59d33d9..7b13892540 100644 --- a/cli/kata-check.go +++ b/cli/kata-check.go @@ -283,6 +283,8 @@ var kataCheckCLICommand = cli.Command{ Usage: "tests if system can run " + project, Action: func(context *cli.Context) error { + setCPUtype() + details := vmContainerCapableDetails{ cpuInfoFile: procCPUInfo, requiredCPUFlags: archRequiredCPUFlags, diff --git a/cli/kata-check_amd64.go b/cli/kata-check_amd64.go index a642602059..d3b6076ae3 100644 --- a/cli/kata-check_amd64.go +++ b/cli/kata-check_amd64.go @@ -7,52 +7,124 @@ package main import ( "github.com/sirupsen/logrus" + "io/ioutil" + "strings" ) const ( cpuFlagsTag = genericCPUFlagsTag archCPUVendorField = genericCPUVendorField archCPUModelField = genericCPUModelField + archGenuineIntel = "GenuineIntel" + archAuthenticAMD = "AuthenticAMD" + msgKernelVM = "Kernel-based Virtual Machine" + msgKernelVirtio = "Host kernel accelerator for virtio" + msgKernelVirtioNet = "Host kernel accelerator for virtio network" ) +// CPU types +const ( + cpuTypeIntel = 0 + cpuTypeAMD = 1 + cpuTypeUnknown = -1 +) + +// cpuType save the CPU type +var cpuType int + // archRequiredCPUFlags maps a CPU flag value to search for and a // human-readable description of that value. -var archRequiredCPUFlags = map[string]string{ - "vmx": "Virtualization support", - "lm": "64Bit CPU", - "sse4_1": "SSE4.1", -} +var archRequiredCPUFlags map[string]string // archRequiredCPUAttribs maps a CPU (non-CPU flag) attribute value to search for // and a human-readable description of that value. -var archRequiredCPUAttribs = map[string]string{ - "GenuineIntel": "Intel Architecture CPU", -} +var archRequiredCPUAttribs map[string]string // archRequiredKernelModules maps a required module name to a human-readable // description of the modules functionality and an optional list of // required module parameters. -var archRequiredKernelModules = map[string]kernelModule{ - "kvm": { - desc: "Kernel-based Virtual Machine", - }, - "kvm_intel": { - desc: "Intel KVM", - parameters: map[string]string{ - "nested": "Y", - // "VMX Unrestricted mode support". This is used - // as a heuristic to determine if the system is - // "new enough" to run a Kata Container - // (atleast a Westmere). - "unrestricted_guest": "Y", - }, - }, - "vhost": { - desc: "Host kernel accelerator for virtio", - }, - "vhost_net": { - desc: "Host kernel accelerator for virtio network", - }, +var archRequiredKernelModules map[string]kernelModule + +func setCPUtype() { + cpuType = getCPUtype() + + if cpuType == cpuTypeUnknown { + kataLog.Fatal("Unknown CPU Type") + exit(1) + } else if cpuType == cpuTypeIntel { + archRequiredCPUFlags = map[string]string{ + "vmx": "Virtualization support", + "lm": "64Bit CPU", + "sse4_1": "SSE4.1", + } + archRequiredCPUAttribs = map[string]string{ + archGenuineIntel: "Intel Architecture CPU", + } + archRequiredKernelModules = map[string]kernelModule{ + "kvm": { + desc: msgKernelVM, + }, + "kvm_intel": { + desc: "Intel KVM", + parameters: map[string]string{ + "nested": "Y", + // "VMX Unrestricted mode support". This is used + // as a heuristic to determine if the system is + // "new enough" to run a Kata Container + // (atleast a Westmere). + "unrestricted_guest": "Y", + }, + }, + "vhost": { + desc: msgKernelVirtio, + }, + "vhost_net": { + desc: msgKernelVirtioNet, + }, + } + } else if cpuType == cpuTypeAMD { + archRequiredCPUFlags = map[string]string{ + "svm": "Virtualization support", + "lm": "64Bit CPU", + "sse4_1": "SSE4.1", + } + archRequiredCPUAttribs = map[string]string{ + archAuthenticAMD: "AMD Architecture CPU", + } + archRequiredKernelModules = map[string]kernelModule{ + "kvm": { + desc: msgKernelVM, + }, + "kvm_amd": { + desc: "AMD KVM", + parameters: map[string]string{ + "nested": "1", + }, + }, + "vhost": { + desc: msgKernelVirtio, + }, + "vhost_net": { + desc: msgKernelVirtioNet, + }, + } + } +} + +func getCPUtype() int { + content, err := ioutil.ReadFile("/proc/cpuinfo") + if err != nil { + kataLog.WithError(err).Error("failed to read file") + return cpuTypeUnknown + } + str := string(content) + if strings.Contains(str, archGenuineIntel) { + return cpuTypeIntel + } else if strings.Contains(str, archAuthenticAMD) { + return cpuTypeAMD + } else { + return cpuTypeUnknown + } } // kvmIsUsable determines if it will be possible to create a full virtual machine diff --git a/cli/kata-check_amd64_test.go b/cli/kata-check_amd64_test.go index 1d28e44937..f8c39784ca 100644 --- a/cli/kata-check_amd64_test.go +++ b/cli/kata-check_amd64_test.go @@ -73,13 +73,26 @@ func TestCCCheckCLIFunction(t *testing.T) { t.Fatal(err) } - cpuData := []testCPUData{ - {"GenuineIntel", "lm vmx sse4_1", false}, - } + var cpuData []testCPUData + var moduleData []testModuleData - moduleData := []testModuleData{ - {filepath.Join(sysModuleDir, "kvm_intel/parameters/unrestricted_guest"), false, "Y"}, - {filepath.Join(sysModuleDir, "kvm_intel/parameters/nested"), false, "Y"}, + if cpuType == cpuTypeIntel { + cpuData = []testCPUData{ + {archGenuineIntel, "lm vmx sse4_1", false}, + } + + moduleData = []testModuleData{ + {filepath.Join(sysModuleDir, "kvm_intel/parameters/unrestricted_guest"), false, "Y"}, + {filepath.Join(sysModuleDir, "kvm_intel/parameters/nested"), false, "Y"}, + } + } else if cpuType == cpuTypeAMD { + cpuData = []testCPUData{ + {archAuthenticAMD, "lm svm sse4_1", false}, + } + + moduleData = []testModuleData{ + {filepath.Join(sysModuleDir, "kvm_amd/parameters/nested"), false, "1"}, + } } devNull, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0666) @@ -175,7 +188,7 @@ func TestCheckCheckKernelModulesNoNesting(t *testing.T) { {filepath.Join(sysModuleDir, "kvm_intel/parameters/nested"), false, "N"}, } - vendor := "GenuineIntel" + vendor := archGenuineIntel flags := "vmx lm sse4_1 hypervisor" _, err = checkKernelModules(requiredModules, archKernelParamHandler) @@ -259,7 +272,7 @@ func TestCheckCheckKernelModulesNoUnrestrictedGuest(t *testing.T) { {filepath.Join(sysModuleDir, "kvm_intel/parameters/unrestricted_guest"), false, "N"}, } - vendor := "GenuineIntel" + vendor := archGenuineIntel flags := "vmx lm sse4_1" _, err = checkKernelModules(requiredModules, archKernelParamHandler) @@ -334,20 +347,40 @@ func TestCheckHostIsVMContainerCapable(t *testing.T) { t.Fatal(err) } - cpuData := []testCPUData{ - {"", "", true}, - {"Intel", "", true}, - {"GenuineIntel", "", true}, - {"GenuineIntel", "lm", true}, - {"GenuineIntel", "lm vmx", true}, - {"GenuineIntel", "lm vmx sse4_1", false}, - } + var cpuData []testCPUData + var moduleData []testModuleData + + if cpuType == cpuTypeIntel { + cpuData = []testCPUData{ + {"", "", true}, + {"Intel", "", true}, + {archGenuineIntel, "", true}, + {archGenuineIntel, "lm", true}, + {archGenuineIntel, "lm vmx", true}, + {archGenuineIntel, "lm vmx sse4_1", false}, + } - moduleData := []testModuleData{ - {filepath.Join(sysModuleDir, "kvm"), true, ""}, - {filepath.Join(sysModuleDir, "kvm_intel"), true, ""}, - {filepath.Join(sysModuleDir, "kvm_intel/parameters/nested"), false, "Y"}, - {filepath.Join(sysModuleDir, "kvm_intel/parameters/unrestricted_guest"), false, "Y"}, + moduleData = []testModuleData{ + {filepath.Join(sysModuleDir, "kvm"), true, ""}, + {filepath.Join(sysModuleDir, "kvm_intel"), true, ""}, + {filepath.Join(sysModuleDir, "kvm_intel/parameters/nested"), false, "Y"}, + {filepath.Join(sysModuleDir, "kvm_intel/parameters/unrestricted_guest"), false, "Y"}, + } + } else if cpuType == cpuTypeAMD { + cpuData = []testCPUData{ + {"", "", true}, + {"AMD", "", true}, + {archAuthenticAMD, "", true}, + {archAuthenticAMD, "lm", true}, + {archAuthenticAMD, "lm svm", true}, + {archAuthenticAMD, "lm svm sse4_1", false}, + } + + moduleData = []testModuleData{ + {filepath.Join(sysModuleDir, "kvm"), true, ""}, + {filepath.Join(sysModuleDir, "kvm_amd"), true, ""}, + {filepath.Join(sysModuleDir, "kvm_amd/parameters/nested"), false, "1"}, + } } setupCheckHostIsVMContainerCapable(assert, cpuInfoFile, cpuData, moduleData) diff --git a/cli/kata-check_arm64.go b/cli/kata-check_arm64.go index 97af7d433a..20c24c4f4f 100644 --- a/cli/kata-check_arm64.go +++ b/cli/kata-check_arm64.go @@ -40,6 +40,9 @@ var archRequiredKernelModules = map[string]kernelModule{ }, } +func setCPUtype() { +} + // kvmIsUsable determines if it will be possible to create a full virtual machine // by creating a minimal VM and then deleting it. func kvmIsUsable() error { diff --git a/cli/kata-check_ppc64le.go b/cli/kata-check_ppc64le.go index 9843df0d37..627a7e9dde 100644 --- a/cli/kata-check_ppc64le.go +++ b/cli/kata-check_ppc64le.go @@ -44,6 +44,9 @@ var archRequiredKernelModules = map[string]kernelModule{ }, } +func setCPUtype() { +} + func archHostCanCreateVMContainer() error { return kvmIsUsable() } From 28b299fb77cb108d20605586ec9b83388e4bad55 Mon Sep 17 00:00:00 2001 From: Jose Carlos Venegas Munoz Date: Thu, 26 Jul 2018 01:02:41 +0000 Subject: [PATCH 25/39] kata-agent: Improve error message. If the grpc connection check fails we only return the grpc error. To make more clear what failed add more information to the error. Signed-off-by: Jose Carlos Venegas Munoz --- virtcontainers/kata_agent.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 896cd63277..d91df8bf92 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -1148,8 +1148,12 @@ func (k *kataAgent) disconnect() error { return nil } +// check grpc server is serving func (k *kataAgent) check() error { _, err := k.sendReq(&grpc.CheckRequest{}) + if err != nil { + err = fmt.Errorf("Failed to check if grpc server is working: %s", err) + } return err } From e4ccf035f98bccde4c57bae13cda069886538da2 Mon Sep 17 00:00:00 2001 From: Jose Carlos Venegas Munoz Date: Thu, 26 Jul 2018 01:22:19 +0000 Subject: [PATCH 26/39] agent: check: Increase timeout check request. In some slow enviroments the agent is taking more than 5 seconds to start to serve grpc request. This was reproducible in a Centos VM with 4 cpus running 8 pods in parallel. Fixes: #516 Signed-off-by: Jose Carlos Venegas Munoz --- virtcontainers/kata_agent.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index d91df8bf92..b9ecc5688f 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -34,6 +34,7 @@ import ( ) var ( + checkRequestTimeout = 30 * time.Second defaultKataSocketName = "kata.sock" defaultKataChannel = "agent.channel.0" defaultKataDeviceID = "channel0" @@ -1197,7 +1198,7 @@ type reqFunc func(context.Context, interface{}, ...golangGrpc.CallOption) (inter func (k *kataAgent) installReqFunc(c *kataclient.AgentClient) { k.reqHandlers = make(map[string]reqFunc) k.reqHandlers["grpc.CheckRequest"] = func(ctx context.Context, req interface{}, opts ...golangGrpc.CallOption) (interface{}, error) { - ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + ctx, cancel := context.WithTimeout(ctx, checkRequestTimeout) defer cancel() return k.client.Check(ctx, req.(*grpc.CheckRequest), opts...) } From 21ae3231a37f928df8c0b8da7870cf43e9db3bef Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Wed, 1 Aug 2018 11:08:13 +0800 Subject: [PATCH 27/39] codecov: remove codecov.yml Now that we have a global team yaml, let's use it. Fixes: #540 Signed-off-by: Peng Tao --- codecov.yml | 11 ----------- 1 file changed, 11 deletions(-) delete mode 100644 codecov.yml diff --git a/codecov.yml b/codecov.yml deleted file mode 100644 index a41de4b6a7..0000000000 --- a/codecov.yml +++ /dev/null @@ -1,11 +0,0 @@ -# -# Copyright (c) 2018 Intel Corporation -# -# SPDX-License-Identifier: Apache-2.0 -# - -coverage: - status: - patch: - default: - enabled: no From f9446e54e1441ff326ed96a1faee1f1b16ed81aa Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Fri, 27 Jul 2018 13:23:36 +0100 Subject: [PATCH 28/39] kernel: Remove initcall_debug boot option Remove the `initcall_debug` boot option from the kernel command-line as we don't need it any more and it generates a ton of boot messages that may well be impacting performance. Fixes #526. Signed-off-by: James O. D. Hunt --- virtcontainers/qemu.go | 1 - virtcontainers/qemu_test.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 262928e7ad..482813eabb 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -70,7 +70,6 @@ var qemuMinorVersion int // agnostic list of kernel parameters var defaultKernelParameters = []Param{ {"panic", "1"}, - {"initcall_debug", ""}, } type operation int diff --git a/virtcontainers/qemu_test.go b/virtcontainers/qemu_test.go index 4470e8b3ac..f388cf6c08 100644 --- a/virtcontainers/qemu_test.go +++ b/virtcontainers/qemu_test.go @@ -52,7 +52,7 @@ func testQemuKernelParameters(t *testing.T, kernelParams []Param, expected strin } func TestQemuKernelParameters(t *testing.T) { - expectedOut := fmt.Sprintf("panic=1 initcall_debug nr_cpus=%d foo=foo bar=bar", MaxQemuVCPUs()) + expectedOut := fmt.Sprintf("panic=1 nr_cpus=%d foo=foo bar=bar", MaxQemuVCPUs()) params := []Param{ { Key: "foo", From 7b5f920b9fd8bcfe232fa15a8f8b8759ab0f1fbf Mon Sep 17 00:00:00 2001 From: Nitesh Konkar Date: Thu, 9 Aug 2018 11:36:48 +0530 Subject: [PATCH 29/39] virtcontainers: ppc64le: Add nvdimm to defaultQemuMachineOption nvdimm is fundamental to get rootfs approach working for Kata Containers on ppc64le. It should be added to the default qemu machine option list. Fixes: #561 Signed-off-by: Nitesh Konkar niteshkonkar@in.ibm.com --- virtcontainers/qemu_ppc64le.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/virtcontainers/qemu_ppc64le.go b/virtcontainers/qemu_ppc64le.go index ba259389eb..e852e4e138 100644 --- a/virtcontainers/qemu_ppc64le.go +++ b/virtcontainers/qemu_ppc64le.go @@ -24,7 +24,7 @@ const defaultQemuPath = "/usr/bin/qemu-system-ppc64le" const defaultQemuMachineType = QemuPseries -const defaultQemuMachineOptions = "accel=kvm,usb=off" +const defaultQemuMachineOptions = "accel=kvm,usb=off,nvdimm" const defaultPCBridgeBus = "pci.0" From a6668af5d2eca32d10c50c77c4d146081aff232d Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Mon, 23 Jul 2018 16:03:04 -0700 Subject: [PATCH 30/39] network: Error out when host-networking is requested Instead of continuing with the network setup, we should detect if host network namespace was requested and error out early. Fixes #499 Signed-off-by: Archana Shinde --- virtcontainers/network.go | 111 +++++++++++++++++++++++++++++++++ virtcontainers/network_test.go | 85 +++++++++++++++++++++++++ 2 files changed, 196 insertions(+) diff --git a/virtcontainers/network.go b/virtcontainers/network.go index 15401af655..96b40a0c26 100644 --- a/virtcontainers/network.go +++ b/virtcontainers/network.go @@ -6,6 +6,7 @@ package virtcontainers import ( + "bufio" "encoding/hex" "encoding/json" "fmt" @@ -579,6 +580,107 @@ func newNetwork(networkType NetworkModel) network { } } +const procMountInfoFile = "/proc/self/mountinfo" + +// getNetNsFromBindMount returns the network namespace for the bind-mounted path +func getNetNsFromBindMount(nsPath string, procMountFile string) (string, error) { + netNsMountType := "nsfs" + + // Resolve all symlinks in the path as the mountinfo file contains + // resolved paths. + nsPath, err := filepath.EvalSymlinks(nsPath) + if err != nil { + return "", err + } + + f, err := os.Open(procMountFile) + if err != nil { + return "", err + } + defer f.Close() + + scanner := bufio.NewScanner(f) + for scanner.Scan() { + text := scanner.Text() + + // Scan the mountinfo file to search for the network namespace path + // This file contains mounts in the eg format: + // "711 26 0:3 net:[4026532009] /run/docker/netns/default rw shared:535 - nsfs nsfs rw" + // + // Reference: https://www.kernel.org/doc/Documentation/filesystems/proc.txt + + // We are interested in the first 9 fields of this file, + // to check for the correct mount type. + fields := strings.Split(text, " ") + if len(fields) < 9 { + continue + } + + // We check here if the mount type is a network namespace mount type, namely "nsfs" + mountTypeFieldIdx := 8 + if fields[mountTypeFieldIdx] != netNsMountType { + continue + } + + // This is the mount point/destination for the mount + mntDestIdx := 4 + if fields[mntDestIdx] != nsPath { + continue + } + + // This is the root/source of the mount + return fields[3], nil + } + + return "", nil +} + +// hostNetworkingRequested checks if the network namespace requested is the +// same as the current process. +func hostNetworkingRequested(configNetNs string) (bool, error) { + var evalNS, nsPath, currentNsPath string + var err error + + // Net namespace provided as "/proc/pid/ns/net" or "/proc//task//ns/net" + if strings.HasPrefix(configNetNs, "/proc") && strings.HasSuffix(configNetNs, "/ns/net") { + if _, err := os.Stat(configNetNs); err != nil { + return false, err + } + + // Here we are trying to resolve the path but it fails because + // namespaces links don't really exist. For this reason, the + // call to EvalSymlinks will fail when it will try to stat the + // resolved path found. As we only care about the path, we can + // retrieve it from the PathError structure. + if _, err = filepath.EvalSymlinks(configNetNs); err != nil { + nsPath = err.(*os.PathError).Path + } else { + return false, fmt.Errorf("Net namespace path %s is not a symlink", configNetNs) + } + + _, evalNS = filepath.Split(nsPath) + + } else { + // Bind-mounted path provided + evalNS, _ = getNetNsFromBindMount(configNetNs, procMountInfoFile) + } + + currentNS := fmt.Sprintf("/proc/%d/task/%d/ns/net", os.Getpid(), unix.Gettid()) + if _, err = filepath.EvalSymlinks(currentNS); err != nil { + currentNsPath = err.(*os.PathError).Path + } else { + return false, fmt.Errorf("Unexpected: Current network namespace path is not a symlink") + } + + _, evalCurrentNS := filepath.Split(currentNsPath) + + if evalNS == evalCurrentNS { + return true, nil + } + + return false, nil +} + func initNetworkCommon(config NetworkConfig) (string, bool, error) { if !config.InterworkingModel.IsValid() || config.InterworkingModel == NetXConnectDefaultModel { config.InterworkingModel = DefaultNetInterworkingModel @@ -593,6 +695,15 @@ func initNetworkCommon(config NetworkConfig) (string, bool, error) { return path, true, nil } + isHostNs, err := hostNetworkingRequested(config.NetNSPath) + if err != nil { + return "", false, err + } + + if isHostNs { + return "", false, fmt.Errorf("Host networking requested, not supported by runtime") + } + return config.NetNSPath, false, nil } diff --git a/virtcontainers/network_test.go b/virtcontainers/network_test.go index f829c3aaab..c2442f4f33 100644 --- a/virtcontainers/network_test.go +++ b/virtcontainers/network_test.go @@ -7,12 +7,16 @@ package virtcontainers import ( "fmt" + "io/ioutil" "net" "os" + "path/filepath" "reflect" + "syscall" "testing" "github.com/containernetworking/plugins/pkg/ns" + "github.com/stretchr/testify/assert" "github.com/vishvananda/netlink" "github.com/vishvananda/netns" ) @@ -488,3 +492,84 @@ func TestVhostUserEndpointAttach(t *testing.T) { t.Fatal(err) } } + +func TestGetNetNsFromBindMount(t *testing.T) { + assert := assert.New(t) + + tmpdir, err := ioutil.TempDir("", "") + assert.NoError(err) + defer os.RemoveAll(tmpdir) + + mountFile := filepath.Join(tmpdir, "mountInfo") + nsPath := filepath.Join(tmpdir, "ns123") + + // Non-existent namespace path + _, err = getNetNsFromBindMount(nsPath, mountFile) + assert.NotNil(err) + + tmpNSPath := filepath.Join(tmpdir, "testNetNs") + f, err := os.Create(tmpNSPath) + assert.NoError(err) + defer f.Close() + + type testData struct { + contents string + expectedResult string + } + + data := []testData{ + {fmt.Sprintf("711 26 0:3 net:[4026532008] %s rw shared:535 - nsfs nsfs rw", tmpNSPath), "net:[4026532008]"}, + {"711 26 0:3 net:[4026532008] /run/netns/ns123 rw shared:535 - tmpfs tmpfs rw", ""}, + {"a a a a a a a - b c d", ""}, + {"", ""}, + } + + for i, d := range data { + err := ioutil.WriteFile(mountFile, []byte(d.contents), 0640) + assert.NoError(err) + + path, err := getNetNsFromBindMount(tmpNSPath, mountFile) + assert.NoError(err, fmt.Sprintf("got %q, test data: %+v", path, d)) + + assert.Equal(d.expectedResult, path, "Test %d, expected %s, got %s", i, d.expectedResult, path) + } +} + +func TestHostNetworkingRequested(t *testing.T) { + if os.Geteuid() != 0 { + t.Skip(testDisabledAsNonRoot) + } + + assert := assert.New(t) + + // Network namespace same as the host + selfNsPath := "/proc/self/ns/net" + isHostNs, err := hostNetworkingRequested(selfNsPath) + assert.NoError(err) + assert.True(isHostNs) + + // Non-existent netns path + nsPath := "/proc/123/ns/net" + _, err = hostNetworkingRequested(nsPath) + assert.Error(err) + + // Bind-mounted Netns + tmpdir, err := ioutil.TempDir("", "") + assert.NoError(err) + defer os.RemoveAll(tmpdir) + + // Create a bind mount to the current network namespace. + tmpFile := filepath.Join(tmpdir, "testNetNs") + f, err := os.Create(tmpFile) + assert.NoError(err) + defer f.Close() + + err = syscall.Mount(selfNsPath, tmpFile, "bind", syscall.MS_BIND, "") + assert.Nil(err) + + isHostNs, err = hostNetworkingRequested(tmpFile) + assert.NoError(err) + assert.True(isHostNs) + + syscall.Unmount(tmpFile, 0) +} From 4021697da1912ec3f2e07c5e261758060a365d8d Mon Sep 17 00:00:00 2001 From: Nitesh Konkar Date: Fri, 10 Aug 2018 17:38:18 +0530 Subject: [PATCH 31/39] cli: Make message of using initrd OR rootfs clearer When starting a kata container, if both initrd and rootfs are provided in the configuration file then the error message presented is "cannot specify an image and an initrd in configuration file" which might be a bit confusing. This fix makes the error message more explicit. Fixes: #563 Signed-off-by: Nitesh Konkar niteshkonkar@in.ibm.com --- cli/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/config.go b/cli/config.go index cc6e9bb006..bbead0ccdb 100644 --- a/cli/config.go +++ b/cli/config.go @@ -311,7 +311,7 @@ func newQemuHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) { if image != "" && initrd != "" { return vc.HypervisorConfig{}, - errors.New("cannot specify an image and an initrd in configuration file") + errors.New("having both an image and an initrd defined in the configuration file is not supported") } firmware, err := h.firmware() From a4de4e6e19c9752eee28c7d90d3f131bcb4b05ea Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Wed, 22 Aug 2018 08:56:53 -0700 Subject: [PATCH 32/39] vc: cni: add SPDX license to OWNERS file Add license file. Signed-off-by: Eric Ernst --- virtcontainers/pkg/cni/OWNERS | 1 + 1 file changed, 1 insertion(+) diff --git a/virtcontainers/pkg/cni/OWNERS b/virtcontainers/pkg/cni/OWNERS index aa02988580..033b57fb5a 100644 --- a/virtcontainers/pkg/cni/OWNERS +++ b/virtcontainers/pkg/cni/OWNERS @@ -1,3 +1,4 @@ +#SPDX-License-Identifier: Apache-2.0 reviewers: - virtcontainers-maintainers From bd5c8fe0f2627f42a5a5f7c38aaea0e9858077fa Mon Sep 17 00:00:00 2001 From: flyflypeng Date: Wed, 20 Jun 2018 07:38:24 +0800 Subject: [PATCH 33/39] virtcontainers: add kata-proxy rollback If some errors occur after kata-proxy start, we need to rollback to kill kata-proxy process Fixes: #297 Signed-off-by: flyflypeng --- virtcontainers/kata_agent.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index b9ecc5688f..2a6317aef5 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -421,6 +421,8 @@ func (k *kataAgent) generateInterfacesAndRoutes(networkNS NetworkNamespace) ([]* } func (k *kataAgent) startProxy(sandbox *Sandbox) error { + var err error + if k.proxy == nil { return errorMissingProxy } @@ -446,10 +448,18 @@ func (k *kataAgent) startProxy(sandbox *Sandbox) error { return err } + // If error occurs after kata-proxy process start, + // then rollback to kill kata-proxy process + defer func() { + if err != nil && pid > 0 { + k.proxy.stop(sandbox, pid) + } + }() + // Fill agent state with proxy information, and store them. k.state.ProxyPid = pid k.state.URL = uri - if err := sandbox.storage.storeAgentState(sandbox.id, k.state); err != nil { + if err = sandbox.storage.storeAgentState(sandbox.id, k.state); err != nil { return err } From 01906eef9932fe6a32fb12978929e157f1615b36 Mon Sep 17 00:00:00 2001 From: flyflypeng Date: Wed, 20 Jun 2018 07:51:05 +0800 Subject: [PATCH 34/39] virtcontainers: add qemu process rollback If some errors occur after qemu process start, then we need to rollback to kill qemu process Fixes: #297 Signed-off-by: flyflypeng --- virtcontainers/api.go | 21 +++++++++++++++++++++ virtcontainers/sandbox.go | 10 ++++++---- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/virtcontainers/api.go b/virtcontainers/api.go index dfaf0c881a..bcb271f4dc 100644 --- a/virtcontainers/api.go +++ b/virtcontainers/api.go @@ -55,6 +55,27 @@ func createSandboxFromConfig(sandboxConfig SandboxConfig) (*Sandbox, error) { return nil, err } + // rollback to stop VM if error occurs + defer func() { + if err != nil { + s.stopVM() + } + }() + + // Once startVM is done, we want to guarantee + // that the sandbox is manageable. For that we need + // to start the sandbox inside the VM. + if err = s.agent.startSandbox(s); err != nil { + return nil, err + } + + // rollback to stop sandbox in VM + defer func() { + if err != nil { + s.agent.stopSandbox(s) + } + }() + // Create Containers if err := s.createContainers(); err != nil { return nil, err diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 9422599821..90af285a38 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -949,10 +949,12 @@ func (s *Sandbox) startVM() error { s.Logger().Info("VM started") - // Once startVM is done, we want to guarantee - // that the sandbox is manageable. For that we need - // to start the sandbox inside the VM. - return s.agent.startSandbox(s) + return nil +} + +// stopVM: stop the sandbox's VM +func (s *Sandbox) stopVM() error { + return s.hypervisor.stopSandbox() } func (s *Sandbox) addContainer(c *Container) error { From 85b16278bd7390afb8084a5fef9f04ebff37a7b4 Mon Sep 17 00:00:00 2001 From: flyflypeng Date: Tue, 19 Jun 2018 23:20:18 +0800 Subject: [PATCH 35/39] virtcontainers: fix kata-agent fail to start If kata-agent doesn't start in VM, we need to do some rollback operations to release related resources. add grpc check() to check kata-agent is running or not Fixes: #297 Signed-off-by: flyflypeng --- virtcontainers/kata_agent.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 2a6317aef5..2fc6a84558 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -483,6 +483,11 @@ func (k *kataAgent) startSandbox(sandbox *Sandbox) error { hostname = hostname[:maxHostnameLen] } + // check grpc server is serving + if err = k.check(); err != nil { + return err + } + // // Setup network interfaces and routes // From 3bbcdbc039dd90395c9a2bce7a256a44a2de22a9 Mon Sep 17 00:00:00 2001 From: flyflypeng Date: Wed, 20 Jun 2018 07:33:45 +0800 Subject: [PATCH 36/39] virtcontainers: add rollback to remove sandbox network If error occurs after sandbox network created successfully, we need to rollback to remove the created sandbox network Fixes: #297 Signed-off-by: flyflypeng Signed-off-by: Eric Ernst --- virtcontainers/api.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/virtcontainers/api.go b/virtcontainers/api.go index bcb271f4dc..edfd7a0d56 100644 --- a/virtcontainers/api.go +++ b/virtcontainers/api.go @@ -39,6 +39,8 @@ func CreateSandbox(sandboxConfig SandboxConfig) (VCSandbox, error) { } func createSandboxFromConfig(sandboxConfig SandboxConfig) (*Sandbox, error) { + var err error + // Create the sandbox. s, err := createSandbox(sandboxConfig) if err != nil { @@ -46,12 +48,19 @@ func createSandboxFromConfig(sandboxConfig SandboxConfig) (*Sandbox, error) { } // Create the sandbox network - if err := s.createNetwork(); err != nil { + if err = s.createNetwork(); err != nil { return nil, err } + // network rollback + defer func() { + if err != nil && s.networkNS.NetNsCreated { + s.removeNetwork() + } + }() + // Start the VM - if err := s.startVM(); err != nil { + if err = s.startVM(); err != nil { return nil, err } @@ -77,12 +86,12 @@ func createSandboxFromConfig(sandboxConfig SandboxConfig) (*Sandbox, error) { }() // Create Containers - if err := s.createContainers(); err != nil { + if err = s.createContainers(); err != nil { return nil, err } // The sandbox is completely created now, we can store it. - if err := s.storeSandbox(); err != nil { + if err = s.storeSandbox(); err != nil { return nil, err } From 35a9430d2948859d210eb2dbdbddf47d49be4b6d Mon Sep 17 00:00:00 2001 From: fupan Date: Tue, 24 Jul 2018 09:49:42 +0800 Subject: [PATCH 37/39] virtconainers: rollback the NetNs when createNetwork failed When createNetwork failed, cleanup the NetNs if it created. Fixes: #508 Signed-off-by: fupan --- virtcontainers/sandbox.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 90af285a38..724cfff54a 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -905,8 +905,20 @@ func (s *Sandbox) Delete() error { } func (s *Sandbox) createNetwork() error { + var netNsPath string + var netNsCreated bool + var networkNS NetworkNamespace + var err error + + //rollback the NetNs when createNetwork failed + defer func() { + if err != nil && netNsPath != "" && netNsCreated { + deleteNetNS(netNsPath) + } + }() + // Initialize the network. - netNsPath, netNsCreated, err := s.network.init(s.config.NetworkConfig) + netNsPath, netNsCreated, err = s.network.init(s.config.NetworkConfig) if err != nil { return err } @@ -919,14 +931,16 @@ func (s *Sandbox) createNetwork() error { } // Add the network - networkNS, err := s.network.add(s, s.config.NetworkConfig, netNsPath, netNsCreated) + networkNS, err = s.network.add(s, s.config.NetworkConfig, netNsPath, netNsCreated) if err != nil { return err } s.networkNS = networkNS // Store the network - return s.storage.storeSandboxNetwork(s.id, networkNS) + err = s.storage.storeSandboxNetwork(s.id, networkNS) + + return err } func (s *Sandbox) removeNetwork() error { From 6374e40ec263f701c20b39371a690d0ca1397d0d Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Wed, 27 Jun 2018 11:58:42 +0100 Subject: [PATCH 38/39] main: Pass runtime CLI command to vc logger Add the runtime CLI command name to the virtcontainers logger so that it is clear when reading virtcontainers log entries which runtime command they refer to. Fixes #448. Signed-off-by: James O. D. Hunt Signed-off-by: Eric Ernst --- cli/main.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cli/main.go b/cli/main.go index 99a4a890eb..ed3bcdf4d9 100644 --- a/cli/main.go +++ b/cli/main.go @@ -235,10 +235,6 @@ func beforeSubcommands(context *cli.Context) error { return fmt.Errorf("unknown log-format %q", context.GlobalString("log-format")) } - setExternalLoggers(kataLog) - - ignoreLogging := false - // Add the name of the sub-command to each log entry for easier // debugging. cmdName := context.Args().First() @@ -246,6 +242,10 @@ func beforeSubcommands(context *cli.Context) error { kataLog = kataLog.WithField("command", cmdName) } + setExternalLoggers(kataLog) + + ignoreLogging := false + if context.NArg() == 1 && context.Args()[0] == envCmd { // simply report the logging setup ignoreLogging = true From d3dfdaade4769ec1f14d6f8aee07bb4dc823099b Mon Sep 17 00:00:00 2001 From: Salvador Fuentes Date: Thu, 12 Jul 2018 09:26:38 -0500 Subject: [PATCH 39/39] versions: Update CRI-O supported version Some test fixes were introduced into the 1.9 and 1.10 branches of cri-o. These fixes will help us minimize random failures. Fixes #481. Signed-off-by: Salvador Fuentes --- versions.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/versions.yaml b/versions.yaml index 78d88b6197..6abf086c9c 100644 --- a/versions.yaml +++ b/versions.yaml @@ -132,9 +132,9 @@ externals: description: | OCI-based Kubernetes Container Runtime Interface implementation url: "https://github.com/kubernetes-incubator/cri-o" - version: "e0dd8a3d4c9e5ebc8b25299950bd5b222e3783d3" + version: "6273bea4c9ed788aeb3d051ebf2d030060c05b6c" meta: - openshift: "v1.9.11" + openshift: "7fcfaf1ec696d14615069707fbe8b36f19cbb8d7" cri-containerd: description: |