diff --git a/.ci/go-no-os-exit.sh b/.ci/go-no-os-exit.sh index 51eac257ab..5f0f98436a 100755 --- a/.ci/go-no-os-exit.sh +++ b/.ci/go-no-os-exit.sh @@ -2,21 +2,28 @@ # 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. + +# Allow the path to check to be over-ridden. +# Default to the current directory. +go_packages=${1:-.} -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` + # 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 +[ -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 diff --git a/Makefile b/Makefile index 10a7f851d2..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) @@ -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 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() diff --git a/cli/create.go b/cli/create.go index 165bf6b698..858d4233fc 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" ) @@ -90,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 { @@ -241,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 { @@ -268,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 { @@ -283,11 +285,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 } 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/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/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/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) 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() } 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..cf285c7c40 100644 --- a/cli/kata-env_test.go +++ b/cli/kata-env_test.go @@ -7,6 +7,8 @@ package main import ( "bytes" + "encoding/json" + "flag" "fmt" "io/ioutil" "os" @@ -668,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{} @@ -717,7 +719,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 } @@ -737,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 { @@ -748,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) } @@ -761,11 +840,18 @@ func TestEnvShowSettingsInvalidFile(t *testing.T) { tmpfile, err := ioutil.TempFile("", "envShowSettings-") assert.NoError(t, err) + defer os.Remove(tmpfile.Name()) // 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) } @@ -782,7 +868,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, } @@ -791,7 +881,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 @@ -815,7 +905,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, } @@ -824,47 +917,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) } @@ -882,7 +1003,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{}{ @@ -906,6 +1028,12 @@ func TestEnvCLIFunction(t *testing.T) { err = fn(ctx) assert.NoError(t, err) + + set.Bool("json", true, "") + ctx = cli.NewContext(app, set, nil) + + err = fn(ctx) + assert.NoError(t, err) } func TestEnvCLIFunctionFail(t *testing.T) { 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/main.go b/cli/main.go index e5b7f61e25..ed3bcdf4d9 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,14 +235,6 @@ 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) - - ignoreLogging := false - // Add the name of the sub-command to each log entry for easier // debugging. cmdName := context.Args().First() @@ -240,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 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/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) 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 { 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") diff --git a/versions.yaml b/versions.yaml index d616dc0b25..6abf086c9c 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" @@ -116,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: | 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..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,22 +48,50 @@ 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 + } + + // 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 { + 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 } @@ -110,7 +140,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/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/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/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..2fc6a84558 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" @@ -419,11 +420,17 @@ func (k *kataAgent) generateInterfacesAndRoutes(networkNS NetworkNamespace) ([]* return ifaces, routes, nil } -func (k *kataAgent) startSandbox(sandbox *Sandbox) error { +func (k *kataAgent) startProxy(sandbox *Sandbox) error { + var err 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 { @@ -441,10 +448,18 @@ func (k *kataAgent) startSandbox(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 } @@ -454,11 +469,25 @@ 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] } + // check grpc server is serving + if err = k.check(); err != nil { + return err + } + // // Setup network interfaces and routes // @@ -1135,8 +1164,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 } @@ -1180,7 +1213,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...) } @@ -1256,6 +1289,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) } 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/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/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) +} 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/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 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/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) + } +} diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 882294ad39..482813eabb 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -64,10 +64,12 @@ const qmpSocket = "qmp.sock" const defaultConsole = "console.sock" +var qemuMajorVersion int +var qemuMinorVersion int + // agnostic list of kernel parameters var defaultKernelParameters = []Param{ {"panic", "1"}, - {"initcall_debug", ""}, } type operation int @@ -456,6 +458,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..e852e4e138 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 { @@ -23,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" @@ -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) } 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", diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 776c0c102e..724cfff54a 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 == "" { @@ -441,7 +452,7 @@ type Sandbox struct { volumes []Volume - containers []*Container + containers map[string]*Container runPath string configPath string @@ -510,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 @@ -519,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 } } @@ -730,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{}, @@ -783,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 } @@ -835,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 } } @@ -856,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. @@ -892,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 } @@ -906,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 { @@ -936,14 +963,19 @@ 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 { - 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 } @@ -1055,8 +1087,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, @@ -1192,6 +1224,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() } 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)