diff --git a/.gitignore b/.gitignore index a8942b24d9..364c0dd8b7 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,7 @@ /cli/config/configuration-qemu-virtiofs.toml /cli/config/configuration-clh.toml /cli/config-generated.go +/cli/containerd-shim-kata-v2/config-generated.go /cli/coverage.html /containerd-shim-kata-v2 /data/kata-collect-data.sh diff --git a/cli/delete.go b/cli/delete.go index c2ce52a468..641ef3727a 100644 --- a/cli/delete.go +++ b/cli/delete.go @@ -10,6 +10,7 @@ import ( "context" "fmt" "os" + "syscall" "github.com/kata-containers/runtime/pkg/katautils" vc "github.com/kata-containers/runtime/virtcontainers" @@ -75,6 +76,11 @@ func delete(ctx context.Context, containerID string, force bool) error { kataLog.Warnf("Failed to get container, force will not fail: %s", err) return nil } + if err.Error() == syscall.ENOENT.Error() { + kataLog.WithField("container", containerID).Info("skipping delete as container does not exist") + katautils.DelContainerIDMapping(ctx, containerID) + return nil + } return err } diff --git a/cli/kill.go b/cli/kill.go index 60fa41e09e..259a72dd52 100644 --- a/cli/kill.go +++ b/cli/kill.go @@ -108,8 +108,11 @@ func kill(ctx context.Context, containerID, signal string, all bool) error { // Checks the MUST and MUST NOT from OCI runtime specification status, sandboxID, err := getExistingContainerInfo(ctx, containerID) - if err != nil { + if err.Error() == syscall.ENOENT.Error() { + kataLog.WithField("container", containerID).Info("skipping kill as container does not exist") + return nil + } return err } diff --git a/virtcontainers/hypervisor.go b/virtcontainers/hypervisor.go index 4bb2eeb7b4..95b4d9282b 100644 --- a/virtcontainers/hypervisor.go +++ b/virtcontainers/hypervisor.go @@ -578,11 +578,6 @@ func (conf *HypervisorConfig) HypervisorCtlAssetPath() (string, error) { return conf.assetPath(types.HypervisorCtlAsset) } -// JailerAssetPath returns the VM Jailer path -func (conf *HypervisorConfig) JailerAssetPath() (string, error) { - return conf.assetPath(types.JailerAsset) -} - // CustomHypervisorAsset returns true if the hypervisor asset is a custom one, false otherwise. func (conf *HypervisorConfig) CustomHypervisorAsset() bool { return conf.isCustomAsset(types.HypervisorAsset) @@ -593,11 +588,6 @@ func (conf *HypervisorConfig) FirmwareAssetPath() (string, error) { return conf.assetPath(types.FirmwareAsset) } -// CustomFirmwareAsset returns true if the firmware asset is a custom one, false otherwise. -func (conf *HypervisorConfig) CustomFirmwareAsset() bool { - return conf.isCustomAsset(types.FirmwareAsset) -} - func appendParam(params []Param, parameter string, value string) []Param { return append(params, Param{parameter, value}) } diff --git a/virtcontainers/hypervisor_test.go b/virtcontainers/hypervisor_test.go index 6f91287aa1..e4ce65df48 100644 --- a/virtcontainers/hypervisor_test.go +++ b/virtcontainers/hypervisor_test.go @@ -456,3 +456,41 @@ func TestGenerateVMSocket(t *testing.T) { assert.NotZero(vsock.ContextID) assert.NotZero(vsock.Port) } + +func TestAssetPath(t *testing.T) { + assert := assert.New(t) + + // Minimal config containing values for all asset annotation options. + // The values are "paths" (start with a slash), but end with the + // annotation name. + cfg := HypervisorConfig{ + HypervisorPath: "/" + "io.katacontainers.config.hypervisor.path", + HypervisorCtlPath: "/" + "io.katacontainers.config.hypervisor.ctlpath", + + KernelPath: "/" + "io.katacontainers.config.hypervisor.kernel", + + ImagePath: "/" + "io.katacontainers.config.hypervisor.image", + InitrdPath: "/" + "io.katacontainers.config.hypervisor.initrd", + + FirmwarePath: "/" + "io.katacontainers.config.hypervisor.firmware", + JailerPath: "/" + "io.katacontainers.config.hypervisor.jailer_path", + } + + for _, asset := range types.AssetTypes() { + msg := fmt.Sprintf("asset: %v", asset) + + annoPath, annoHash, err := asset.Annotations() + assert.NoError(err, msg) + + msg += fmt.Sprintf(", annotation path: %v, annotation hash: %v", annoPath, annoHash) + + p, err := cfg.assetPath(asset) + assert.NoError(err, msg) + + assert.NotEqual(p, annoPath, msg) + assert.NotEqual(p, annoHash, msg) + + expected := fmt.Sprintf("/%s", annoPath) + assert.Equal(expected, p, msg) + } +} diff --git a/virtcontainers/pkg/annotations/annotations.go b/virtcontainers/pkg/annotations/annotations.go index 10ce7833e2..33eaaa0c2b 100644 --- a/virtcontainers/pkg/annotations/annotations.go +++ b/virtcontainers/pkg/annotations/annotations.go @@ -41,6 +41,9 @@ const ( // HypervisorPath is a sandbox annotation for passing a per container path pointing at the hypervisor that will run the container VM. HypervisorPath = kataAnnotHypervisorPrefix + "path" + // HypervisorCtlPath is a sandbox annotation for passing a per container path pointing at the hypervisor control binary that will run the container VM. + HypervisorCtlPath = kataAnnotHypervisorPrefix + "ctlpath" + // JailerPath is a sandbox annotation for passing a per container path pointing at the jailer that will constrain the container VM. JailerPath = kataAnnotHypervisorPrefix + "jailer_path" @@ -59,6 +62,9 @@ const ( // HypervisorHash is an sandbox annotation for passing a container hypervisor binary SHA-512 hash value. HypervisorHash = kataAnnotHypervisorPrefix + "hypervisor_hash" + // HypervisorCtlHash is a sandbox annotation for passing a container hypervisor control binary SHA-512 hash value. + HypervisorCtlHash = kataAnnotHypervisorPrefix + "hypervisorctl_hash" + // JailerHash is an sandbox annotation for passing a jailer binary SHA-512 hash value. JailerHash = kataAnnotHypervisorPrefix + "jailer_hash" diff --git a/virtcontainers/pkg/oci/utils.go b/virtcontainers/pkg/oci/utils.go index 9701df3d55..527c483c3f 100644 --- a/virtcontainers/pkg/oci/utils.go +++ b/virtcontainers/pkg/oci/utils.go @@ -335,7 +335,11 @@ func SandboxID(spec specs.Spec) (string, error) { } func addAnnotations(ocispec specs.Spec, config *vc.SandboxConfig) error { - addAssetAnnotations(ocispec, config) + err := addAssetAnnotations(ocispec, config) + if err != nil { + return err + } + if err := addHypervisorConfigOverrides(ocispec, config); err != nil { return err } @@ -350,17 +354,10 @@ func addAnnotations(ocispec specs.Spec, config *vc.SandboxConfig) error { return nil } -func addAssetAnnotations(ocispec specs.Spec, config *vc.SandboxConfig) { - assetAnnotations := []string{ - vcAnnotations.KernelPath, - vcAnnotations.ImagePath, - vcAnnotations.InitrdPath, - vcAnnotations.FirmwarePath, - vcAnnotations.KernelHash, - vcAnnotations.ImageHash, - vcAnnotations.InitrdHash, - vcAnnotations.FirmwareHash, - vcAnnotations.AssetHashType, +func addAssetAnnotations(ocispec specs.Spec, config *vc.SandboxConfig) error { + assetAnnotations, err := types.AssetAnnotations() + if err != nil { + return err } for _, a := range assetAnnotations { @@ -371,6 +368,8 @@ func addAssetAnnotations(ocispec specs.Spec, config *vc.SandboxConfig) { config.Annotations[a] = value } + + return nil } func addHypervisorConfigOverrides(ocispec specs.Spec, config *vc.SandboxConfig) error { diff --git a/virtcontainers/pkg/oci/utils_test.go b/virtcontainers/pkg/oci/utils_test.go index 1a90e8a145..851abfba7c 100644 --- a/virtcontainers/pkg/oci/utils_test.go +++ b/virtcontainers/pkg/oci/utils_test.go @@ -665,12 +665,26 @@ func TestAddAssetAnnotations(t *testing.T) { assert := assert.New(t) expectedAnnotations := map[string]string{ - vcAnnotations.KernelPath: "/abc/rgb/kernel", - vcAnnotations.ImagePath: "/abc/rgb/image", - vcAnnotations.InitrdPath: "/abc/rgb/initrd", - vcAnnotations.KernelHash: "3l2353we871g", - vcAnnotations.ImageHash: "52ss2550983", - vcAnnotations.AssetHashType: "sha", + vcAnnotations.FirmwarePath: "/some/where", + vcAnnotations.FirmwareHash: "ffff", + + vcAnnotations.HypervisorPath: "/some/where", + vcAnnotations.HypervisorHash: "bbbbb", + + vcAnnotations.HypervisorCtlPath: "/some/where/else", + vcAnnotations.HypervisorCtlHash: "cc", + + vcAnnotations.ImagePath: "/abc/rgb/image", + vcAnnotations.ImageHash: "52ss2550983", + + vcAnnotations.InitrdPath: "/abc/rgb/initrd", + vcAnnotations.InitrdHash: "aaaa", + + vcAnnotations.JailerPath: "/foo/bar", + vcAnnotations.JailerHash: "dddd", + + vcAnnotations.KernelPath: "/abc/rgb/kernel", + vcAnnotations.KernelHash: "3l2353we871g", } config := vc.SandboxConfig{ diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index dd79274931..b5f1e9f3d6 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -1965,7 +1965,7 @@ func genericBridges(number uint32, machineType string) []types.Bridge { case QemuPC: bt = types.PCI case QemuVirt: - bt = types.PCIE + bt = types.PCI case QemuPseries: bt = types.PCI case QemuCCWVirtio: diff --git a/virtcontainers/qemu_arm64_test.go b/virtcontainers/qemu_arm64_test.go index 4035115829..728b8e0ff8 100644 --- a/virtcontainers/qemu_arm64_test.go +++ b/virtcontainers/qemu_arm64_test.go @@ -119,7 +119,7 @@ func TestQemuArm64AppendBridges(t *testing.T) { expectedOut := []govmmQemu.Device{ govmmQemu.BridgeDevice{ - Type: govmmQemu.PCIEBridge, + Type: govmmQemu.PCIBridge, Bus: defaultBridgeBus, ID: bridges[0].ID, Chassis: 1, diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index a7420529fe..908e753ef6 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -416,31 +416,24 @@ func createAssets(ctx context.Context, sandboxConfig *SandboxConfig) error { span, _ := trace(ctx, "createAssets") defer span.Finish() - kernel, err := types.NewAsset(sandboxConfig.Annotations, types.KernelAsset) - if err != nil { - return err - } + for _, name := range types.AssetTypes() { + a, err := types.NewAsset(sandboxConfig.Annotations, name) + if err != nil { + return err + } - image, err := types.NewAsset(sandboxConfig.Annotations, types.ImageAsset) - if err != nil { - return err + if err := sandboxConfig.HypervisorConfig.addCustomAsset(a); err != nil { + return err + } } - initrd, err := types.NewAsset(sandboxConfig.Annotations, types.InitrdAsset) - if err != nil { - return err - } + _, imageErr := sandboxConfig.HypervisorConfig.assetPath(types.ImageAsset) + _, initrdErr := sandboxConfig.HypervisorConfig.assetPath(types.InitrdAsset) - if image != nil && initrd != nil { + if imageErr != nil && initrdErr != nil { return fmt.Errorf("%s and %s cannot be both set", types.ImageAsset, types.InitrdAsset) } - for _, a := range []*types.Asset{kernel, image, initrd} { - if err := sandboxConfig.HypervisorConfig.addCustomAsset(a); err != nil { - return err - } - } - return nil } diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index 9f9d50ac1c..47f3b0d4a3 100644 --- a/virtcontainers/sandbox_test.go +++ b/virtcontainers/sandbox_test.go @@ -843,51 +843,127 @@ var assetContentWrongHash = "92549f8d2018a95a294d28a65e795ed7d1a9d150009a28cea10 func TestSandboxCreateAssets(t *testing.T) { assert := assert.New(t) + type testData struct { + assetType types.AssetType + annotations map[string]string + } + tmpfile, err := ioutil.TempFile("", "virtcontainers-test-") assert.Nil(err) + filename := tmpfile.Name() + defer func() { tmpfile.Close() - os.Remove(tmpfile.Name()) // clean up + os.Remove(filename) // clean up }() _, err = tmpfile.Write(assetContent) assert.Nil(err) originalKernelPath := filepath.Join(testDir, testKernel) + originalImagePath := filepath.Join(testDir, testImage) + originalInitrdPath := filepath.Join(testDir, testInitrd) + originalFirmwarePath := filepath.Join(testDir, testFirmware) + originalHypervisorPath := filepath.Join(testDir, testHypervisor) + originalHypervisorCtlPath := filepath.Join(testDir, testHypervisorCtl) + originalJailerPath := filepath.Join(testDir, testJailer) hc := HypervisorConfig{ - KernelPath: originalKernelPath, - ImagePath: filepath.Join(testDir, testImage), + KernelPath: originalKernelPath, + ImagePath: originalImagePath, + InitrdPath: originalInitrdPath, + FirmwarePath: originalFirmwarePath, + HypervisorPath: originalHypervisorPath, + HypervisorCtlPath: originalHypervisorCtlPath, + JailerPath: originalJailerPath, } - p := &SandboxConfig{ - Annotations: map[string]string{ - annotations.KernelPath: tmpfile.Name(), - annotations.KernelHash: assetContentHash, + data := []testData{ + { + types.FirmwareAsset, + map[string]string{ + annotations.FirmwarePath: filename, + annotations.FirmwareHash: assetContentHash, + }, + }, + { + types.HypervisorAsset, + map[string]string{ + annotations.HypervisorPath: filename, + annotations.HypervisorHash: assetContentHash, + }, + }, + { + types.HypervisorCtlAsset, + map[string]string{ + annotations.HypervisorCtlPath: filename, + annotations.HypervisorCtlHash: assetContentHash, + }, + }, + { + types.ImageAsset, + map[string]string{ + annotations.ImagePath: filename, + annotations.ImageHash: assetContentHash, + }, + }, + { + types.InitrdAsset, + map[string]string{ + annotations.InitrdPath: filename, + annotations.InitrdHash: assetContentHash, + }, + }, + { + types.JailerAsset, + map[string]string{ + annotations.JailerPath: filename, + annotations.JailerHash: assetContentHash, + }, + }, + { + types.KernelAsset, + map[string]string{ + annotations.KernelPath: filename, + annotations.KernelHash: assetContentHash, + }, }, - - HypervisorConfig: hc, } - err = createAssets(context.Background(), p) - assert.Nil(err) + for i, d := range data { + msg := fmt.Sprintf("test[%d]: %+v", i, d) - a, ok := p.HypervisorConfig.customAssets[types.KernelAsset] - assert.True(ok) - assert.Equal(a.Path(), tmpfile.Name()) + config := &SandboxConfig{ + Annotations: d.annotations, + HypervisorConfig: hc, + } - p = &SandboxConfig{ - Annotations: map[string]string{ - annotations.KernelPath: tmpfile.Name(), - annotations.KernelHash: assetContentWrongHash, - }, + err = createAssets(context.Background(), config) + assert.NoError(err, msg) - HypervisorConfig: hc, - } + a, ok := config.HypervisorConfig.customAssets[d.assetType] + assert.True(ok, msg) + assert.Equal(a.Path(), filename, msg) + + // Now test with invalid hashes + badHashAnnotations := make(map[string]string) + for k, v := range d.annotations { + if strings.HasSuffix(k, "_hash") { + badHashAnnotations[k] = assetContentWrongHash + } else { + badHashAnnotations[k] = v + } + } + + config = &SandboxConfig{ + Annotations: badHashAnnotations, + HypervisorConfig: hc, + } - err = createAssets(context.Background(), p) - assert.NotNil(err) + err = createAssets(context.Background(), config) + assert.Error(err, msg) + } } func testFindContainerFailure(t *testing.T, sandbox *Sandbox, cid string) { diff --git a/virtcontainers/types/asset.go b/virtcontainers/types/asset.go index eaeaf418cd..b95b19aa65 100644 --- a/virtcontainers/types/asset.go +++ b/virtcontainers/types/asset.go @@ -18,6 +18,60 @@ import ( // AssetType describe a type of assets. type AssetType string +const ( + // KernelAsset is a kernel asset. + KernelAsset AssetType = "kernel" + + // ImageAsset is an image asset. + ImageAsset AssetType = "image" + + // InitrdAsset is an initrd asset. + InitrdAsset AssetType = "initrd" + + // HypervisorAsset is an hypervisor asset. + HypervisorAsset AssetType = "hypervisor" + + // HypervisorCtlAsset is a hypervisor control asset. + HypervisorCtlAsset AssetType = "hypervisorctl" + + // JailerAsset is a jailer asset. + JailerAsset AssetType = "jailer" + + // FirmwareAsset is a firmware asset. + FirmwareAsset AssetType = "firmware" +) + +// AssetTypes returns a list of all known asset types. +// +// XXX: New asset types *MUST* be added here. +func AssetTypes() []AssetType { + return []AssetType{ + FirmwareAsset, + HypervisorAsset, + HypervisorCtlAsset, + ImageAsset, + InitrdAsset, + JailerAsset, + KernelAsset, + } +} + +// AssetAnnotations returns all annotations for all asset types. +func AssetAnnotations() ([]string, error) { + var result []string + + for _, at := range AssetTypes() { + aPath, aHash, err := at.Annotations() + if err != nil { + return []string{}, err + } + + result = append(result, []string{aPath, aHash}...) + } + + return result, nil +} + // Annotations returns the path and hash annotations for a given Asset type. func (t AssetType) Annotations() (string, string, error) { switch t { @@ -29,6 +83,8 @@ func (t AssetType) Annotations() (string, string, error) { return annotations.InitrdPath, annotations.InitrdHash, nil case HypervisorAsset: return annotations.HypervisorPath, annotations.HypervisorHash, nil + case HypervisorCtlAsset: + return annotations.HypervisorCtlPath, annotations.HypervisorCtlHash, nil case JailerAsset: return annotations.JailerPath, annotations.JailerHash, nil case FirmwareAsset: @@ -38,28 +94,6 @@ func (t AssetType) Annotations() (string, string, error) { return "", "", fmt.Errorf("Wrong asset type %s", t) } -const ( - // KernelAsset is a kernel asset. - KernelAsset AssetType = "kernel" - - // ImageAsset is an image asset. - ImageAsset AssetType = "image" - - // InitrdAsset is an intird asset. - InitrdAsset AssetType = "initrd" - - // HypervisorAsset is an hypervisor asset. - HypervisorAsset AssetType = "hypervisor" - - // HypervisorCtlAsset is an hypervisor control asset. - HypervisorCtlAsset AssetType = "hypervisorctl" - // JailerAsset is an jailer asset. - JailerAsset AssetType = "jailer" - - // FirmwareAsset is a firmware asset. - FirmwareAsset AssetType = "firmware" -) - // Asset represents a virtcontainers asset. type Asset struct { path string @@ -83,19 +117,10 @@ func (a *Asset) Valid() bool { return false } - switch a.kind { - case KernelAsset: - return true - case ImageAsset: - return true - case InitrdAsset: - return true - case HypervisorAsset: - return true - case JailerAsset: - return true - case FirmwareAsset: - return true + for _, at := range AssetTypes() { + if at == a.kind { + return true + } } return false diff --git a/virtcontainers/virtcontainers_test.go b/virtcontainers/virtcontainers_test.go index 3d10476210..08eef22468 100644 --- a/virtcontainers/virtcontainers_test.go +++ b/virtcontainers/virtcontainers_test.go @@ -29,6 +29,8 @@ const testKernel = "kernel" const testInitrd = "initrd" const testImage = "image" const testHypervisor = "hypervisor" +const testJailer = "jailer" +const testFirmware = "firmware" const testVirtiofsd = "virtiofsd" const testHypervisorCtl = "hypervisorctl" const testBundle = "bundle"