From 613fd0fb60a4a88da24012b7a422a30a3f30271d Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 22 Nov 2019 14:56:18 +0000 Subject: [PATCH] virtcontainers: rename GetOCISpec to GetPatchedOCISpec GetOCISpec returns a patched version of the original OCI spec, it was modified to support: * capabilities * Ephemeral storage * k8s empty dir In order to avoid consusions and make api clear, rename GetOCISpec to GetPatchedOCISpec and ContainerConfig.Spec to ContainerConfig.CustomSpec fixes #2252 Signed-off-by: Julio Montes --- virtcontainers/api.go | 2 +- virtcontainers/api_test.go | 4 +-- virtcontainers/cgroups_test.go | 4 +-- virtcontainers/container.go | 18 ++++++++++---- virtcontainers/kata_agent.go | 4 +-- virtcontainers/kata_agent_test.go | 2 +- virtcontainers/pkg/oci/utils.go | 5 +++- virtcontainers/pkg/oci/utils_test.go | 2 +- virtcontainers/sandbox.go | 37 +++++++++++----------------- virtcontainers/sandbox_test.go | 6 ++--- 10 files changed, 43 insertions(+), 41 deletions(-) diff --git a/virtcontainers/api.go b/virtcontainers/api.go index acd73a4480..3af197302b 100644 --- a/virtcontainers/api.go +++ b/virtcontainers/api.go @@ -620,7 +620,7 @@ func statusContainer(sandbox *Sandbox, containerID string) (ContainerStatus, err PID: container.process.Pid, StartTime: container.process.StartTime, RootFs: container.config.RootFs.Target, - Spec: container.GetOCISpec(), + Spec: container.GetPatchedOCISpec(), Annotations: container.config.Annotations, }, nil } diff --git a/virtcontainers/api_test.go b/virtcontainers/api_test.go index a190c5409f..3e883f0f86 100644 --- a/virtcontainers/api_test.go +++ b/virtcontainers/api_test.go @@ -81,7 +81,7 @@ func newTestSandboxConfigNoop() SandboxConfig { RootFs: RootFs{Target: bundlePath, Mounted: true}, Cmd: newBasicTestCmd(), Annotations: containerAnnotations, - Spec: emptySpec, + CustomSpec: emptySpec, } // Sets the hypervisor configuration. @@ -717,7 +717,7 @@ func newTestContainerConfigNoop(contID string) ContainerConfig { RootFs: RootFs{Target: filepath.Join(testDir, testBundle), Mounted: true}, Cmd: newBasicTestCmd(), Annotations: containerAnnotations, - Spec: newEmptySpec(), + CustomSpec: newEmptySpec(), } return container diff --git a/virtcontainers/cgroups_test.go b/virtcontainers/cgroups_test.go index d8eeace586..6fb73c4845 100644 --- a/virtcontainers/cgroups_test.go +++ b/virtcontainers/cgroups_test.go @@ -175,7 +175,7 @@ func TestUpdateCgroups(t *testing.T) { }, config: &ContainerConfig{ Annotations: containerAnnotations, - Spec: newEmptySpec(), + CustomSpec: newEmptySpec(), }, }, "xyz": { @@ -184,7 +184,7 @@ func TestUpdateCgroups(t *testing.T) { }, config: &ContainerConfig{ Annotations: containerAnnotations, - Spec: newEmptySpec(), + CustomSpec: newEmptySpec(), }, }, } diff --git a/virtcontainers/container.go b/virtcontainers/container.go index c890de9afb..0c8c184260 100644 --- a/virtcontainers/container.go +++ b/virtcontainers/container.go @@ -252,7 +252,7 @@ type ContainerConfig struct { Resources specs.LinuxResources // Raw OCI specification, it won't be saved to disk. - Spec *specs.Spec `json:"-"` + CustomSpec *specs.Spec `json:"-"` } // valid checks that the container configuration is valid. @@ -406,9 +406,17 @@ func (c *Container) GetAnnotations() map[string]string { return c.config.Annotations } -// GetOCISpec returns container's OCI specification -func (c *Container) GetOCISpec() *specs.Spec { - return c.config.Spec +// GetPatchedOCISpec returns container's OCI specification +// This OCI specification was patched when the sandbox was created +// by containerCapabilities(), SetEphemeralStorageType() and others +// in order to support: +// * capabilities +// * Ephemeral storage +// * k8s empty dir +// If you need the original (vanilla) OCI spec, +// use compatoci.GetContainerSpec() instead. +func (c *Container) GetPatchedOCISpec() *specs.Spec { + return c.config.CustomSpec } // storeContainer stores a container config. @@ -1469,7 +1477,7 @@ func (c *Container) detachDevices() error { // cgroupsCreate creates cgroups on the host for the associated container func (c *Container) cgroupsCreate() (err error) { - spec := c.GetOCISpec() + spec := c.GetPatchedOCISpec() if spec == nil { return errorMissingOCISpec } diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 781e0c6d6e..161ebe3fe5 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -746,7 +746,7 @@ func (k *kataAgent) setProxyFromGrpc(proxy proxy, pid int, url string) { } func (k *kataAgent) getDNS(sandbox *Sandbox) ([]string, error) { - ociSpec := sandbox.GetOCISpec() + ociSpec := sandbox.GetPatchedOCISpec() if ociSpec == nil { k.Logger().Debug("Sandbox OCI spec not found. Sandbox DNS will not be set.") return nil, nil @@ -1283,7 +1283,7 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process, ctrStorages = append(ctrStorages, rootfs) } - ociSpec := c.GetOCISpec() + ociSpec := c.GetPatchedOCISpec() if ociSpec == nil { return nil, errorMissingOCISpec } diff --git a/virtcontainers/kata_agent_test.go b/virtcontainers/kata_agent_test.go index b25e06c143..7f5e99d4e0 100644 --- a/virtcontainers/kata_agent_test.go +++ b/virtcontainers/kata_agent_test.go @@ -728,7 +728,7 @@ func TestAgentCreateContainer(t *testing.T) { Fstype: "xfs", }, config: &ContainerConfig{ - Spec: &specs.Spec{}, + CustomSpec: &specs.Spec{}, Annotations: map[string]string{}, }, } diff --git a/virtcontainers/pkg/oci/utils.go b/virtcontainers/pkg/oci/utils.go index 6619753e44..51db648be0 100644 --- a/virtcontainers/pkg/oci/utils.go +++ b/virtcontainers/pkg/oci/utils.go @@ -861,7 +861,10 @@ func ContainerConfig(ocispec specs.Spec, bundlePath, cid, console string, detach Mounts: containerMounts(ocispec), DeviceInfos: deviceInfos, Resources: *ocispec.Linux.Resources, - Spec: &ocispec, + + // This is a custom OCI spec modified at SetEphemeralStorageType() + // to support ephemeral storage and k8s empty dir. + CustomSpec: &ocispec, } cType, err := ContainerType(ocispec) diff --git a/virtcontainers/pkg/oci/utils_test.go b/virtcontainers/pkg/oci/utils_test.go index dc64051cbb..5bd09b7219 100644 --- a/virtcontainers/pkg/oci/utils_test.go +++ b/virtcontainers/pkg/oci/utils_test.go @@ -156,7 +156,7 @@ func TestMinimalSandboxConfig(t *testing.T) { Resources: specs.LinuxResources{Devices: []specs.LinuxDeviceCgroup{ {Allow: false, Type: "", Major: (*int64)(nil), Minor: (*int64)(nil), Access: "rwm"}, }}, - Spec: &spec, + CustomSpec: &spec, } expectedNetworkConfig := vc.NetworkConfig{} diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 4d52220910..1dbfdeefd3 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -1130,7 +1130,7 @@ func (s *Sandbox) fetchContainers() error { if err != nil { return err } - contConfig.Spec = &spec + contConfig.CustomSpec = &spec s.config.Containers[i] = contConfig c, err := newContainer(s, &s.config.Containers[i]) @@ -2213,7 +2213,7 @@ func (s *Sandbox) cpuResources() *specs.LinuxCPU { // setupSandboxCgroup creates and joins sandbox cgroups for the sandbox config func (s *Sandbox) setupSandboxCgroup() error { - spec := s.GetOCISpec() + spec := s.GetPatchedOCISpec() if spec == nil { return errorMissingOCISpec @@ -2242,9 +2242,16 @@ func (s *Sandbox) setupSandboxCgroup() error { return nil } -func (s *Sandbox) sandboxContConf() *ContainerConfig { - var podSandboxConfig *ContainerConfig - +// GetPatchedOCISpec returns sandbox's OCI specification +// This OCI specification was patched when the sandbox was created +// by containerCapabilities(), SetEphemeralStorageType() and others +// in order to support: +// * capabilities +// * Ephemeral storage +// * k8s empty dir +// If you need the original (vanilla) OCI spec, +// use compatoci.GetContainerSpec() instead. +func (s *Sandbox) GetPatchedOCISpec() *specs.Spec { if s.config == nil { return nil } @@ -2254,25 +2261,9 @@ func (s *Sandbox) sandboxContConf() *ContainerConfig { // cgroup path from this container. for _, cConfig := range s.config.Containers { if cConfig.Annotations[annotations.ContainerTypeKey] == string(PodSandbox) { - podSandboxConfig = &cConfig - break + return cConfig.CustomSpec } } - if podSandboxConfig == nil { - return nil - } - - return podSandboxConfig -} - -// GetOCISpec returns sandbox's OCI specification -func (s *Sandbox) GetOCISpec() *specs.Spec { - conf := s.sandboxContConf() - if conf == nil { - return nil - } - - // First container is sandbox container as default - return conf.Spec + return nil } diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index e6d38a310b..08d43449d7 100644 --- a/virtcontainers/sandbox_test.go +++ b/virtcontainers/sandbox_test.go @@ -649,7 +649,7 @@ func TestContainerStateSetFstype(t *testing.T) { { ID: "100", Annotations: containerAnnotations, - Spec: newEmptySpec(), + CustomSpec: newEmptySpec(), }, } @@ -1524,7 +1524,7 @@ func TestSandbox_SetupSandboxCgroup(t *testing.T) { sandboxContainer.Annotations[annotations.ContainerTypeKey] = string(PodSandbox) emptyJSONLinux := ContainerConfig{ - Spec: newEmptySpec(), + CustomSpec: newEmptySpec(), } emptyJSONLinux.Annotations = make(map[string]string) emptyJSONLinux.Annotations[annotations.ContainerTypeKey] = string(PodSandbox) @@ -1532,7 +1532,7 @@ func TestSandbox_SetupSandboxCgroup(t *testing.T) { cloneSpec1 := newEmptySpec() cloneSpec1.Linux.CgroupsPath = "/myRuntime/myContainer" successfulContainer := ContainerConfig{ - Spec: cloneSpec1, + CustomSpec: cloneSpec1, } successfulContainer.Annotations = make(map[string]string) successfulContainer.Annotations[annotations.ContainerTypeKey] = string(PodSandbox)