From 469e098543b02be93a29c08768b8fd1cc3f3cd73 Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Mon, 23 May 2022 12:27:55 -0700 Subject: [PATCH 01/27] 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 dbdfdac30..fd6df4c3c 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 0bcac2137..a33e37a1a 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 c8e48953f..a91505d28 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 02/27] 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 f6da05ec5..1c3ebc147 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 6984b73be..85ec05f15 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 25012d3f3..434e32041 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 119c4667c..18ccd7ae2 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 ec475e86e..31452200d 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 83e776d28..f4a0b934e 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 5e89ae8a6..0159a993d 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 a91505d28..7440c8258 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 a995f1f77..550362afd 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 a9db8efc0..a96661d43 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 03/27] 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 14f0f3ecf..36a2f618b 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 86868038e..63870b232 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 527c9cfbc..5676f4451 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 dbdfdac30..c0ffb9926 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 0bcac2137..65b541e73 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 000000000..63ca4a6d2 --- /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 000000000..e98f261a0 --- /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 000000000..4e4f984c0 --- /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 000000000..596576591 --- /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 000000000..49d808a9e --- /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 000000000..a3f457699 --- /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 000000000..3d07711ce --- /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 000000000..c8500cc6f --- /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 000000000..438d9eff8 --- /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 000000000..f29473ba0 --- /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 8c27d91df..fb015d1c2 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 119c4667c..e39885dac 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 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 04/27] 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 6984b73be..801d53a9b 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 c8e48953f..f2c501ac3 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 b50d73a91..b932189e8 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 05/27] 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 757d0a48a..fa07b87de 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 41d4ae493..5d2d9c2f1 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 b6892bd17..8761d8a02 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 702b71aad..115cd19cc 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 06/27] 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 e39885dac..17fc7a875 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 ec475e86e..424518915 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 f2c501ac3..f0cac0a89 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 07/27] 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 000000000..e074d59ea --- /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 000000000..f1f20d315 --- /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 000000000..609e52fd7 --- /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 000000000..49558f6a9 --- /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 550362afd..d53de2656 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 08/27] 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 748c5b4e5..61f9236b9 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 44fd32187..cdd4db998 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 09/27] 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 8a24c29bf..ebb742790 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 a995f1f77..d1705d379 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 10/27] 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 dbdfdac30..8522274da 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 11/27] 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 8522274da..d342bb3b3 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 12/27] 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 d342bb3b3..43cf0da25 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 13/27] 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 43cf0da25..ef0b7649a 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 14/27] 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 0bcac2137..4cc7eb931 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 15/27] 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 d6738f66f..c8aeec0ce 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 16/27] 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 f980bc5a5..3ab861420 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 95c7dc59d..f8f98c3c4 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 eba829e2d..6b14a94c7 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 994de7c43..121d5ea4d 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 35b489920..32195bcce 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 e456d3727..ffcaa0715 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 15b456113..b1e4cf2a9 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 71423cf0c..5fb8ea1d5 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 2ddd42d11..6fe136b5d 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 f4fc3e503..5149b0423 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 17/27] 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 2062e51b9..fec12ac1d 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 18/27] 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 bcf2096d2..10f1a0021 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 19/27] 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 f7f1eeeb7..3cb2c158f 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 20/27] 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 8cc34dba0..5b073ff07 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 21/27] 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 71423cf0c..53503b322 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 2ddd42d11..7caa51aa2 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 22/27] 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 5f1aa5606..ace9891d2 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 bcf2096d2..ba9981069 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 23/27] 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 5f1aa5606..584207cf8 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 4386dde5c..6a023e093 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 24/27] 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 f47f2df4b..3998b166c 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 25/27] 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 f1d9d0a2f..000000000 --- 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 346e4c0ee..000000000 --- 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 26/27] 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 23f468b7e..3fef9b564 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.5.0-alpha2 +2.5.0-rc0 From 3bafafec58ec5d124483a839e59b33d568e5b86f Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Wed, 6 Jul 2022 11:14:27 +0800 Subject: [PATCH 27/27] 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 fec12ac1d..fbdb02b6d 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