From 3ece4130c9b3f325a30d45d825599c639147bf43 Mon Sep 17 00:00:00 2001 From: bin liu Date: Tue, 30 Jun 2020 15:34:34 +0800 Subject: [PATCH] runtime: clean up shim abstraction This PR will delete shim abstraction from sandbox. Fixes: #364 Signed-off-by: bin liu --- src/runtime/cli/kata-check.go | 42 --- src/runtime/cli/kata-check_test.go | 92 ----- src/runtime/cli/kata-env.go | 40 --- src/runtime/cli/kata-env_test.go | 156 -------- src/runtime/cli/main_test.go | 1 - src/runtime/containerd-shim-v2/utils_test.go | 1 - src/runtime/pkg/katautils/config.go | 80 +---- src/runtime/pkg/katautils/config_test.go | 179 +--------- src/runtime/pkg/katautils/create_test.go | 1 - src/runtime/virtcontainers/api.go | 36 -- src/runtime/virtcontainers/container.go | 42 --- src/runtime/virtcontainers/kata_agent.go | 43 +-- .../virtcontainers/kata_builtin_shim.go | 15 - src/runtime/virtcontainers/kata_shim.go | 61 ---- src/runtime/virtcontainers/kata_shim_test.go | 332 ------------------ src/runtime/virtcontainers/noop_shim.go | 14 - src/runtime/virtcontainers/noop_shim_test.go | 24 -- src/runtime/virtcontainers/persist.go | 22 -- src/runtime/virtcontainers/pkg/oci/utils.go | 6 - .../virtcontainers/pkg/oci/utils_test.go | 2 - src/runtime/virtcontainers/sandbox.go | 5 +- src/runtime/virtcontainers/shim.go | 281 --------------- src/runtime/virtcontainers/shim_test.go | 237 ------------- 23 files changed, 14 insertions(+), 1698 deletions(-) delete mode 100644 src/runtime/virtcontainers/kata_builtin_shim.go delete mode 100644 src/runtime/virtcontainers/kata_shim.go delete mode 100644 src/runtime/virtcontainers/kata_shim_test.go delete mode 100644 src/runtime/virtcontainers/noop_shim.go delete mode 100644 src/runtime/virtcontainers/noop_shim_test.go delete mode 100644 src/runtime/virtcontainers/shim.go delete mode 100644 src/runtime/virtcontainers/shim_test.go diff --git a/src/runtime/cli/kata-check.go b/src/runtime/cli/kata-check.go index 0cfe35da21..31ebfe20fe 100644 --- a/src/runtime/cli/kata-check.go +++ b/src/runtime/cli/kata-check.go @@ -362,16 +362,6 @@ var kataCheckCLICommand = cli.Command{ fmt.Println(successMessageCreate) } - strict := context.Bool("strict") - if strict { - err = checkVersionConsistencyInComponents(runtimeConfig) - if err != nil { - return err - } - - fmt.Println(successMessageVersion) - } - return nil }, } @@ -469,35 +459,3 @@ func genericCheckKVMExtensions(extensions map[string]kvmExtension) (map[string]i return results, nil } - -// checkVersionConsistencyInComponents checks version consistency in Kata Components. -func checkVersionConsistencyInComponents(config oci.RuntimeConfig) error { - proxyInfo := getProxyInfo(config) - - shimInfo, err := getShimInfo(config) - if err != nil { - return err - } - shimVersionInfo := shimInfo.Version - - runtimeVersionInfo := constructVersionInfo(version) - - // kata-proxy exists - if proxyInfo.Type != string(vc.NoProxyType) { - proxyVersionInfo := proxyInfo.Version - if !versionEqual(proxyVersionInfo, runtimeVersionInfo) || !versionEqual(shimVersionInfo, runtimeVersionInfo) { - return fmt.Errorf("there exists version inconsistency in kata components. kata-proxy: v%d.%d.%d, kata-shim: v%d.%d.%d, kata-runtime: v%d.%d.%d", - proxyVersionInfo.Major, proxyVersionInfo.Minor, proxyVersionInfo.Patch, - shimVersionInfo.Major, shimVersionInfo.Minor, shimVersionInfo.Patch, - runtimeVersionInfo.Major, runtimeVersionInfo.Minor, runtimeVersionInfo.Patch) - } - } else { - if !versionEqual(shimVersionInfo, runtimeVersionInfo) { - return fmt.Errorf("there exists version inconsistency in kata components. kata-shim: v%d.%d.%d, kata-runtime: v%d.%d.%d", - shimVersionInfo.Major, shimVersionInfo.Minor, shimVersionInfo.Patch, - runtimeVersionInfo.Major, runtimeVersionInfo.Minor, runtimeVersionInfo.Patch) - } - } - - return nil -} diff --git a/src/runtime/cli/kata-check_test.go b/src/runtime/cli/kata-check_test.go index c1e986ccc3..632b26a16c 100644 --- a/src/runtime/cli/kata-check_test.go +++ b/src/runtime/cli/kata-check_test.go @@ -19,7 +19,6 @@ import ( ktu "github.com/kata-containers/kata-containers/src/runtime/pkg/katatestutils" "github.com/kata-containers/kata-containers/src/runtime/pkg/katautils" - vc "github.com/kata-containers/kata-containers/src/runtime/virtcontainers" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/urfave/cli" @@ -906,94 +905,3 @@ func TestArchRequiredKernelModules(t *testing.T) { assert.EqualValues(count, expectedCount) } - -func TestCheckVersionConsistencyInComponents(t *testing.T) { - type testData struct { - proxyExist bool - expectError bool - shimVersion string - proxyVersion string - runtimeVersion string - } - - data := []testData{ - { - true, - true, - "kata-shim version 0.2.0-rc0-xxxxxxxxxxxxx", - "kata-proxy version 0.1.0-rc0-xxxxxxxxxxxxx", - "0.2.0-rc0", - }, - { - true, - true, - "kata-shim version 0.1.0-rc0-xxxxxxxxxxxxx", - "kata-proxy version 0.2.0-rc0-xxxxxxxxxxxxx", - "0.2.0-rc0", - }, - { - true, - true, - "kata-shim version 0.1.0-rc0-xxxxxxxxxxxxx", - "kata-proxy version 0.1.0-rc0-xxxxxxxxxxxxx", - "0.2.0-rc0", - }, - { - true, - false, - "kata-shim version 0.2.0-rc0-xxxxxxxxxxxxx", - "kata-proxy version 0.2.0-rc0-xxxxxxxxxxxxx", - "0.2.0-rc0", - }, - { - false, - true, - "kata-shim version 0.1.0-rc0-xxxxxxxxxxxxx", - "", - "0.2.0-rc0", - }, - { - false, - false, - "kata-shim version 0.2.0-rc0-xxxxxxxxxxxxx", - "", - "0.2.0-rc0", - }, - { - false, - false, - "kata-shim version 0.2.0-xxxxxxxxxxxxx", - "", - "0.2.0", - }, - } - - origVersion := version - for _, d := range data { - tmpdir, err := ioutil.TempDir("", "") - assert.NoError(t, err) - defer os.RemoveAll(tmpdir) - - testShimVersion = d.shimVersion - if d.proxyExist { - testProxyVersion = d.proxyVersion - } - _, config, err := makeRuntimeConfig(tmpdir) - assert.NoError(t, err) - if !d.proxyExist { - config.ProxyType = vc.NoProxyType - } - version = d.runtimeVersion - defer func() { - version = origVersion - }() - - err = checkVersionConsistencyInComponents(config) - if d.expectError { - assert.Error(t, err, fmt.Sprintf("%+v", d)) - continue - } else { - assert.NoError(t, err, fmt.Sprintf("%+v", d)) - } - } -} diff --git a/src/runtime/cli/kata-env.go b/src/runtime/cli/kata-env.go index 12afdd6491..161b24d52d 100644 --- a/src/runtime/cli/kata-env.go +++ b/src/runtime/cli/kata-env.go @@ -113,14 +113,6 @@ type ProxyInfo struct { Debug bool } -// ShimInfo stores shim details -type ShimInfo struct { - Type string - Version VersionInfo - Path string - Debug bool -} - // AgentInfo stores agent details type AgentInfo struct { Type string @@ -166,7 +158,6 @@ type EnvInfo struct { Kernel KernelInfo Initrd InitrdInfo Proxy ProxyInfo - Shim ShimInfo Agent AgentInfo Host HostInfo Netmon NetmonInfo @@ -308,31 +299,6 @@ func getCommandVersion(cmd string) (string, error) { return katautils.RunCommand([]string{cmd, "--version"}) } -func getShimInfo(config oci.RuntimeConfig) (ShimInfo, error) { - shimConfig, ok := config.ShimConfig.(vc.ShimConfig) - if !ok { - return ShimInfo{}, errors.New("cannot determine shim config") - } - - shimPath := shimConfig.Path - - var shimVersionInfo VersionInfo - if version, err := getCommandVersion(shimConfig.Path); err != nil { - shimVersionInfo = unknownVersionInfo - } else { - shimVersionInfo = constructVersionInfo(version) - } - - shim := ShimInfo{ - Type: string(config.ShimType), - Version: shimVersionInfo, - Path: shimPath, - Debug: shimConfig.Debug, - } - - return shim, nil -} - func getAgentInfo(config oci.RuntimeConfig) (AgentInfo, error) { agent := AgentInfo{ Type: string(config.AgentType), @@ -400,11 +366,6 @@ func getEnvInfo(configFile string, config oci.RuntimeConfig) (env EnvInfo, err e netmon := getNetmonInfo(config) - shim, err := getShimInfo(config) - if err != nil { - return EnvInfo{}, err - } - agent, err := getAgentInfo(config) if err != nil { return EnvInfo{}, err @@ -433,7 +394,6 @@ func getEnvInfo(configFile string, config oci.RuntimeConfig) (env EnvInfo, err e Kernel: kernel, Initrd: initrd, Proxy: proxy, - Shim: shim, Agent: agent, Host: host, Netmon: netmon, diff --git a/src/runtime/cli/kata-env_test.go b/src/runtime/cli/kata-env_test.go index 46da0ff3c9..4b60d6dc23 100644 --- a/src/runtime/cli/kata-env_test.go +++ b/src/runtime/cli/kata-env_test.go @@ -32,7 +32,6 @@ import ( var ( testProxyVersion = "proxy version 0.1" - testShimVersion = "shim version 0.1" testNetmonVersion = "netmon version 0.1" testHypervisorVersion = "QEMU emulator version 2.7.0+git.741f430a96-6.1, Copyright (c) 2003-2016 Fabrice Bellard and the QEMU Project developers" ) @@ -43,7 +42,6 @@ var ( proxyDebug = false runtimeDebug = false runtimeTrace = false - shimDebug = false netmonDebug = false agentDebug = false agentTrace = false @@ -86,7 +84,6 @@ func makeRuntimeConfig(prefixDir string) (configFile string, config oci.RuntimeC imagePath := filepath.Join(prefixDir, "image") kernelParams := "foo=bar xyz" machineType := "machineType" - shimPath := filepath.Join(prefixDir, "shim") proxyPath := filepath.Join(prefixDir, "proxy") netmonPath := filepath.Join(prefixDir, "netmon") disableBlock := true @@ -112,11 +109,6 @@ func makeRuntimeConfig(prefixDir string) (configFile string, config oci.RuntimeC } } - err = makeVersionBinary(shimPath, testShimVersion) - if err != nil { - return "", oci.RuntimeConfig{}, err - } - err = makeVersionBinary(proxyPath, testProxyVersion) if err != nil { return "", oci.RuntimeConfig{}, err @@ -145,7 +137,6 @@ func makeRuntimeConfig(prefixDir string) (configFile string, config oci.RuntimeC ImagePath: imagePath, KernelParams: kernelParams, MachineType: machineType, - ShimPath: shimPath, ProxyPath: proxyPath, NetmonPath: netmonPath, LogPath: logPath, @@ -164,7 +155,6 @@ func makeRuntimeConfig(prefixDir string) (configFile string, config oci.RuntimeC RuntimeDebug: runtimeDebug, RuntimeTrace: runtimeTrace, ProxyDebug: proxyDebug, - ShimDebug: shimDebug, NetmonDebug: netmonDebug, AgentDebug: agentDebug, AgentTrace: agentTrace, @@ -206,22 +196,6 @@ func getExpectedNetmonDetails(config oci.RuntimeConfig) (NetmonInfo, error) { }, nil } -func getExpectedShimDetails(config oci.RuntimeConfig) (ShimInfo, error) { - shimConfig, ok := config.ShimConfig.(vc.ShimConfig) - if !ok { - return ShimInfo{}, fmt.Errorf("failed to get shim config") - } - - shimPath := shimConfig.Path - - return ShimInfo{ - Type: string(config.ShimType), - Version: constructVersionInfo(testShimVersion), - Path: shimPath, - Debug: shimConfig.Debug, - }, nil -} - func getExpectedAgentDetails(config oci.RuntimeConfig) (AgentInfo, error) { agentConfig, ok := config.AgentConfig.(vc.KataAgentConfig) @@ -385,11 +359,6 @@ func getExpectedSettings(config oci.RuntimeConfig, tmpdir, configFile string) (E return EnvInfo{}, err } - shim, err := getExpectedShimDetails(config) - if err != nil { - return EnvInfo{}, err - } - agent, err := getExpectedAgentDetails(config) if err != nil { return EnvInfo{}, err @@ -416,7 +385,6 @@ func getExpectedSettings(config oci.RuntimeConfig, tmpdir, configFile string) (E Image: image, Kernel: kernel, Proxy: proxy, - Shim: shim, Agent: agent, Host: host, Netmon: netmon, @@ -521,7 +489,6 @@ func TestEnvGetEnvInfo(t *testing.T) { proxyDebug = toggle runtimeDebug = toggle runtimeTrace = toggle - shimDebug = toggle agentDebug = toggle agentTrace = toggle @@ -562,22 +529,6 @@ func TestEnvGetEnvInfoNoHypervisorVersion(t *testing.T) { assert.Equal(expectedEnv, env) } -func TestEnvGetEnvInfoShimError(t *testing.T) { - assert := assert.New(t) - - tmpdir, err := ioutil.TempDir("", "") - assert.NoError(err) - defer os.RemoveAll(tmpdir) - - configFile, config, err := makeRuntimeConfig(tmpdir) - assert.NoError(err) - - config.ShimConfig = "invalid shim config" - - _, err = getEnvInfo(configFile, config) - assert.Error(err) -} - func TestEnvGetEnvInfoAgentError(t *testing.T) { assert := assert.New(t) @@ -759,71 +710,6 @@ func TestEnvGetNetmonInfoNoVersion(t *testing.T) { assert.Equal(t, expectedNetmon, netmon) } -func TestEnvGetShimInfo(t *testing.T) { - tmpdir, err := ioutil.TempDir("", "") - if err != nil { - panic(err) - } - defer os.RemoveAll(tmpdir) - - _, config, err := makeRuntimeConfig(tmpdir) - assert.NoError(t, err) - - expectedShim, err := getExpectedShimDetails(config) - assert.NoError(t, err) - - shim, err := getShimInfo(config) - assert.NoError(t, err) - - assert.Equal(t, expectedShim, shim) -} - -func TestEnvGetShimInfoNoVersion(t *testing.T) { - tmpdir, err := ioutil.TempDir("", "") - if err != nil { - panic(err) - } - defer os.RemoveAll(tmpdir) - - _, config, err := makeRuntimeConfig(tmpdir) - assert.NoError(t, err) - - expectedShim, err := getExpectedShimDetails(config) - assert.NoError(t, err) - - shimPath := expectedShim.Path - - // ensure querying the shim version fails - err = createFile(shimPath, `#!/bin/sh - exit 1`) - assert.NoError(t, err) - - expectedShim.Version = unknownVersionInfo - - shim, err := getShimInfo(config) - assert.NoError(t, err) - - assert.Equal(t, expectedShim, shim) -} - -func TestEnvGetShimInfoInvalidType(t *testing.T) { - tmpdir, err := ioutil.TempDir("", "") - if err != nil { - panic(err) - } - defer os.RemoveAll(tmpdir) - - _, config, err := makeRuntimeConfig(tmpdir) - assert.NoError(t, err) - - _, err = getExpectedShimDetails(config) - assert.NoError(t, err) - - config.ShimConfig = "foo" - _, err = getShimInfo(config) - assert.Error(t, err) -} - func TestEnvGetAgentInfo(t *testing.T) { tmpdir, err := ioutil.TempDir("", "") if err != nil { @@ -891,12 +777,6 @@ func testEnvShowTOMLSettings(t *testing.T, tmpdir string, tmpfile *os.File) erro Debug: false, } - shim := ShimInfo{ - Type: "shim-type", - Version: constructVersionInfo(testShimVersion), - Path: "/resolved/shim/path", - } - agent := AgentInfo{ Type: "agent-type", } @@ -910,7 +790,6 @@ func testEnvShowTOMLSettings(t *testing.T, tmpdir string, tmpfile *os.File) erro Image: image, Kernel: kernel, Proxy: proxy, - Shim: shim, Agent: agent, Host: expectedHostDetails, } @@ -960,12 +839,6 @@ func testEnvShowJSONSettings(t *testing.T, tmpdir string, tmpfile *os.File) erro Debug: false, } - shim := ShimInfo{ - Type: "shim-type", - Version: constructVersionInfo(testShimVersion), - Path: "/resolved/shim/path", - } - agent := AgentInfo{ Type: "agent-type", } @@ -979,7 +852,6 @@ func testEnvShowJSONSettings(t *testing.T, tmpdir string, tmpfile *os.File) erro Image: image, Kernel: kernel, Proxy: proxy, - Shim: shim, Agent: agent, Host: expectedHostDetails, } @@ -1083,34 +955,6 @@ func TestEnvHandleSettings(t *testing.T) { assert.NoError(t, err) } -func TestEnvHandleSettingsInvalidShimConfig(t *testing.T) { - assert := assert.New(t) - - tmpdir, err := ioutil.TempDir("", "") - assert.NoError(err) - defer os.RemoveAll(tmpdir) - - configFile, config, err := makeRuntimeConfig(tmpdir) - assert.NoError(err) - - _, err = getExpectedSettings(config, tmpdir, configFile) - assert.NoError(err) - - config.ShimConfig = "invalid shim config" - - ctx := createCLIContext(nil) - ctx.App.Name = "foo" - ctx.App.Metadata["configFile"] = configFile - ctx.App.Metadata["runtimeConfig"] = config - - tmpfile, err := ioutil.TempFile("", "") - assert.NoError(err) - defer os.Remove(tmpfile.Name()) - - err = handleSettings(tmpfile, ctx) - assert.Error(err) -} - func TestEnvHandleSettingsInvalidParams(t *testing.T) { assert := assert.New(t) diff --git a/src/runtime/cli/main_test.go b/src/runtime/cli/main_test.go index 08d98d8aa8..ba2fbfb52d 100644 --- a/src/runtime/cli/main_test.go +++ b/src/runtime/cli/main_test.go @@ -244,7 +244,6 @@ func newTestRuntimeConfig(dir, consolePath string, create bool) (oci.RuntimeConf HypervisorConfig: hypervisorConfig, AgentType: vc.KataContainersAgent, ProxyType: vc.KataProxyType, - ShimType: vc.KataShimType, Console: consolePath, }, nil } diff --git a/src/runtime/containerd-shim-v2/utils_test.go b/src/runtime/containerd-shim-v2/utils_test.go index d1a5226302..5cf14502b8 100644 --- a/src/runtime/containerd-shim-v2/utils_test.go +++ b/src/runtime/containerd-shim-v2/utils_test.go @@ -211,7 +211,6 @@ func newTestRuntimeConfig(dir, consolePath string, create bool) (oci.RuntimeConf HypervisorConfig: hypervisorConfig, AgentType: vc.KataContainersAgent, ProxyType: vc.KataBuiltInProxyType, - ShimType: vc.KataBuiltInShimType, Console: consolePath, }, nil } diff --git a/src/runtime/pkg/katautils/config.go b/src/runtime/pkg/katautils/config.go index 405bfc6d01..8c076bebc8 100644 --- a/src/runtime/pkg/katautils/config.go +++ b/src/runtime/pkg/katautils/config.go @@ -30,7 +30,6 @@ const ( var ( defaultProxy = vc.KataProxyType - defaultShim = vc.KataShimType // if true, enable opentracing support. tracing = false @@ -42,7 +41,7 @@ var ( // // [.] // -// The components are hypervisor, proxy, shim and agent. For example, +// The components are hypervisor, proxy and agent. For example, // // [proxy.kata] // @@ -57,9 +56,6 @@ const ( // supported proxy component types kataProxyTableType = "kata" - // supported shim component types - kataShimTableType = "kata" - // supported agent component types kataAgentTableType = "kata" @@ -70,7 +66,6 @@ const ( type tomlConfig struct { Hypervisor map[string]hypervisor Proxy map[string]proxy - Shim map[string]shim Agent map[string]agent Runtime runtime Factory factory @@ -149,12 +144,6 @@ type runtime struct { InterNetworkModel string `toml:"internetworking_model"` } -type shim struct { - Path string `toml:"path"` - Debug bool `toml:"enable_debug"` - Tracing bool `toml:"enable_tracing"` -} - type agent struct { Debug bool `toml:"enable_debug"` Tracing bool `toml:"enable_tracing"` @@ -462,24 +451,6 @@ func (p proxy) debug() bool { return p.Debug } -func (s shim) path() (string, error) { - p := s.Path - - if p == "" { - p = defaultShimPath - } - - return ResolvePath(p) -} - -func (s shim) debug() bool { - return s.Debug -} - -func (s shim) trace() bool { - return s.Tracing -} - func (a agent) debug() bool { return a.Debug } @@ -883,19 +854,6 @@ func newFactoryConfig(f factory) (oci.FactoryConfig, error) { }, nil } -func newShimConfig(s shim) (vc.ShimConfig, error) { - path, err := s.path() - if err != nil { - return vc.ShimConfig{}, err - } - - return vc.ShimConfig{ - Path: path, - Debug: s.debug(), - Trace: s.trace(), - }, nil -} - func updateRuntimeConfigHypervisor(configPath string, tomlConf tomlConfig, config *oci.RuntimeConfig) error { for k, hypervisor := range tomlConf.Hypervisor { var err error @@ -996,32 +954,6 @@ func updateRuntimeConfigAgent(configPath string, tomlConf tomlConfig, config *oc return nil } -func updateRuntimeConfigShim(configPath string, tomlConf tomlConfig, config *oci.RuntimeConfig, builtIn bool) error { - if builtIn { - config.ShimType = vc.KataBuiltInShimType - config.ShimConfig = vc.ShimConfig{} - return nil - } - - for k, shim := range tomlConf.Shim { - switch k { - case kataShimTableType: - config.ShimType = vc.KataShimType - default: - return fmt.Errorf("%s shim is not supported", k) - } - - shConfig, err := newShimConfig(shim) - if err != nil { - return fmt.Errorf("%v: %v", configPath, err) - } - - config.ShimConfig = shConfig - } - - return nil -} - // SetKernelParams adds the user-specified kernel parameters (from the // configuration file) to the defaults so that the former take priority. func SetKernelParams(runtimeConfig *oci.RuntimeConfig) error { @@ -1096,10 +1028,6 @@ func updateRuntimeConfig(configPath string, tomlConf tomlConfig, config *oci.Run return err } - if err := updateRuntimeConfigShim(configPath, tomlConf, config, builtIn); err != nil { - return err - } - fConfig, err := newFactoryConfig(tomlConf.Factory) if err != nil { return fmt.Errorf("%v: %v", configPath, err) @@ -1177,7 +1105,6 @@ func initConfig() (config oci.RuntimeConfig, err error) { AgentType: defaultAgent, AgentConfig: defaultAgentConfig, ProxyType: defaultProxy, - ShimType: defaultShim, } return config, nil @@ -1320,11 +1247,6 @@ func checkNetNsConfig(config oci.RuntimeConfig) error { if config.InterNetworkModel != vc.NetXConnectNoneModel { return fmt.Errorf("config disable_new_netns only works with 'none' internetworking_model") } - } else if shim, ok := config.ShimConfig.(vc.ShimConfig); ok && shim.Trace { - // Normally, the shim runs in a separate network namespace. - // But when tracing, the shim process needs to be able to talk - // to the Jaeger agent running in the host network namespace. - return errors.New("Shim tracing requires disable_new_netns for Jaeger agent communication") } return nil diff --git a/src/runtime/pkg/katautils/config_test.go b/src/runtime/pkg/katautils/config_test.go index 7ffd68436d..f354a5943f 100644 --- a/src/runtime/pkg/katautils/config_test.go +++ b/src/runtime/pkg/katautils/config_test.go @@ -31,7 +31,6 @@ var ( proxyDebug = false runtimeDebug = false runtimeTrace = false - shimDebug = false netmonDebug = false agentDebug = false agentTrace = false @@ -72,7 +71,6 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf kernelPath := path.Join(dir, "kernel") kernelParams := "foo=bar xyz" imagePath := path.Join(dir, "image") - shimPath := path.Join(dir, "shim") proxyPath := path.Join(dir, "proxy") netmonPath := path.Join(dir, "netmon") logDir := path.Join(dir, "logs") @@ -94,7 +92,6 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf ImagePath: imagePath, KernelParams: kernelParams, MachineType: machineType, - ShimPath: shimPath, ProxyPath: proxyPath, NetmonPath: netmonPath, LogPath: logPath, @@ -113,7 +110,6 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf RuntimeDebug: runtimeDebug, RuntimeTrace: runtimeTrace, ProxyDebug: proxyDebug, - ShimDebug: shimDebug, NetmonDebug: netmonDebug, AgentDebug: agentDebug, AgentTrace: agentTrace, @@ -137,7 +133,7 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf return config, err } - files := []string{hypervisorPath, kernelPath, imagePath, shimPath, proxyPath} + files := []string{hypervisorPath, kernelPath, imagePath, proxyPath} for _, file := range files { // create the resource (which must be >0 bytes) @@ -179,10 +175,6 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf Path: proxyPath, } - shimConfig := vc.ShimConfig{ - Path: shimPath, - } - netmonConfig := vc.NetmonConfig{ Path: netmonPath, Debug: false, @@ -204,9 +196,6 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf ProxyType: defaultProxy, ProxyConfig: proxyConfig, - ShimType: defaultShim, - ShimConfig: shimConfig, - NetmonConfig: netmonConfig, DisableNewNetNs: disableNewNetNs, @@ -410,28 +399,6 @@ func TestConfigLoadConfigurationFailMissingKernel(t *testing.T) { }) } -func TestConfigLoadConfigurationFailMissingShim(t *testing.T) { - tmpdir, err := ioutil.TempDir(testDir, "runtime-config-") - assert.NoError(t, err) - defer os.RemoveAll(tmpdir) - - testLoadConfiguration(t, tmpdir, - func(config testRuntimeConfig, configFile string, ignoreLogging bool) (bool, error) { - expectFail := true - - shimConfig, ok := config.RuntimeConfig.ShimConfig.(vc.ShimConfig) - if !ok { - return expectFail, fmt.Errorf("cannot determine shim config") - } - err = os.Remove(shimConfig.Path) - if err != nil { - return expectFail, err - } - - return expectFail, nil - }) -} - func TestConfigLoadConfigurationFailUnreadableConfig(t *testing.T) { if tc.NotValid(ktu.NeedNonRoot()) { t.Skip(ktu.TestDisabledNeedNonRoot) @@ -517,7 +484,6 @@ func TestMinimalRuntimeConfig(t *testing.T) { } defer os.RemoveAll(dir) - shimPath := path.Join(dir, "shim") proxyPath := path.Join(dir, "proxy") hypervisorPath := path.Join(dir, "hypervisor") defaultHypervisorPath = hypervisorPath @@ -565,9 +531,6 @@ func TestMinimalRuntimeConfig(t *testing.T) { [proxy.kata] path = "` + proxyPath + `" - [shim.kata] - path = "` + shimPath + `" - [agent.kata] [netmon] @@ -580,16 +543,6 @@ func TestMinimalRuntimeConfig(t *testing.T) { t.Fatal(err) } - _, config, err := LoadConfiguration(configPath, false, false) - if err == nil { - t.Fatalf("Expected loadConfiguration to fail as shim path does not exist: %+v", config) - } - - err = createEmptyFile(shimPath) - if err != nil { - t.Error(err) - } - err = createEmptyFile(proxyPath) if err != nil { t.Error(err) @@ -610,7 +563,7 @@ func TestMinimalRuntimeConfig(t *testing.T) { t.Error(err) } - _, config, err = LoadConfiguration(configPath, false, false) + _, config, err := LoadConfiguration(configPath, false, false) if err != nil { t.Fatal(err) } @@ -641,10 +594,6 @@ func TestMinimalRuntimeConfig(t *testing.T) { Path: proxyPath, } - expectedShimConfig := vc.ShimConfig{ - Path: shimPath, - } - expectedNetmonConfig := vc.NetmonConfig{ Path: netmonPath, Debug: false, @@ -666,9 +615,6 @@ func TestMinimalRuntimeConfig(t *testing.T) { ProxyType: defaultProxy, ProxyConfig: expectedProxyConfig, - ShimType: defaultShim, - ShimConfig: expectedShimConfig, - NetmonConfig: expectedNetmonConfig, FactoryConfig: expectedFactoryConfig, @@ -693,7 +639,6 @@ func TestMinimalRuntimeConfigWithVsock(t *testing.T) { imagePath := path.Join(dir, "image.img") initrdPath := path.Join(dir, "initrd.img") proxyPath := path.Join(dir, "proxy") - shimPath := path.Join(dir, "shim") hypervisorPath := path.Join(dir, "hypervisor") kernelPath := path.Join(dir, "kernel") @@ -716,7 +661,7 @@ func TestMinimalRuntimeConfigWithVsock(t *testing.T) { defaultHypervisorPath = hypervisorPath defaultKernelPath = kernelPath - for _, file := range []string{proxyPath, shimPath, hypervisorPath, kernelPath, imagePath} { + for _, file := range []string{proxyPath, hypervisorPath, kernelPath, imagePath} { err = WriteFile(file, "foo", testFileMode) if err != nil { t.Fatal(err) @@ -733,9 +678,6 @@ func TestMinimalRuntimeConfigWithVsock(t *testing.T) { [proxy.kata] path = "` + proxyPath + `" - [shim.kata] - path = "` + shimPath + `" - [agent.kata] ` orgVHostVSockDevicePath := utils.VHostVSockDevicePath @@ -1073,39 +1015,6 @@ func TestNewClhHypervisorConfig(t *testing.T) { } -func TestNewShimConfig(t *testing.T) { - dir, err := ioutil.TempDir(testDir, "shim-config-") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) - - shimPath := path.Join(dir, "shim") - - shim := shim{ - Path: shimPath, - } - - _, err = newShimConfig(shim) - if err == nil { - t.Fatalf("Expected newShimConfig to fail as no paths exist") - } - - err = createEmptyFile(shimPath) - if err != nil { - t.Error(err) - } - - shConfig, err := newShimConfig(shim) - if err != nil { - t.Fatalf("newShimConfig failed unexpectedly: %v", err) - } - - if shConfig.Path != shimPath { - t.Errorf("Expected shim path %v, got %v", shimPath, shConfig.Path) - } -} - func TestHypervisorDefaults(t *testing.T) { assert := assert.New(t) @@ -1368,50 +1277,6 @@ func TestProxyDefaults(t *testing.T) { assert.True(p.debug()) } -func TestShimDefaults(t *testing.T) { - assert := assert.New(t) - - tmpdir, err := ioutil.TempDir(testDir, "") - assert.NoError(err) - defer os.RemoveAll(tmpdir) - - testShimPath := filepath.Join(tmpdir, "shim") - testShimLinkPath := filepath.Join(tmpdir, "shim-link") - - err = createEmptyFile(testShimPath) - assert.NoError(err) - - err = syscall.Symlink(testShimPath, testShimLinkPath) - assert.NoError(err) - - savedShimPath := defaultShimPath - - defer func() { - defaultShimPath = savedShimPath - }() - - defaultShimPath = testShimPath - s := shim{} - p, err := s.path() - assert.NoError(err) - assert.Equal(p, defaultShimPath, "default shim path wrong") - - // test path resolution - defaultShimPath = testShimLinkPath - s = shim{} - p, err = s.path() - assert.NoError(err) - assert.Equal(p, testShimPath) - - assert.False(s.debug()) - s.Debug = true - assert.True(s.debug()) - - assert.False(s.trace()) - s.Tracing = true - assert.True(s.trace()) -} - func TestAgentDefaults(t *testing.T) { assert := assert.New(t) @@ -1865,41 +1730,3 @@ func TestCheckFactoryConfig(t *testing.T) { } } } - -func TestCheckNetNsConfigShimTrace(t *testing.T) { - assert := assert.New(t) - - type testData struct { - networkModel vc.NetInterworkingModel - disableNetNs bool - shimTrace bool - expectError bool - } - - data := []testData{ - {vc.NetXConnectMacVtapModel, false, false, false}, - {vc.NetXConnectMacVtapModel, false, true, true}, - {vc.NetXConnectMacVtapModel, true, true, true}, - {vc.NetXConnectMacVtapModel, true, false, true}, - {vc.NetXConnectNoneModel, true, false, false}, - {vc.NetXConnectNoneModel, true, true, false}, - } - - for i, d := range data { - config := oci.RuntimeConfig{ - DisableNewNetNs: d.disableNetNs, - InterNetworkModel: d.networkModel, - ShimConfig: vc.ShimConfig{ - Trace: d.shimTrace, - }, - } - - err := checkNetNsConfig(config) - - if d.expectError { - assert.Error(err, "test %d (%+v)", i, d) - } else { - assert.NoError(err, "test %d (%+v)", i, d) - } - } -} diff --git a/src/runtime/pkg/katautils/create_test.go b/src/runtime/pkg/katautils/create_test.go index 0d32d5091e..e376c129c4 100644 --- a/src/runtime/pkg/katautils/create_test.go +++ b/src/runtime/pkg/katautils/create_test.go @@ -107,7 +107,6 @@ func newTestRuntimeConfig(dir, consolePath string, create bool) (oci.RuntimeConf HypervisorConfig: hypervisorConfig, AgentType: vc.KataContainersAgent, ProxyType: vc.KataProxyType, - ShimType: vc.KataShimType, Console: consolePath, }, nil } diff --git a/src/runtime/virtcontainers/api.go b/src/runtime/virtcontainers/api.go index 8bbbd371f7..f8b9af55dd 100644 --- a/src/runtime/virtcontainers/api.go +++ b/src/runtime/virtcontainers/api.go @@ -577,44 +577,8 @@ func StatusContainer(ctx context.Context, sandboxID, containerID string) (Contai return statusContainer(s, containerID) } -// This function might have to stop the container if it realizes the shim -// process is not running anymore. This might be caused by two different -// reasons, either the process inside the VM exited and the shim terminated -// accordingly, or the shim has been killed directly and we need to make sure -// that we properly stop the container process inside the VM. -// -// When a container needs to be stopped because of those reasons, we want this -// to happen atomically from a sandbox perspective. That's why we cannot afford -// to take a read or read/write lock based on the situation, as it would break -// the initial locking from the caller. Instead, a read/write lock has to be -// taken from the caller, even if we simply return the container status without -// taking any action regarding the container. func statusContainer(sandbox *Sandbox, containerID string) (ContainerStatus, error) { if container, ok := sandbox.containers[containerID]; ok { - // We have to check for the process state to make sure - // we update the status in case the process is supposed - // to be running but has been killed or terminated. - if (container.state.State == types.StateReady || - container.state.State == types.StateRunning || - container.state.State == types.StatePaused) && - container.process.Pid > 0 { - - running, err := isShimRunning(container.process.Pid) - if err != nil { - return ContainerStatus{}, err - } - - if !running { - virtLog.WithFields(logrus.Fields{ - "state": container.state.State, - "pid": container.process.Pid}). - Info("container isn't running") - if err := container.stop(true); err != nil { - return ContainerStatus{}, err - } - } - } - return ContainerStatus{ ID: container.id, State: container.state, diff --git a/src/runtime/virtcontainers/container.go b/src/runtime/virtcontainers/container.go index 0030b1a1e1..f438b3c530 100644 --- a/src/runtime/virtcontainers/container.go +++ b/src/runtime/virtcontainers/container.go @@ -1051,51 +1051,9 @@ func (c *Container) stop(force bool) error { return err } - defer func() { - span, _ := c.trace("stopShim") - defer span.Finish() - - // If shim is still running something went wrong - // Make sure we stop the shim process - if running, _ := isShimRunning(c.process.Pid); running { - l := c.Logger() - l.Error("Failed to stop container so stopping dangling shim") - if err := stopShim(c.process.Pid); err != nil { - l.WithError(err).Warn("failed to stop shim") - } - } - - }() - - // Here we expect that stop() has been called because the container - // process returned or because it received a signal. In case of a - // signal, we want to give it some time to end the container process. - // However, if the signal didn't reach its goal, the caller still - // expects this container to be stopped, that's why we should not - // return an error, but instead try to kill it forcefully. - if err := waitForShim(c.process.Pid); err != nil { - // Force the container to be killed. - if err := c.kill(syscall.SIGKILL, true); err != nil && !force { - return err - } - - // Wait for the end of container process. We expect this call - // to succeed. Indeed, we have already given a second chance - // to the container by trying to kill it with SIGKILL, there - // is no reason to try to go further if we got an error. - if err := waitForShim(c.process.Pid); err != nil && !force { - return err - } - } - // Force the container to be killed. For most of the cases, this // should not matter and it should return an error that will be // ignored. - // But for the specific case where the shim has been SIGKILL'ed, - // the container is still running inside the VM. And this is why - // this signal will ensure the container will get killed to match - // the state of the shim. This will allow the following call to - // stopContainer() to succeed in such particular case. c.kill(syscall.SIGKILL, true) // Since the agent has supported the MultiWaitProcess, it's better to diff --git a/src/runtime/virtcontainers/kata_agent.go b/src/runtime/virtcontainers/kata_agent.go index 1726bba0da..9c66c68d00 100644 --- a/src/runtime/virtcontainers/kata_agent.go +++ b/src/runtime/virtcontainers/kata_agent.go @@ -209,7 +209,6 @@ type KataAgentState struct { } type kataAgent struct { - shim shim proxy proxy // lock protects the client pointer @@ -337,11 +336,6 @@ func (k *kataAgent) init(ctx context.Context, sandbox *Sandbox, config interface return false, err } - k.shim, err = newShim(sandbox.config.ShimType) - if err != nil { - return false, err - } - k.proxyBuiltIn = isProxyBuiltIn(sandbox.config.ProxyType) // Fetch agent runtime info. @@ -573,19 +567,7 @@ func (k *kataAgent) exec(sandbox *Sandbox, c Container, cmd types.Cmd) (*Process return nil, err } - enterNSList := []ns.Namespace{ - { - PID: c.process.Pid, - Type: ns.NSTypeNet, - }, - { - PID: c.process.Pid, - Type: ns.NSTypePID, - }, - } - - return prepareAndStartShim(sandbox, k.shim, c.id, req.ExecId, - k.state.URL, "", cmd, []ns.NSType{}, enterNSList) + return buildProcessFromExecID(req.ExecId) } func (k *kataAgent) updateInterface(ifc *vcTypes.Interface) (*vcTypes.Interface, error) { @@ -1458,8 +1440,6 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process, return nil, err } - createNSList := []ns.NSType{ns.NSTypePID} - enterNSList := []ns.Namespace{} if sandbox.networkNS.NetNsPath != "" { enterNSList = append(enterNSList, ns.Namespace{ @@ -1468,20 +1448,15 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process, }) } - // Ask to the shim to print the agent logs, if it's the process who monitors the sandbox and use_vsock is true (no proxy) - // Don't read the console socket if agent debug console is enabled. - var consoleURL string - if sandbox.config.HypervisorConfig.UseVSock && - c.GetAnnotations()[vcAnnotations.ContainerTypeKey] == string(PodSandbox) && - !k.hasAgentDebugConsole(sandbox) { - consoleURL, err = sandbox.hypervisor.getSandboxConsole(sandbox.id) - if err != nil { - return nil, err - } - } + return buildProcessFromExecID(req.ExecId) +} - return prepareAndStartShim(sandbox, k.shim, c.id, req.ExecId, - k.state.URL, consoleURL, c.config.Cmd, createNSList, enterNSList) +func buildProcessFromExecID(token string) (*Process, error) { + return &Process{ + Token: token, + StartTime: time.Now().UTC(), + Pid: -1, + }, nil } // handleEphemeralStorage handles ephemeral storages by diff --git a/src/runtime/virtcontainers/kata_builtin_shim.go b/src/runtime/virtcontainers/kata_builtin_shim.go deleted file mode 100644 index a51da030f2..0000000000 --- a/src/runtime/virtcontainers/kata_builtin_shim.go +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright (c) 2018 HyperHQ Inc. -// -// SPDX-License-Identifier: Apache-2.0 -// - -package virtcontainers - -type kataBuiltInShim struct{} - -// start is the kataBuiltInShim start implementation for kata builtin shim. -// It does nothing. The shim functionality is provided by the virtcontainers -// library. -func (s *kataBuiltInShim) start(sandbox *Sandbox, params ShimParams) (int, error) { - return -1, nil -} diff --git a/src/runtime/virtcontainers/kata_shim.go b/src/runtime/virtcontainers/kata_shim.go deleted file mode 100644 index d8c46adafc..0000000000 --- a/src/runtime/virtcontainers/kata_shim.go +++ /dev/null @@ -1,61 +0,0 @@ -// Copyright (c) 2017 Intel Corporation -// -// SPDX-License-Identifier: Apache-2.0 -// - -package virtcontainers - -import ( - "fmt" -) - -type kataShim struct{} - -// start is the ccShim start implementation. -// It starts the cc-shim binary with URL and token flags provided by -// the proxy. -func (s *kataShim) start(sandbox *Sandbox, params ShimParams) (int, error) { - if sandbox.config == nil { - return -1, fmt.Errorf("Sandbox config cannot be nil") - } - - config, ok := newShimConfig(*(sandbox.config)).(ShimConfig) - if !ok { - return -1, fmt.Errorf("Wrong shim config type, should be ShimConfig type") - } - - if config.Path == "" { - return -1, fmt.Errorf("Shim path cannot be empty") - } - - if params.URL == "" { - return -1, fmt.Errorf("URL cannot be empty") - } - - if params.Container == "" { - return -1, fmt.Errorf("Container cannot be empty") - } - - if params.Token == "" { - return -1, fmt.Errorf("Process token cannot be empty") - } - - args := []string{config.Path, "-agent", params.URL, "-container", params.Container, "-exec-id", params.Token} - - if params.Terminal { - args = append(args, "-terminal") - } - - if config.Debug { - args = append(args, "-log", "debug") - if params.ConsoleURL != "" { - args = append(args, "-agent-logs-socket", params.ConsoleURL) - } - } - - if config.Trace { - args = append(args, "-trace") - } - - return startShim(args, params) -} diff --git a/src/runtime/virtcontainers/kata_shim_test.go b/src/runtime/virtcontainers/kata_shim_test.go deleted file mode 100644 index 162919ce87..0000000000 --- a/src/runtime/virtcontainers/kata_shim_test.go +++ /dev/null @@ -1,332 +0,0 @@ -// Copyright (c) 2017 Intel Corporation -// -// SPDX-License-Identifier: Apache-2.0 -// - -package virtcontainers - -import ( - "fmt" - "io" - "io/ioutil" - "os" - "path/filepath" - "strings" - "syscall" - "testing" - "time" - "unsafe" - - . "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/mock" - "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/utils" - "github.com/stretchr/testify/assert" -) - -const ( - testContainer = "testContainer" -) - -var ( - testKataShimPath = "/usr/bin/virtcontainers/bin/test/kata-shim" - testKataShimProxyURL = "foo:///foo/kata-containers/proxy.sock" - testWrongConsolePath = "/foo/wrong-console" -) - -func getMockKataShimBinPath() string { - if DefaultMockKataShimBinPath == "" { - return testKataShimPath - } - - return DefaultMockKataShimBinPath -} - -func testKataShimStart(t *testing.T, sandbox *Sandbox, params ShimParams, expectFail bool) { - s := &kataShim{} - assert := assert.New(t) - - pid, err := s.start(sandbox, params) - if expectFail { - assert.Error(err) - assert.Equal(pid, -1) - } else { - assert.NoError(err) - assert.NotEqual(pid, -1) - } -} - -func TestKataShimStartNilSandboxConfigFailure(t *testing.T) { - testKataShimStart(t, &Sandbox{}, ShimParams{}, true) -} - -func TestKataShimStartNilShimConfigFailure(t *testing.T) { - sandbox := &Sandbox{ - config: &SandboxConfig{}, - } - - testKataShimStart(t, sandbox, ShimParams{}, true) -} - -func TestKataShimStartShimPathEmptyFailure(t *testing.T) { - sandbox := &Sandbox{ - config: &SandboxConfig{ - ShimType: KataShimType, - ShimConfig: ShimConfig{}, - }, - } - - testKataShimStart(t, sandbox, ShimParams{}, true) -} - -func TestKataShimStartShimTypeInvalid(t *testing.T) { - sandbox := &Sandbox{ - config: &SandboxConfig{ - ShimType: "foo", - ShimConfig: ShimConfig{}, - }, - } - - testKataShimStart(t, sandbox, ShimParams{}, true) -} - -func TestKataShimStartParamsTokenEmptyFailure(t *testing.T) { - sandbox := &Sandbox{ - config: &SandboxConfig{ - ShimType: KataShimType, - ShimConfig: ShimConfig{ - Path: getMockKataShimBinPath(), - }, - }, - } - - testKataShimStart(t, sandbox, ShimParams{}, true) -} - -func TestKataShimStartParamsURLEmptyFailure(t *testing.T) { - sandbox := &Sandbox{ - config: &SandboxConfig{ - ShimType: KataShimType, - ShimConfig: ShimConfig{ - Path: getMockKataShimBinPath(), - }, - }, - } - - params := ShimParams{ - Token: "testToken", - } - - testKataShimStart(t, sandbox, params, true) -} - -func TestKataShimStartParamsContainerEmptyFailure(t *testing.T) { - sandbox := &Sandbox{ - config: &SandboxConfig{ - ShimType: KataShimType, - ShimConfig: ShimConfig{ - Path: getMockKataShimBinPath(), - }, - }, - } - - params := ShimParams{ - Token: "testToken", - URL: "unix://is/awesome", - } - - testKataShimStart(t, sandbox, params, true) -} - -func TestKataShimStartParamsInvalidCommand(t *testing.T) { - dir, err := ioutil.TempDir("", "") - assert.NoError(t, err) - defer os.RemoveAll(dir) - - cmd := filepath.Join(dir, "does-not-exist") - - sandbox := &Sandbox{ - config: &SandboxConfig{ - ShimType: KataShimType, - ShimConfig: ShimConfig{ - Path: cmd, - }, - }, - } - - params := ShimParams{ - Token: "testToken", - URL: "http://foo", - } - - testKataShimStart(t, sandbox, params, true) -} - -func startKataShimStartWithoutConsoleSuccessful(t *testing.T, detach bool) (*os.File, *os.File, *os.File, *Sandbox, ShimParams, error) { - saveStdout := os.Stdout - rStdout, wStdout, err := os.Pipe() - if err != nil { - return nil, nil, nil, &Sandbox{}, ShimParams{}, err - } - - os.Stdout = wStdout - - sandbox := &Sandbox{ - config: &SandboxConfig{ - ShimType: KataShimType, - ShimConfig: ShimConfig{ - Path: getMockKataShimBinPath(), - }, - }, - } - - params := ShimParams{ - Container: testContainer, - Token: "testToken", - URL: testKataShimProxyURL, - Detach: detach, - } - - return rStdout, wStdout, saveStdout, sandbox, params, nil -} - -func TestKataShimStartSuccessful(t *testing.T) { - rStdout, wStdout, saveStdout, sandbox, params, err := startKataShimStartWithoutConsoleSuccessful(t, false) - assert := assert.New(t) - assert.NoError(err) - - defer func() { - os.Stdout = saveStdout - rStdout.Close() - wStdout.Close() - }() - - testKataShimStart(t, sandbox, params, false) - - bufStdout := make([]byte, 1024) - _, err = rStdout.Read(bufStdout) - assert.NoError(err) - assert.True(strings.Contains(string(bufStdout), ShimStdoutOutput)) -} - -func TestKataShimStartDetachSuccessful(t *testing.T) { - rStdout, wStdout, saveStdout, sandbox, params, err := startKataShimStartWithoutConsoleSuccessful(t, true) - assert.NoError(t, err) - - defer func() { - os.Stdout = saveStdout - wStdout.Close() - rStdout.Close() - }() - - testKataShimStart(t, sandbox, params, false) - - readCh := make(chan error, 1) - go func() { - defer close(readCh) - bufStdout := make([]byte, 1024) - n, err := rStdout.Read(bufStdout) - if err != nil && err != io.EOF { - readCh <- err - return - } - - if n > 0 { - readCh <- fmt.Errorf("Not expecting to read anything, Got %q", string(bufStdout)) - return - } - - readCh <- nil - }() - - select { - case err := <-readCh: - assert.NoError(t, err) - case <-time.After(time.Duration(20) * time.Millisecond): - return - } -} - -func TestKataShimStartWithConsoleNonExistingFailure(t *testing.T) { - sandbox := &Sandbox{ - config: &SandboxConfig{ - ShimType: KataShimType, - ShimConfig: ShimConfig{ - Path: getMockKataShimBinPath(), - }, - }, - } - - params := ShimParams{ - Token: "testToken", - URL: testKataShimProxyURL, - Console: testWrongConsolePath, - } - - testKataShimStart(t, sandbox, params, true) -} - -// unlockpt unlocks the slave pseudoterminal device corresponding to the master pseudoterminal referred to by f. -func unlockpt(f *os.File) error { - var u int32 - - return utils.Ioctl(f.Fd(), syscall.TIOCSPTLCK, uintptr(unsafe.Pointer(&u))) -} - -// ptsname retrieves the name of the first available pts for the given master. -func ptsname(f *os.File) (string, error) { - var n int32 - - if err := utils.Ioctl(f.Fd(), syscall.TIOCGPTN, uintptr(unsafe.Pointer(&n))); err != nil { - return "", err - } - - return fmt.Sprintf("/dev/pts/%d", n), nil -} - -func newConsole() (*os.File, string, error) { - master, err := os.OpenFile("/dev/ptmx", syscall.O_RDWR|syscall.O_NOCTTY|syscall.O_CLOEXEC, 0) - if err != nil { - return nil, "", err - } - - console, err := ptsname(master) - if err != nil { - return nil, "", err - } - - if err := unlockpt(master); err != nil { - return nil, "", err - } - - if err := os.Chmod(console, 0600); err != nil { - return nil, "", err - } - - return master, console, nil -} - -func TestKataShimStartWithConsoleSuccessful(t *testing.T) { - defer cleanUp() - - master, console, err := newConsole() - t.Logf("Console created for tests:%s\n", console) - assert.NoError(t, err) - - sandbox := &Sandbox{ - config: &SandboxConfig{ - ShimType: KataShimType, - ShimConfig: ShimConfig{ - Path: getMockKataShimBinPath(), - }, - }, - } - - params := ShimParams{ - Container: testContainer, - Token: "testToken", - URL: testKataShimProxyURL, - Console: console, - } - - testKataShimStart(t, sandbox, params, false) - master.Close() -} diff --git a/src/runtime/virtcontainers/noop_shim.go b/src/runtime/virtcontainers/noop_shim.go deleted file mode 100644 index e700c07c1f..0000000000 --- a/src/runtime/virtcontainers/noop_shim.go +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) 2017 Intel Corporation -// -// SPDX-License-Identifier: Apache-2.0 -// - -package virtcontainers - -type noopShim struct{} - -// start is the noopShim start implementation for testing purpose. -// It does nothing. -func (s *noopShim) start(sandbox *Sandbox, params ShimParams) (int, error) { - return 0, nil -} diff --git a/src/runtime/virtcontainers/noop_shim_test.go b/src/runtime/virtcontainers/noop_shim_test.go deleted file mode 100644 index 0626a4a8b7..0000000000 --- a/src/runtime/virtcontainers/noop_shim_test.go +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright (c) 2017 Intel Corporation -// -// SPDX-License-Identifier: Apache-2.0 -// - -package virtcontainers - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestNoopShimStart(t *testing.T) { - assert := assert.New(t) - s := &noopShim{} - sandbox := &Sandbox{} - params := ShimParams{} - expected := 0 - - pid, err := s.start(sandbox, params) - assert.NoError(err) - assert.Equal(pid, expected) -} diff --git a/src/runtime/virtcontainers/persist.go b/src/runtime/virtcontainers/persist.go index f545eeb62c..028b36f88e 100644 --- a/src/runtime/virtcontainers/persist.go +++ b/src/runtime/virtcontainers/persist.go @@ -185,7 +185,6 @@ func (s *Sandbox) dumpConfig(ss *persistapi.SandboxState) { Path: sconfig.ProxyConfig.Path, Debug: sconfig.ProxyConfig.Debug, }, - ShimType: string(sconfig.ShimType), NetworkConfig: persistapi.NetworkConfig{ NetNSPath: sconfig.NetworkConfig.NetNSPath, NetNsCreated: sconfig.NetworkConfig.NetNsCreated, @@ -273,19 +272,6 @@ func (s *Sandbox) dumpConfig(ss *persistapi.SandboxState) { } } - if sconfig.ShimType == "kataShim" { - var shim ShimConfig - err := mapstructure.Decode(sconfig.ShimConfig, &shim) - if err != nil { - s.Logger().WithError(err).Error("internal error: ShimConfig failed to decode") - } else { - ss.Config.KataShimConfig = &persistapi.ShimConfig{ - Path: shim.Path, - Debug: shim.Debug, - } - } - } - for _, contConf := range sconfig.Containers { ss.Config.ContainerConfigs = append(ss.Config.ContainerConfigs, persistapi.ContainerConfig{ ID: contConf.ID, @@ -475,7 +461,6 @@ func loadSandboxConfig(id string) (*SandboxConfig, error) { Path: savedConf.ProxyConfig.Path, Debug: savedConf.ProxyConfig.Debug, }, - ShimType: ShimType(savedConf.ShimType), NetworkConfig: NetworkConfig{ NetNSPath: savedConf.NetworkConfig.NetNSPath, NetNsCreated: savedConf.NetworkConfig.NetNsCreated, @@ -558,13 +543,6 @@ func loadSandboxConfig(id string) (*SandboxConfig, error) { } } - if savedConf.ShimType == "kataShim" { - sconfig.ShimConfig = ShimConfig{ - Path: savedConf.KataShimConfig.Path, - Debug: savedConf.KataShimConfig.Debug, - } - } - for _, contConf := range savedConf.ContainerConfigs { sconfig.Containers = append(sconfig.Containers, ContainerConfig{ ID: contConf.ID, diff --git a/src/runtime/virtcontainers/pkg/oci/utils.go b/src/runtime/virtcontainers/pkg/oci/utils.go index 2c1537ba71..1601e2169c 100644 --- a/src/runtime/virtcontainers/pkg/oci/utils.go +++ b/src/runtime/virtcontainers/pkg/oci/utils.go @@ -102,9 +102,6 @@ type RuntimeConfig struct { ProxyType vc.ProxyType ProxyConfig vc.ProxyConfig - ShimType vc.ShimType - ShimConfig interface{} - Console string //Determines how the VM should be connected to the @@ -853,9 +850,6 @@ func SandboxConfig(ocispec specs.Spec, runtime RuntimeConfig, bundlePath, cid, c ProxyType: runtime.ProxyType, ProxyConfig: runtime.ProxyConfig, - ShimType: runtime.ShimType, - ShimConfig: runtime.ShimConfig, - NetworkConfig: networkConfig, Containers: []vc.ContainerConfig{containerConfig}, diff --git a/src/runtime/virtcontainers/pkg/oci/utils_test.go b/src/runtime/virtcontainers/pkg/oci/utils_test.go index d97dbf468e..a86c0b34d8 100644 --- a/src/runtime/virtcontainers/pkg/oci/utils_test.go +++ b/src/runtime/virtcontainers/pkg/oci/utils_test.go @@ -73,7 +73,6 @@ func TestMinimalSandboxConfig(t *testing.T) { HypervisorType: vc.QemuHypervisor, AgentType: vc.KataContainersAgent, ProxyType: vc.KataProxyType, - ShimType: vc.KataShimType, Console: consolePath, } @@ -173,7 +172,6 @@ func TestMinimalSandboxConfig(t *testing.T) { HypervisorType: vc.QemuHypervisor, AgentType: vc.KataContainersAgent, ProxyType: vc.KataProxyType, - ShimType: vc.KataShimType, NetworkConfig: expectedNetworkConfig, diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index 3e70779174..999405c615 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -25,7 +25,6 @@ import ( "github.com/sirupsen/logrus" "github.com/vishvananda/netlink" - "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/agent/protocols/grpc" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/device/api" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/device/config" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/device/drivers" @@ -33,6 +32,7 @@ import ( exp "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/experimental" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/persist" persistapi "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/persist/api" + "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/agent/protocols/grpc" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/annotations" vccgroups "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/cgroups" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/compatoci" @@ -88,9 +88,6 @@ type SandboxConfig struct { ProxyType ProxyType ProxyConfig ProxyConfig - ShimType ShimType - ShimConfig interface{} - NetworkConfig NetworkConfig // Volumes is a list of shared volumes between the host and the Sandbox. diff --git a/src/runtime/virtcontainers/shim.go b/src/runtime/virtcontainers/shim.go deleted file mode 100644 index 919f58e839..0000000000 --- a/src/runtime/virtcontainers/shim.go +++ /dev/null @@ -1,281 +0,0 @@ -// Copyright (c) 2017 Intel Corporation -// -// SPDX-License-Identifier: Apache-2.0 -// - -package virtcontainers - -import ( - "fmt" - "os" - "os/exec" - "syscall" - "time" - - ns "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/nsenter" - "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/types" - "github.com/mitchellh/mapstructure" - "github.com/sirupsen/logrus" -) - -// ShimType describes a shim type. -type ShimType string - -const ( - // NoopShimType is the noopShim. - NoopShimType ShimType = "noopShim" - - // KataShimType is the Kata Containers shim type. - KataShimType ShimType = "kataShim" - - // KataBuiltInShimType is the Kata Containers builtin shim type. - KataBuiltInShimType ShimType = "kataBuiltInShim" -) - -var waitForShimTimeout = 10.0 -var consoleFileMode = os.FileMode(0660) - -// ShimParams is the structure providing specific parameters needed -// for the execution of the shim binary. -type ShimParams struct { - Container string - Token string - URL string - Console string - ConsoleURL string - Terminal bool - Detach bool - PID int - CreateNS []ns.NSType - EnterNS []ns.Namespace -} - -// ShimConfig is the structure providing specific configuration -// for shim implementations. -type ShimConfig struct { - Path string - Debug bool - Trace bool -} - -// Set sets a shim type based on the input string. -func (pType *ShimType) Set(value string) error { - switch value { - case "noopShim": - *pType = NoopShimType - case "kataShim": - *pType = KataShimType - case "kataBuiltInShim": - *pType = KataBuiltInShimType - default: - return fmt.Errorf("Unknown shim type %s", value) - } - return nil -} - -// String converts a shim type to a string. -func (pType *ShimType) String() string { - switch *pType { - case NoopShimType: - return string(NoopShimType) - case KataShimType: - return string(KataShimType) - case KataBuiltInShimType: - return string(KataBuiltInShimType) - default: - return "" - } -} - -// newShim returns a shim from a shim type. -func newShim(pType ShimType) (shim, error) { - switch pType { - case NoopShimType: - return &noopShim{}, nil - case KataShimType: - return &kataShim{}, nil - case KataBuiltInShimType: - return &kataBuiltInShim{}, nil - default: - return &noopShim{}, nil - } -} - -// newShimConfig returns a shim config from a generic SandboxConfig interface. -func newShimConfig(config SandboxConfig) interface{} { - switch config.ShimType { - case NoopShimType, KataBuiltInShimType: - return nil - case KataShimType: - var shimConfig ShimConfig - err := mapstructure.Decode(config.ShimConfig, &shimConfig) - if err != nil { - return err - } - return shimConfig - default: - return nil - } -} - -func shimLogger() *logrus.Entry { - return virtLog.WithField("subsystem", "shim") -} - -func signalShim(pid int, sig syscall.Signal) error { - if pid <= 0 { - return nil - } - - shimLogger().WithFields( - logrus.Fields{ - "shim-pid": pid, - "shim-signal": sig, - }).Info("Signalling shim") - - return syscall.Kill(pid, sig) -} - -func stopShim(pid int) error { - if pid <= 0 { - return nil - } - - if err := signalShim(pid, syscall.SIGKILL); err != nil && err != syscall.ESRCH { - return err - } - - return nil -} - -func prepareAndStartShim(sandbox *Sandbox, shim shim, cid, token, url, consoleURL string, cmd types.Cmd, - createNSList []ns.NSType, enterNSList []ns.Namespace) (*Process, error) { - process := &Process{ - Token: token, - StartTime: time.Now().UTC(), - } - - shimParams := ShimParams{ - Container: cid, - Token: token, - URL: url, - Console: cmd.Console, - Terminal: cmd.Interactive, - Detach: cmd.Detach, - CreateNS: createNSList, - EnterNS: enterNSList, - ConsoleURL: consoleURL, - } - - pid, err := shim.start(sandbox, shimParams) - if err != nil { - return nil, err - } - - process.Pid = pid - - return process, nil -} - -func startShim(args []string, params ShimParams) (int, error) { - cmd := exec.Command(args[0], args[1:]...) - - if !params.Detach { - cmd.Stdin = os.Stdin - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - } - - cloneFlags := 0 - for _, nsType := range params.CreateNS { - cloneFlags |= ns.CloneFlagsTable[nsType] - } - - cmd.SysProcAttr = &syscall.SysProcAttr{ - Cloneflags: uintptr(cloneFlags), - } - - var f *os.File - var err error - if params.Console != "" { - f, err = os.OpenFile(params.Console, os.O_RDWR, consoleFileMode) - if err != nil { - return -1, err - } - - cmd.Stdin = f - cmd.Stdout = f - cmd.Stderr = f - // Create Session - cmd.SysProcAttr.Setsid = true - // Set Controlling terminal to Ctty - cmd.SysProcAttr.Setctty = true - cmd.SysProcAttr.Ctty = int(f.Fd()) - } - defer func() { - if f != nil { - f.Close() - } - }() - - if err := ns.NsEnter(params.EnterNS, func() error { - return cmd.Start() - }); err != nil { - return -1, err - } - - return cmd.Process.Pid, nil -} - -func isShimRunning(pid int) (bool, error) { - if pid <= 0 { - return false, nil - } - - process, err := os.FindProcess(pid) - if err != nil { - return false, err - } - - if err := process.Signal(syscall.Signal(0)); err != nil { - return false, nil - } - - return true, nil -} - -// waitForShim waits for the end of the shim unless it reaches the timeout -// first, returning an error in that case. -func waitForShim(pid int) error { - if pid <= 0 { - return nil - } - - tInit := time.Now() - for { - running, err := isShimRunning(pid) - if err != nil { - return err - } - - if !running { - break - } - - if time.Since(tInit).Seconds() >= waitForShimTimeout { - return fmt.Errorf("Shim still running, timeout %f s has been reached", waitForShimTimeout) - } - - // Let's avoid to run a too busy loop - time.Sleep(time.Duration(100) * time.Millisecond) - } - - return nil -} - -// shim is the virtcontainers shim interface. -type shim interface { - // start starts the shim relying on its configuration and on - // parameters provided. - start(sandbox *Sandbox, params ShimParams) (int, error) -} diff --git a/src/runtime/virtcontainers/shim_test.go b/src/runtime/virtcontainers/shim_test.go deleted file mode 100644 index e9bd027c66..0000000000 --- a/src/runtime/virtcontainers/shim_test.go +++ /dev/null @@ -1,237 +0,0 @@ -// Copyright (c) 2017 Intel Corporation -// -// SPDX-License-Identifier: Apache-2.0 -// - -package virtcontainers - -import ( - "os/exec" - "syscall" - "testing" - - "github.com/stretchr/testify/assert" -) - -const ( - testRunningProcess = "sleep" -) - -func testSetShimType(t *testing.T, value string, expected ShimType) { - var shimType ShimType - assert := assert.New(t) - - err := (&shimType).Set(value) - assert.NoError(err) - assert.Equal(shimType, expected) -} - -func TestSetKataShimType(t *testing.T) { - testSetShimType(t, "kataShim", KataShimType) -} - -func TestSetNoopShimType(t *testing.T) { - testSetShimType(t, "noopShim", NoopShimType) -} - -func TestSetUnknownShimType(t *testing.T) { - var shimType ShimType - assert := assert.New(t) - - unknownType := "unknown" - - err := (&shimType).Set(unknownType) - assert.Error(err) - assert.NotEqual(shimType, NoopShimType) -} - -func testStringFromShimType(t *testing.T, shimType ShimType, expected string) { - shimTypeStr := (&shimType).String() - assert := assert.New(t) - assert.Equal(shimTypeStr, expected) -} - -func TestStringFromKataShimType(t *testing.T) { - shimType := KataShimType - testStringFromShimType(t, shimType, "kataShim") -} - -func TestStringFromNoopShimType(t *testing.T) { - shimType := NoopShimType - testStringFromShimType(t, shimType, "noopShim") -} - -func TestStringFromKataBuiltInShimType(t *testing.T) { - shimType := KataBuiltInShimType - testStringFromShimType(t, shimType, "kataBuiltInShim") -} - -func TestStringFromUnknownShimType(t *testing.T) { - var shimType ShimType - testStringFromShimType(t, shimType, "") -} - -func testNewShimFromShimType(t *testing.T, shimType ShimType, expected shim) { - assert := assert.New(t) - result, err := newShim(shimType) - assert.NoError(err) - assert.Exactly(result, expected) -} - -func TestNewShimFromKataShimType(t *testing.T) { - shimType := KataShimType - expectedShim := &kataShim{} - testNewShimFromShimType(t, shimType, expectedShim) -} - -func TestNewShimFromNoopShimType(t *testing.T) { - shimType := NoopShimType - expectedShim := &noopShim{} - testNewShimFromShimType(t, shimType, expectedShim) -} - -func TestNewShimFromKataBuiltInShimType(t *testing.T) { - shimType := KataBuiltInShimType - expectedShim := &kataBuiltInShim{} - testNewShimFromShimType(t, shimType, expectedShim) -} - -func TestNewShimFromUnknownShimType(t *testing.T) { - var shimType ShimType - assert := assert.New(t) - - _, err := newShim(shimType) - assert.NoError(err) -} - -func testNewShimConfigFromSandboxConfig(t *testing.T, sandboxConfig SandboxConfig, expected interface{}) { - assert := assert.New(t) - result := newShimConfig(sandboxConfig) - assert.Exactly(result, expected) -} - -func TestNewShimConfigFromKataShimSandboxConfig(t *testing.T) { - shimConfig := ShimConfig{} - - sandboxConfig := SandboxConfig{ - ShimType: KataShimType, - ShimConfig: shimConfig, - } - - testNewShimConfigFromSandboxConfig(t, sandboxConfig, shimConfig) -} - -func TestNewShimConfigFromNoopShimSandboxConfig(t *testing.T) { - sandboxConfig := SandboxConfig{ - ShimType: NoopShimType, - } - - testNewShimConfigFromSandboxConfig(t, sandboxConfig, nil) -} - -func TestNewShimConfigFromKataBuiltInShimSandboxConfig(t *testing.T) { - sandboxConfig := SandboxConfig{ - ShimType: KataBuiltInShimType, - } - - testNewShimConfigFromSandboxConfig(t, sandboxConfig, nil) -} - -func TestNewShimConfigFromUnknownShimSandboxConfig(t *testing.T) { - var shimType ShimType - - sandboxConfig := SandboxConfig{ - ShimType: shimType, - } - - testNewShimConfigFromSandboxConfig(t, sandboxConfig, nil) -} - -func testRunSleep0AndGetPid(t *testing.T) int { - assert := assert.New(t) - - binPath, err := exec.LookPath(testRunningProcess) - assert.NoError(err) - - cmd := exec.Command(binPath, "0") - err = cmd.Start() - assert.NoError(err) - - pid := cmd.Process.Pid - err = cmd.Wait() - assert.NoError(err) - - return pid -} - -func testRunSleep999AndGetCmd(t *testing.T) *exec.Cmd { - assert := assert.New(t) - - binPath, err := exec.LookPath(testRunningProcess) - assert.NoError(err) - - cmd := exec.Command(binPath, "999") - err = cmd.Start() - assert.NoError(err) - return cmd -} - -func TestStopShimSuccessfulProcessNotRunning(t *testing.T) { - assert := assert.New(t) - pid := testRunSleep0AndGetPid(t) - assert.NoError(stopShim(pid)) -} - -func TestStopShimSuccessfulProcessRunning(t *testing.T) { - assert := assert.New(t) - cmd := testRunSleep999AndGetCmd(t) - assert.NoError(stopShim(cmd.Process.Pid)) -} - -func testIsShimRunning(t *testing.T, pid int, expected bool) { - assert := assert.New(t) - running, err := isShimRunning(pid) - assert.NoError(err) - assert.Equal(running, expected) -} - -func TestIsShimRunningFalse(t *testing.T) { - pid := testRunSleep0AndGetPid(t) - - testIsShimRunning(t, pid, false) -} - -func TestIsShimRunningTrue(t *testing.T) { - cmd := testRunSleep999AndGetCmd(t) - assert := assert.New(t) - - testIsShimRunning(t, cmd.Process.Pid, true) - - err := syscall.Kill(cmd.Process.Pid, syscall.SIGKILL) - assert.NoError(err) -} - -func TestWaitForShimInvalidPidSuccessful(t *testing.T) { - wrongValuesList := []int{0, -1, -100} - assert := assert.New(t) - - for _, val := range wrongValuesList { - err := waitForShim(val) - assert.NoError(err) - } -} - -func TestWaitForShimNotRunningSuccessful(t *testing.T) { - pid := testRunSleep0AndGetPid(t) - assert := assert.New(t) - assert.NoError(waitForShim(pid)) -} - -func TestWaitForShimRunningForTooLongFailure(t *testing.T) { - cmd := testRunSleep999AndGetCmd(t) - assert := assert.New(t) - - waitForShimTimeout = 0.1 - assert.Error(waitForShim(cmd.Process.Pid)) - assert.NoError(syscall.Kill(cmd.Process.Pid, syscall.SIGKILL)) -}