From 0e2459d13ea10edb37dad5097757603e657057af Mon Sep 17 00:00:00 2001 From: Suraj Deshmukh Date: Thu, 21 Apr 2022 18:44:28 +0530 Subject: [PATCH 01/36] docs: Add cgroupDriver for containerd This commit updates the "Run Kata Containers with Kubernetes" to include cgroupDriver configuration via "KubeletConfiguration". Without this setting kubeadm defaults to systemd cgroupDriver. Containerd with Kata cannot spawn conntainers with systemd cgroup driver. Fixes: #4262 Signed-off-by: Suraj Deshmukh --- docs/how-to/run-kata-with-k8s.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/docs/how-to/run-kata-with-k8s.md b/docs/how-to/run-kata-with-k8s.md index fd53838b88..baee63bffe 100644 --- a/docs/how-to/run-kata-with-k8s.md +++ b/docs/how-to/run-kata-with-k8s.md @@ -99,7 +99,18 @@ $ sudo systemctl restart kubelet $ sudo kubeadm init --ignore-preflight-errors=all --cri-socket /var/run/crio/crio.sock --pod-network-cidr=10.244.0.0/16 # If using containerd -$ sudo kubeadm init --ignore-preflight-errors=all --cri-socket /run/containerd/containerd.sock --pod-network-cidr=10.244.0.0/16 +$ cat < Date: Mon, 23 May 2022 12:27:55 -0700 Subject: [PATCH 02/36] katautils: don't do validation when loading hypervisor config Policy for whats valid/invalid within the config varies by VMM, host, and by silicon architecture. Let's keep katautils simple for just translating a toml to the hypervisor config structure, and leave validation to virtcontainers. Without this change, we're doing duplicate validation. Signed-off-by: Eric Ernst --- src/runtime/pkg/katautils/config.go | 43 +++++++++++------------- src/runtime/pkg/katautils/config_test.go | 12 +++---- src/runtime/virtcontainers/qemu.go | 1 - 3 files changed, 26 insertions(+), 30 deletions(-) diff --git a/src/runtime/pkg/katautils/config.go b/src/runtime/pkg/katautils/config.go index dbdfdac303..fd6df4c3ca 100644 --- a/src/runtime/pkg/katautils/config.go +++ b/src/runtime/pkg/katautils/config.go @@ -220,7 +220,7 @@ func (h hypervisor) initrd() (string, error) { p := h.Initrd if p == "" { - return "", errors.New("initrd is not set") + return "", nil } return ResolvePath(p) @@ -230,7 +230,7 @@ func (h hypervisor) image() (string, error) { p := h.Image if p == "" { - return "", errors.New("image is not set") + return "", nil } return ResolvePath(p) @@ -474,24 +474,6 @@ func (h hypervisor) vhostUserStorePath() string { return h.VhostUserStorePath } -func (h hypervisor) getInitrdAndImage() (initrd string, image string, err error) { - initrd, errInitrd := h.initrd() - - image, errImage := h.image() - - if h.ConfidentialGuest && h.MachineType == vc.QemuCCWVirtio { - if image != "" || initrd != "" { - return "", "", errors.New("Neither the image nor initrd path may be set for Secure Execution") - } - } else if image != "" && initrd != "" { - return "", "", errors.New("having both an image and an initrd defined in the configuration file is not supported") - } else if errInitrd != nil && errImage != nil { - return "", "", fmt.Errorf("Either initrd or image must be set to a valid path (initrd: %v) (image: %v)", errInitrd, errImage) - } - - return -} - func (h hypervisor) getDiskRateLimiterBwMaxRate() int64 { return h.DiskRateLimiterBwMaxRate } @@ -601,7 +583,12 @@ func newFirecrackerHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) { return vc.HypervisorConfig{}, err } - initrd, image, err := h.getInitrdAndImage() + initrd, err := h.initrd() + if err != nil { + return vc.HypervisorConfig{}, err + } + + image, err := h.image() if err != nil { return vc.HypervisorConfig{}, err } @@ -663,7 +650,12 @@ func newQemuHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) { return vc.HypervisorConfig{}, err } - initrd, image, err := h.getInitrdAndImage() + initrd, err := h.initrd() + if err != nil { + return vc.HypervisorConfig{}, err + } + + image, err := h.image() if err != nil { return vc.HypervisorConfig{}, err } @@ -857,7 +849,12 @@ func newClhHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) { return vc.HypervisorConfig{}, err } - initrd, image, err := h.getInitrdAndImage() + initrd, err := h.initrd() + if err != nil { + return vc.HypervisorConfig{}, err + } + + image, err := h.image() if err != nil { return vc.HypervisorConfig{}, err } diff --git a/src/runtime/pkg/katautils/config_test.go b/src/runtime/pkg/katautils/config_test.go index 0bcac2137f..a33e37a1a4 100644 --- a/src/runtime/pkg/katautils/config_test.go +++ b/src/runtime/pkg/katautils/config_test.go @@ -1017,7 +1017,7 @@ func TestHypervisorDefaultsKernel(t *testing.T) { assert.Equal(h.kernelParams(), kernelParams, "custom hypervisor kernel parameterms wrong") } -// The default initrd path is not returned by h.initrd() +// The default initrd path is not returned by h.initrd(), it isn't an error if path isn't provided func TestHypervisorDefaultsInitrd(t *testing.T) { assert := assert.New(t) @@ -1041,18 +1041,18 @@ func TestHypervisorDefaultsInitrd(t *testing.T) { defaultInitrdPath = testInitrdPath h := hypervisor{} p, err := h.initrd() - assert.Error(err) + assert.NoError(err) assert.Equal(p, "", "default Image path wrong") // test path resolution defaultInitrdPath = testInitrdLinkPath h = hypervisor{} p, err = h.initrd() - assert.Error(err) + assert.NoError(err) assert.Equal(p, "") } -// The default image path is not returned by h.image() +// The default image path is not returned by h.image(), it isn't an error if path isn't provided func TestHypervisorDefaultsImage(t *testing.T) { assert := assert.New(t) @@ -1076,14 +1076,14 @@ func TestHypervisorDefaultsImage(t *testing.T) { defaultImagePath = testImagePath h := hypervisor{} p, err := h.image() - assert.Error(err) + assert.NoError(err) assert.Equal(p, "", "default Image path wrong") // test path resolution defaultImagePath = testImageLinkPath h = hypervisor{} p, err = h.image() - assert.Error(err) + assert.NoError(err) assert.Equal(p, "") } diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index c8e48953f6..a91505d285 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -513,7 +513,6 @@ func (q *qemu) CreateVM(ctx context.Context, id string, network Network, hypervi span, ctx := katatrace.Trace(ctx, q.Logger(), "CreateVM", qemuTracingTags, map[string]string{"VM_ID": q.id}) defer span.End() - // Breaks hypervisor abstraction Has Kata Specific logic: See within if err := q.setup(ctx, id, hypervisorConfig); err != nil { return err } From bdf5e5229b751c868d6c49ab966f6de8547cb38e Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Mon, 23 May 2022 15:58:47 -0700 Subject: [PATCH 03/36] virtcontainers: validate hypervisor config outside of hypervisor itself Depending on the user of it, the hypervisor from hypervisor interface could have differing view on what is valid or not. To help decouple, let's instead check the hypervisor config validity as part of the sandbox creation, rather than as part of the CreateVM call within the hypervisor interface implementation. Fixes: #4251 Signed-off-by: Eric Ernst --- src/runtime/virtcontainers/acrn.go | 4 - src/runtime/virtcontainers/clh.go | 5 - src/runtime/virtcontainers/fc.go | 5 - src/runtime/virtcontainers/hypervisor.go | 55 -------- src/runtime/virtcontainers/hypervisor_test.go | 118 ------------------ src/runtime/virtcontainers/mock_hypervisor.go | 4 - .../virtcontainers/mock_hypervisor_test.go | 16 +-- src/runtime/virtcontainers/qemu.go | 5 - src/runtime/virtcontainers/sandbox.go | 60 +++++++++ src/runtime/virtcontainers/vm.go | 3 +- 10 files changed, 64 insertions(+), 211 deletions(-) diff --git a/src/runtime/virtcontainers/acrn.go b/src/runtime/virtcontainers/acrn.go index f6da05ec58..1c3ebc1476 100644 --- a/src/runtime/virtcontainers/acrn.go +++ b/src/runtime/virtcontainers/acrn.go @@ -348,10 +348,6 @@ func (a *Acrn) createDummyVirtioBlkDev(ctx context.Context, devices []Device) ([ } func (a *Acrn) setConfig(config *HypervisorConfig) error { - if err := config.Valid(); err != nil { - return err - } - a.config = *config return nil diff --git a/src/runtime/virtcontainers/clh.go b/src/runtime/virtcontainers/clh.go index 6984b73be5..85ec05f15a 100644 --- a/src/runtime/virtcontainers/clh.go +++ b/src/runtime/virtcontainers/clh.go @@ -268,11 +268,6 @@ var clhDebugKernelParams = []Param{ //########################################################### func (clh *cloudHypervisor) setConfig(config *HypervisorConfig) error { - err := config.Valid() - if err != nil { - return err - } - clh.config = *config return nil diff --git a/src/runtime/virtcontainers/fc.go b/src/runtime/virtcontainers/fc.go index 25012d3f3d..434e320417 100644 --- a/src/runtime/virtcontainers/fc.go +++ b/src/runtime/virtcontainers/fc.go @@ -189,11 +189,6 @@ func (fc *firecracker) truncateID(id string) string { } func (fc *firecracker) setConfig(config *HypervisorConfig) error { - err := config.Valid() - if err != nil { - return err - } - fc.config = *config return nil diff --git a/src/runtime/virtcontainers/hypervisor.go b/src/runtime/virtcontainers/hypervisor.go index 119c4667c3..18ccd7ae26 100644 --- a/src/runtime/virtcontainers/hypervisor.go +++ b/src/runtime/virtcontainers/hypervisor.go @@ -568,61 +568,6 @@ func (conf *HypervisorConfig) CheckTemplateConfig() error { return nil } -func (conf *HypervisorConfig) Valid() error { - // Kata specific checks. Should be done outside the hypervisor - if conf.KernelPath == "" { - return fmt.Errorf("Missing kernel path") - } - - if conf.ConfidentialGuest && conf.HypervisorMachineType == QemuCCWVirtio { - if conf.ImagePath != "" || conf.InitrdPath != "" { - fmt.Println("yes, failing") - return fmt.Errorf("Neither the image or initrd path may be set for Secure Execution") - } - } else if conf.ImagePath == "" && conf.InitrdPath == "" { - return fmt.Errorf("Missing image and initrd path") - } else if conf.ImagePath != "" && conf.InitrdPath != "" { - return fmt.Errorf("Image and initrd path cannot be both set") - } - - if err := conf.CheckTemplateConfig(); err != nil { - return err - } - - if conf.NumVCPUs == 0 { - conf.NumVCPUs = defaultVCPUs - } - - if conf.MemorySize == 0 { - conf.MemorySize = defaultMemSzMiB - } - - if conf.DefaultBridges == 0 { - conf.DefaultBridges = defaultBridges - } - - if conf.BlockDeviceDriver == "" { - conf.BlockDeviceDriver = defaultBlockDriver - } else if conf.BlockDeviceDriver == config.VirtioBlock && conf.HypervisorMachineType == QemuCCWVirtio { - conf.BlockDeviceDriver = config.VirtioBlockCCW - } - - if conf.DefaultMaxVCPUs == 0 || conf.DefaultMaxVCPUs > defaultMaxVCPUs { - conf.DefaultMaxVCPUs = defaultMaxVCPUs - } - - if conf.ConfidentialGuest && conf.NumVCPUs != conf.DefaultMaxVCPUs { - hvLogger.Warnf("Confidential guests do not support hotplugging of vCPUs. Setting DefaultMaxVCPUs to NumVCPUs (%d)", conf.NumVCPUs) - conf.DefaultMaxVCPUs = conf.NumVCPUs - } - - if conf.Msize9p == 0 && conf.SharedFS != config.VirtioFS { - conf.Msize9p = defaultMsize9p - } - - return nil -} - // AddKernelParam allows the addition of new kernel parameters to an existing // hypervisor configuration. func (conf *HypervisorConfig) AddKernelParam(p Param) error { diff --git a/src/runtime/virtcontainers/hypervisor_test.go b/src/runtime/virtcontainers/hypervisor_test.go index ec475e86eb..31452200d3 100644 --- a/src/runtime/virtcontainers/hypervisor_test.go +++ b/src/runtime/virtcontainers/hypervisor_test.go @@ -86,124 +86,6 @@ func TestNewHypervisorFromUnknownHypervisorType(t *testing.T) { assert.Nil(hy) } -func testHypervisorConfigValid(t *testing.T, hypervisorConfig *HypervisorConfig, success bool) { - err := hypervisorConfig.Valid() - assert := assert.New(t) - assert.False(success && err != nil) - assert.False(!success && err == nil) -} - -func TestHypervisorConfigNoKernelPath(t *testing.T) { - hypervisorConfig := &HypervisorConfig{ - KernelPath: "", - ImagePath: fmt.Sprintf("%s/%s", testDir, testImage), - HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor), - } - - testHypervisorConfigValid(t, hypervisorConfig, false) -} - -func TestHypervisorConfigNoImagePath(t *testing.T) { - hypervisorConfig := &HypervisorConfig{ - KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), - ImagePath: "", - HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor), - } - - testHypervisorConfigValid(t, hypervisorConfig, false) -} - -func TestHypervisorConfigNoHypervisorPath(t *testing.T) { - hypervisorConfig := &HypervisorConfig{ - KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), - ImagePath: fmt.Sprintf("%s/%s", testDir, testImage), - HypervisorPath: "", - } - - testHypervisorConfigValid(t, hypervisorConfig, true) -} - -func TestHypervisorConfigIsValid(t *testing.T) { - hypervisorConfig := &HypervisorConfig{ - KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), - ImagePath: fmt.Sprintf("%s/%s", testDir, testImage), - HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor), - } - - testHypervisorConfigValid(t, hypervisorConfig, true) -} - -func TestHypervisorConfigBothInitrdAndImage(t *testing.T) { - hypervisorConfig := &HypervisorConfig{ - KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), - ImagePath: fmt.Sprintf("%s/%s", testDir, testImage), - InitrdPath: fmt.Sprintf("%s/%s", testDir, testInitrd), - HypervisorPath: "", - } - - testHypervisorConfigValid(t, hypervisorConfig, false) -} - -func TestHypervisorConfigSecureExecution(t *testing.T) { - hypervisorConfig := &HypervisorConfig{ - KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), - InitrdPath: fmt.Sprintf("%s/%s", testDir, testInitrd), - ConfidentialGuest: true, - HypervisorMachineType: QemuCCWVirtio, - } - - // Secure Execution should only specify a kernel (encrypted image contains all components) - testHypervisorConfigValid(t, hypervisorConfig, false) -} - -func TestHypervisorConfigValidTemplateConfig(t *testing.T) { - hypervisorConfig := &HypervisorConfig{ - KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), - ImagePath: fmt.Sprintf("%s/%s", testDir, testImage), - HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor), - BootToBeTemplate: true, - BootFromTemplate: true, - } - testHypervisorConfigValid(t, hypervisorConfig, false) - - hypervisorConfig.BootToBeTemplate = false - testHypervisorConfigValid(t, hypervisorConfig, false) - hypervisorConfig.MemoryPath = "foobar" - testHypervisorConfigValid(t, hypervisorConfig, false) - hypervisorConfig.DevicesStatePath = "foobar" - testHypervisorConfigValid(t, hypervisorConfig, true) - - hypervisorConfig.BootFromTemplate = false - hypervisorConfig.BootToBeTemplate = true - testHypervisorConfigValid(t, hypervisorConfig, true) - hypervisorConfig.MemoryPath = "" - testHypervisorConfigValid(t, hypervisorConfig, false) -} - -func TestHypervisorConfigDefaults(t *testing.T) { - assert := assert.New(t) - hypervisorConfig := &HypervisorConfig{ - KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), - ImagePath: fmt.Sprintf("%s/%s", testDir, testImage), - HypervisorPath: "", - } - testHypervisorConfigValid(t, hypervisorConfig, true) - - hypervisorConfigDefaultsExpected := &HypervisorConfig{ - KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), - ImagePath: fmt.Sprintf("%s/%s", testDir, testImage), - HypervisorPath: "", - NumVCPUs: defaultVCPUs, - MemorySize: defaultMemSzMiB, - DefaultBridges: defaultBridges, - BlockDeviceDriver: defaultBlockDriver, - DefaultMaxVCPUs: defaultMaxVCPUs, - Msize9p: defaultMsize9p, - } - - assert.Exactly(hypervisorConfig, hypervisorConfigDefaultsExpected) -} - func TestAppendParams(t *testing.T) { assert := assert.New(t) paramList := []Param{ diff --git a/src/runtime/virtcontainers/mock_hypervisor.go b/src/runtime/virtcontainers/mock_hypervisor.go index 83e776d289..f4a0b934ec 100644 --- a/src/runtime/virtcontainers/mock_hypervisor.go +++ b/src/runtime/virtcontainers/mock_hypervisor.go @@ -31,10 +31,6 @@ func (m *mockHypervisor) HypervisorConfig() HypervisorConfig { } func (m *mockHypervisor) setConfig(config *HypervisorConfig) error { - if err := config.Valid(); err != nil { - return err - } - return nil } diff --git a/src/runtime/virtcontainers/mock_hypervisor_test.go b/src/runtime/virtcontainers/mock_hypervisor_test.go index 5e89ae8a69..0159a993dd 100644 --- a/src/runtime/virtcontainers/mock_hypervisor_test.go +++ b/src/runtime/virtcontainers/mock_hypervisor_test.go @@ -21,9 +21,9 @@ func TestMockHypervisorCreateVM(t *testing.T) { config: &SandboxConfig{ ID: "mock_sandbox", HypervisorConfig: HypervisorConfig{ - KernelPath: "", - ImagePath: "", - HypervisorPath: "", + KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), + ImagePath: fmt.Sprintf("%s/%s", testDir, testImage), + HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor), }, }, } @@ -33,16 +33,6 @@ func TestMockHypervisorCreateVM(t *testing.T) { ctx := context.Background() - // wrong config - err = m.CreateVM(ctx, sandbox.config.ID, network, &sandbox.config.HypervisorConfig) - assert.Error(err) - - sandbox.config.HypervisorConfig = HypervisorConfig{ - KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), - ImagePath: fmt.Sprintf("%s/%s", testDir, testImage), - HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor), - } - err = m.CreateVM(ctx, sandbox.config.ID, network, &sandbox.config.HypervisorConfig) assert.NoError(err) } diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index a91505d285..7440c82587 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -461,11 +461,6 @@ func (q *qemu) setupFileBackedMem(knobs *govmmQemu.Knobs, memory *govmmQemu.Memo } func (q *qemu) setConfig(config *HypervisorConfig) error { - err := config.Valid() - if err != nil { - return err - } - q.config = *config return nil diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index a995f1f77b..550362afdd 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -593,6 +593,10 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor s.Logger().WithError(err).Debug("restore sandbox failed") } + if err := validateHypervisorConfig(&sandboxConfig.HypervisorConfig); err != nil { + return nil, err + } + // store doesn't require hypervisor to be stored immediately if err = s.hypervisor.CreateVM(ctx, s.id, s.network, &sandboxConfig.HypervisorConfig); err != nil { return nil, err @@ -605,6 +609,62 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor return s, nil } +func validateHypervisorConfig(conf *HypervisorConfig) error { + if conf.KernelPath == "" { + return fmt.Errorf("Missing kernel path") + } + + QemuCCWVirtio := "" + if conf.ConfidentialGuest && conf.HypervisorMachineType == QemuCCWVirtio { + if conf.ImagePath != "" || conf.InitrdPath != "" { + fmt.Println("yes, failing") + return fmt.Errorf("Neither the image or initrd path may be set for Secure Execution") + } + } else if conf.ImagePath == "" && conf.InitrdPath == "" { + return fmt.Errorf("Missing image and initrd path") + } else if conf.ImagePath != "" && conf.InitrdPath != "" { + return fmt.Errorf("Image and initrd path cannot be both set") + } + + if err := conf.CheckTemplateConfig(); err != nil { + return err + } + + if conf.NumVCPUs == 0 { + conf.NumVCPUs = defaultVCPUs + } + + if conf.MemorySize == 0 { + conf.MemorySize = defaultMemSzMiB + } + + if conf.DefaultBridges == 0 { + conf.DefaultBridges = defaultBridges + } + + if conf.BlockDeviceDriver == "" { + conf.BlockDeviceDriver = defaultBlockDriver + } else if conf.BlockDeviceDriver == config.VirtioBlock && conf.HypervisorMachineType == QemuCCWVirtio { + conf.BlockDeviceDriver = config.VirtioBlockCCW + } + + if conf.DefaultMaxVCPUs == 0 || conf.DefaultMaxVCPUs > defaultMaxVCPUs { + conf.DefaultMaxVCPUs = defaultMaxVCPUs + } + + if conf.ConfidentialGuest && conf.NumVCPUs != conf.DefaultMaxVCPUs { + hvLogger.Warnf("Confidential guests do not support hotplugging of vCPUs. Setting DefaultMaxVCPUs to NumVCPUs (%d)", conf.NumVCPUs) + conf.DefaultMaxVCPUs = conf.NumVCPUs + } + + if conf.Msize9p == 0 && conf.SharedFS != config.VirtioFS { + conf.Msize9p = defaultMsize9p + } + + return nil + +} + func (s *Sandbox) createResourceController() error { var err error cgroupPath := "" diff --git a/src/runtime/virtcontainers/vm.go b/src/runtime/virtcontainers/vm.go index a9db8efc06..a96661d434 100644 --- a/src/runtime/virtcontainers/vm.go +++ b/src/runtime/virtcontainers/vm.go @@ -42,9 +42,8 @@ type VMConfig struct { HypervisorConfig HypervisorConfig } -// Valid Check VMConfig validity. func (c *VMConfig) Valid() error { - return c.HypervisorConfig.Valid() + return validateHypervisorConfig(&c.HypervisorConfig) } // ToGrpc convert VMConfig struct to grpc format pb.GrpcVMConfig. From afdc9604249336a0715b6c28717bb628e535db2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Mon, 27 Jun 2022 15:18:27 +0200 Subject: [PATCH 04/36] hypervisor: Add default_maxmemory configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let's add a `default_maxmemory` configuration, which allows the admins to set the maximum amount of memory to be used by a VM, considering the initial amount + whatever ends up being hotplugged via the pod limits. By default this value is 0 (zero), and it means that the whole physical RAM is the limit. Fixes: #4516 Signed-off-by: Fabiano Fidêncio --- src/runtime/go.mod | 1 + src/runtime/go.sum | 2 + src/runtime/pkg/katatestutils/utils.go | 1 + src/runtime/pkg/katautils/config.go | 20 +++++++ src/runtime/pkg/katautils/config_test.go | 4 ++ .../vendor/github.com/pbnjay/memory/LICENSE | 29 +++++++++ .../vendor/github.com/pbnjay/memory/README.md | 41 +++++++++++++ .../vendor/github.com/pbnjay/memory/doc.go | 24 ++++++++ .../vendor/github.com/pbnjay/memory/go.mod | 3 + .../github.com/pbnjay/memory/memory_bsd.go | 19 ++++++ .../github.com/pbnjay/memory/memory_darwin.go | 49 +++++++++++++++ .../github.com/pbnjay/memory/memory_linux.go | 29 +++++++++ .../pbnjay/memory/memory_windows.go | 60 +++++++++++++++++++ .../github.com/pbnjay/memory/memsysctl.go | 21 +++++++ .../vendor/github.com/pbnjay/memory/stub.go | 10 ++++ src/runtime/vendor/modules.txt | 3 + src/runtime/virtcontainers/hypervisor.go | 3 + 17 files changed, 319 insertions(+) create mode 100644 src/runtime/vendor/github.com/pbnjay/memory/LICENSE create mode 100644 src/runtime/vendor/github.com/pbnjay/memory/README.md create mode 100644 src/runtime/vendor/github.com/pbnjay/memory/doc.go create mode 100644 src/runtime/vendor/github.com/pbnjay/memory/go.mod create mode 100644 src/runtime/vendor/github.com/pbnjay/memory/memory_bsd.go create mode 100644 src/runtime/vendor/github.com/pbnjay/memory/memory_darwin.go create mode 100644 src/runtime/vendor/github.com/pbnjay/memory/memory_linux.go create mode 100644 src/runtime/vendor/github.com/pbnjay/memory/memory_windows.go create mode 100644 src/runtime/vendor/github.com/pbnjay/memory/memsysctl.go create mode 100644 src/runtime/vendor/github.com/pbnjay/memory/stub.go diff --git a/src/runtime/go.mod b/src/runtime/go.mod index 14f0f3ecf3..36a2f618b9 100644 --- a/src/runtime/go.mod +++ b/src/runtime/go.mod @@ -33,6 +33,7 @@ require ( github.com/opencontainers/runc v1.1.2 github.com/opencontainers/runtime-spec v1.0.3-0.20210326190908-1c3f411f0417 github.com/opencontainers/selinux v1.10.1 + github.com/pbnjay/memory v0.0.0-20210728143218-7b4eea64cf58 github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.11.1 github.com/prometheus/client_model v0.2.0 diff --git a/src/runtime/go.sum b/src/runtime/go.sum index 86868038e9..63870b2325 100644 --- a/src/runtime/go.sum +++ b/src/runtime/go.sum @@ -767,6 +767,8 @@ github.com/opencontainers/selinux v1.10.1 h1:09LIPVRP3uuZGQvgR+SgMSNBd1Eb3vlRbGq github.com/opencontainers/selinux v1.10.1/go.mod h1:2i0OySw99QjzBBQByd1Gr9gSjvuho1lHsJxIJ3gGbJI= github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o= github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= +github.com/pbnjay/memory v0.0.0-20210728143218-7b4eea64cf58 h1:onHthvaw9LFnH4t2DcNVpwGmV9E1BkGknEliJkfwQj0= +github.com/pbnjay/memory v0.0.0-20210728143218-7b4eea64cf58/go.mod h1:DXv8WO4yhMYhSNPKjeNKa5WY9YCIEBRbNzFFPJbWO6Y= github.com/pborman/uuid v1.2.0 h1:J7Q5mO4ysT1dv8hyrUGHb9+ooztCXu1D8MY8DZYsu3g= github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k= github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic= diff --git a/src/runtime/pkg/katatestutils/utils.go b/src/runtime/pkg/katatestutils/utils.go index 527c9cfbc7..5676f4451c 100644 --- a/src/runtime/pkg/katatestutils/utils.go +++ b/src/runtime/pkg/katatestutils/utils.go @@ -226,6 +226,7 @@ type RuntimeConfigOptions struct { DefaultVCPUCount uint32 DefaultMaxVCPUCount uint32 DefaultMemSize uint32 + DefaultMaxMemorySize uint64 DefaultMsize9p uint32 DisableBlock bool EnableIOThreads bool diff --git a/src/runtime/pkg/katautils/config.go b/src/runtime/pkg/katautils/config.go index dbdfdac303..c0ffb99267 100644 --- a/src/runtime/pkg/katautils/config.go +++ b/src/runtime/pkg/katautils/config.go @@ -24,6 +24,7 @@ import ( vc "github.com/kata-containers/kata-containers/src/runtime/virtcontainers" exp "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/experimental" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/utils" + "github.com/pbnjay/memory" "github.com/sirupsen/logrus" ) @@ -121,6 +122,7 @@ type hypervisor struct { DefaultMaxVCPUs uint32 `toml:"default_maxvcpus"` MemorySize uint32 `toml:"default_memory"` MemSlots uint32 `toml:"memory_slots"` + DefaultMaxMemorySize uint64 `toml:"default_maxmemory"` DefaultBridges uint32 `toml:"default_bridges"` Msize9p uint32 `toml:"msize_9p"` PCIeRootPort uint32 `toml:"pcie_root_port"` @@ -400,6 +402,20 @@ func (h hypervisor) defaultMemOffset() uint64 { return offset } +func (h hypervisor) defaultMaxMemSz() uint64 { + hostMemory := memory.TotalMemory() / 1024 / 1024 //MiB + + if h.DefaultMaxMemorySize == 0 { + return hostMemory + } + + if h.DefaultMaxMemorySize > hostMemory { + return hostMemory + } + + return h.DefaultMaxMemorySize +} + func (h hypervisor) defaultBridges() uint32 { if h.DefaultBridges == 0 { return defaultBridgesCount @@ -635,6 +651,7 @@ func newFirecrackerHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) { DefaultMaxVCPUs: h.defaultMaxVCPUs(), MemorySize: h.defaultMemSz(), MemSlots: h.defaultMemSlots(), + DefaultMaxMemorySize: h.defaultMaxMemSz(), EntropySource: h.GetEntropySource(), EntropySourceList: h.EntropySourceList, DefaultBridges: h.defaultBridges(), @@ -736,6 +753,7 @@ func newQemuHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) { MemorySize: h.defaultMemSz(), MemSlots: h.defaultMemSlots(), MemOffset: h.defaultMemOffset(), + DefaultMaxMemorySize: h.defaultMaxMemSz(), VirtioMem: h.VirtioMem, EntropySource: h.GetEntropySource(), EntropySourceList: h.EntropySourceList, @@ -833,6 +851,7 @@ func newAcrnHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) { DefaultMaxVCPUs: h.defaultMaxVCPUs(), MemorySize: h.defaultMemSz(), MemSlots: h.defaultMemSlots(), + DefaultMaxMemorySize: h.defaultMaxMemSz(), EntropySource: h.GetEntropySource(), EntropySourceList: h.EntropySourceList, DefaultBridges: h.defaultBridges(), @@ -910,6 +929,7 @@ func newClhHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) { MemorySize: h.defaultMemSz(), MemSlots: h.defaultMemSlots(), MemOffset: h.defaultMemOffset(), + DefaultMaxMemorySize: h.defaultMaxMemSz(), VirtioMem: h.VirtioMem, EntropySource: h.GetEntropySource(), EntropySourceList: h.EntropySourceList, diff --git a/src/runtime/pkg/katautils/config_test.go b/src/runtime/pkg/katautils/config_test.go index 0bcac2137f..65b541e73c 100644 --- a/src/runtime/pkg/katautils/config_test.go +++ b/src/runtime/pkg/katautils/config_test.go @@ -22,6 +22,7 @@ import ( "github.com/kata-containers/kata-containers/src/runtime/pkg/oci" vc "github.com/kata-containers/kata-containers/src/runtime/virtcontainers" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/utils" + "github.com/pbnjay/memory" "github.com/stretchr/testify/assert" ) @@ -85,6 +86,7 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf sharedFS := "virtio-9p" virtioFSdaemon := path.Join(dir, "virtiofsd") epcSize := int64(0) + maxMemory := uint64(memory.TotalMemory() / 1024 / 1024) configFileOptions := ktu.RuntimeConfigOptions{ Hypervisor: "qemu", @@ -104,6 +106,7 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf DefaultVCPUCount: defaultVCPUCount, DefaultMaxVCPUCount: defaultMaxVCPUCount, DefaultMemSize: defaultMemSize, + DefaultMaxMemorySize: maxMemory, DefaultMsize9p: defaultMsize9p, HypervisorDebug: hypervisorDebug, RuntimeDebug: runtimeDebug, @@ -153,6 +156,7 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf NumVCPUs: defaultVCPUCount, DefaultMaxVCPUs: getCurrentCpuNum(), MemorySize: defaultMemSize, + DefaultMaxMemorySize: maxMemory, DisableBlockDeviceUse: disableBlockDevice, BlockDeviceDriver: defaultBlockDeviceDriver, DefaultBridges: defaultBridgesCount, diff --git a/src/runtime/vendor/github.com/pbnjay/memory/LICENSE b/src/runtime/vendor/github.com/pbnjay/memory/LICENSE new file mode 100644 index 0000000000..63ca4a6d24 --- /dev/null +++ b/src/runtime/vendor/github.com/pbnjay/memory/LICENSE @@ -0,0 +1,29 @@ +BSD 3-Clause License + +Copyright (c) 2017, Jeremy Jay +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are met: + +* Redistributions of source code must retain the above copyright notice, this + list of conditions and the following disclaimer. + +* Redistributions in binary form must reproduce the above copyright notice, + this list of conditions and the following disclaimer in the documentation + and/or other materials provided with the distribution. + +* Neither the name of the copyright holder nor the names of its + contributors may be used to endorse or promote products derived from + this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/src/runtime/vendor/github.com/pbnjay/memory/README.md b/src/runtime/vendor/github.com/pbnjay/memory/README.md new file mode 100644 index 0000000000..e98f261a0f --- /dev/null +++ b/src/runtime/vendor/github.com/pbnjay/memory/README.md @@ -0,0 +1,41 @@ +# memory + +Package `memory` provides two methods reporting total physical system memory +accessible to the kernel, and free memory available to the running application. + +This package has no external dependency besides the standard library and default operating system tools. + +Documentation: +[![GoDoc](https://godoc.org/github.com/pbnjay/memory?status.svg)](https://godoc.org/github.com/pbnjay/memory) + +This is useful for dynamic code to minimize thrashing and other contention, similar to the stdlib `runtime.NumCPU` +See some history of the proposal at https://github.com/golang/go/issues/21816 + + +## Example + +```go +fmt.Printf("Total system memory: %d\n", memory.TotalMemory()) +fmt.Printf("Free memory: %d\n", memory.FreeMemory()) +``` + + +## Testing + +Tested/working on: + - macOS 10.12.6 (16G29), 10.15.7 (19H2) + - Windows 10 1511 (10586.1045) + - Linux RHEL (3.10.0-327.3.1.el7.x86_64) + - Raspberry Pi 3 (ARMv8) on Raspbian, ODROID-C1+ (ARMv7) on Ubuntu, C.H.I.P + (ARMv7). + - Amazon Linux 2 aarch64 (m6a.large, 4.14.203-156.332.amzn2.aarch64) + +Tested on virtual machines: + - Windows 7 SP1 386 + - Debian stretch 386 + - NetBSD 7.1 amd64 + 386 + - OpenBSD 6.1 amd64 + 386 + - FreeBSD 11.1 amd64 + 386 + - DragonFly BSD 4.8.1 amd64 + +If you have access to untested systems please test and report success/bugs. diff --git a/src/runtime/vendor/github.com/pbnjay/memory/doc.go b/src/runtime/vendor/github.com/pbnjay/memory/doc.go new file mode 100644 index 0000000000..4e4f984c0f --- /dev/null +++ b/src/runtime/vendor/github.com/pbnjay/memory/doc.go @@ -0,0 +1,24 @@ +// Package memory provides a single method reporting total system memory +// accessible to the kernel. +package memory + +// TotalMemory returns the total accessible system memory in bytes. +// +// The total accessible memory is installed physical memory size minus reserved +// areas for the kernel and hardware, if such reservations are reported by +// the operating system. +// +// If accessible memory size could not be determined, then 0 is returned. +func TotalMemory() uint64 { + return sysTotalMemory() +} + +// FreeMemory returns the total free system memory in bytes. +// +// The total free memory is installed physical memory size minus reserved +// areas for other applications running on the same system. +// +// If free memory size could not be determined, then 0 is returned. +func FreeMemory() uint64 { + return sysFreeMemory() +} diff --git a/src/runtime/vendor/github.com/pbnjay/memory/go.mod b/src/runtime/vendor/github.com/pbnjay/memory/go.mod new file mode 100644 index 0000000000..5965765912 --- /dev/null +++ b/src/runtime/vendor/github.com/pbnjay/memory/go.mod @@ -0,0 +1,3 @@ +module github.com/pbnjay/memory + +go 1.16 diff --git a/src/runtime/vendor/github.com/pbnjay/memory/memory_bsd.go b/src/runtime/vendor/github.com/pbnjay/memory/memory_bsd.go new file mode 100644 index 0000000000..49d808a9e7 --- /dev/null +++ b/src/runtime/vendor/github.com/pbnjay/memory/memory_bsd.go @@ -0,0 +1,19 @@ +// +build freebsd openbsd dragonfly netbsd + +package memory + +func sysTotalMemory() uint64 { + s, err := sysctlUint64("hw.physmem") + if err != nil { + return 0 + } + return s +} + +func sysFreeMemory() uint64 { + s, err := sysctlUint64("hw.usermem") + if err != nil { + return 0 + } + return s +} diff --git a/src/runtime/vendor/github.com/pbnjay/memory/memory_darwin.go b/src/runtime/vendor/github.com/pbnjay/memory/memory_darwin.go new file mode 100644 index 0000000000..a3f4576990 --- /dev/null +++ b/src/runtime/vendor/github.com/pbnjay/memory/memory_darwin.go @@ -0,0 +1,49 @@ +// +build darwin + +package memory + +import ( + "os/exec" + "regexp" + "strconv" +) + +func sysTotalMemory() uint64 { + s, err := sysctlUint64("hw.memsize") + if err != nil { + return 0 + } + return s +} + +func sysFreeMemory() uint64 { + cmd := exec.Command("vm_stat") + outBytes, err := cmd.Output() + if err != nil { + return 0 + } + + rePageSize := regexp.MustCompile("page size of ([0-9]*) bytes") + reFreePages := regexp.MustCompile("Pages free: *([0-9]*)\\.") + + // default: page size of 4096 bytes + matches := rePageSize.FindSubmatchIndex(outBytes) + pageSize := uint64(4096) + if len(matches) == 4 { + pageSize, err = strconv.ParseUint(string(outBytes[matches[2]:matches[3]]), 10, 64) + if err != nil { + return 0 + } + } + + // ex: Pages free: 1126961. + matches = reFreePages.FindSubmatchIndex(outBytes) + freePages := uint64(0) + if len(matches) == 4 { + freePages, err = strconv.ParseUint(string(outBytes[matches[2]:matches[3]]), 10, 64) + if err != nil { + return 0 + } + } + return freePages * pageSize +} diff --git a/src/runtime/vendor/github.com/pbnjay/memory/memory_linux.go b/src/runtime/vendor/github.com/pbnjay/memory/memory_linux.go new file mode 100644 index 0000000000..3d07711ce5 --- /dev/null +++ b/src/runtime/vendor/github.com/pbnjay/memory/memory_linux.go @@ -0,0 +1,29 @@ +// +build linux + +package memory + +import "syscall" + +func sysTotalMemory() uint64 { + in := &syscall.Sysinfo_t{} + err := syscall.Sysinfo(in) + if err != nil { + return 0 + } + // If this is a 32-bit system, then these fields are + // uint32 instead of uint64. + // So we always convert to uint64 to match signature. + return uint64(in.Totalram) * uint64(in.Unit) +} + +func sysFreeMemory() uint64 { + in := &syscall.Sysinfo_t{} + err := syscall.Sysinfo(in) + if err != nil { + return 0 + } + // If this is a 32-bit system, then these fields are + // uint32 instead of uint64. + // So we always convert to uint64 to match signature. + return uint64(in.Freeram) * uint64(in.Unit) +} diff --git a/src/runtime/vendor/github.com/pbnjay/memory/memory_windows.go b/src/runtime/vendor/github.com/pbnjay/memory/memory_windows.go new file mode 100644 index 0000000000..c8500cc6f3 --- /dev/null +++ b/src/runtime/vendor/github.com/pbnjay/memory/memory_windows.go @@ -0,0 +1,60 @@ +// +build windows + +package memory + +import ( + "syscall" + "unsafe" +) + +// omitting a few fields for brevity... +// https://msdn.microsoft.com/en-us/library/windows/desktop/aa366589(v=vs.85).aspx +type memStatusEx struct { + dwLength uint32 + dwMemoryLoad uint32 + ullTotalPhys uint64 + ullAvailPhys uint64 + unused [5]uint64 +} + +func sysTotalMemory() uint64 { + kernel32, err := syscall.LoadDLL("kernel32.dll") + if err != nil { + return 0 + } + // GetPhysicallyInstalledSystemMemory is simpler, but broken on + // older versions of windows (and uses this under the hood anyway). + globalMemoryStatusEx, err := kernel32.FindProc("GlobalMemoryStatusEx") + if err != nil { + return 0 + } + msx := &memStatusEx{ + dwLength: 64, + } + r, _, _ := globalMemoryStatusEx.Call(uintptr(unsafe.Pointer(msx))) + if r == 0 { + return 0 + } + return msx.ullTotalPhys +} + +func sysFreeMemory() uint64 { + kernel32, err := syscall.LoadDLL("kernel32.dll") + if err != nil { + return 0 + } + // GetPhysicallyInstalledSystemMemory is simpler, but broken on + // older versions of windows (and uses this under the hood anyway). + globalMemoryStatusEx, err := kernel32.FindProc("GlobalMemoryStatusEx") + if err != nil { + return 0 + } + msx := &memStatusEx{ + dwLength: 64, + } + r, _, _ := globalMemoryStatusEx.Call(uintptr(unsafe.Pointer(msx))) + if r == 0 { + return 0 + } + return msx.ullAvailPhys +} diff --git a/src/runtime/vendor/github.com/pbnjay/memory/memsysctl.go b/src/runtime/vendor/github.com/pbnjay/memory/memsysctl.go new file mode 100644 index 0000000000..438d9eff8e --- /dev/null +++ b/src/runtime/vendor/github.com/pbnjay/memory/memsysctl.go @@ -0,0 +1,21 @@ +// +build darwin freebsd openbsd dragonfly netbsd + +package memory + +import ( + "syscall" + "unsafe" +) + +func sysctlUint64(name string) (uint64, error) { + s, err := syscall.Sysctl(name) + if err != nil { + return 0, err + } + // hack because the string conversion above drops a \0 + b := []byte(s) + if len(b) < 8 { + b = append(b, 0) + } + return *(*uint64)(unsafe.Pointer(&b[0])), nil +} diff --git a/src/runtime/vendor/github.com/pbnjay/memory/stub.go b/src/runtime/vendor/github.com/pbnjay/memory/stub.go new file mode 100644 index 0000000000..f29473ba08 --- /dev/null +++ b/src/runtime/vendor/github.com/pbnjay/memory/stub.go @@ -0,0 +1,10 @@ +// +build !linux,!darwin,!windows,!freebsd,!dragonfly,!netbsd,!openbsd + +package memory + +func sysTotalMemory() uint64 { + return 0 +} +func sysFreeMemory() uint64 { + return 0 +} diff --git a/src/runtime/vendor/modules.txt b/src/runtime/vendor/modules.txt index 8c27d91df4..fb015d1c20 100644 --- a/src/runtime/vendor/modules.txt +++ b/src/runtime/vendor/modules.txt @@ -259,6 +259,9 @@ github.com/opencontainers/selinux/go-selinux github.com/opencontainers/selinux/go-selinux/label github.com/opencontainers/selinux/pkg/pwalk github.com/opencontainers/selinux/pkg/pwalkdir +# github.com/pbnjay/memory v0.0.0-20210728143218-7b4eea64cf58 +## explicit +github.com/pbnjay/memory # github.com/pkg/errors v0.9.1 ## explicit github.com/pkg/errors diff --git a/src/runtime/virtcontainers/hypervisor.go b/src/runtime/virtcontainers/hypervisor.go index 119c4667c3..e39885dac8 100644 --- a/src/runtime/virtcontainers/hypervisor.go +++ b/src/runtime/virtcontainers/hypervisor.go @@ -441,6 +441,9 @@ type HypervisorConfig struct { // DefaultMem specifies default memory size in MiB for the VM. MemorySize uint32 + // DefaultMaxMemorySize specifies the maximum amount of RAM in MiB for the VM. + DefaultMaxMemorySize uint64 + // DefaultBridges specifies default number of bridges for the VM. // Bridges can be used to hot plug devices DefaultBridges uint32 From 1a78c3df2eeea4a7186a9807c1cbcfe94642f106 Mon Sep 17 00:00:00 2001 From: Gabriela Cervantes Date: Tue, 28 Jun 2022 15:10:39 +0000 Subject: [PATCH 05/36] packaging: Remove unused kata docker configure script This PR removes an unused kata configure docker script which was used in packaging for kata 1.x but not longer being used in kata 2.x Fixes #4546 Signed-off-by: Gabriela Cervantes --- .../scripts/kata-configure-docker.sh | 146 ------------------ 1 file changed, 146 deletions(-) delete mode 100644 tools/packaging/static-build/scripts/kata-configure-docker.sh diff --git a/tools/packaging/static-build/scripts/kata-configure-docker.sh b/tools/packaging/static-build/scripts/kata-configure-docker.sh deleted file mode 100644 index 72fbc28e01..0000000000 --- a/tools/packaging/static-build/scripts/kata-configure-docker.sh +++ /dev/null @@ -1,146 +0,0 @@ -#!/usr/bin/env bash -# Copyright (c) 2019 Intel Corporation -# -# SPDX-License-Identifier: Apache-2.0 -# - -# Description: Script to configure Docker for the static -# version of Kata Containers. - -[ -z "${DEBUG}" ] || set -x -set -o errexit -set -o nounset -set -o pipefail - -docker_config_dir="/etc/docker" -docker_config_file="${docker_config_file:-${docker_config_dir}/daemon.json}" - -# The static version of Kata Containers is entirely contained within -# this directory. -readonly static_base_dir="/opt/kata" - -# Path to runtime in static archive file -readonly runtime_path="${static_base_dir}/bin/kata-runtime" - -die() -{ - local msg="$*" - echo >&2 "ERROR: $msg" - exit 1 -} - -info() -{ - local msg="$*" - echo >&2 "INFO: $msg" -} - -configure_docker() -{ - local file="$1" - [ -z "$file" ] && die "need file" - - mkdir -p "${docker_config_dir}" - - if [ -e "$docker_config_file" ] - then - local today=$(date '+%Y-%m-%d') - local backup="${docker_config_file}.${today}" - - info "Backing up original Docker config file '$docker_config_file' to '$backup'" - - sudo cp "${docker_config_file}" "${docker_config_file}.${today}" - else - # Create a minimal valid JSON document - echo "{}" > "${docker_config_file}" - fi - - local config_files=$(tar tvf "$file" |\ - grep "/configuration-.*\.toml" |\ - grep -v -- '->' |\ - awk '{print $NF}' |\ - sed 's/^\.//g' || true) - - [ -z "$config_files" ] && die "cannot find any configuration files in '$file'" - - local config - local -a runtimes - - for config in $(echo "$config_files" | tr '\n' ' ') - do - local runtime - runtime=$(echo "$config" |\ - awk -F \/ '{print $NF}' |\ - sed -e 's/configuration/kata/g' -e 's/\.toml//g') - - runtimes+=("$runtime") - - local result - result=$(cat "$docker_config_file" |\ - jq \ - --arg config "$config" \ - --arg runtime "$runtime" \ - --arg runtime_path "$runtime_path" \ - '.runtimes[$runtime] = {path: $runtime_path, "runtimeArgs": ["--config", $config]}') - - echo "$result" > "$docker_config_file" - done - - info "Validating $docker_config_file" - - jq -S . "$docker_config_file" &>/dev/null - - info "Restarting Docker to apply new configuration" - - $chronic sudo systemctl restart docker - - info "Docker configured for the following additional runtimes: ${runtimes[@]}" -} - -setup() -{ - source "/etc/os-release" || source "/usr/lib/os-release" - - # Used to manipulate $docker_config_file - local pkg="jq" - - case "$ID" in - opensuse*) distro="opensuse" ;; - *) distro="$ID" ;; - esac - - # Use chronic(1) if available - chronic= - command -v chronic && chronic=chronic - - if command -v "$pkg" &>/dev/null - then - return 0 - fi - - info "Cannot find $pkg command so installing package" - - case "$distro" in - centos|rhel) $chronic sudo -E yum -y install "$pkg" ;; - debian|ubuntu) $chronic sudo -E apt-get --no-install-recommends install -y "$pkg" ;; - fedora) $chronic sudo -E dnf -y install "$pkg" ;; - opensuse|sles) $chronic sudo -E zypper -y install "$pkg" ;; - *) die "do not know how to install command $pkg' for distro '$distro'" ;; - esac -} - -main() -{ - local file="$1" - [ -z "$file" ] && die "need full path to Kata Containers static archive file" - - echo "$file" | grep -q "^kata-static-.*\.tar.xz" || die "invalid file: '$file'" - - [ $(id -u) -eq 0 ] || die "must be run as root" - - setup - - configure_docker "$file" -} - -main "$@" From 58ff2bd5c9b11cce8b62470afc633eeb87a3ef51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Mon, 27 Jun 2022 22:09:21 +0200 Subject: [PATCH 06/36] clh,qemu: Adapt to using default_maxmemory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let's adapt Cloud Hypervisor's and QEMU's code to properly behave to the newly added `default_maxmemory` config. While implementing this, a change of behaviour (or a bug fix, depending on how you see it) has been introduced as if a pod requests more memory than the amount avaiable in the host, instead of failing to start the pod, we simply hotplug the maximum amount of memory available, mimicing better the runc behaviour. Fixes: #4516 Signed-off-by: Fabiano Fidêncio --- src/runtime/virtcontainers/clh.go | 12 +++++----- src/runtime/virtcontainers/qemu.go | 29 ++++++------------------- src/runtime/virtcontainers/qemu_test.go | 11 +++++----- 3 files changed, 20 insertions(+), 32 deletions(-) diff --git a/src/runtime/virtcontainers/clh.go b/src/runtime/virtcontainers/clh.go index 6984b73be5..801d53a9bd 100644 --- a/src/runtime/virtcontainers/clh.go +++ b/src/runtime/virtcontainers/clh.go @@ -480,12 +480,9 @@ func (clh *cloudHypervisor) CreateVM(ctx context.Context, id string, network Net // Enable hugepages if needed clh.vmconfig.Memory.Hugepages = func(b bool) *bool { return &b }(clh.config.HugePages) if !clh.config.ConfidentialGuest { - hostMemKb, err := GetHostMemorySizeKb(procMemInfo) - if err != nil { - return nil - } + hotplugSize := clh.config.DefaultMaxMemorySize // OpenAPI only supports int64 values - clh.vmconfig.Memory.HotplugSize = func(i int64) *int64 { return &i }(int64((utils.MemUnit(hostMemKb) * utils.KiB).ToBytes())) + clh.vmconfig.Memory.HotplugSize = func(i int64) *int64 { return &i }(int64((utils.MemUnit(hotplugSize) * utils.MiB).ToBytes())) } // Set initial amount of cpu's for the virtual machine clh.vmconfig.Cpus = chclient.NewCpusConfig(int32(clh.config.NumVCPUs), int32(clh.config.DefaultMaxVCPUs)) @@ -882,6 +879,11 @@ func (clh *cloudHypervisor) ResizeMemory(ctx context.Context, reqMemMB uint32, m return 0, MemoryDevice{}, err } + maxHotplugSize := utils.MemUnit(*info.Config.Memory.HotplugSize) * utils.Byte + if reqMemMB > uint32(maxHotplugSize.ToMiB()) { + reqMemMB = uint32(maxHotplugSize.ToMiB()) + } + currentMem := utils.MemUnit(info.Config.Memory.Size) * utils.Byte newMem := utils.MemUnit(reqMemMB) * utils.MiB diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index c8e48953f6..f2c501ac3a 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -315,11 +315,7 @@ func (q *qemu) hostMemMB() (uint64, error) { } func (q *qemu) memoryTopology() (govmmQemu.Memory, error) { - hostMemMb, err := q.hostMemMB() - if err != nil { - return govmmQemu.Memory{}, err - } - + hostMemMb := q.config.DefaultMaxMemorySize memMb := uint64(q.config.MemorySize) return q.arch.memoryTopology(memMb, hostMemMb, uint8(q.config.MemSlots)), nil @@ -779,12 +775,8 @@ func (q *qemu) getMemArgs() (bool, string, string, error) { } func (q *qemu) setupVirtioMem(ctx context.Context) error { - maxMem, err := q.hostMemMB() - if err != nil { - return err - } // backend memory size must be multiple of 4Mib - sizeMB := (int(maxMem) - int(q.config.MemorySize)) >> 2 << 2 + sizeMB := (int(q.config.DefaultMaxMemorySize) - int(q.config.MemorySize)) >> 2 << 2 share, target, memoryBack, err := q.getMemArgs() if err != nil { @@ -1970,8 +1962,6 @@ func (q *qemu) hotplugMemory(memDev *MemoryDevice, op Operation) (int, error) { return 0, err } - currentMemory := int(q.config.MemorySize) + q.state.HotpluggedMemory - if memDev.SizeMB == 0 { memLog.Debug("hotplug is not required") return 0, nil @@ -1985,17 +1975,7 @@ func (q *qemu) hotplugMemory(memDev *MemoryDevice, op Operation) (int, error) { return 0, nil case AddDevice: memLog.WithField("operation", "add").Debugf("Requested to add memory: %d MB", memDev.SizeMB) - maxMem, err := q.hostMemMB() - if err != nil { - return 0, err - } - // Don't exceed the maximum amount of memory - if currentMemory+memDev.SizeMB > int(maxMem) { - // Fixme: return a typed error - return 0, fmt.Errorf("Unable to hotplug %d MiB memory, the SB has %d MiB and the maximum amount is %d MiB", - memDev.SizeMB, currentMemory, maxMem) - } memoryAdded, err := q.hotplugAddMemory(memDev) if err != nil { return memoryAdded, err @@ -2231,6 +2211,11 @@ func (q *qemu) ResizeMemory(ctx context.Context, reqMemMB uint32, memoryBlockSiz case currentMemory < reqMemMB: //hotplug addMemMB := reqMemMB - currentMemory + + if currentMemory+addMemMB > uint32(q.config.DefaultMaxMemorySize) { + addMemMB = uint32(q.config.DefaultMaxMemorySize) - currentMemory + } + memHotplugMB, err := calcHotplugMemMiBSize(addMemMB, memoryBlockSizeMB) if err != nil { return currentMemory, MemoryDevice{}, err diff --git a/src/runtime/virtcontainers/qemu_test.go b/src/runtime/virtcontainers/qemu_test.go index b50d73a917..b932189e8d 100644 --- a/src/runtime/virtcontainers/qemu_test.go +++ b/src/runtime/virtcontainers/qemu_test.go @@ -21,6 +21,7 @@ import ( "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/persist" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/types" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/utils" + "github.com/pbnjay/memory" "github.com/pkg/errors" "github.com/stretchr/testify/assert" ) @@ -172,20 +173,20 @@ func TestQemuCPUTopology(t *testing.T) { func TestQemuMemoryTopology(t *testing.T) { mem := uint32(1000) + maxMem := memory.TotalMemory() / 1024 / 1024 //MiB slots := uint32(8) assert := assert.New(t) q := &qemu{ arch: &qemuArchBase{}, config: HypervisorConfig{ - MemorySize: mem, - MemSlots: slots, + MemorySize: mem, + MemSlots: slots, + DefaultMaxMemorySize: maxMem, }, } - hostMemKb, err := GetHostMemorySizeKb(procMemInfo) - assert.NoError(err) - memMax := fmt.Sprintf("%dM", int(float64(hostMemKb)/1024)) + memMax := fmt.Sprintf("%dM", int(maxMem)) expectedOut := govmmQemu.Memory{ Size: fmt.Sprintf("%dM", mem), From 0939f5181bfa6ba59f52d15b94c9f62a3911046a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Mon, 27 Jun 2022 15:02:00 +0200 Subject: [PATCH 07/36] config: Expose default_maxmemory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Expose the newly added `default_maxmemory` to the project's Makefile and to the configuration files. Fixes: #4516 Signed-off-by: Fabiano Fidêncio --- src/runtime/Makefile | 3 +++ src/runtime/config/configuration-clh.toml.in | 6 ++++++ src/runtime/config/configuration-fc.toml.in | 7 +++++++ src/runtime/config/configuration-qemu.toml.in | 6 ++++++ 4 files changed, 22 insertions(+) diff --git a/src/runtime/Makefile b/src/runtime/Makefile index 757d0a48a9..fa07b87deb 100644 --- a/src/runtime/Makefile +++ b/src/runtime/Makefile @@ -158,6 +158,8 @@ DEFMEMSZ := 2048 # - vm template memory # - hugepage memory DEFMEMSLOTS := 10 +# Default maximum memory in MiB +DEFMAXMEMSZ := 0 #Default number of bridges DEFBRIDGES := 1 DEFENABLEANNOTATIONS := [\"enable_iommu\"] @@ -442,6 +444,7 @@ USER_VARS += DEFMAXVCPUS USER_VARS += DEFMAXVCPUS_ACRN USER_VARS += DEFMEMSZ USER_VARS += DEFMEMSLOTS +USER_VARS += DEFMAXMEMSZ USER_VARS += DEFBRIDGES USER_VARS += DEFNETWORKMODEL_ACRN USER_VARS += DEFNETWORKMODEL_CLH diff --git a/src/runtime/config/configuration-clh.toml.in b/src/runtime/config/configuration-clh.toml.in index 41d4ae4930..5d2d9c2f10 100644 --- a/src/runtime/config/configuration-clh.toml.in +++ b/src/runtime/config/configuration-clh.toml.in @@ -105,6 +105,12 @@ default_memory = @DEFMEMSZ@ # This is will determine the times that memory will be hotadded to sandbox/VM. #memory_slots = @DEFMEMSLOTS@ +# Default maximum memory in MiB per SB / VM +# unspecified or == 0 --> will be set to the actual amount of physical RAM +# > 0 <= amount of physical RAM --> will be set to the specified number +# > amount of physical RAM --> will be set to the actual amount of physical RAM +default_maxmemory = @DEFMAXMEMSZ@ + # Shared file system type: # - virtio-fs (default) # - virtio-fs-nydus diff --git a/src/runtime/config/configuration-fc.toml.in b/src/runtime/config/configuration-fc.toml.in index b6892bd17c..8761d8a02e 100644 --- a/src/runtime/config/configuration-fc.toml.in +++ b/src/runtime/config/configuration-fc.toml.in @@ -91,6 +91,7 @@ default_bridges = @DEFBRIDGES@ # Default memory size in MiB for SB/VM. # If unspecified then it will be set @DEFMEMSZ@ MiB. default_memory = @DEFMEMSZ@ + # # Default memory slots per SB/VM. # If unspecified then it will be set @DEFMEMSLOTS@. @@ -104,6 +105,12 @@ default_memory = @DEFMEMSZ@ # Default 0 #memory_offset = 0 +# Default maximum memory in MiB per SB / VM +# unspecified or == 0 --> will be set to the actual amount of physical RAM +# > 0 <= amount of physical RAM --> will be set to the specified number +# > amount of physical RAM --> will be set to the actual amount of physical RAM +default_maxmemory = @DEFMAXMEMSZ@ + # Block storage driver to be used for the hypervisor in case the container # rootfs is backed by a block device. This is virtio-scsi, virtio-blk # or nvdimm. diff --git a/src/runtime/config/configuration-qemu.toml.in b/src/runtime/config/configuration-qemu.toml.in index 702b71aadd..115cd19ccd 100644 --- a/src/runtime/config/configuration-qemu.toml.in +++ b/src/runtime/config/configuration-qemu.toml.in @@ -134,6 +134,12 @@ default_memory = @DEFMEMSZ@ # This is will determine the times that memory will be hotadded to sandbox/VM. #memory_slots = @DEFMEMSLOTS@ +# Default maximum memory in MiB per SB / VM +# unspecified or == 0 --> will be set to the actual amount of physical RAM +# > 0 <= amount of physical RAM --> will be set to the specified number +# > amount of physical RAM --> will be set to the actual amount of physical RAM +default_maxmemory = @DEFMAXMEMSZ@ + # The size in MiB will be plused to max memory of hypervisor. # It is the memory address space for the NVDIMM devie. # If set block storage driver (block_device_driver) to "nvdimm", From 323271403e7b44e60ed81f16eeaecd6020999778 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Mon, 27 Jun 2022 18:46:11 +0200 Subject: [PATCH 08/36] virtcontainers: Remove unused function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While working on the previous commits, some of the functions become non-used. Let's simply remove them. Signed-off-by: Fabiano Fidêncio --- src/runtime/virtcontainers/hypervisor.go | 35 ------------- src/runtime/virtcontainers/hypervisor_test.go | 50 ------------------- src/runtime/virtcontainers/qemu.go | 12 ----- 3 files changed, 97 deletions(-) diff --git a/src/runtime/virtcontainers/hypervisor.go b/src/runtime/virtcontainers/hypervisor.go index e39885dac8..17fc7a8757 100644 --- a/src/runtime/virtcontainers/hypervisor.go +++ b/src/runtime/virtcontainers/hypervisor.go @@ -11,7 +11,6 @@ import ( "fmt" "os" "runtime" - "strconv" "strings" "github.com/pkg/errors" @@ -50,7 +49,6 @@ const ( // MockHypervisor is a mock hypervisor for testing purposes MockHypervisor HypervisorType = "mock" - procMemInfo = "/proc/meminfo" procCPUInfo = "/proc/cpuinfo" defaultVCPUs = 1 @@ -799,39 +797,6 @@ func DeserializeParams(parameters []string) []Param { return params } -func GetHostMemorySizeKb(memInfoPath string) (uint64, error) { - f, err := os.Open(memInfoPath) - if err != nil { - return 0, err - } - defer f.Close() - - scanner := bufio.NewScanner(f) - for scanner.Scan() { - // Expected format: ["MemTotal:", "1234", "kB"] - parts := strings.Fields(scanner.Text()) - - // Sanity checks: Skip malformed entries. - if len(parts) < 3 || parts[0] != "MemTotal:" || parts[2] != "kB" { - continue - } - - sizeKb, err := strconv.ParseUint(parts[1], 0, 64) - if err != nil { - continue - } - - return sizeKb, nil - } - - // Handle errors that may have occurred during the reading of the file. - if err := scanner.Err(); err != nil { - return 0, err - } - - return 0, fmt.Errorf("unable get MemTotal from %s", memInfoPath) -} - // CheckCmdline checks whether an option or parameter is present in the kernel command line. // Search is case-insensitive. // Takes path to file that contains the kernel command line, desired option, and permitted values diff --git a/src/runtime/virtcontainers/hypervisor_test.go b/src/runtime/virtcontainers/hypervisor_test.go index ec475e86eb..424518915c 100644 --- a/src/runtime/virtcontainers/hypervisor_test.go +++ b/src/runtime/virtcontainers/hypervisor_test.go @@ -8,7 +8,6 @@ package virtcontainers import ( "fmt" "os" - "path/filepath" "testing" ktu "github.com/kata-containers/kata-containers/src/runtime/pkg/katatestutils" @@ -372,55 +371,6 @@ func TestAddKernelParamInvalid(t *testing.T) { assert.Error(err) } -func TestGetHostMemorySizeKb(t *testing.T) { - assert := assert.New(t) - type testData struct { - contents string - expectedResult int - expectError bool - } - - data := []testData{ - { - ` - MemTotal: 1 kB - MemFree: 2 kB - SwapTotal: 3 kB - SwapFree: 4 kB - `, - 1024, - false, - }, - { - ` - MemFree: 2 kB - SwapTotal: 3 kB - SwapFree: 4 kB - `, - 0, - true, - }, - } - - dir := t.TempDir() - - file := filepath.Join(dir, "meminfo") - _, err := GetHostMemorySizeKb(file) - assert.Error(err) - - for _, d := range data { - err = os.WriteFile(file, []byte(d.contents), os.FileMode(0640)) - assert.NoError(err) - defer os.Remove(file) - - hostMemKb, err := GetHostMemorySizeKb(file) - - assert.False((d.expectError && err == nil)) - assert.False((!d.expectError && err != nil)) - assert.NotEqual(hostMemKb, d.expectedResult) - } -} - func TestCheckCmdline(t *testing.T) { assert := assert.New(t) diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index f2c501ac3a..f0cac0a897 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -302,18 +302,6 @@ func (q *qemu) cpuTopology() govmmQemu.SMP { return q.arch.cpuTopology(q.config.NumVCPUs, q.config.DefaultMaxVCPUs) } -func (q *qemu) hostMemMB() (uint64, error) { - hostMemKb, err := GetHostMemorySizeKb(procMemInfo) - if err != nil { - return 0, fmt.Errorf("Unable to read memory info: %s", err) - } - if hostMemKb == 0 { - return 0, fmt.Errorf("Error host memory size 0") - } - - return hostMemKb / 1024, nil -} - func (q *qemu) memoryTopology() (govmmQemu.Memory, error) { hostMemMb := q.config.DefaultMaxMemorySize memMb := uint64(q.config.MemorySize) From 5f936f268f0fc878963c80e2c75aa5ac9117f341 Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Mon, 23 May 2022 17:15:50 -0700 Subject: [PATCH 09/36] virtcontainers: config validation is host specific Ideally this config validation would be in a seperate package (katautils?), but that would introduce circular dependency since we'd call it from vc, and it depends on vc types (which, shouldn't be vc, but probably a hypervisor package instead). Signed-off-by: Eric Ernst --- .../hypervisor_config_darwin.go | 33 +++++ .../virtcontainers/hypervisor_config_linux.go | 67 ++++++++++ .../hypervisor_config_linux_test.go | 114 ++++++++++++++++++ .../virtcontainers/hypervisor_config_test.go | 30 +++++ src/runtime/virtcontainers/sandbox.go | 56 --------- 5 files changed, 244 insertions(+), 56 deletions(-) create mode 100644 src/runtime/virtcontainers/hypervisor_config_darwin.go create mode 100644 src/runtime/virtcontainers/hypervisor_config_linux.go create mode 100644 src/runtime/virtcontainers/hypervisor_config_linux_test.go create mode 100644 src/runtime/virtcontainers/hypervisor_config_test.go diff --git a/src/runtime/virtcontainers/hypervisor_config_darwin.go b/src/runtime/virtcontainers/hypervisor_config_darwin.go new file mode 100644 index 0000000000..e074d59ea6 --- /dev/null +++ b/src/runtime/virtcontainers/hypervisor_config_darwin.go @@ -0,0 +1,33 @@ +// Copyright (c) 2022 Apple Inc. +// +// SPDX-License-Identifier: Apache-2.0 +// + +package virtcontainers + +import ( + "fmt" +) + +func validateHypervisorConfig(conf *HypervisorConfig) error { + + if conf.KernelPath == "" { + return fmt.Errorf("Missing kernel path") + } + + if conf.ImagePath == "" && conf.InitrdPath == "" { + return fmt.Errorf("Missing image and initrd path") + } else if conf.ImagePath != "" && conf.InitrdPath != "" { + return fmt.Errorf("Image and initrd path cannot be both set") + } + + if conf.NumVCPUs == 0 { + conf.NumVCPUs = defaultVCPUs + } + + if conf.MemorySize == 0 { + conf.MemorySize = defaultMemSzMiB + } + + return nil +} diff --git a/src/runtime/virtcontainers/hypervisor_config_linux.go b/src/runtime/virtcontainers/hypervisor_config_linux.go new file mode 100644 index 0000000000..f1f20d3151 --- /dev/null +++ b/src/runtime/virtcontainers/hypervisor_config_linux.go @@ -0,0 +1,67 @@ +// Copyright (c) 2022 Apple Inc. +// +// SPDX-License-Identifier: Apache-2.0 +// + +package virtcontainers + +import ( + "fmt" + + "github.com/kata-containers/kata-containers/src/runtime/pkg/device/config" +) + +func validateHypervisorConfig(conf *HypervisorConfig) error { + + if conf.KernelPath == "" { + return fmt.Errorf("Missing kernel path") + } + + if conf.ConfidentialGuest && conf.HypervisorMachineType == QemuCCWVirtio { + if conf.ImagePath != "" || conf.InitrdPath != "" { + fmt.Println("yes, failing") + return fmt.Errorf("Neither the image or initrd path may be set for Secure Execution") + } + } else if conf.ImagePath == "" && conf.InitrdPath == "" { + return fmt.Errorf("Missing image and initrd path") + } else if conf.ImagePath != "" && conf.InitrdPath != "" { + return fmt.Errorf("Image and initrd path cannot be both set") + } + + if err := conf.CheckTemplateConfig(); err != nil { + return err + } + + if conf.NumVCPUs == 0 { + conf.NumVCPUs = defaultVCPUs + } + + if conf.MemorySize == 0 { + conf.MemorySize = defaultMemSzMiB + } + + if conf.DefaultBridges == 0 { + conf.DefaultBridges = defaultBridges + } + + if conf.BlockDeviceDriver == "" { + conf.BlockDeviceDriver = defaultBlockDriver + } else if conf.BlockDeviceDriver == config.VirtioBlock && conf.HypervisorMachineType == QemuCCWVirtio { + conf.BlockDeviceDriver = config.VirtioBlockCCW + } + + if conf.DefaultMaxVCPUs == 0 || conf.DefaultMaxVCPUs > defaultMaxVCPUs { + conf.DefaultMaxVCPUs = defaultMaxVCPUs + } + + if conf.ConfidentialGuest && conf.NumVCPUs != conf.DefaultMaxVCPUs { + hvLogger.Warnf("Confidential guests do not support hotplugging of vCPUs. Setting DefaultMaxVCPUs to NumVCPUs (%d)", conf.NumVCPUs) + conf.DefaultMaxVCPUs = conf.NumVCPUs + } + + if conf.Msize9p == 0 && conf.SharedFS != config.VirtioFS { + conf.Msize9p = defaultMsize9p + } + + return nil +} diff --git a/src/runtime/virtcontainers/hypervisor_config_linux_test.go b/src/runtime/virtcontainers/hypervisor_config_linux_test.go new file mode 100644 index 0000000000..609e52fd73 --- /dev/null +++ b/src/runtime/virtcontainers/hypervisor_config_linux_test.go @@ -0,0 +1,114 @@ +// Copyright (c) 2022 Apple Inc. +// +// SPDX-License-Identifier: Apache-2.0 +// + +package virtcontainers + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestHypervisorConfigNoImagePath(t *testing.T) { + hypervisorConfig := &HypervisorConfig{ + KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), + ImagePath: "", + HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor), + } + + testHypervisorConfigValid(t, hypervisorConfig, false) +} + +func TestHypervisorConfigNoHypervisorPath(t *testing.T) { + hypervisorConfig := &HypervisorConfig{ + KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), + ImagePath: fmt.Sprintf("%s/%s", testDir, testImage), + HypervisorPath: "", + } + + testHypervisorConfigValid(t, hypervisorConfig, true) +} + +func TestHypervisorConfigIsValid(t *testing.T) { + hypervisorConfig := &HypervisorConfig{ + KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), + ImagePath: fmt.Sprintf("%s/%s", testDir, testImage), + HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor), + } + + testHypervisorConfigValid(t, hypervisorConfig, true) +} + +func TestHypervisorConfigBothInitrdAndImage(t *testing.T) { + hypervisorConfig := &HypervisorConfig{ + KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), + ImagePath: fmt.Sprintf("%s/%s", testDir, testImage), + InitrdPath: fmt.Sprintf("%s/%s", testDir, testInitrd), + HypervisorPath: "", + } + + testHypervisorConfigValid(t, hypervisorConfig, false) +} + +func TestHypervisorConfigSecureExecution(t *testing.T) { + hypervisorConfig := &HypervisorConfig{ + KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), + InitrdPath: fmt.Sprintf("%s/%s", testDir, testInitrd), + ConfidentialGuest: true, + HypervisorMachineType: QemuCCWVirtio, + } + + // Secure Execution should only specify a kernel (encrypted image contains all components) + testHypervisorConfigValid(t, hypervisorConfig, false) +} + +func TestHypervisorConfigValidTemplateConfig(t *testing.T) { + hypervisorConfig := &HypervisorConfig{ + KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), + ImagePath: fmt.Sprintf("%s/%s", testDir, testImage), + HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor), + BootToBeTemplate: true, + BootFromTemplate: true, + } + testHypervisorConfigValid(t, hypervisorConfig, false) + + hypervisorConfig.BootToBeTemplate = false + testHypervisorConfigValid(t, hypervisorConfig, false) + hypervisorConfig.MemoryPath = "foobar" + testHypervisorConfigValid(t, hypervisorConfig, false) + hypervisorConfig.DevicesStatePath = "foobar" + testHypervisorConfigValid(t, hypervisorConfig, true) + + hypervisorConfig.BootFromTemplate = false + hypervisorConfig.BootToBeTemplate = true + testHypervisorConfigValid(t, hypervisorConfig, true) + hypervisorConfig.MemoryPath = "" + testHypervisorConfigValid(t, hypervisorConfig, false) +} + +func TestHypervisorConfigDefaults(t *testing.T) { + assert := assert.New(t) + hypervisorConfig := &HypervisorConfig{ + KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), + ImagePath: fmt.Sprintf("%s/%s", testDir, testImage), + HypervisorPath: "", + } + testHypervisorConfigValid(t, hypervisorConfig, true) + + hypervisorConfigDefaultsExpected := &HypervisorConfig{ + KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), + ImagePath: fmt.Sprintf("%s/%s", testDir, testImage), + HypervisorPath: "", + NumVCPUs: defaultVCPUs, + MemorySize: defaultMemSzMiB, + DefaultBridges: defaultBridges, + BlockDeviceDriver: defaultBlockDriver, + DefaultMaxVCPUs: defaultMaxVCPUs, + Msize9p: defaultMsize9p, + } + + assert.Exactly(hypervisorConfig, hypervisorConfigDefaultsExpected) +} diff --git a/src/runtime/virtcontainers/hypervisor_config_test.go b/src/runtime/virtcontainers/hypervisor_config_test.go new file mode 100644 index 0000000000..49558f6a97 --- /dev/null +++ b/src/runtime/virtcontainers/hypervisor_config_test.go @@ -0,0 +1,30 @@ +// Copyright (c) 2022 Apple Inc. +// +// SPDX-License-Identifier: Apache-2.0 +// + +package virtcontainers + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func testHypervisorConfigValid(t *testing.T, hypervisorConfig *HypervisorConfig, success bool) { + err := validateHypervisorConfig(hypervisorConfig) + assert := assert.New(t) + assert.False(success && err != nil) + assert.False(!success && err == nil) +} + +func TestHypervisorConfigNoKernelPath(t *testing.T) { + hypervisorConfig := &HypervisorConfig{ + KernelPath: "", + ImagePath: fmt.Sprintf("%s/%s", testDir, testImage), + HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor), + } + + testHypervisorConfigValid(t, hypervisorConfig, false) +} diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index 550362afdd..d53de26566 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -609,62 +609,6 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor return s, nil } -func validateHypervisorConfig(conf *HypervisorConfig) error { - if conf.KernelPath == "" { - return fmt.Errorf("Missing kernel path") - } - - QemuCCWVirtio := "" - if conf.ConfidentialGuest && conf.HypervisorMachineType == QemuCCWVirtio { - if conf.ImagePath != "" || conf.InitrdPath != "" { - fmt.Println("yes, failing") - return fmt.Errorf("Neither the image or initrd path may be set for Secure Execution") - } - } else if conf.ImagePath == "" && conf.InitrdPath == "" { - return fmt.Errorf("Missing image and initrd path") - } else if conf.ImagePath != "" && conf.InitrdPath != "" { - return fmt.Errorf("Image and initrd path cannot be both set") - } - - if err := conf.CheckTemplateConfig(); err != nil { - return err - } - - if conf.NumVCPUs == 0 { - conf.NumVCPUs = defaultVCPUs - } - - if conf.MemorySize == 0 { - conf.MemorySize = defaultMemSzMiB - } - - if conf.DefaultBridges == 0 { - conf.DefaultBridges = defaultBridges - } - - if conf.BlockDeviceDriver == "" { - conf.BlockDeviceDriver = defaultBlockDriver - } else if conf.BlockDeviceDriver == config.VirtioBlock && conf.HypervisorMachineType == QemuCCWVirtio { - conf.BlockDeviceDriver = config.VirtioBlockCCW - } - - if conf.DefaultMaxVCPUs == 0 || conf.DefaultMaxVCPUs > defaultMaxVCPUs { - conf.DefaultMaxVCPUs = defaultMaxVCPUs - } - - if conf.ConfidentialGuest && conf.NumVCPUs != conf.DefaultMaxVCPUs { - hvLogger.Warnf("Confidential guests do not support hotplugging of vCPUs. Setting DefaultMaxVCPUs to NumVCPUs (%d)", conf.NumVCPUs) - conf.DefaultMaxVCPUs = conf.NumVCPUs - } - - if conf.Msize9p == 0 && conf.SharedFS != config.VirtioFS { - conf.Msize9p = defaultMsize9p - } - - return nil - -} - func (s *Sandbox) createResourceController() error { var err error cgroupPath := "" From e5be5cb086515ca7539cc6ee35e9d56bddea89f8 Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Mon, 27 Jun 2022 10:30:35 -0700 Subject: [PATCH 10/36] runtime: device: cleanup outdated comments Prior device config move didn't update the comments. Let's address this, and make sure comments match the new path... Signed-off-by: Eric Ernst --- src/runtime/pkg/device/config/config.go | 4 ++-- src/runtime/pkg/device/config/pmem.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/runtime/pkg/device/config/config.go b/src/runtime/pkg/device/config/config.go index 748c5b4e51..61f9236b9c 100644 --- a/src/runtime/pkg/device/config/config.go +++ b/src/runtime/pkg/device/config/config.go @@ -444,7 +444,7 @@ func getVhostUserDevName(dirname string, majorNum, minorNum uint32) (string, err // DeviceState is a structure which represents host devices // plugged to a hypervisor, one Device can be shared among containers in POD -// Refs: virtcontainers/device/drivers/generic.go:GenericDevice +// Refs: pkg/device/drivers/generic.go:GenericDevice type DeviceState struct { // DriverOptions is specific options for each device driver // for example, for BlockDevice, we can set DriverOptions["block-driver"]="virtio-blk" @@ -459,7 +459,7 @@ type DeviceState struct { ID string // Type is used to specify driver type - // Refs: virtcontainers/device/config/config.go:DeviceType + // Refs: pkg/device/config/config.go:DeviceType Type string // Type of device: c, b, u or p diff --git a/src/runtime/pkg/device/config/pmem.go b/src/runtime/pkg/device/config/pmem.go index 44fd321873..cdd4db9988 100644 --- a/src/runtime/pkg/device/config/pmem.go +++ b/src/runtime/pkg/device/config/pmem.go @@ -26,7 +26,7 @@ const ( ) var ( - pmemLog = logrus.WithField("source", "virtcontainers/device/config") + pmemLog = logrus.WithField("source", "pkg/device/config") ) // SetLogger sets up a logger for this pkg From ab5f1c9564fcdf4d5797db9b3f59333061ec6df2 Mon Sep 17 00:00:00 2001 From: liubin Date: Wed, 29 Jun 2022 12:33:32 +0800 Subject: [PATCH 11/36] shim: set a non-zero return code if the wait process call failed. Return code is an int32 type, so if an error occurred, the default value may be zero, this value will be created as a normal exit code. Set return code to 255 will let the caller(for example Kubernetes) know that there are some problems with the pod/container. Fixes: #4419 Signed-off-by: liubin --- src/runtime/pkg/containerd-shim-v2/wait.go | 5 +++++ src/runtime/virtcontainers/sandbox.go | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/runtime/pkg/containerd-shim-v2/wait.go b/src/runtime/pkg/containerd-shim-v2/wait.go index 8a24c29bfa..ebb742790d 100644 --- a/src/runtime/pkg/containerd-shim-v2/wait.go +++ b/src/runtime/pkg/containerd-shim-v2/wait.go @@ -53,6 +53,11 @@ func wait(ctx context.Context, s *service, c *container, execID string) (int32, "container": c.id, "pid": processID, }).Error("Wait for process failed") + + // set return code if wait failed + if ret == 0 { + ret = exitCode255 + } } timeStamp := time.Now() diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index a995f1f77b..d1705d3791 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -1590,7 +1590,7 @@ func (s *Sandbox) ResumeContainer(ctx context.Context, containerID string) error } // createContainers registers all containers, create the -// containers in the guest and starts one shim per container. +// containers in the guest. func (s *Sandbox) createContainers(ctx context.Context) error { span, ctx := katatrace.Trace(ctx, s.Logger(), "createContainers", sandboxTracingTags, map[string]string{"sandbox_id": s.id}) defer span.End() From 20f11877bed31a0efbda204ebd9c50f8eed02b93 Mon Sep 17 00:00:00 2001 From: Pavel Mores Date: Thu, 14 Apr 2022 19:00:54 +0200 Subject: [PATCH 12/36] runtime: Add framework to manipulate config structs via reflection For 'tomlConfig' substructures stored in Golang maps - 'hypervisor' and 'agent' - BurntSushi doesn't preserve their previous contents as it does for substructures stored directly (e.g. 'runtime'). We use reflection to work around this. This commit adds three primitive operations to work with struct fields identified by their `toml:"..."` tags - one to get a field value, one to set a field value and one to assign a source struct field value to the corresponding field of a target. Signed-off-by: Pavel Mores --- src/runtime/pkg/katautils/config.go | 49 +++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/runtime/pkg/katautils/config.go b/src/runtime/pkg/katautils/config.go index dbdfdac303..8522274dae 100644 --- a/src/runtime/pkg/katautils/config.go +++ b/src/runtime/pkg/katautils/config.go @@ -12,6 +12,7 @@ import ( "fmt" "os" "path/filepath" + "reflect" goruntime "runtime" "strings" @@ -1308,6 +1309,54 @@ func decodeConfig(configPath string) (tomlConfig, string, error) { return tomlConf, resolved, nil } + +// Copies a TOML value of the source field identified by its TOML key to the +// corresponding field of the target. Basically +// 'target[tomlKeyName] = source[tomlKeyNmae]'. +func copyFieldValue(source reflect.Value, tomlKeyName string, target reflect.Value) error { + val, err := getValue(source, tomlKeyName) + if err != nil { + return fmt.Errorf("error getting key %q from a decoded drop-in conf file: %s", tomlKeyName, err) + } + err = setValue(target, tomlKeyName, val) + if err != nil { + return fmt.Errorf("error setting key %q to a new value '%v': %s", tomlKeyName, val.Interface(), err) + } + return nil +} + +// The first argument is expected to be a reflect.Value of a tomlConfig +// substructure (hypervisor, agent), the second argument is a TOML key +// corresponding to the substructure field whose TOML value is queried. +// Return value corresponds to 'tomlConfStruct[tomlKey]'. +func getValue(tomlConfStruct reflect.Value, tomlKey string) (reflect.Value, error) { + tomlConfStructType := tomlConfStruct.Type() + for j := 0; j < tomlConfStruct.NumField(); j++ { + fieldTomlTag := tomlConfStructType.Field(j).Tag.Get("toml") + if fieldTomlTag == tomlKey { + return tomlConfStruct.Field(j), nil + } + } + return reflect.Value{}, fmt.Errorf("key %q not found", tomlKey) +} + +// The first argument is expected to be a reflect.Value of a tomlConfig +// substructure (hypervisor, agent), the second argument is a TOML key +// corresponding to the substructure field whose TOML value is to be changed, +// the third argument is a reflect.Value representing the new TOML value. +// An equivalent of 'tomlConfStruct[tomlKey] = newVal'. +func setValue(tomlConfStruct reflect.Value, tomlKey string, newVal reflect.Value) error { + tomlConfStructType := tomlConfStruct.Type() + for j := 0; j < tomlConfStruct.NumField(); j++ { + fieldTomlTag := tomlConfStructType.Field(j).Tag.Get("toml") + if fieldTomlTag == tomlKey { + tomlConfStruct.Field(j).Set(newVal) + return nil + } + } + return fmt.Errorf("key %q not found", tomlKey) +} + // checkConfig checks the validity of the specified config. func checkConfig(config oci.RuntimeConfig) error { if err := checkNetNsConfig(config); err != nil { From 2c1efcc697051cdee49d4152be049c36b37c9de4 Mon Sep 17 00:00:00 2001 From: Pavel Mores Date: Thu, 14 Apr 2022 19:14:22 +0200 Subject: [PATCH 13/36] runtime: Add helpers to copy fields between tomlConfig instances These functions take a TOML key - an array of individual components, e.g. ["agent" "kata" "enable_tracing"], as returned by BurntSushi - and two 'tomlConfig' instances. They copy the value of the struct field identified by the key from the source instance to the target one if necessary. This is only done if the TOML key points to structures stored in maps by 'tomlConfig', i.e. 'hypervisor' and 'agent'. Nothing needs to be done in other cases. Signed-off-by: Pavel Mores --- src/runtime/pkg/katautils/config.go | 53 +++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/runtime/pkg/katautils/config.go b/src/runtime/pkg/katautils/config.go index 8522274dae..d342bb3b35 100644 --- a/src/runtime/pkg/katautils/config.go +++ b/src/runtime/pkg/katautils/config.go @@ -1310,6 +1310,59 @@ func decodeConfig(configPath string) (tomlConfig, string, error) { } +func applyKey(sourceConf tomlConfig, key []string, targetConf *tomlConfig) error { + // Any key that might need treatment provided by this function has to have + // (at least) three components: [ map_name map_key_name field_toml_tag ], + // e.g. [agent kata enable_tracing] or [hypervisor qemu confidential_guest]. + if len(key) < 3 { + return nil + } + switch key[0] { + case "agent": + return applyAgentKey(sourceConf, key[1:], targetConf) + case "hypervisor": + return applyHypervisorKey(sourceConf, key[1:], targetConf) + // The table the 'key' is in is not stored in a map so no special handling + // is needed. + } + return nil +} + +// Both of the following functions copy the value of a 'sourceConf' field +// identified by the TOML tag in 'key' into the corresponding field in +// 'targetConf'. +func applyAgentKey(sourceConf tomlConfig, key []string, targetConf *tomlConfig) error { + agentName := key[0] + tomlKeyName := key[1] + + sourceAgentConf := sourceConf.Agent[agentName] + targetAgentConf := targetConf.Agent[agentName] + + err := copyFieldValue(reflect.ValueOf(&sourceAgentConf).Elem(), tomlKeyName, reflect.ValueOf(&targetAgentConf).Elem()) + if err != nil { + return err + } + + targetConf.Agent[agentName] = targetAgentConf + return nil +} + +func applyHypervisorKey(sourceConf tomlConfig, key []string, targetConf *tomlConfig) error { + hypervisorName := key[0] + tomlKeyName := key[1] + + sourceHypervisorConf := sourceConf.Hypervisor[hypervisorName] + targetHypervisorConf := targetConf.Hypervisor[hypervisorName] + + err := copyFieldValue(reflect.ValueOf(&sourceHypervisorConf).Elem(), tomlKeyName, reflect.ValueOf(&targetHypervisorConf).Elem()) + if err != nil { + return err + } + + targetConf.Hypervisor[hypervisorName] = targetHypervisorConf + return nil +} + // Copies a TOML value of the source field identified by its TOML key to the // corresponding field of the target. Basically // 'target[tomlKeyName] = source[tomlKeyNmae]'. From 0f9856c4654b2b68fd71517423f2d4f8c0b3e48c Mon Sep 17 00:00:00 2001 From: Pavel Mores Date: Thu, 14 Apr 2022 19:29:41 +0200 Subject: [PATCH 14/36] runtime: Scan drop-in directory, read files and decode them updateFromDropIn() uses the infrastructure built by previous commits to ensure no contents of 'tomlConfig' are lost during decoding. To do this, we preserve the current contents of our tomlConfig in a clone and decode a drop-in into the original. At this point, the original instance is updated but its Agent and/or Hypervisor fields are potentially damaged. To merge, we update the clone's Agent/Hypervisor from the original instance. Now the clone has the desired Agent/Hypervisor and the original instance has the rest, so to finish, we just need to move the clone's Agent/Hypervisor to the original. Signed-off-by: Pavel Mores --- src/runtime/pkg/katautils/config.go | 79 +++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/src/runtime/pkg/katautils/config.go b/src/runtime/pkg/katautils/config.go index d342bb3b35..43cf0da253 100644 --- a/src/runtime/pkg/katautils/config.go +++ b/src/runtime/pkg/katautils/config.go @@ -10,6 +10,7 @@ package katautils import ( "errors" "fmt" + "io/ioutil" "os" "path/filepath" "reflect" @@ -177,6 +178,20 @@ type agent struct { DialTimeout uint32 `toml:"dial_timeout"` } +func (orig *tomlConfig) Clone() tomlConfig { + clone := *orig + clone.Hypervisor = make(map[string]hypervisor) + clone.Agent = make(map[string]agent) + + for key, value := range orig.Hypervisor { + clone.Hypervisor[key] = value + } + for key, value := range orig.Agent { + clone.Agent[key] = value + } + return clone +} + func (h hypervisor) path() (string, error) { p := h.Path @@ -1309,6 +1324,70 @@ func decodeConfig(configPath string) (tomlConfig, string, error) { return tomlConf, resolved, nil } +func decodeDropIns(mainConfigPath string, tomlConf *tomlConfig) error { + configDir := filepath.Dir(mainConfigPath) + dropInDir := filepath.Join(configDir, "config.d") + + files, err := ioutil.ReadDir(dropInDir) + if err != nil { + if !os.IsNotExist(err) { + return fmt.Errorf("error reading %q directory: %s", dropInDir, err) + } else { + return nil + } + } + + for _, file := range files { + dropInFpath := filepath.Join(dropInDir, file.Name()) + + err = updateFromDropIn(dropInFpath, tomlConf) + if err != nil { + return err + } + } + + return nil +} + +func updateFromDropIn(dropInFpath string, tomlConf *tomlConfig) error { + configData, err := os.ReadFile(dropInFpath) + if err != nil { + return fmt.Errorf("error reading file %q: %s", dropInFpath, err) + } + + // Ordinarily, BurntSushi only updates fields of tomlConfig that are + // changed by the file and leaves the rest alone. This doesn't apply + // though to tomlConfig substructures that are stored in maps. Their + // previous contents are erased by toml.Decode() and only fields changed by + // the file are set. To work around this, a bit of juggling is needed to + // preserve the previous contents and merge them manually with the incoming + // changes afterwards, using reflection. + tomlConfOrig := tomlConf.Clone() + + var md toml.MetaData + md, err = toml.Decode(string(configData), &tomlConf) + + if err != nil { + return fmt.Errorf("error decoding file %q: %s", dropInFpath, err) + } + + if len(md.Undecoded()) > 0 { + msg := fmt.Sprintf("warning: undecoded keys in %q: %+v", dropInFpath, md.Undecoded()) + kataUtilsLogger.Warn(msg) + } + + for _, key := range md.Keys() { + err = applyKey(*tomlConf, key, &tomlConfOrig) + if err != nil { + return fmt.Errorf("error applying key '%+v' from drop-in file %q: %s", key, dropInFpath, err) + } + } + + tomlConf.Hypervisor = tomlConfOrig.Hypervisor + tomlConf.Agent = tomlConfOrig.Agent + + return nil +} func applyKey(sourceConf tomlConfig, key []string, targetConf *tomlConfig) error { // Any key that might need treatment provided by this function has to have From 99f5ca80fcbb917439e89b688815482f7c1ea675 Mon Sep 17 00:00:00 2001 From: Pavel Mores Date: Thu, 14 Apr 2022 19:46:47 +0200 Subject: [PATCH 15/36] runtime: Plug drop-in decoding into decodeConfig() Fixes #4108 Signed-off-by: Pavel Mores --- src/runtime/pkg/katautils/config.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/runtime/pkg/katautils/config.go b/src/runtime/pkg/katautils/config.go index 43cf0da253..ef0b7649a5 100644 --- a/src/runtime/pkg/katautils/config.go +++ b/src/runtime/pkg/katautils/config.go @@ -1321,6 +1321,11 @@ func decodeConfig(configPath string) (tomlConfig, string, error) { return tomlConf, resolved, err } + err = decodeDropIns(resolved, &tomlConf) + if err != nil { + return tomlConf, resolved, err + } + return tomlConf, resolved, nil } From c656457e908f250059befa7f68bf090882c843d7 Mon Sep 17 00:00:00 2001 From: Pavel Mores Date: Thu, 21 Apr 2022 13:43:01 +0200 Subject: [PATCH 16/36] runtime: Add tests of drop-in config file decoding The tests ensure that interactions between drop-ins and the base configuration.toml and among drop-ins themselves work as intended, basically that files are evaluated in the correct order (base file first, then drop-ins in alphabetical order) and the last one to set a specific key wins. Signed-off-by: Pavel Mores --- src/runtime/pkg/katautils/config_test.go | 66 ++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/src/runtime/pkg/katautils/config_test.go b/src/runtime/pkg/katautils/config_test.go index 0bcac2137f..4cc7eb9315 100644 --- a/src/runtime/pkg/katautils/config_test.go +++ b/src/runtime/pkg/katautils/config_test.go @@ -1654,3 +1654,69 @@ func TestValidateBindMounts(t *testing.T) { } } } + +func TestLoadDropInConfiguration(t *testing.T) { + tmpdir := t.TempDir() + + // Test Runtime and Hypervisor to represent structures stored directly and + // in maps, respectively. For each of them, test + // - a key that's only set in the base config file + // - a key that's only set in a drop-in + // - a key that's set in the base config file and then changed by a drop-in + // - a key that's set in a drop-in and then overridden by another drop-in + // Avoid default values to reduce the risk of mistaking a result of + // something having gone wrong with the expected value. + + runtimeConfigFileData := ` +[hypervisor.qemu] +path = "/usr/bin/qemu-kvm" +default_bridges = 3 +[runtime] +enable_debug = true +internetworking_model="tcfilter" +` + dropInData := ` +[hypervisor.qemu] +default_vcpus = 2 +default_bridges = 4 +shared_fs = "virtio-fs" +[runtime] +sandbox_cgroup_only=true +internetworking_model="macvtap" +vfio_mode="guest-kernel" +` + dropInOverrideData := ` +[hypervisor.qemu] +shared_fs = "virtio-9p" +[runtime] +vfio_mode="vfio" +` + + configPath := path.Join(tmpdir, "runtime.toml") + err := createConfig(configPath, runtimeConfigFileData) + assert.NoError(t, err) + + dropInDir := path.Join(tmpdir, "config.d") + err = os.Mkdir(dropInDir, os.FileMode(0777)) + assert.NoError(t, err) + + dropInPath := path.Join(dropInDir, "10-base") + err = createConfig(dropInPath, dropInData) + assert.NoError(t, err) + + dropInOverridePath := path.Join(dropInDir, "10-override") + err = createConfig(dropInOverridePath, dropInOverrideData) + assert.NoError(t, err) + + config, _, err := decodeConfig(configPath) + assert.NoError(t, err) + + assert.Equal(t, config.Hypervisor["qemu"].Path, "/usr/bin/qemu-kvm") + assert.Equal(t, config.Hypervisor["qemu"].NumVCPUs, int32(2)) + assert.Equal(t, config.Hypervisor["qemu"].DefaultBridges, uint32(4)) + assert.Equal(t, config.Hypervisor["qemu"].SharedFS, "virtio-9p") + assert.Equal(t, config.Runtime.Debug, true) + assert.Equal(t, config.Runtime.SandboxCgroupOnly, true) + assert.Equal(t, config.Runtime.InterNetworkModel, "macvtap") + assert.Equal(t, config.Runtime.VfioMode, "vfio") +} From 96553e8bd2a94f416563eae75b43d167e9fb0dea Mon Sep 17 00:00:00 2001 From: Pavel Mores Date: Wed, 11 May 2022 13:39:12 +0200 Subject: [PATCH 17/36] runtime: Add documentation of drop-in config file fragments Added user manual for the drop-in config file fragments feature. Signed-off-by: Pavel Mores --- src/runtime/README.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/runtime/README.md b/src/runtime/README.md index d6738f66fb..c8aeec0ce7 100644 --- a/src/runtime/README.md +++ b/src/runtime/README.md @@ -87,6 +87,27 @@ following locations (in order): > **Note:** For both binaries, the first path that exists will be used. +#### Drop-in configuration file fragments + +To enable changing configuration without changing the configuration file +itself, drop-in configuration file fragments are supported. Once a +configuration file is parsed, if there is a subdirectory called `config.d` in +the same directory as the configuration file its contents will be loaded +in alphabetical order and each item will be parsed as a config file. Settings +loaded from these configuration file fragments override settings loaded from +the main configuration file and earlier fragments. Users are encouraged to use +familiar naming conventions to order the fragments (e.g. `config.d/10-this`, +`config.d/20-that` etc.). + +Non-existent or empty `config.d` directory is not an error (in other words, not +using configuration file fragments is fine). On the other hand, if fragments +are used, they must be valid - any errors while parsing fragments (unreadable +fragment files, contents not valid TOML) are treated the same as errors +while parsing the main configuration file. A `config.d` subdirectory affects +only the `configuration.toml` _in the same directory_. For fragments in +`config.d` to be parsed, there has to be a valid main configuration file _in +that location_ (it can be empty though). + ### Hypervisor specific configuration Kata Containers supports multiple hypervisors so your `configuration.toml` From a5a25ed13d5fe14e077b913005e8ae2c40cf6b28 Mon Sep 17 00:00:00 2001 From: liubin Date: Wed, 29 Jun 2022 17:20:22 +0800 Subject: [PATCH 18/36] runtime: delete Console from Cmd type There is much code related to this property, but it is not used anymore. Fixes: #4553 Signed-off-by: liubin --- src/runtime/cmd/kata-runtime/factory_test.go | 6 ++--- src/runtime/cmd/kata-runtime/main_test.go | 5 +---- src/runtime/pkg/containerd-shim-v2/create.go | 4 ++-- .../pkg/containerd-shim-v2/create_test.go | 12 +++++----- .../pkg/containerd-shim-v2/utils_test.go | 4 +--- src/runtime/pkg/katautils/create.go | 8 +++---- src/runtime/pkg/katautils/create_test.go | 22 +++++++++---------- src/runtime/pkg/oci/utils.go | 8 +++---- src/runtime/pkg/oci/utils_test.go | 12 +--------- src/runtime/virtcontainers/types/sandbox.go | 1 - 10 files changed, 31 insertions(+), 51 deletions(-) diff --git a/src/runtime/cmd/kata-runtime/factory_test.go b/src/runtime/cmd/kata-runtime/factory_test.go index f980bc5a56..3ab861420b 100644 --- a/src/runtime/cmd/kata-runtime/factory_test.go +++ b/src/runtime/cmd/kata-runtime/factory_test.go @@ -44,7 +44,7 @@ func TestFactoryCLIFunctionInit(t *testing.T) { tmpdir := t.TempDir() - runtimeConfig, err := newTestRuntimeConfig(tmpdir, testConsole, true) + runtimeConfig, err := newTestRuntimeConfig(tmpdir, true) assert.NoError(err) set := flag.NewFlagSet("", 0) @@ -91,7 +91,7 @@ func TestFactoryCLIFunctionDestroy(t *testing.T) { tmpdir := t.TempDir() - runtimeConfig, err := newTestRuntimeConfig(tmpdir, testConsole, true) + runtimeConfig, err := newTestRuntimeConfig(tmpdir, true) assert.NoError(err) set := flag.NewFlagSet("", 0) @@ -123,7 +123,7 @@ func TestFactoryCLIFunctionStatus(t *testing.T) { tmpdir := t.TempDir() - runtimeConfig, err := newTestRuntimeConfig(tmpdir, testConsole, true) + runtimeConfig, err := newTestRuntimeConfig(tmpdir, true) assert.NoError(err) set := flag.NewFlagSet("", 0) diff --git a/src/runtime/cmd/kata-runtime/main_test.go b/src/runtime/cmd/kata-runtime/main_test.go index 95c7dc59d5..f8f98c3c47 100644 --- a/src/runtime/cmd/kata-runtime/main_test.go +++ b/src/runtime/cmd/kata-runtime/main_test.go @@ -33,8 +33,6 @@ const ( testDirMode = os.FileMode(0750) testFileMode = os.FileMode(0640) testExeFileMode = os.FileMode(0750) - - testConsole = "/dev/pts/999" ) var ( @@ -151,7 +149,7 @@ func newTestHypervisorConfig(dir string, create bool) (vc.HypervisorConfig, erro } // newTestRuntimeConfig creates a new RuntimeConfig -func newTestRuntimeConfig(dir, consolePath string, create bool) (oci.RuntimeConfig, error) { +func newTestRuntimeConfig(dir string, create bool) (oci.RuntimeConfig, error) { if dir == "" { return oci.RuntimeConfig{}, errors.New("BUG: need directory") } @@ -164,7 +162,6 @@ func newTestRuntimeConfig(dir, consolePath string, create bool) (oci.RuntimeConf return oci.RuntimeConfig{ HypervisorType: vc.QemuHypervisor, HypervisorConfig: hypervisorConfig, - Console: consolePath, }, nil } diff --git a/src/runtime/pkg/containerd-shim-v2/create.go b/src/runtime/pkg/containerd-shim-v2/create.go index eba829e2dd..6b14a94c7a 100644 --- a/src/runtime/pkg/containerd-shim-v2/create.go +++ b/src/runtime/pkg/containerd-shim-v2/create.go @@ -144,7 +144,7 @@ func create(ctx context.Context, s *service, r *taskAPI.CreateTaskRequest) (*con // ctx will be canceled after this rpc service call, but the sandbox will live // across multiple rpc service calls. // - sandbox, _, err := katautils.CreateSandbox(s.ctx, vci, *ociSpec, *s.config, rootFs, r.ID, bundlePath, "", disableOutput, false) + sandbox, _, err := katautils.CreateSandbox(s.ctx, vci, *ociSpec, *s.config, rootFs, r.ID, bundlePath, disableOutput, false) if err != nil { return nil, err } @@ -179,7 +179,7 @@ func create(ctx context.Context, s *service, r *taskAPI.CreateTaskRequest) (*con } }() - _, err = katautils.CreateContainer(ctx, s.sandbox, *ociSpec, rootFs, r.ID, bundlePath, "", disableOutput, runtimeConfig.DisableGuestEmptyDir) + _, err = katautils.CreateContainer(ctx, s.sandbox, *ociSpec, rootFs, r.ID, bundlePath, disableOutput, runtimeConfig.DisableGuestEmptyDir) if err != nil { return nil, err } diff --git a/src/runtime/pkg/containerd-shim-v2/create_test.go b/src/runtime/pkg/containerd-shim-v2/create_test.go index 994de7c436..121d5ea4db 100644 --- a/src/runtime/pkg/containerd-shim-v2/create_test.go +++ b/src/runtime/pkg/containerd-shim-v2/create_test.go @@ -51,7 +51,7 @@ func TestCreateSandboxSuccess(t *testing.T) { tmpdir, bundlePath, ociConfigFile := ktu.SetupOCIConfigFile(t) - runtimeConfig, err := newTestRuntimeConfig(tmpdir, testConsole, true) + runtimeConfig, err := newTestRuntimeConfig(tmpdir, true) assert.NoError(err) spec, err := compatoci.ParseConfigJSON(bundlePath) @@ -99,7 +99,7 @@ func TestCreateSandboxFail(t *testing.T) { tmpdir, bundlePath, ociConfigFile := ktu.SetupOCIConfigFile(t) - runtimeConfig, err := newTestRuntimeConfig(tmpdir, testConsole, true) + runtimeConfig, err := newTestRuntimeConfig(tmpdir, true) assert.NoError(err) spec, err := compatoci.ParseConfigJSON(bundlePath) @@ -136,7 +136,7 @@ func TestCreateSandboxConfigFail(t *testing.T) { tmpdir, bundlePath, _ := ktu.SetupOCIConfigFile(t) - runtimeConfig, err := newTestRuntimeConfig(tmpdir, testConsole, true) + runtimeConfig, err := newTestRuntimeConfig(tmpdir, true) assert.NoError(err) spec, err := compatoci.ParseConfigJSON(bundlePath) @@ -185,7 +185,7 @@ func TestCreateContainerSuccess(t *testing.T) { tmpdir, bundlePath, ociConfigFile := ktu.SetupOCIConfigFile(t) - runtimeConfig, err := newTestRuntimeConfig(tmpdir, testConsole, true) + runtimeConfig, err := newTestRuntimeConfig(tmpdir, true) assert.NoError(err) spec, err := compatoci.ParseConfigJSON(bundlePath) @@ -224,7 +224,7 @@ func TestCreateContainerFail(t *testing.T) { tmpdir, bundlePath, ociConfigFile := ktu.SetupOCIConfigFile(t) - runtimeConfig, err := newTestRuntimeConfig(tmpdir, testConsole, true) + runtimeConfig, err := newTestRuntimeConfig(tmpdir, true) assert.NoError(err) spec, err := compatoci.ParseConfigJSON(bundlePath) @@ -274,7 +274,7 @@ func TestCreateContainerConfigFail(t *testing.T) { tmpdir, bundlePath, ociConfigFile := ktu.SetupOCIConfigFile(t) - runtimeConfig, err := newTestRuntimeConfig(tmpdir, testConsole, true) + runtimeConfig, err := newTestRuntimeConfig(tmpdir, true) assert.NoError(err) spec, err := compatoci.ParseConfigJSON(bundlePath) diff --git a/src/runtime/pkg/containerd-shim-v2/utils_test.go b/src/runtime/pkg/containerd-shim-v2/utils_test.go index 35b489920b..32195bcce0 100644 --- a/src/runtime/pkg/containerd-shim-v2/utils_test.go +++ b/src/runtime/pkg/containerd-shim-v2/utils_test.go @@ -28,7 +28,6 @@ const ( testSandboxID = "777-77-77777777" testContainerID = "42" - testConsole = "/dev/pts/888" testContainerTypeAnnotation = "io.kubernetes.cri.container-type" testSandboxIDAnnotation = "io.kubernetes.cri.sandbox-id" @@ -91,7 +90,7 @@ func newTestHypervisorConfig(dir string, create bool) (vc.HypervisorConfig, erro } // newTestRuntimeConfig creates a new RuntimeConfig -func newTestRuntimeConfig(dir, consolePath string, create bool) (oci.RuntimeConfig, error) { +func newTestRuntimeConfig(dir string, create bool) (oci.RuntimeConfig, error) { if dir == "" { return oci.RuntimeConfig{}, errors.New("BUG: need directory") } @@ -104,7 +103,6 @@ func newTestRuntimeConfig(dir, consolePath string, create bool) (oci.RuntimeConf return oci.RuntimeConfig{ HypervisorType: vc.QemuHypervisor, HypervisorConfig: hypervisorConfig, - Console: consolePath, }, nil } diff --git a/src/runtime/pkg/katautils/create.go b/src/runtime/pkg/katautils/create.go index e456d37276..ffcaa07154 100644 --- a/src/runtime/pkg/katautils/create.go +++ b/src/runtime/pkg/katautils/create.go @@ -111,12 +111,12 @@ func SetEphemeralStorageType(ociSpec specs.Spec, disableGuestEmptyDir bool) spec // CreateSandbox create a sandbox container func CreateSandbox(ctx context.Context, vci vc.VC, ociSpec specs.Spec, runtimeConfig oci.RuntimeConfig, rootFs vc.RootFs, - containerID, bundlePath, console string, disableOutput, systemdCgroup bool) (_ vc.VCSandbox, _ vc.Process, err error) { + containerID, bundlePath string, disableOutput, systemdCgroup bool) (_ vc.VCSandbox, _ vc.Process, err error) { span, ctx := katatrace.Trace(ctx, nil, "CreateSandbox", createTracingTags) katatrace.AddTags(span, "container_id", containerID) defer span.End() - sandboxConfig, err := oci.SandboxConfig(ociSpec, runtimeConfig, bundlePath, containerID, console, disableOutput, systemdCgroup) + sandboxConfig, err := oci.SandboxConfig(ociSpec, runtimeConfig, bundlePath, containerID, disableOutput, systemdCgroup) if err != nil { return nil, vc.Process{}, err } @@ -219,7 +219,7 @@ func checkForFIPS(sandboxConfig *vc.SandboxConfig) error { } // CreateContainer create a container -func CreateContainer(ctx context.Context, sandbox vc.VCSandbox, ociSpec specs.Spec, rootFs vc.RootFs, containerID, bundlePath, console string, disableOutput bool, disableGuestEmptyDir bool) (vc.Process, error) { +func CreateContainer(ctx context.Context, sandbox vc.VCSandbox, ociSpec specs.Spec, rootFs vc.RootFs, containerID, bundlePath string, disableOutput bool, disableGuestEmptyDir bool) (vc.Process, error) { var c vc.VCContainer span, ctx := katatrace.Trace(ctx, nil, "CreateContainer", createTracingTags) @@ -228,7 +228,7 @@ func CreateContainer(ctx context.Context, sandbox vc.VCSandbox, ociSpec specs.Sp ociSpec = SetEphemeralStorageType(ociSpec, disableGuestEmptyDir) - contConfig, err := oci.ContainerConfig(ociSpec, bundlePath, containerID, console, disableOutput) + contConfig, err := oci.ContainerConfig(ociSpec, bundlePath, containerID, disableOutput) if err != nil { return vc.Process{}, err } diff --git a/src/runtime/pkg/katautils/create_test.go b/src/runtime/pkg/katautils/create_test.go index 15b4561137..b1e4cf2a90 100644 --- a/src/runtime/pkg/katautils/create_test.go +++ b/src/runtime/pkg/katautils/create_test.go @@ -28,7 +28,6 @@ import ( ) const ( - testConsole = "/dev/pts/999" testContainerTypeAnnotation = "io.kubernetes.cri-o.ContainerType" testSandboxIDAnnotation = "io.kubernetes.cri-o.SandboxID" testContainerTypeContainer = "container" @@ -50,7 +49,7 @@ func init() { } // newTestRuntimeConfig creates a new RuntimeConfig -func newTestRuntimeConfig(dir, consolePath string, create bool) (oci.RuntimeConfig, error) { +func newTestRuntimeConfig(dir string, create bool) (oci.RuntimeConfig, error) { if dir == "" { return oci.RuntimeConfig{}, errors.New("BUG: need directory") } @@ -63,7 +62,6 @@ func newTestRuntimeConfig(dir, consolePath string, create bool) (oci.RuntimeConf return oci.RuntimeConfig{ HypervisorType: vc.QemuHypervisor, HypervisorConfig: hypervisorConfig, - Console: consolePath, }, nil } @@ -213,7 +211,7 @@ func TestCreateSandboxConfigFail(t *testing.T) { tmpdir, bundlePath, _ := ktu.SetupOCIConfigFile(t) - runtimeConfig, err := newTestRuntimeConfig(tmpdir, testConsole, true) + runtimeConfig, err := newTestRuntimeConfig(tmpdir, true) assert.NoError(err) spec, err := compatoci.ParseConfigJSON(bundlePath) @@ -233,7 +231,7 @@ func TestCreateSandboxConfigFail(t *testing.T) { rootFs := vc.RootFs{Mounted: true} - _, _, err = CreateSandbox(context.Background(), testingImpl, spec, runtimeConfig, rootFs, testContainerID, bundlePath, testConsole, true, true) + _, _, err = CreateSandbox(context.Background(), testingImpl, spec, runtimeConfig, rootFs, testContainerID, bundlePath, true, true) assert.Error(err) } @@ -246,7 +244,7 @@ func TestCreateSandboxFail(t *testing.T) { tmpdir, bundlePath, _ := ktu.SetupOCIConfigFile(t) - runtimeConfig, err := newTestRuntimeConfig(tmpdir, testConsole, true) + runtimeConfig, err := newTestRuntimeConfig(tmpdir, true) assert.NoError(err) spec, err := compatoci.ParseConfigJSON(bundlePath) @@ -254,7 +252,7 @@ func TestCreateSandboxFail(t *testing.T) { rootFs := vc.RootFs{Mounted: true} - _, _, err = CreateSandbox(context.Background(), testingImpl, spec, runtimeConfig, rootFs, testContainerID, bundlePath, testConsole, true, true) + _, _, err = CreateSandbox(context.Background(), testingImpl, spec, runtimeConfig, rootFs, testContainerID, bundlePath, true, true) assert.Error(err) assert.True(vcmock.IsMockError(err)) } @@ -268,7 +266,7 @@ func TestCreateSandboxAnnotations(t *testing.T) { tmpdir, bundlePath, _ := ktu.SetupOCIConfigFile(t) - runtimeConfig, err := newTestRuntimeConfig(tmpdir, testConsole, true) + runtimeConfig, err := newTestRuntimeConfig(tmpdir, true) assert.NoError(err) spec, err := compatoci.ParseConfigJSON(bundlePath) @@ -290,7 +288,7 @@ func TestCreateSandboxAnnotations(t *testing.T) { testingImpl.CreateSandboxFunc = nil }() - sandbox, _, err := CreateSandbox(context.Background(), testingImpl, spec, runtimeConfig, rootFs, testContainerID, bundlePath, testConsole, true, true) + sandbox, _, err := CreateSandbox(context.Background(), testingImpl, spec, runtimeConfig, rootFs, testContainerID, bundlePath, true, true) assert.NoError(err) netNsPath, err := sandbox.Annotations("nerdctl/network-namespace") @@ -356,7 +354,7 @@ func TestCreateContainerContainerConfigFail(t *testing.T) { rootFs := vc.RootFs{Mounted: true} for _, disableOutput := range []bool{true, false} { - _, err = CreateContainer(context.Background(), mockSandbox, spec, rootFs, testContainerID, bundlePath, testConsole, disableOutput, false) + _, err = CreateContainer(context.Background(), mockSandbox, spec, rootFs, testContainerID, bundlePath, disableOutput, false) assert.Error(err) assert.False(vcmock.IsMockError(err)) assert.True(strings.Contains(err.Error(), containerType)) @@ -383,7 +381,7 @@ func TestCreateContainerFail(t *testing.T) { rootFs := vc.RootFs{Mounted: true} for _, disableOutput := range []bool{true, false} { - _, err = CreateContainer(context.Background(), mockSandbox, spec, rootFs, testContainerID, bundlePath, testConsole, disableOutput, false) + _, err = CreateContainer(context.Background(), mockSandbox, spec, rootFs, testContainerID, bundlePath, disableOutput, false) assert.Error(err) assert.True(vcmock.IsMockError(err)) } @@ -417,7 +415,7 @@ func TestCreateContainer(t *testing.T) { rootFs := vc.RootFs{Mounted: true} for _, disableOutput := range []bool{true, false} { - _, err = CreateContainer(context.Background(), mockSandbox, spec, rootFs, testContainerID, bundlePath, testConsole, disableOutput, false) + _, err = CreateContainer(context.Background(), mockSandbox, spec, rootFs, testContainerID, bundlePath, disableOutput, false) assert.NoError(err) } } diff --git a/src/runtime/pkg/oci/utils.go b/src/runtime/pkg/oci/utils.go index 71423cf0cc..5fb8ea1d5d 100644 --- a/src/runtime/pkg/oci/utils.go +++ b/src/runtime/pkg/oci/utils.go @@ -105,7 +105,6 @@ type RuntimeConfig struct { //Experimental features enabled Experimental []exp.Feature - Console string JaegerEndpoint string JaegerUser string JaegerPassword string @@ -861,8 +860,8 @@ func addAgentConfigOverrides(ocispec specs.Spec, config *vc.SandboxConfig) error // SandboxConfig converts an OCI compatible runtime configuration file // to a virtcontainers sandbox configuration structure. -func SandboxConfig(ocispec specs.Spec, runtime RuntimeConfig, bundlePath, cid, console string, detach, systemdCgroup bool) (vc.SandboxConfig, error) { - containerConfig, err := ContainerConfig(ocispec, bundlePath, cid, console, detach) +func SandboxConfig(ocispec specs.Spec, runtime RuntimeConfig, bundlePath, cid string, detach, systemdCgroup bool) (vc.SandboxConfig, error) { + containerConfig, err := ContainerConfig(ocispec, bundlePath, cid, detach) if err != nil { return vc.SandboxConfig{}, err } @@ -947,7 +946,7 @@ func SandboxConfig(ocispec specs.Spec, runtime RuntimeConfig, bundlePath, cid, c // ContainerConfig converts an OCI compatible runtime configuration // file to a virtcontainers container configuration structure. -func ContainerConfig(ocispec specs.Spec, bundlePath, cid, console string, detach bool) (vc.ContainerConfig, error) { +func ContainerConfig(ocispec specs.Spec, bundlePath, cid string, detach bool) (vc.ContainerConfig, error) { rootfs := vc.RootFs{Target: ocispec.Root.Path, Mounted: true} if !filepath.IsAbs(rootfs.Target) { rootfs.Target = filepath.Join(bundlePath, ocispec.Root.Path) @@ -962,7 +961,6 @@ func ContainerConfig(ocispec specs.Spec, bundlePath, cid, console string, detach User: strconv.FormatUint(uint64(ocispec.Process.User.UID), 10), PrimaryGroup: strconv.FormatUint(uint64(ocispec.Process.User.GID), 10), Interactive: ocispec.Process.Terminal, - Console: console, Detach: detach, NoNewPrivileges: ocispec.Process.NoNewPrivileges, } diff --git a/src/runtime/pkg/oci/utils_test.go b/src/runtime/pkg/oci/utils_test.go index 2ddd42d111..6fe136b5d4 100644 --- a/src/runtime/pkg/oci/utils_test.go +++ b/src/runtime/pkg/oci/utils_test.go @@ -38,7 +38,6 @@ const ( var ( tempRoot = "" tempBundlePath = "" - consolePath = "" ) func createConfig(fileName string, fileData string) (string, error) { @@ -72,7 +71,6 @@ func TestMinimalSandboxConfig(t *testing.T) { runtimeConfig := RuntimeConfig{ HypervisorType: vc.QemuHypervisor, - Console: consolePath, } capList := []string{"CAP_AUDIT_WRITE", "CAP_KILL", "CAP_NET_BIND_SERVICE"} @@ -94,7 +92,6 @@ func TestMinimalSandboxConfig(t *testing.T) { PrimaryGroup: "0", SupplementaryGroups: []string{"10", "29"}, Interactive: true, - Console: consolePath, NoNewPrivileges: true, Capabilities: &specs.LinuxCapabilities{ Bounding: capList, @@ -181,7 +178,7 @@ func TestMinimalSandboxConfig(t *testing.T) { SystemdCgroup: true, } - sandboxConfig, err := SandboxConfig(spec, runtimeConfig, tempBundlePath, containerID, consolePath, false, true) + sandboxConfig, err := SandboxConfig(spec, runtimeConfig, tempBundlePath, containerID, false, true) assert.NoError(err) assert.Exactly(sandboxConfig, expectedSandboxConfig) @@ -452,7 +449,6 @@ func TestMain(m *testing.M) { } tempBundlePath = filepath.Join(tempRoot, "ocibundle") - consolePath = filepath.Join(tempRoot, "console") /* Create temp bundle directory if necessary */ err = os.MkdirAll(tempBundlePath, dirMode) @@ -513,7 +509,6 @@ func TestAddAssetAnnotations(t *testing.T) { runtimeConfig := RuntimeConfig{ HypervisorType: vc.QemuHypervisor, - Console: consolePath, } // Try annotations without enabling them first @@ -567,7 +562,6 @@ func TestAddAgentAnnotations(t *testing.T) { runtimeConfig := RuntimeConfig{ HypervisorType: vc.QemuHypervisor, - Console: consolePath, } ocispec.Annotations[vcAnnotations.KernelModules] = strings.Join(expectedAgentConfig.KernelModules, KernelModulesSeparator) @@ -594,7 +588,6 @@ func TestContainerPipeSizeAnnotation(t *testing.T) { runtimeConfig := RuntimeConfig{ HypervisorType: vc.QemuHypervisor, - Console: consolePath, } ocispec.Annotations[vcAnnotations.AgentContainerPipeSize] = "foo" @@ -629,7 +622,6 @@ func TestAddHypervisorAnnotations(t *testing.T) { runtimeConfig := RuntimeConfig{ HypervisorType: vc.QemuHypervisor, - Console: consolePath, } runtimeConfig.HypervisorConfig.EnableAnnotations = []string{".*"} runtimeConfig.HypervisorConfig.FileBackedMemRootList = []string{"/dev/shm*"} @@ -743,7 +735,6 @@ func TestAddProtectedHypervisorAnnotations(t *testing.T) { runtimeConfig := RuntimeConfig{ HypervisorType: vc.QemuHypervisor, - Console: consolePath, } ocispec.Annotations[vcAnnotations.KernelParams] = "vsyscall=emulate iommu=on" err := addAnnotations(ocispec, &config, runtimeConfig) @@ -809,7 +800,6 @@ func TestAddRuntimeAnnotations(t *testing.T) { runtimeConfig := RuntimeConfig{ HypervisorType: vc.QemuHypervisor, - Console: consolePath, } ocispec.Annotations[vcAnnotations.DisableGuestSeccomp] = "true" diff --git a/src/runtime/virtcontainers/types/sandbox.go b/src/runtime/virtcontainers/types/sandbox.go index f4fc3e503f..5149b04232 100644 --- a/src/runtime/virtcontainers/types/sandbox.go +++ b/src/runtime/virtcontainers/types/sandbox.go @@ -314,7 +314,6 @@ type Cmd struct { User string PrimaryGroup string WorkDir string - Console string Args []string Envs []EnvVar From 433816cca2cd72c9ecbb458688c238d8cfe6149e Mon Sep 17 00:00:00 2001 From: Derek Lee Date: Wed, 29 Jun 2022 16:20:19 -0700 Subject: [PATCH 19/36] ci/cd: update check-commit-message Recently added check-commit-message to the tests repository. Minor changes were also made to action. For consistency's sake, copied changes over to here as well. tests - https://github.com/kata-containers/tests/pull/4878 Minor Changes: 1. Body length check is now 75 and consistent with guidelines 2. Lines without spaces are not counted in body length check Fixes #4559 Signed-off-by: Derek Lee --- .github/workflows/commit-message-check.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/commit-message-check.yaml b/.github/workflows/commit-message-check.yaml index 2062e51b9c..fec12ac1dd 100644 --- a/.github/workflows/commit-message-check.yaml +++ b/.github/workflows/commit-message-check.yaml @@ -63,7 +63,8 @@ jobs: # the entire commit message. # # - Body lines *can* be longer than the maximum if they start - # with a non-alphabetic character. + # with a non-alphabetic character or if there is no whitespace in + # the line. # # This allows stack traces, log files snippets, emails, long URLs, # etc to be specified. Some of these naturally "work" as they start @@ -74,7 +75,7 @@ jobs: # # - A SoB comment can be any length (as it is unreasonable to penalise # people with long names/email addresses :) - pattern: '^.+(\n([a-zA-Z].{0,149}|[^a-zA-Z\n].*|Signed-off-by:.*|))+$' + pattern: '^.+(\n([a-zA-Z].{0,72}|[^a-zA-Z\n].*|[^\s\n]*|Signed-off-by:.*|))+$' error: 'Body line too long (max 72)' post_error: ${{ env.error_msg }} From 2a4fbd6d8c8abe8fa0b315dd86115850d1b1c774 Mon Sep 17 00:00:00 2001 From: quanweiZhou Date: Fri, 17 Jun 2022 16:51:10 +0800 Subject: [PATCH 20/36] agent: enhance get handled signal For runC, send the signal to the init process directly. For kata, we try to send `SIGKILL` instead of `SIGTERM` when the process has not installed the handler for `SIGTERM`. The `is_signal_handled` function determine which signal the container process has been handled. But currently `is_signal_handled` is only catching (SigCgt). While the container process is ignoring (SigIgn) or blocking (SigBlk) also should not be converted from the `SIGTERM` to `SIGKILL`. For example, when using terminationGracePeriodSeconds the k8s will send SIGTERM first and then send `SIGKILL`, in this case, the container ignores the `SIGTERM`, so we should send the `SIGTERM` not the `SIGKILL` to the container. Fixes: #4478 Signed-off-by: quanweiZhou --- src/agent/src/rpc.rs | 50 ++++++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/src/agent/src/rpc.rs b/src/agent/src/rpc.rs index bcf2096d2b..10f1a0021b 100644 --- a/src/agent/src/rpc.rs +++ b/src/agent/src/rpc.rs @@ -1793,34 +1793,25 @@ fn is_signal_handled(proc_status_file: &str, signum: u32) -> bool { let sig_mask: u64 = 1 << shift_count; let reader = BufReader::new(file); - // Read the file line by line using the lines() iterator from std::io::BufRead. - for (_index, line) in reader.lines().enumerate() { - let line = match line { - Ok(l) => l, - Err(_) => { - warn!(sl!(), "failed to read file {}", proc_status_file); - return false; - } - }; - if line.starts_with("SigCgt:") { + // read lines start with SigBlk/SigIgn/SigCgt and check any match the signal mask + reader + .lines() + .flatten() + .filter(|line| { + line.starts_with("SigBlk:") + || line.starts_with("SigIgn:") + || line.starts_with("SigCgt:") + }) + .any(|line| { let mask_vec: Vec<&str> = line.split(':').collect(); - if mask_vec.len() != 2 { - warn!(sl!(), "parse the SigCgt field failed"); - return false; - } - let sig_cgt_str = mask_vec[1].trim(); - let sig_cgt_mask = match u64::from_str_radix(sig_cgt_str, 16) { - Ok(h) => h, - Err(_) => { - warn!(sl!(), "failed to parse the str {} to hex", sig_cgt_str); - return false; + if mask_vec.len() == 2 { + let sig_str = mask_vec[1].trim(); + if let Ok(sig) = u64::from_str_radix(sig_str, 16) { + return sig & sig_mask == sig_mask; } - }; - - return (sig_cgt_mask & sig_mask) == sig_mask; - } - } - false + } + false + }) } fn do_mem_hotplug_by_probe(addrs: &[u64]) -> Result<()> { @@ -2642,7 +2633,12 @@ OtherField:other TestData { status_file_data: Some("SigBlk:0000000000000001"), signum: 1, - result: false, + result: true, + }, + TestData { + status_file_data: Some("SigIgn:0000000000000001"), + signum: 1, + result: true, }, TestData { status_file_data: None, From 48ccd4233932652afa11ad4e6b127153326ac3de Mon Sep 17 00:00:00 2001 From: Manabu Sugimoto Date: Thu, 30 Jun 2022 13:53:13 +0900 Subject: [PATCH 21/36] ci: Set safe.directory against tests repository Set `safe.directory` against `kata-containers/tests` repository before checkout because the user in the docker container is root, but the `tests` repository on the host machine is usually owned by the normal user. This works when we already have the `tests` repository which is not owned by root on the host machine and try to create a rootfs using Docker (`USE_DOCKER=true`). Fixes: #4561 Signed-off-by: Manabu Sugimoto --- ci/lib.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ci/lib.sh b/ci/lib.sh index f7f1eeeb75..3cb2c158f6 100644 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -18,6 +18,13 @@ clone_tests_repo() { if [ -d "$tests_repo_dir" ]; then [ -n "${CI:-}" ] && return + # git config --global --add safe.directory will always append + # the target to .gitconfig without checking the existence of + # the target, so it's better to check it before adding the target repo. + local sd="$(git config --global --get safe.directory ${tests_repo_dir} || true)" + if [ -z "${sd}" ]; then + git config --global --add safe.directory ${tests_repo_dir} + fi pushd "${tests_repo_dir}" git checkout "${branch}" git pull From 4e48509ed90fb62aeb1d9d133416eef813257e9b Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Thu, 30 Jun 2022 20:52:44 -0700 Subject: [PATCH 22/36] build: Set safe.directory for runtime repo While doing a docker build for shim-v2, we see this: ``` fatal: unsafe repository ('/home/${user}/go/src/github.com/kata-containers/kata-containers' is owned by someone else) To add an exception for this directory, call: git config --global --add safe.directory /home/${user}/go/src/github.com/kata-containers/kata-containers ``` This is because the docker container build is run as root while the runtime repo is checked out as normal user. Unlike this error causing the rootfs build to error out, the error here does not really cause `make shim-v2-tarball` to fail. However its good to get rid of this error message showing during the make process. Fixes: #4572 Signed-off-by: Archana Shinde --- tools/packaging/static-build/shim-v2/build.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/packaging/static-build/shim-v2/build.sh b/tools/packaging/static-build/shim-v2/build.sh index 8cc34dba04..5b073ff07c 100755 --- a/tools/packaging/static-build/shim-v2/build.sh +++ b/tools/packaging/static-build/shim-v2/build.sh @@ -29,12 +29,12 @@ fi sudo docker run --rm -i -v "${repo_root_dir}:${repo_root_dir}" \ -w "${repo_root_dir}/src/runtime" \ "${container_image}" \ - bash -c "make PREFIX=${PREFIX} QEMUCMD=qemu-system-${arch}" + bash -c "git config --global --add safe.directory ${repo_root_dir} && make PREFIX=${PREFIX} QEMUCMD=qemu-system-${arch}" sudo docker run --rm -i -v "${repo_root_dir}:${repo_root_dir}" \ -w "${repo_root_dir}/src/runtime" \ "${container_image}" \ - bash -c "make PREFIX="${PREFIX}" DESTDIR="${DESTDIR}" install" + bash -c "git config --global --add safe.directory ${repo_root_dir} && make PREFIX="${PREFIX}" DESTDIR="${DESTDIR}" install" sudo sed -i -e '/^initrd =/d' "${DESTDIR}/${PREFIX}/share/defaults/kata-containers/configuration-qemu.toml" sudo sed -i -e '/^initrd =/d' "${DESTDIR}/${PREFIX}/share/defaults/kata-containers/configuration-fc.toml" From 1f363a386c6d97a60c61dae88a992b9eb57858bb Mon Sep 17 00:00:00 2001 From: liubin Date: Wed, 29 Jun 2022 16:43:16 +0800 Subject: [PATCH 23/36] runtime: overwrite mount type to bind for bind mounts Some clients like nerdctl may pass mount type of none for volumes/bind mounts, this will lead to container start fails. Referring to runc, it overwrites the mount type to bind and ignores the input value. Fixes: #4548 Signed-off-by: liubin --- src/runtime/pkg/oci/utils.go | 20 +++++--- src/runtime/pkg/oci/utils_test.go | 76 +++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 6 deletions(-) diff --git a/src/runtime/pkg/oci/utils.go b/src/runtime/pkg/oci/utils.go index 71423cf0cc..53503b3222 100644 --- a/src/runtime/pkg/oci/utils.go +++ b/src/runtime/pkg/oci/utils.go @@ -187,16 +187,27 @@ func cmdEnvs(spec specs.Spec, envs []types.EnvVar) []types.EnvVar { func newMount(m specs.Mount) vc.Mount { readonly := false + bind := false for _, flag := range m.Options { - if flag == "ro" { + switch flag { + case "rbind", "bind": + bind = true + case "ro": readonly = true - break } } + + // normal bind mounts, set type to bind. + // https://github.com/opencontainers/runc/blob/v1.1.3/libcontainer/specconv/spec_linux.go#L512-L520 + mountType := m.Type + if mountType != vc.KataEphemeralDevType && mountType != vc.KataLocalDevType && bind { + mountType = "bind" + } + return vc.Mount{ Source: m.Source, Destination: m.Destination, - Type: m.Type, + Type: mountType, Options: m.Options, ReadOnly: readonly, } @@ -913,9 +924,6 @@ func SandboxConfig(ocispec specs.Spec, runtime RuntimeConfig, bundlePath, cid, c DisableGuestSeccomp: runtime.DisableGuestSeccomp, - // Q: Is this really necessary? @weizhang555 - // Spec: &ocispec, - Experimental: runtime.Experimental, } diff --git a/src/runtime/pkg/oci/utils_test.go b/src/runtime/pkg/oci/utils_test.go index 2ddd42d111..7caa51aa28 100644 --- a/src/runtime/pkg/oci/utils_test.go +++ b/src/runtime/pkg/oci/utils_test.go @@ -1210,3 +1210,79 @@ func TestCalculateSandboxSizing(t *testing.T) { assert.Equal(tt.expectedMem, mem, "unexpected memory") } } + +func TestNewMount(t *testing.T) { + assert := assert.New(t) + + testCases := []struct { + out vc.Mount + in specs.Mount + }{ + { + in: specs.Mount{ + Source: "proc", + Destination: "/proc", + Type: "proc", + Options: nil, + }, + out: vc.Mount{ + Source: "proc", + Destination: "/proc", + Type: "proc", + Options: nil, + }, + }, + { + in: specs.Mount{ + Source: "proc", + Destination: "/proc", + Type: "proc", + Options: []string{"ro"}, + }, + out: vc.Mount{ + Source: "proc", + Destination: "/proc", + Type: "proc", + Options: []string{"ro"}, + ReadOnly: true, + }, + }, + { + in: specs.Mount{ + Source: "/abc", + Destination: "/def", + Type: "none", + Options: []string{"bind"}, + }, + out: vc.Mount{ + Source: "/abc", + Destination: "/def", + Type: "bind", + Options: []string{"bind"}, + }, + }, { + in: specs.Mount{ + Source: "/abc", + Destination: "/def", + Type: "none", + Options: []string{"rbind"}, + }, + out: vc.Mount{ + Source: "/abc", + Destination: "/def", + Type: "bind", + Options: []string{"rbind"}, + }, + }, + } + + for _, tt := range testCases { + actualMount := newMount(tt.in) + + assert.Equal(tt.out.Source, actualMount.Source, "unexpected mount source") + assert.Equal(tt.out.Destination, actualMount.Destination, "unexpected mount destination") + assert.Equal(tt.out.Type, actualMount.Type, "unexpected mount type") + assert.Equal(tt.out.Options, actualMount.Options, "unexpected mount options") + assert.Equal(tt.out.ReadOnly, actualMount.ReadOnly, "unexpected mount ReadOnly") + } +} From acd3302bef19ee26c1c4dbe2715fc632f30c9f6f Mon Sep 17 00:00:00 2001 From: Manabu Sugimoto Date: Fri, 1 Jul 2022 09:53:20 +0900 Subject: [PATCH 24/36] agent: Run OCI poststart hooks after a container is launched Run the OCI `poststart` hooks must be called after the user-specified process is executed but before the `start` operation returns in accordance with OCI runtime spec. Fixes: #4575 Signed-off-by: Manabu Sugimoto --- src/agent/rustjail/src/container.rs | 52 +++++++++++++++-------------- src/agent/src/rpc.rs | 2 +- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/agent/rustjail/src/container.rs b/src/agent/rustjail/src/container.rs index 5f1aa56066..ace9891d28 100644 --- a/src/agent/rustjail/src/container.rs +++ b/src/agent/rustjail/src/container.rs @@ -222,7 +222,7 @@ pub trait BaseContainer { async fn start(&mut self, p: Process) -> Result<()>; async fn run(&mut self, p: Process) -> Result<()>; async fn destroy(&mut self) -> Result<()>; - fn exec(&mut self) -> Result<()>; + async fn exec(&mut self) -> Result<()>; } // LinuxContainer protected by Mutex @@ -634,11 +634,6 @@ fn do_init_child(cwfd: RawFd) -> Result<()> { capabilities::drop_privileges(cfd_log, c)?; } - if init { - // notify parent to run poststart hooks - write_sync(cwfd, SYNC_SUCCESS, "")?; - } - let args = oci_process.args.to_vec(); let env = oci_process.env.to_vec(); @@ -1054,7 +1049,7 @@ impl BaseContainer for LinuxContainer { self.start(p).await?; if init { - self.exec()?; + self.exec().await?; self.status.transition(ContainerState::Running); } @@ -1102,7 +1097,7 @@ impl BaseContainer for LinuxContainer { Ok(()) } - fn exec(&mut self) -> Result<()> { + async fn exec(&mut self) -> Result<()> { let fifo = format!("{}/{}", &self.root, EXEC_FIFO_FILENAME); let fd = fcntl::open(fifo.as_str(), OFlag::O_WRONLY, Mode::from_bits_truncate(0))?; let data: &[u8] = &[0]; @@ -1114,6 +1109,26 @@ impl BaseContainer for LinuxContainer { .as_secs(); self.status.transition(ContainerState::Running); + + let spec = self + .config + .spec + .as_ref() + .ok_or_else(|| anyhow!("OCI spec was not found"))?; + let st = self.oci_state()?; + + // run poststart hook + if spec.hooks.is_some() { + info!(self.logger, "poststart hook"); + let hooks = spec + .hooks + .as_ref() + .ok_or_else(|| anyhow!("OCI hooks were not found"))?; + for h in hooks.poststart.iter() { + execute_hook(&self.logger, h, &st).await?; + } + } + unistd::close(fd)?; Ok(()) @@ -1335,20 +1350,6 @@ async fn join_namespaces( // notify child run prestart hooks completed info!(logger, "notify child run prestart hook completed!"); write_async(pipe_w, SYNC_SUCCESS, "").await?; - - info!(logger, "notify child parent ready to run poststart hook!"); - // wait to run poststart hook - read_async(pipe_r).await?; - info!(logger, "get ready to run poststart hook!"); - - // run poststart hook - if spec.hooks.is_some() { - info!(logger, "poststart hook"); - let hooks = spec.hooks.as_ref().unwrap(); - for h in hooks.poststart.iter() { - execute_hook(&logger, h, st).await?; - } - } } info!(logger, "wait for child process ready to run exec"); @@ -2090,9 +2091,10 @@ mod tests { assert!(ret.is_ok(), "Expecting Ok, Got {:?}", ret); } - #[test] - fn test_linuxcontainer_exec() { - let ret = new_linux_container_and_then(|mut c: LinuxContainer| c.exec()); + #[tokio::test] + async fn test_linuxcontainer_exec() { + let (c, _dir) = new_linux_container(); + let ret = c.unwrap().exec().await; assert!(ret.is_err(), "Expecting Err, Got {:?}", ret); } diff --git a/src/agent/src/rpc.rs b/src/agent/src/rpc.rs index bcf2096d2b..ba9981069f 100644 --- a/src/agent/src/rpc.rs +++ b/src/agent/src/rpc.rs @@ -269,7 +269,7 @@ impl AgentService { .get_container(&cid) .ok_or_else(|| anyhow!("Invalid container id"))?; - ctr.exec()?; + ctr.exec().await?; if sid == cid { return Ok(()); From fbb2e9bce9746ee0c28a5ccdb14eebd20285cbec Mon Sep 17 00:00:00 2001 From: Manabu Sugimoto Date: Mon, 4 Jul 2022 13:14:06 +0900 Subject: [PATCH 25/36] agent: Replace some libc functions with nix ones Replace `libc::setgroups()`, `libc::fchown()`, and `libc::sethostname()` functions with nix crate ones for safety and maintainability. Fixes: #4579 Signed-off-by: Manabu Sugimoto --- src/agent/rustjail/src/container.rs | 33 ++++++++++++----------------- src/agent/src/main.rs | 17 +-------------- 2 files changed, 15 insertions(+), 35 deletions(-) diff --git a/src/agent/rustjail/src/container.rs b/src/agent/rustjail/src/container.rs index 5f1aa56066..584207cf86 100644 --- a/src/agent/rustjail/src/container.rs +++ b/src/agent/rustjail/src/container.rs @@ -587,14 +587,20 @@ fn do_init_child(cwfd: RawFd) -> Result<()> { // only change stdio devices owner when user // isn't root. - if guser.uid != 0 { - set_stdio_permissions(guser.uid)?; + if !uid.is_root() { + set_stdio_permissions(uid)?; } setid(uid, gid)?; if !guser.additional_gids.is_empty() { - setgroups(guser.additional_gids.as_slice()).map_err(|e| { + let gids: Vec = guser + .additional_gids + .iter() + .map(|gid| Gid::from_raw(*gid)) + .collect(); + + unistd::setgroups(&gids).map_err(|e| { let _ = write_sync( cwfd, SYNC_FAILED, @@ -730,7 +736,7 @@ fn do_init_child(cwfd: RawFd) -> Result<()> { // within the container to the specified user. // The ownership needs to match because it is created outside of // the container and needs to be localized. -fn set_stdio_permissions(uid: libc::uid_t) -> Result<()> { +fn set_stdio_permissions(uid: Uid) -> Result<()> { let meta = fs::metadata("/dev/null")?; let fds = [ std::io::stdin().as_raw_fd(), @@ -745,19 +751,13 @@ fn set_stdio_permissions(uid: libc::uid_t) -> Result<()> { continue; } - // According to the POSIX specification, -1 is used to indicate that owner and group - // are not to be changed. Since uid_t and gid_t are unsigned types, we have to wrap - // around to get -1. - let gid = 0u32.wrapping_sub(1); - // We only change the uid owner (as it is possible for the mount to // prefer a different gid, and there's no reason for us to change it). // The reason why we don't just leave the default uid=X mount setup is // that users expect to be able to actually use their console. Without // this code, you couldn't effectively run as a non-root user inside a // container and also have a console set up. - let res = unsafe { libc::fchown(*fd, uid, gid) }; - Errno::result(res).map_err(|e| anyhow!(e).context("set stdio permissions failed"))?; + unistd::fchown(*fd, Some(uid), None).with_context(|| "set stdio permissions failed")?; } Ok(()) @@ -1477,12 +1477,6 @@ impl LinuxContainer { } } -fn setgroups(grps: &[libc::gid_t]) -> Result<()> { - let ret = unsafe { libc::setgroups(grps.len(), grps.as_ptr() as *const libc::gid_t) }; - Errno::result(ret).map(drop)?; - Ok(()) -} - use std::fs::OpenOptions; use std::io::Write; @@ -1652,6 +1646,7 @@ mod tests { use super::*; use crate::process::Process; use crate::skip_if_not_root; + use nix::unistd::Uid; use std::fs; use std::os::unix::fs::MetadataExt; use std::os::unix::io::AsRawFd; @@ -1797,7 +1792,7 @@ mod tests { let old_uid = meta.uid(); let uid = 1000; - set_stdio_permissions(uid).unwrap(); + set_stdio_permissions(Uid::from_raw(uid)).unwrap(); let meta = fs::metadata("/dev/stdin").unwrap(); assert_eq!(meta.uid(), uid); @@ -1809,7 +1804,7 @@ mod tests { assert_eq!(meta.uid(), uid); // restore the uid - set_stdio_permissions(old_uid).unwrap(); + set_stdio_permissions(Uid::from_raw(old_uid)).unwrap(); } #[test] diff --git a/src/agent/src/main.rs b/src/agent/src/main.rs index 4386dde5cf..6a023e093e 100644 --- a/src/agent/src/main.rs +++ b/src/agent/src/main.rs @@ -27,7 +27,6 @@ use nix::unistd::{self, dup, Pid}; use std::env; use std::ffi::OsStr; use std::fs::{self, File}; -use std::os::unix::ffi::OsStrExt; use std::os::unix::fs as unixfs; use std::os::unix::io::AsRawFd; use std::path::Path; @@ -382,27 +381,13 @@ fn init_agent_as_init(logger: &Logger, unified_cgroup_hierarchy: bool) -> Result let contents_array: Vec<&str> = contents.split(' ').collect(); let hostname = contents_array[0].trim(); - if sethostname(OsStr::new(hostname)).is_err() { + if unistd::sethostname(OsStr::new(hostname)).is_err() { warn!(logger, "failed to set hostname"); } Ok(()) } -#[instrument] -fn sethostname(hostname: &OsStr) -> Result<()> { - let size = hostname.len() as usize; - - let result = - unsafe { libc::sethostname(hostname.as_bytes().as_ptr() as *const libc::c_char, size) }; - - if result != 0 { - Err(anyhow!("failed to set hostname")) - } else { - Ok(()) - } -} - // The Rust standard library had suppressed the default SIGPIPE behavior, // see https://github.com/rust-lang/rust/pull/13158. // Since the parent's signal handler would be inherited by it's child process, From 0ddb34a38d16900137de2633732882073904012d Mon Sep 17 00:00:00 2001 From: "haining.cao" Date: Fri, 1 Jul 2022 18:04:36 +0800 Subject: [PATCH 26/36] oci: fix serde skip serializing condition There is an extra space on the serde serialization condition. Fixes: #4578 Signed-off-by: haining.cao --- src/libs/oci/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/oci/src/lib.rs b/src/libs/oci/src/lib.rs index f47f2df4be..3998b166c1 100644 --- a/src/libs/oci/src/lib.rs +++ b/src/libs/oci/src/lib.rs @@ -43,7 +43,7 @@ pub struct Spec { pub process: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub root: Option, - #[serde(default, skip_serializing_if = "String:: is_empty")] + #[serde(default, skip_serializing_if = "String::is_empty")] pub hostname: String, #[serde(default, skip_serializing_if = "Vec::is_empty")] pub mounts: Vec, From f4eea832a1973a4ebe42c32bbbad54628f6b3435 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Tue, 5 Jul 2022 22:23:05 +0200 Subject: [PATCH 27/36] release: Adapt kata-deploy for 2.5.0-rc0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit kata-deploy files must be adapted to a new release. The cases where it happens are when the release goes from -> to: * main -> stable: * kata-deploy-stable / kata-cleanup-stable: are removed * stable -> stable: * kata-deploy / kata-cleanup: bump the release to the new one. There are no changes when doing an alpha release, as the files on the "main" branch always point to the "latest" and "stable" tags. Signed-off-by: Fabiano Fidêncio --- .../base/kata-cleanup-stable.yaml | 46 ------------- .../kata-deploy/base/kata-deploy-stable.yaml | 69 ------------------- 2 files changed, 115 deletions(-) delete mode 100644 tools/packaging/kata-deploy/kata-cleanup/base/kata-cleanup-stable.yaml delete mode 100644 tools/packaging/kata-deploy/kata-deploy/base/kata-deploy-stable.yaml diff --git a/tools/packaging/kata-deploy/kata-cleanup/base/kata-cleanup-stable.yaml b/tools/packaging/kata-deploy/kata-cleanup/base/kata-cleanup-stable.yaml deleted file mode 100644 index f1d9d0a2f9..0000000000 --- a/tools/packaging/kata-deploy/kata-cleanup/base/kata-cleanup-stable.yaml +++ /dev/null @@ -1,46 +0,0 @@ ---- -apiVersion: apps/v1 -kind: DaemonSet -metadata: - name: kubelet-kata-cleanup - namespace: kube-system -spec: - selector: - matchLabels: - name: kubelet-kata-cleanup - template: - metadata: - labels: - name: kubelet-kata-cleanup - spec: - serviceAccountName: kata-label-node - nodeSelector: - katacontainers.io/kata-runtime: cleanup - containers: - - name: kube-kata-cleanup - image: quay.io/kata-containers/kata-deploy:stable - imagePullPolicy: Always - command: [ "bash", "-c", "/opt/kata-artifacts/scripts/kata-deploy.sh reset" ] - env: - - name: NODE_NAME - valueFrom: - fieldRef: - fieldPath: spec.nodeName - securityContext: - privileged: false - volumeMounts: - - name: dbus - mountPath: /var/run/dbus - - name: systemd - mountPath: /run/systemd - volumes: - - name: dbus - hostPath: - path: /var/run/dbus - - name: systemd - hostPath: - path: /run/systemd - updateStrategy: - rollingUpdate: - maxUnavailable: 1 - type: RollingUpdate diff --git a/tools/packaging/kata-deploy/kata-deploy/base/kata-deploy-stable.yaml b/tools/packaging/kata-deploy/kata-deploy/base/kata-deploy-stable.yaml deleted file mode 100644 index 346e4c0ee2..0000000000 --- a/tools/packaging/kata-deploy/kata-deploy/base/kata-deploy-stable.yaml +++ /dev/null @@ -1,69 +0,0 @@ ---- -apiVersion: apps/v1 -kind: DaemonSet -metadata: - name: kata-deploy - namespace: kube-system -spec: - selector: - matchLabels: - name: kata-deploy - template: - metadata: - labels: - name: kata-deploy - spec: - serviceAccountName: kata-label-node - containers: - - name: kube-kata - image: quay.io/kata-containers/kata-deploy:stable - imagePullPolicy: Always - lifecycle: - preStop: - exec: - command: ["bash", "-c", "/opt/kata-artifacts/scripts/kata-deploy.sh cleanup"] - command: [ "bash", "-c", "/opt/kata-artifacts/scripts/kata-deploy.sh install" ] - env: - - name: NODE_NAME - valueFrom: - fieldRef: - fieldPath: spec.nodeName - securityContext: - privileged: false - volumeMounts: - - name: crio-conf - mountPath: /etc/crio/ - - name: containerd-conf - mountPath: /etc/containerd/ - - name: kata-artifacts - mountPath: /opt/kata/ - - name: dbus - mountPath: /var/run/dbus - - name: systemd - mountPath: /run/systemd - - name: local-bin - mountPath: /usr/local/bin/ - volumes: - - name: crio-conf - hostPath: - path: /etc/crio/ - - name: containerd-conf - hostPath: - path: /etc/containerd/ - - name: kata-artifacts - hostPath: - path: /opt/kata/ - type: DirectoryOrCreate - - name: dbus - hostPath: - path: /var/run/dbus - - name: systemd - hostPath: - path: /run/systemd - - name: local-bin - hostPath: - path: /usr/local/bin/ - updateStrategy: - rollingUpdate: - maxUnavailable: 1 - type: RollingUpdate From 2d29791c198503ae55a6ae36e4b71ff0e64d5075 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Tue, 5 Jul 2022 22:23:05 +0200 Subject: [PATCH 28/36] release: Kata Containers 2.5.0-rc0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Drop in cfg files support - agent: enhance get handled signal - oci: fix serde skip serializing condition - agent: Run OCI poststart hooks after a container is launched - agent: Replace some libc functions with nix ones - runtime: overwrite mount type to bind for bind mounts - build: Set safe.directory for runtime repo - ci/cd: update check-commit-message - Set safe.directory against tests repository - runtime: delete Console from Cmd type - Add `default_maxmemory` config option - shim: set a non-zero return code if the wait process call failed. - Refactor how hypervisor config validation is handled - packaging: Remove unused kata docker configure script - kata-with-k8s: Add cgroupDriver for containerd - shim: support shim v2 logging plugin - device package cleanup/refactor - versions: Update kernel to latest LTS version 5.15.48 - agent: Allow BUILD_TYPE=debug - Fix clippy warnings and update agent's vendored code - block: Leverage multiqueue for virtio-block - kernel: Add CONFIG_EFI=y as part of the TDX fragments - runtime: Add heuristic to get the right value(s) for mem-reserve - runtime: enable sandbox feature on qemu - snap: fix snap build on ppc64le - packaging: Remove unused publish kata image script - rootfs: Fix chronyd.service failing on boot - tracing: Remove whitespace from root span - workflow: Removing man-db, workflow kept failing - docs: Update outdated URLs and keep them available - runtime: fix error when trying to parse sandbox sizing annotations - snap: Fix debug cli option - deps: Resolve dependabot bumps of containerd, crossbeam-utils, regex - Allow Cloud Hypervisor to run under the `container_kvm_t` - docs: Update containerd url link - agent: refactor reading file timing for debugging - safe-path: fix clippy warning - kernel building: efi_secret module - runtime: Switch to using the rust version of virtiofsd (all arches but powerpc) - shim: change the log level for GetOOMEvent call failures - docs: Add more kata monitor details - Allow io.katacontainers.config.hypervisor.enable_iommu annotation by … - versions: Bump virtiofsd to v1.3.0 - docs: Add storage limits to arch doc - docs: Update source for cri-tools - tools: Enable extra detail on error - docs: Add agent-ctl examples section f4eea832a release: Adapt kata-deploy for 2.5.0-rc0 0ddb34a38 oci: fix serde skip serializing condition fbb2e9bce agent: Replace some libc functions with nix ones acd3302be agent: Run OCI poststart hooks after a container is launched 1f363a386 runtime: overwrite mount type to bind for bind mounts 4e48509ed build: Set safe.directory for runtime repo 48ccd4233 ci: Set safe.directory against tests repository 2a4fbd6d8 agent: enhance get handled signal 433816cca ci/cd: update check-commit-message a5a25ed13 runtime: delete Console from Cmd type 96553e8bd runtime: Add documentation of drop-in config file fragments c656457e9 runtime: Add tests of drop-in config file decoding 99f5ca80f runtime: Plug drop-in decoding into decodeConfig() 0f9856c46 runtime: Scan drop-in directory, read files and decode them 2c1efcc69 runtime: Add helpers to copy fields between tomlConfig instances 20f11877b runtime: Add framework to manipulate config structs via reflection ab5f1c956 shim: set a non-zero return code if the wait process call failed. e5be5cb08 runtime: device: cleanup outdated comments 5f936f268 virtcontainers: config validation is host specific 323271403 virtcontainers: Remove unused function 0939f5181 config: Expose default_maxmemory 58ff2bd5c clh,qemu: Adapt to using default_maxmemory 1a78c3df2 packaging: Remove unused kata docker configure script afdc96042 hypervisor: Add default_maxmemory configuration 4e30e11b3 shim: support shim v2 logging plugin bdf5e5229 virtcontainers: validate hypervisor config outside of hypervisor itself 469e09854 katautils: don't do validation when loading hypervisor config e32bf5331 device: deduplicate state structures f97d9b45c runtime: device/persist: drop persist dependency from device pkgs f9e96c650 runtime: device: move to top level package 3880e0c07 agent: refactor reading file timing for debugging c70d3a2c3 agent: Update the dependencies 612fd79ba random: Fix "nonminimal-bool" clippy warning d4417f210 netlink: Fix "or-fun-call" clippy warnings 93874cb3b packaging: Restrict kernel patches applied to top-level dir 07b1367c2 versions: Update kernel to latest LTS version 5.15.48 1b7d36fdb agent: Allow BUILD_TYPE=debug 9ff10c083 kernel: Add CONFIG_EFI=y as part of the TDX fragments e227b4c40 block: Leverage multiqueue for virtio-block e7e7dc9df runtime: Add heuristic to get the right value(s) for mem-reserve c7dd10e5e packaging: Remove unused publish kata image script 0bbbe7068 snap: fix snap build on ppc64le ef925d40c runtime: enable sandbox feature on qemu 28995301b tracing: Remove whitespace from root span 9941588c0 workflow: Removing man-db, workflow kept failing 90a7763ac snap: Fix debug cli option a305bafee docs: Update outdated URLs and keep them available bee770343 docs: Update containerd url link ac5dbd859 clh: Improve logging related to the net dev addition 0b75522e1 network: Set queues to 1 to ensure we get the network fds 93b61e0f0 network: Add FFI_NO_PI to the netlink flags bf3ddc125 clh: Pass the tuntap fds down to Cloud Hypervisor 55ed32e92 clh: Take care of the VmAdNetdPut request ourselves 01fe09a4e clh: Hotplug the network devices 2e0753833 clh: Expose VmAddNetPut 1ef0b7ded runtime: Switch to using the rust version of virtiofsd (all but power) bb26bd73b safe-path: fix clippy warning 1a5ba31cb agent: refactor reading file timing for debugging 721ca72a6 runtime: fix error when trying to parse sandbox sizing annotations 9773838c0 virtiofsd: export env vars needed for building it b0e090f40 versions: Bump virtiofsd to v1.3.0 db5048d52 kernel: build efi_secret module for SEV 1b845978f docs: Add storage limits to arch doc 412441308 docs: Add more kata monitor details eff4e1017 shim: change the log level for GetOOMEvent call failures 5d7fb7b7b build(deps): bump github.com/containerd/containerd in /src/runtime d0ca2fcbb build(deps): bump crossbeam-utils in /src/tools/trace-forwarder a60dcff4d build(deps): bump regex from 1.5.4 to 1.5.6 in /src/tools/agent-ctl dbf50672e build(deps): bump crossbeam-utils in /src/tools/agent-ctl 8e2847bd5 build(deps): bump crossbeam-utils from 0.8.6 to 0.8.8 in /src/libs e9ada165f build(deps): bump regex from 1.5.4 to 1.5.5 in /src/agent adad9cef1 build(deps): bump crossbeam-utils from 0.8.5 to 0.8.8 in /src/agent 34bcef884 docs: Add agent-ctl examples section 815157bf0 docs: Remove erroneous whitespace f5099620f tools: Enable extra detail on error 8f10e13e0 config: Allow enable_iommu pod annotation by default 7ae11cad6 docs: Update source for cri-tools 0e2459d13 docs: Add cgroupDriver for containerd 1b7fd19ac rootfs: Fix chronyd.service failing on boot Signed-off-by: Fabiano Fidêncio --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 23f468b7ea..3fef9b5648 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.5.0-alpha2 +2.5.0-rc0 From 5010c643c44371584c13918fcb543b19ed9d1e1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Tue, 5 Jul 2022 22:23:49 +0200 Subject: [PATCH 29/36] release: Revert kata-deploy changes after 2.5.0-rc0 release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As 2.5.0-rc0 has been released, let's switch the kata-deploy / kata-cleanup tags back to "latest", and re-add the kata-deploy-stable and the kata-cleanup-stable files. Signed-off-by: Fabiano Fidêncio --- .../base/kata-cleanup-stable.yaml | 46 +++++++++++++ .../kata-deploy/base/kata-deploy-stable.yaml | 69 +++++++++++++++++++ 2 files changed, 115 insertions(+) create mode 100644 tools/packaging/kata-deploy/kata-cleanup/base/kata-cleanup-stable.yaml create mode 100644 tools/packaging/kata-deploy/kata-deploy/base/kata-deploy-stable.yaml diff --git a/tools/packaging/kata-deploy/kata-cleanup/base/kata-cleanup-stable.yaml b/tools/packaging/kata-deploy/kata-cleanup/base/kata-cleanup-stable.yaml new file mode 100644 index 0000000000..f1d9d0a2f9 --- /dev/null +++ b/tools/packaging/kata-deploy/kata-cleanup/base/kata-cleanup-stable.yaml @@ -0,0 +1,46 @@ +--- +apiVersion: apps/v1 +kind: DaemonSet +metadata: + name: kubelet-kata-cleanup + namespace: kube-system +spec: + selector: + matchLabels: + name: kubelet-kata-cleanup + template: + metadata: + labels: + name: kubelet-kata-cleanup + spec: + serviceAccountName: kata-label-node + nodeSelector: + katacontainers.io/kata-runtime: cleanup + containers: + - name: kube-kata-cleanup + image: quay.io/kata-containers/kata-deploy:stable + imagePullPolicy: Always + command: [ "bash", "-c", "/opt/kata-artifacts/scripts/kata-deploy.sh reset" ] + env: + - name: NODE_NAME + valueFrom: + fieldRef: + fieldPath: spec.nodeName + securityContext: + privileged: false + volumeMounts: + - name: dbus + mountPath: /var/run/dbus + - name: systemd + mountPath: /run/systemd + volumes: + - name: dbus + hostPath: + path: /var/run/dbus + - name: systemd + hostPath: + path: /run/systemd + updateStrategy: + rollingUpdate: + maxUnavailable: 1 + type: RollingUpdate diff --git a/tools/packaging/kata-deploy/kata-deploy/base/kata-deploy-stable.yaml b/tools/packaging/kata-deploy/kata-deploy/base/kata-deploy-stable.yaml new file mode 100644 index 0000000000..346e4c0ee2 --- /dev/null +++ b/tools/packaging/kata-deploy/kata-deploy/base/kata-deploy-stable.yaml @@ -0,0 +1,69 @@ +--- +apiVersion: apps/v1 +kind: DaemonSet +metadata: + name: kata-deploy + namespace: kube-system +spec: + selector: + matchLabels: + name: kata-deploy + template: + metadata: + labels: + name: kata-deploy + spec: + serviceAccountName: kata-label-node + containers: + - name: kube-kata + image: quay.io/kata-containers/kata-deploy:stable + imagePullPolicy: Always + lifecycle: + preStop: + exec: + command: ["bash", "-c", "/opt/kata-artifacts/scripts/kata-deploy.sh cleanup"] + command: [ "bash", "-c", "/opt/kata-artifacts/scripts/kata-deploy.sh install" ] + env: + - name: NODE_NAME + valueFrom: + fieldRef: + fieldPath: spec.nodeName + securityContext: + privileged: false + volumeMounts: + - name: crio-conf + mountPath: /etc/crio/ + - name: containerd-conf + mountPath: /etc/containerd/ + - name: kata-artifacts + mountPath: /opt/kata/ + - name: dbus + mountPath: /var/run/dbus + - name: systemd + mountPath: /run/systemd + - name: local-bin + mountPath: /usr/local/bin/ + volumes: + - name: crio-conf + hostPath: + path: /etc/crio/ + - name: containerd-conf + hostPath: + path: /etc/containerd/ + - name: kata-artifacts + hostPath: + path: /opt/kata/ + type: DirectoryOrCreate + - name: dbus + hostPath: + path: /var/run/dbus + - name: systemd + hostPath: + path: /run/systemd + - name: local-bin + hostPath: + path: /usr/local/bin/ + updateStrategy: + rollingUpdate: + maxUnavailable: 1 + type: RollingUpdate From 3bafafec58ec5d124483a839e59b33d568e5b86f Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Wed, 6 Jul 2022 11:14:27 +0800 Subject: [PATCH 30/36] action: extend commit message line limit to 150 bytes So that we can add move info there and few people use such small terminals nowadays. Fixes: #4596 Signed-off-by: Peng Tao --- .github/workflows/commit-message-check.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/commit-message-check.yaml b/.github/workflows/commit-message-check.yaml index fec12ac1dd..fbdb02b6df 100644 --- a/.github/workflows/commit-message-check.yaml +++ b/.github/workflows/commit-message-check.yaml @@ -75,8 +75,8 @@ jobs: # # - A SoB comment can be any length (as it is unreasonable to penalise # people with long names/email addresses :) - pattern: '^.+(\n([a-zA-Z].{0,72}|[^a-zA-Z\n].*|[^\s\n]*|Signed-off-by:.*|))+$' - error: 'Body line too long (max 72)' + pattern: '^.+(\n([a-zA-Z].{0,150}|[^a-zA-Z\n].*|[^\s\n]*|Signed-off-by:.*|))+$' + error: 'Body line too long (max 150)' post_error: ${{ env.error_msg }} - name: Check Fixes From 4d89476c916b062a0b614f781763ae9d020fd303 Mon Sep 17 00:00:00 2001 From: Manabu Sugimoto Date: Wed, 6 Jul 2022 15:35:24 +0900 Subject: [PATCH 31/36] runtime: Fix DisableSelinux config Enable Kata runtime to handle `disable_selinux` flag properly in order to be able to change the status by the runtime configuration whether the runtime applies the SELinux label to VMM process. Fixes: #4599 Signed-off-by: Manabu Sugimoto --- src/runtime/config/configuration-acrn.toml.in | 6 +++--- src/runtime/config/configuration-clh.toml.in | 6 +++--- src/runtime/config/configuration-fc.toml.in | 6 +++--- src/runtime/config/configuration-qemu.toml.in | 6 +++--- src/runtime/pkg/katautils/config.go | 3 +++ 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/runtime/config/configuration-acrn.toml.in b/src/runtime/config/configuration-acrn.toml.in index f0be92ad08..5f1368ce82 100644 --- a/src/runtime/config/configuration-acrn.toml.in +++ b/src/runtime/config/configuration-acrn.toml.in @@ -118,6 +118,9 @@ block_device_driver = "@DEFBLOCKSTORAGEDRIVER_ACRN@" # but it will not abort container execution. #guest_hook_path = "/usr/share/oci/hooks" +# disable applying SELinux on the VMM process (default false) +disable_selinux=@DEFDISABLESELINUX@ + [agent.@PROJECT_TYPE@] # If enabled, make the agent display debug-level messages. # (default: disabled) @@ -186,9 +189,6 @@ internetworking_model="@DEFNETWORKMODEL_ACRN@" # (default: true) disable_guest_seccomp=@DEFDISABLEGUESTSECCOMP@ -# disable applying SELinux on the VMM process (default false) -disable_selinux=@DEFDISABLESELINUX@ - # If enabled, the runtime will create opentracing.io traces and spans. # (See https://www.jaegertracing.io/docs/getting-started). # (default: disabled) diff --git a/src/runtime/config/configuration-clh.toml.in b/src/runtime/config/configuration-clh.toml.in index 5d2d9c2f10..f09c095f0e 100644 --- a/src/runtime/config/configuration-clh.toml.in +++ b/src/runtime/config/configuration-clh.toml.in @@ -39,6 +39,9 @@ image = "@IMAGEPATH@" # Default false # confidential_guest = true +# disable applying SELinux on the VMM process (default false) +disable_selinux=@DEFDISABLESELINUX@ + # Path to the firmware. # If you want Cloud Hypervisor to use a specific firmware, set its path below. # This is option is only used when confidential_guest is enabled. @@ -319,9 +322,6 @@ internetworking_model="@DEFNETWORKMODEL_CLH@" # (default: true) disable_guest_seccomp=@DEFDISABLEGUESTSECCOMP@ -# disable applying SELinux on the VMM process (default false) -disable_selinux=@DEFDISABLESELINUX@ - # If enabled, the runtime will create opentracing.io traces and spans. # (See https://www.jaegertracing.io/docs/getting-started). # (default: disabled) diff --git a/src/runtime/config/configuration-fc.toml.in b/src/runtime/config/configuration-fc.toml.in index 8761d8a02e..b7f349c0dd 100644 --- a/src/runtime/config/configuration-fc.toml.in +++ b/src/runtime/config/configuration-fc.toml.in @@ -221,6 +221,9 @@ valid_entropy_sources = @DEFVALIDENTROPYSOURCES@ # Default 0-sized value means unlimited rate. #tx_rate_limiter_max_rate = 0 +# disable applying SELinux on the VMM process (default false) +disable_selinux=@DEFDISABLESELINUX@ + [factory] # VM templating support. Once enabled, new VMs are created from template # using vm cloning. They will share the same initial kernel, initramfs and @@ -309,9 +312,6 @@ internetworking_model="@DEFNETWORKMODEL_FC@" # (default: true) disable_guest_seccomp=@DEFDISABLEGUESTSECCOMP@ -# disable applying SELinux on the VMM process (default false) -disable_selinux=@DEFDISABLESELINUX@ - # If enabled, the runtime will create opentracing.io traces and spans. # (See https://www.jaegertracing.io/docs/getting-started). # (default: disabled) diff --git a/src/runtime/config/configuration-qemu.toml.in b/src/runtime/config/configuration-qemu.toml.in index 115cd19ccd..3ec44c8b6e 100644 --- a/src/runtime/config/configuration-qemu.toml.in +++ b/src/runtime/config/configuration-qemu.toml.in @@ -406,6 +406,9 @@ valid_entropy_sources = @DEFVALIDENTROPYSOURCES@ # use legacy serial for guest console if available and implemented for architecture. Default false #use_legacy_serial = true +# disable applying SELinux on the VMM process (default false) +disable_selinux=@DEFDISABLESELINUX@ + [factory] # VM templating support. Once enabled, new VMs are created from template # using vm cloning. They will share the same initial kernel, initramfs and @@ -523,9 +526,6 @@ internetworking_model="@DEFNETWORKMODEL_QEMU@" # (default: true) disable_guest_seccomp=@DEFDISABLEGUESTSECCOMP@ -# disable applying SELinux on the VMM process (default false) -disable_selinux=@DEFDISABLESELINUX@ - # If enabled, the runtime will create opentracing.io traces and spans. # (See https://www.jaegertracing.io/docs/getting-started). # (default: disabled) diff --git a/src/runtime/pkg/katautils/config.go b/src/runtime/pkg/katautils/config.go index 4edc0c1108..0903c8ea9e 100644 --- a/src/runtime/pkg/katautils/config.go +++ b/src/runtime/pkg/katautils/config.go @@ -669,6 +669,7 @@ func newFirecrackerHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) { RxRateLimiterMaxRate: rxRateLimiterMaxRate, TxRateLimiterMaxRate: txRateLimiterMaxRate, EnableAnnotations: h.EnableAnnotations, + DisableSeLinux: h.DisableSeLinux, }, nil } @@ -805,6 +806,7 @@ func newQemuHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) { GuestSwap: h.GuestSwap, Rootless: h.Rootless, LegacySerial: h.LegacySerial, + DisableSeLinux: h.DisableSeLinux, }, nil } @@ -869,6 +871,7 @@ func newAcrnHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) { BlockDeviceDriver: blockDriver, DisableVhostNet: h.DisableVhostNet, GuestHookPath: h.guestHookPath(), + DisableSeLinux: h.DisableSeLinux, EnableAnnotations: h.EnableAnnotations, }, nil } From 0e40ecf383850c6eeb23ac2e80702f0f13a76d7a Mon Sep 17 00:00:00 2001 From: Derek Lee Date: Fri, 24 Jun 2022 10:08:41 -0700 Subject: [PATCH 32/36] tools/snap: simplify nproc Replaces calls of nproc with nproc with nproc ${CI:+--ignore 1} to run nproc with one less processing unit than the maximum to prevent DOS-ing the local machine. If process is being run in a container (determined via whether $CI is null), all processing units avaliable will be used. Fixes #3967 Signed-off-by: Derek Lee --- docs/Developer-Guide.md | 2 +- docs/use-cases/using-Intel-QAT-and-kata.md | 4 ++-- snap/snapcraft.yaml | 4 ++-- tools/osbuilder/dockerfiles/QAT/run.sh | 4 ++-- tools/packaging/kata-deploy/local-build/Makefile | 2 +- tools/packaging/kernel/build-kernel.sh | 4 ++-- tools/packaging/static-build/qemu/Dockerfile | 2 +- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/Developer-Guide.md b/docs/Developer-Guide.md index 05664b7248..c1c2d62ab1 100644 --- a/docs/Developer-Guide.md +++ b/docs/Developer-Guide.md @@ -425,7 +425,7 @@ To build utilizing the same options as Kata, you should make use of the `configu $ cd $your_qemu_directory $ $packaging_dir/scripts/configure-hypervisor.sh kata-qemu > kata.cfg $ eval ./configure "$(cat kata.cfg)" -$ make -j $(nproc) +$ make -j $(nproc --ignore=1) $ sudo -E make install ``` diff --git a/docs/use-cases/using-Intel-QAT-and-kata.md b/docs/use-cases/using-Intel-QAT-and-kata.md index 0bcd2e3bbd..d029de3672 100644 --- a/docs/use-cases/using-Intel-QAT-and-kata.md +++ b/docs/use-cases/using-Intel-QAT-and-kata.md @@ -279,8 +279,8 @@ $ export KERNEL_EXTRAVERSION=$(awk '/^EXTRAVERSION =/{print $NF}' $GOPATH/$LINUX $ export KERNEL_ROOTFS_DIR=${KERNEL_MAJOR_VERSION}.${KERNEL_PATHLEVEL}.${KERNEL_SUBLEVEL}${KERNEL_EXTRAVERSION} $ cd $QAT_SRC $ KERNEL_SOURCE_ROOT=$GOPATH/$LINUX_VER ./configure --enable-icp-sriov=guest -$ sudo -E make all -j$(nproc) -$ sudo -E make INSTALL_MOD_PATH=$ROOTFS_DIR qat-driver-install -j$(nproc) +$ sudo -E make all -j $($(nproc ${CI:+--ignore 1})) +$ sudo -E make INSTALL_MOD_PATH=$ROOTFS_DIR qat-driver-install -j $($(nproc ${CI:+--ignore 1})) ``` The `usdm_drv` module also needs to be copied into the rootfs modules path and diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index 77f27b15f9..b462755080 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -193,7 +193,7 @@ parts: # Setup and build kernel ./build-kernel.sh -v "${kernel_version}" -d setup cd ${kernel_dir_prefix}* - make -j $(($(nproc)-1)) EXTRAVERSION=".container" + make -j $(nproc ${CI:+--ignore 1}) EXTRAVERSION=".container" kernel_suffix="${kernel_version}.container" kata_kernel_dir="${SNAPCRAFT_PART_INSTALL}/usr/share/kata-containers" @@ -282,7 +282,7 @@ parts: esac # build and install - make -j $(($(nproc)-1)) + make -j $(nproc ${CI:+--ignore 1}) make install DESTDIR="${SNAPCRAFT_PART_INSTALL}" prime: - -snap/ diff --git a/tools/osbuilder/dockerfiles/QAT/run.sh b/tools/osbuilder/dockerfiles/QAT/run.sh index b30c34c8b0..59194a2bcb 100755 --- a/tools/osbuilder/dockerfiles/QAT/run.sh +++ b/tools/osbuilder/dockerfiles/QAT/run.sh @@ -90,14 +90,14 @@ build_qat_drivers() KERNEL_ROOTFS_DIR=${KERNEL_MAJOR_VERSION}.${KERNEL_PATHLEVEL}.${KERNEL_SUBLEVEL}${KERNEL_EXTRAVERSION} cd $QAT_SRC KERNEL_SOURCE_ROOT=${linux_kernel_path} ./configure ${QAT_CONFIGURE_OPTIONS} - make all -j$(nproc) + make all -j $($(nproc ${CI:+--ignore 1})) } add_qat_to_rootfs() { /bin/echo -e "\n\e[1;42mCopy driver modules to rootfs\e[0m" cd $QAT_SRC - sudo -E make INSTALL_MOD_PATH=${ROOTFS_DIR} qat-driver-install -j$(nproc) + sudo -E make INSTALL_MOD_PATH=${ROOTFS_DIR} qat-driver-install -j$(nproc --ignore=1) sudo cp $QAT_SRC/build/usdm_drv.ko ${ROOTFS_DIR}/lib/modules/${KERNEL_ROOTFS_DIR}/updates/drivers sudo depmod -a -b ${ROOTFS_DIR} ${KERNEL_ROOTFS_DIR} cd ${kata_repo_path}/tools/osbuilder/image-builder diff --git a/tools/packaging/kata-deploy/local-build/Makefile b/tools/packaging/kata-deploy/local-build/Makefile index 9d09db049c..9f9bdcd6c5 100644 --- a/tools/packaging/kata-deploy/local-build/Makefile +++ b/tools/packaging/kata-deploy/local-build/Makefile @@ -19,7 +19,7 @@ $(MK_DIR)/dockerbuild/install_yq.sh: $(MK_DIR)/kata-deploy-copy-yq-installer.sh all-parallel: $(MK_DIR)/dockerbuild/install_yq.sh - ${MAKE} -f $(MK_PATH) all -j$$(( $$(nproc) - 1 )) V= + ${MAKE} -f $(MK_PATH) all -j $(shell nproc ${CI:+--ignore 1}) V= all: cloud-hypervisor-tarball \ firecracker-tarball \ diff --git a/tools/packaging/kernel/build-kernel.sh b/tools/packaging/kernel/build-kernel.sh index 4e58a45ef1..9b9a008d8f 100755 --- a/tools/packaging/kernel/build-kernel.sh +++ b/tools/packaging/kernel/build-kernel.sh @@ -418,9 +418,9 @@ build_kernel() { [ -n "${arch_target}" ] || arch_target="$(uname -m)" arch_target=$(arch_to_kernel "${arch_target}") pushd "${kernel_path}" >>/dev/null - make -j $(nproc) ARCH="${arch_target}" + make -j $(nproc ${CI:+--ignore 1}) ARCH="${arch_target}" if [ "${conf_guest}" == "sev" ]; then - make -j $(nproc --ignore=1) INSTALL_MOD_STRIP=1 INSTALL_MOD_PATH=${kernel_path} modules_install + make -j $(nproc ${CI:+--ignore 1}) INSTALL_MOD_STRIP=1 INSTALL_MOD_PATH=${kernel_path} modules_install fi [ "$arch_target" != "powerpc" ] && ([ -e "arch/${arch_target}/boot/bzImage" ] || [ -e "arch/${arch_target}/boot/Image.gz" ]) [ -e "vmlinux" ] diff --git a/tools/packaging/static-build/qemu/Dockerfile b/tools/packaging/static-build/qemu/Dockerfile index 61cc6ce951..27d088303e 100644 --- a/tools/packaging/static-build/qemu/Dockerfile +++ b/tools/packaging/static-build/qemu/Dockerfile @@ -74,6 +74,6 @@ RUN git clone --depth=1 "${QEMU_REPO}" qemu && \ /root/patch_qemu.sh "${QEMU_VERSION}" "/root/kata_qemu/patches" && \ (PREFIX="${PREFIX}" /root/configure-hypervisor.sh -s "kata-qemu${BUILD_SUFFIX}" | xargs ./configure \ --with-pkgversion="kata-static${BUILD_SUFFIX}") && \ - make -j"$(nproc)" && \ + make -j"$(nproc ${CI:+--ignore 1})" && \ make install DESTDIR="${QEMU_DESTDIR}" && \ /root/static-build/scripts/qemu-build-post.sh From efdb92366b633f8ca573f31df7ff2ca1ac067980 Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Tue, 5 Jul 2022 14:46:43 -0700 Subject: [PATCH 33/36] build: Fix clh source build as normal user While running make as non-privileged user, the make errors out with the following message: "INFO: Build cloud-hypervisor enabling the following features: tdx Got permission denied while trying to connect to the Docker daemon socket at unix:///var/run/docker.sock: Post "http://%2Fvar%2Frun%2Fdocker.sock/v1.24/images/create?fromImage=cloudhypervisor%2Fdev&tag=20220524-0": dial unix /var/run/docker.sock: connect: permission denied" Even though the user may be part of docker group, the clh build from source does a docker in docker build. It is necessary for the user of the nested container to be part of docker build for the build to succeed. Fixes #4594 Signed-off-by: Archana Shinde --- .../kata-deploy/local-build/dockerbuild/Dockerfile | 7 ++++++- .../local-build/kata-deploy-binaries-in-docker.sh | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/tools/packaging/kata-deploy/local-build/dockerbuild/Dockerfile b/tools/packaging/kata-deploy/local-build/dockerbuild/Dockerfile index 5514d9a640..06a4a93ac9 100644 --- a/tools/packaging/kata-deploy/local-build/dockerbuild/Dockerfile +++ b/tools/packaging/kata-deploy/local-build/dockerbuild/Dockerfile @@ -23,8 +23,13 @@ RUN apt-get update && \ ARG IMG_USER=kata-builder ARG UID=1000 ARG GID=1000 +# gid of the docker group on the host, required for running docker in docker builds. +ARG HOST_DOCKER_GID + RUN if [ ${IMG_USER} != "root" ]; then groupadd --gid=${GID} ${IMG_USER};fi RUN if [ ${IMG_USER} != "root" ]; then adduser ${IMG_USER} --uid=${UID} --gid=${GID};fi +RUN if [ ${IMG_USER} != "root" ] && [ ! -z ${HOST_DOCKER_GID} ]; then groupadd --gid=${HOST_DOCKER_GID} docker_on_host;fi +RUN if [ ${IMG_USER} != "root" ] && [ ! -z ${HOST_DOCKER_GID} ]; then usermod -a -G docker_on_host ${IMG_USER};fi RUN sh -c "echo '${IMG_USER} ALL=NOPASSWD: ALL' >> /etc/sudoers" #FIXME: gcc is required as agent is build out of a container build. @@ -40,4 +45,4 @@ RUN apt-get update && \ apt-get clean && rm -rf /var/lib/apt/lists ENV USER ${IMG_USER} -USER ${UID}:${GID} +USER ${IMG_USER} diff --git a/tools/packaging/kata-deploy/local-build/kata-deploy-binaries-in-docker.sh b/tools/packaging/kata-deploy/local-build/kata-deploy-binaries-in-docker.sh index 4035ff9cbd..24d19c936f 100755 --- a/tools/packaging/kata-deploy/local-build/kata-deploy-binaries-in-docker.sh +++ b/tools/packaging/kata-deploy/local-build/kata-deploy-binaries-in-docker.sh @@ -20,17 +20,27 @@ if [ "${script_dir}" != "${PWD}" ]; then ln -sf "${script_dir}/build" "${PWD}/build" fi +# This is the gid of the "docker" group on host. In case of docker in docker builds +# for some of the targets (clh builds from source), the nested container user needs to +# be part of this group. +docker_gid=$(getent group docker | cut -d: -f3 || { echo >&2 "Missing docker group, docker needs to be installed" && false; }) + +# If docker gid is the effective group id of the user, do not pass it as +# an additional group. +if [ ${docker_gid} == ${gid} ]; then + docker_gid="" +fi + docker build -q -t build-kata-deploy \ --build-arg IMG_USER="${USER}" \ --build-arg UID=${uid} \ --build-arg GID=${gid} \ + --build-arg HOST_DOCKER_GID=${docker_gid} \ "${script_dir}/dockerbuild/" docker run \ -v /var/run/docker.sock:/var/run/docker.sock \ - --user ${uid}:${gid} \ --env USER=${USER} -v "${kata_dir}:${kata_dir}" \ --rm \ -w ${script_dir} \ build-kata-deploy "${kata_deploy_create}" $@ - From 2551924bda926ec9f2ee210323f6b034f5a77acf Mon Sep 17 00:00:00 2001 From: liubin Date: Thu, 7 Jul 2022 11:34:04 +0800 Subject: [PATCH 34/36] docs: delete CRI containerd plugin statement There is no independent CRI containerd plugin for new containerd, the related documentation should be updated too. Fixes: #4605 Signed-off-by: liubin --- docs/how-to/README.md | 2 +- docs/how-to/how-to-set-prometheus-in-k8s.md | 2 +- ...md => how-to-use-k8s-with-containerd-and-kata.md} | 12 ++++++------ docs/how-to/privileged.md | 2 +- docs/how-to/run-kata-with-k8s.md | 6 +++--- 5 files changed, 12 insertions(+), 12 deletions(-) rename docs/how-to/{how-to-use-k8s-with-cri-containerd-and-kata.md => how-to-use-k8s-with-containerd-and-kata.md} (93%) diff --git a/docs/how-to/README.md b/docs/how-to/README.md index 6dd163ce73..aa09b49c73 100644 --- a/docs/how-to/README.md +++ b/docs/how-to/README.md @@ -5,7 +5,7 @@ - [Run Kata containers with `crictl`](run-kata-with-crictl.md) - [Run Kata Containers with Kubernetes](run-kata-with-k8s.md) - [How to use Kata Containers and Containerd](containerd-kata.md) -- [How to use Kata Containers and CRI (containerd) with Kubernetes](how-to-use-k8s-with-cri-containerd-and-kata.md) +- [How to use Kata Containers and containerd with Kubernetes](how-to-use-k8s-with-containerd-and-kata.md) - [Kata Containers and service mesh for Kubernetes](service-mesh.md) - [How to import Kata Containers logs into Fluentd](how-to-import-kata-logs-with-fluentd.md) diff --git a/docs/how-to/how-to-set-prometheus-in-k8s.md b/docs/how-to/how-to-set-prometheus-in-k8s.md index 2090c3bd51..e61db2d1ca 100644 --- a/docs/how-to/how-to-set-prometheus-in-k8s.md +++ b/docs/how-to/how-to-set-prometheus-in-k8s.md @@ -19,7 +19,7 @@ Also you should ensure that `kubectl` working correctly. > **Note**: More information about Kubernetes integrations: > - [Run Kata Containers with Kubernetes](run-kata-with-k8s.md) > - [How to use Kata Containers and Containerd](containerd-kata.md) -> - [How to use Kata Containers and CRI (containerd plugin) with Kubernetes](how-to-use-k8s-with-cri-containerd-and-kata.md) +> - [How to use Kata Containers and containerd with Kubernetes](how-to-use-k8s-with-containerd-and-kata.md) ## Configure Prometheus diff --git a/docs/how-to/how-to-use-k8s-with-cri-containerd-and-kata.md b/docs/how-to/how-to-use-k8s-with-containerd-and-kata.md similarity index 93% rename from docs/how-to/how-to-use-k8s-with-cri-containerd-and-kata.md rename to docs/how-to/how-to-use-k8s-with-containerd-and-kata.md index bacfdccc86..de7a34ef61 100644 --- a/docs/how-to/how-to-use-k8s-with-cri-containerd-and-kata.md +++ b/docs/how-to/how-to-use-k8s-with-containerd-and-kata.md @@ -1,15 +1,15 @@ -# How to use Kata Containers and CRI (containerd plugin) with Kubernetes +# How to use Kata Containers and containerd with Kubernetes This document describes how to set up a single-machine Kubernetes (k8s) cluster. The Kubernetes cluster will use the -[CRI containerd](https://github.com/containerd/containerd/) and -[Kata Containers](https://katacontainers.io) to launch untrusted workloads. +[containerd](https://github.com/containerd/containerd/) and +[Kata Containers](https://katacontainers.io) to launch workloads. ## Requirements - Kubernetes, Kubelet, `kubeadm` -- containerd with `cri` plug-in +- containerd - Kata Containers > **Note:** For information about the supported versions of these components, @@ -149,7 +149,7 @@ $ sudo -E kubectl taint nodes --all node-role.kubernetes.io/master- ## Create runtime class for Kata Containers -By default, all pods are created with the default runtime configured in CRI containerd plugin. +By default, all pods are created with the default runtime configured in containerd. From Kubernetes v1.12, users can use [`RuntimeClass`](https://kubernetes.io/docs/concepts/containers/runtime-class/#runtime-class) to specify a different runtime for Pods. ```bash @@ -166,7 +166,7 @@ $ sudo -E kubectl apply -f runtime.yaml ## Run pod in Kata Containers -If a pod has the `runtimeClassName` set to `kata`, the CRI plugin runs the pod with the +If a pod has the `runtimeClassName` set to `kata`, the CRI runs the pod with the [Kata Containers runtime](../../src/runtime/README.md). - Create an pod configuration that using Kata Containers runtime diff --git a/docs/how-to/privileged.md b/docs/how-to/privileged.md index 10868f9a31..048509ff17 100644 --- a/docs/how-to/privileged.md +++ b/docs/how-to/privileged.md @@ -40,7 +40,7 @@ See below example config: ConfigPath = "/opt/kata/share/defaults/kata-containers/configuration.toml" ``` - - [Kata Containers with Containerd and CRI documentation](how-to-use-k8s-with-cri-containerd-and-kata.md) + - [How to use Kata Containers and containerd with Kubernetes](how-to-use-k8s-with-containerd-and-kata.md) - [Containerd CRI config documentation](https://github.com/containerd/containerd/blob/main/docs/cri/config.md) #### CRI-O diff --git a/docs/how-to/run-kata-with-k8s.md b/docs/how-to/run-kata-with-k8s.md index baee63bffe..4e5c58d5a2 100644 --- a/docs/how-to/run-kata-with-k8s.md +++ b/docs/how-to/run-kata-with-k8s.md @@ -15,7 +15,7 @@ After choosing one CRI implementation, you must make the appropriate configurati to ensure it integrates with Kata Containers. Kata Containers 1.5 introduced the `shimv2` for containerd 1.2.0, reducing the components -required to spawn pods and containers, and this is the preferred way to run Kata Containers with Kubernetes ([as documented here](../how-to/how-to-use-k8s-with-cri-containerd-and-kata.md#configure-containerd-to-use-kata-containers)). +required to spawn pods and containers, and this is the preferred way to run Kata Containers with Kubernetes ([as documented here](../how-to/how-to-use-k8s-with-containerd-and-kata.md#configure-containerd-to-use-kata-containers)). An equivalent shim implementation for CRI-O is planned. @@ -57,7 +57,7 @@ content shown below: To customize containerd to select Kata Containers runtime, follow our "Configure containerd to use Kata Containers" internal documentation -[here](../how-to/how-to-use-k8s-with-cri-containerd-and-kata.md#configure-containerd-to-use-kata-containers). +[here](../how-to/how-to-use-k8s-with-containerd-and-kata.md#configure-containerd-to-use-kata-containers). ## Install Kubernetes @@ -85,7 +85,7 @@ Environment="KUBELET_EXTRA_ARGS=--container-runtime=remote --runtime-request-tim Environment="KUBELET_EXTRA_ARGS=--container-runtime=remote --runtime-request-timeout=15m --container-runtime-endpoint=unix:///run/containerd/containerd.sock" ``` For more information about containerd see the "Configure Kubelet to use containerd" -documentation [here](../how-to/how-to-use-k8s-with-cri-containerd-and-kata.md#configure-kubelet-to-use-containerd). +documentation [here](../how-to/how-to-use-k8s-with-containerd-and-kata.md#configure-kubelet-to-use-containerd). ## Run a Kubernetes pod with Kata Containers From e57a1c831ebc087ffdaaecdfe742a0133a7595b8 Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Tue, 5 Jul 2022 14:58:10 -0700 Subject: [PATCH 35/36] build: Mark git repos as safe for build This is not an issue when the build is run as non-privilged user. Marking these as safe in case where the build may be run as root or some other user. Signed-off-by: Archana Shinde --- .../packaging/static-build/cloud-hypervisor/build-static-clh.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/packaging/static-build/cloud-hypervisor/build-static-clh.sh b/tools/packaging/static-build/cloud-hypervisor/build-static-clh.sh index 82ad3b42b9..8cb1a6e79f 100755 --- a/tools/packaging/static-build/cloud-hypervisor/build-static-clh.sh +++ b/tools/packaging/static-build/cloud-hypervisor/build-static-clh.sh @@ -56,6 +56,7 @@ build_clh_from_source() { repo_dir="${repo_dir//.git}" rm -rf "${repo_dir}" git clone "${cloud_hypervisor_repo}" + git config --global --add safe.directory "$PWD/repo_dir" pushd "${repo_dir}" if [ -n "${cloud_hypervisor_pr}" ]; then From 57c2d8b749796646f2dd5a6752e5af656b435630 Mon Sep 17 00:00:00 2001 From: Gabriela Cervantes Date: Thu, 7 Jul 2022 21:48:18 +0000 Subject: [PATCH 36/36] docs: Update URL links for containerd documentation This PR updates some url links related with containerd documentation. Fixes #4615 Signed-off-by: Gabriela Cervantes --- docs/how-to/containerd-kata.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/how-to/containerd-kata.md b/docs/how-to/containerd-kata.md index ffb7614a09..2ac0613c6d 100644 --- a/docs/how-to/containerd-kata.md +++ b/docs/how-to/containerd-kata.md @@ -132,9 +132,9 @@ The `RuntimeClass` is suggested. The following configuration includes two runtime classes: - `plugins.cri.containerd.runtimes.runc`: the runc, and it is the default runtime. -- `plugins.cri.containerd.runtimes.kata`: The function in containerd (reference [the document here](https://github.com/containerd/containerd/tree/master/runtime/v2#binary-naming)) +- `plugins.cri.containerd.runtimes.kata`: The function in containerd (reference [the document here](https://github.com/containerd/containerd/tree/main/runtime/v2#binary-naming)) where the dot-connected string `io.containerd.kata.v2` is translated to `containerd-shim-kata-v2` (i.e. the - binary name of the Kata implementation of [Containerd Runtime V2 (Shim API)](https://github.com/containerd/containerd/tree/master/runtime/v2)). + binary name of the Kata implementation of [Containerd Runtime V2 (Shim API)](https://github.com/containerd/containerd/tree/main/runtime/v2)). ```toml [plugins.cri.containerd]