utils: update container type handling

Today we assume that if the CRI/upper layer doesn't provide a container
type annotation, it should be treated as a sandbox. Up to this point, a
sandbox with a pause container in CRI context and a single container
(ala ctr run) are treated the same.

For VM sizing and container constraining, it'll be useful to know if
this is a sandbox or if this is a single container.

In updating this, we cleanup the type handling tests and we update the
containerd annotations vendoring.

Fixes: #2926

Signed-off-by: Eric Ernst <eric_ernst@apple.com>

src/runtime/pkg/containerd-shim-v2/create.go

@@ -70,7 +70,7 @@ func create(ctx context.Context, s *service, r *taskAPI.CreateTaskRequest) (*con
 	rootfs := filepath.Join(r.Bundle, "rootfs")
 
 	switch containerType {
-	case vc.PodSandbox:
+	case vc.PodSandbox, vc.SingleContainer:
 		if s.sandbox != nil {
 			return nil, fmt.Errorf("cannot create another sandbox in sandbox: %s", s.sandbox.ID())
 		}

src/runtime/pkg/containerd-shim-v2/service.go

@@ -345,7 +345,7 @@ func (s *service) Cleanup(ctx context.Context) (_ *taskAPI.DeleteResponse, err e
 	}
 
 	switch containerType {
-	case vc.PodSandbox:
+	case vc.PodSandbox, vc.SingleContainer:
 		err = cleanupContainer(spanCtx, s.id, s.id, path)
 		if err != nil {
 			return nil, err

src/runtime/pkg/oci/utils.go

@@ -17,7 +17,7 @@ import (
 	"strings"
 	"syscall"
 
-	criContainerdAnnotations "github.com/containerd/cri-containerd/pkg/annotations"
+	ctrAnnotations "github.com/containerd/containerd/pkg/cri/annotations"
 	crioAnnotations "github.com/cri-o/cri-o/pkg/annotations"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/sirupsen/logrus"
@@ -42,19 +42,19 @@ var (
 
 	// CRIContainerTypeKeyList lists all the CRI keys that could define
 	// the container type from annotations in the config.json.
-	CRIContainerTypeKeyList = []string{criContainerdAnnotations.ContainerType, crioAnnotations.ContainerType, dockershimAnnotations.ContainerTypeLabelKey}
+	CRIContainerTypeKeyList = []string{ctrAnnotations.ContainerType, crioAnnotations.ContainerType, dockershimAnnotations.ContainerTypeLabelKey}
 
 	// CRISandboxNameKeyList lists all the CRI keys that could define
 	// the sandbox ID (sandbox ID) from annotations in the config.json.
-	CRISandboxNameKeyList = []string{criContainerdAnnotations.SandboxID, crioAnnotations.SandboxID, dockershimAnnotations.SandboxIDLabelKey}
+	CRISandboxNameKeyList = []string{ctrAnnotations.SandboxID, crioAnnotations.SandboxID, dockershimAnnotations.SandboxIDLabelKey}
 
 	// CRIContainerTypeList lists all the maps from CRI ContainerTypes annotations
 	// to a virtcontainers ContainerType.
 	CRIContainerTypeList = []annotationContainerType{
 		{crioAnnotations.ContainerTypeSandbox, vc.PodSandbox},
 		{crioAnnotations.ContainerTypeContainer, vc.PodContainer},
-		{criContainerdAnnotations.ContainerTypeSandbox, vc.PodSandbox},
-		{criContainerdAnnotations.ContainerTypeContainer, vc.PodContainer},
+		{ctrAnnotations.ContainerTypeSandbox, vc.PodSandbox},
+		{ctrAnnotations.ContainerTypeContainer, vc.PodContainer},
 		{dockershimAnnotations.ContainerTypeLabelSandbox, vc.PodSandbox},
 		{dockershimAnnotations.ContainerTypeLabelContainer, vc.PodContainer},
 	}
@@ -317,7 +317,7 @@ func networkConfig(ocispec specs.Spec, config RuntimeConfig) (vc.NetworkConfig,
 }
 
 // ContainerType returns the type of container and if the container type was
-// found from CRI servers annotations.
+// found from CRI server's annotations in the container spec.
 func ContainerType(spec specs.Spec) (vc.ContainerType, error) {
 	for _, key := range CRIContainerTypeKeyList {
 		containerTypeVal, ok := spec.Annotations[key]
@@ -331,11 +331,10 @@ func ContainerType(spec specs.Spec) (vc.ContainerType, error) {
 			}
 
 		}
-
 		return vc.UnknownContainerType, fmt.Errorf("Unknown container type %s", containerTypeVal)
 	}
 
-	return vc.PodSandbox, nil
+	return vc.SingleContainer, nil
 }
 
 func GetSandboxConfigPath(annotations map[string]string) string {

src/runtime/pkg/oci/utils_test.go

@@ -16,7 +16,7 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/cri-o/cri-o/pkg/annotations"
+	ctrAnnotations "github.com/containerd/containerd/pkg/cri/annotations"
 	crioAnnotations "github.com/cri-o/cri-o/pkg/annotations"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/stretchr/testify/assert"
@@ -25,6 +25,7 @@ import (
 	vc "github.com/kata-containers/kata-containers/src/runtime/virtcontainers"
 	"github.com/kata-containers/kata-containers/src/runtime/virtcontainers/device/config"
 	vcAnnotations "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/annotations"
+	dockerAnnotations "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/annotations/dockershim"
 	"github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/compatoci"
 	"github.com/kata-containers/kata-containers/src/runtime/virtcontainers/types"
 )
@@ -152,7 +153,7 @@ func TestMinimalSandboxConfig(t *testing.T) {
 		Cmd:            expectedCmd,
 		Annotations: map[string]string{
 			vcAnnotations.BundlePathKey:    tempBundlePath,
-			vcAnnotations.ContainerTypeKey: string(vc.PodSandbox),
+			vcAnnotations.ContainerTypeKey: string(vc.SingleContainer),
 		},
 		Mounts:      expectedMounts,
 		DeviceInfos: expectedDeviceInfo,
@@ -188,51 +189,100 @@ func TestMinimalSandboxConfig(t *testing.T) {
 	assert.NoError(os.Remove(configPath))
 }
 
-func testContainerTypeSuccessful(t *testing.T, ociSpec specs.Spec, expected vc.ContainerType) {
-	containerType, err := ContainerType(ociSpec)
+func TestContainerType(t *testing.T) {
 	assert := assert.New(t)
-
-	assert.NoError(err)
-	assert.Equal(containerType, expected)
-}
-
-func TestContainerTypePodSandbox(t *testing.T) {
-	var ociSpec specs.Spec
-
-	ociSpec.Annotations = map[string]string{
-		annotations.ContainerType: annotations.ContainerTypeSandbox,
+	tests := []struct {
+		description     string
+		annotationKey   string
+		annotationValue string
+		expectedType    vc.ContainerType
+		expectedErr     bool
+	}{
+		{
+			description:     "no annotation, expect single container",
+			annotationKey:   "",
+			annotationValue: "",
+			expectedType:    vc.SingleContainer,
+			expectedErr:     false,
+		},
+		{
+			description:     "unexpected annotation, expect error",
+			annotationKey:   ctrAnnotations.ContainerType,
+			annotationValue: "foo",
+			expectedType:    vc.UnknownContainerType,
+			expectedErr:     true,
+		},
+		{
+			description:     "containerd sandbox",
+			annotationKey:   ctrAnnotations.ContainerType,
+			annotationValue: string(ctrAnnotations.ContainerTypeSandbox),
+			expectedType:    vc.PodSandbox,
+			expectedErr:     false,
+		},
+		{
+			description:     "containerd container",
+			annotationKey:   ctrAnnotations.ContainerType,
+			annotationValue: string(ctrAnnotations.ContainerTypeContainer),
+			expectedType:    vc.PodContainer,
+			expectedErr:     false,
+		},
+		{
+			description:     "crio unexpected annotation, expect error",
+			annotationKey:   crioAnnotations.ContainerType,
+			annotationValue: "foo",
+			expectedType:    vc.UnknownContainerType,
+			expectedErr:     true,
+		},
+		{
+			description:     "crio sandbox",
+			annotationKey:   crioAnnotations.ContainerType,
+			annotationValue: string(crioAnnotations.ContainerTypeSandbox),
+			expectedType:    vc.PodSandbox,
+			expectedErr:     false,
+		},
+		{
+			description:     "crio container",
+			annotationKey:   crioAnnotations.ContainerType,
+			annotationValue: string(crioAnnotations.ContainerTypeContainer),
+			expectedType:    vc.PodContainer,
+			expectedErr:     false,
+		},
+		{
+			description:     "dockershim unexpected annotation, expect error",
+			annotationKey:   dockerAnnotations.ContainerTypeLabelKey,
+			annotationValue: "foo",
+			expectedType:    vc.UnknownContainerType,
+			expectedErr:     true,
+		},
+		{
+			description:     "dockershim sandbox",
+			annotationKey:   dockerAnnotations.ContainerTypeLabelKey,
+			annotationValue: string(dockerAnnotations.ContainerTypeLabelSandbox),
+			expectedType:    vc.PodSandbox,
+			expectedErr:     false,
+		},
+		{
+			description:     "dockershim container",
+			annotationKey:   dockerAnnotations.ContainerTypeLabelKey,
+			annotationValue: string(dockerAnnotations.ContainerTypeLabelContainer),
+			expectedType:    vc.PodContainer,
+			expectedErr:     false,
+		},
 	}
-
-	testContainerTypeSuccessful(t, ociSpec, vc.PodSandbox)
-}
-
-func TestContainerTypePodContainer(t *testing.T) {
-	var ociSpec specs.Spec
-
-	ociSpec.Annotations = map[string]string{
-		annotations.ContainerType: annotations.ContainerTypeContainer,
+	for _, tt := range tests {
+		ociSpec := specs.Spec{
+			Annotations: map[string]string{
+				tt.annotationKey: tt.annotationValue,
+			},
 		}
-
-	testContainerTypeSuccessful(t, ociSpec, vc.PodContainer)
-}
-
-func TestContainerTypePodSandboxEmptyAnnotation(t *testing.T) {
-	testContainerTypeSuccessful(t, specs.Spec{}, vc.PodSandbox)
-}
-
-func TestContainerTypeFailure(t *testing.T) {
-	var ociSpec specs.Spec
-	expected := vc.UnknownContainerType
-	unknownType := "unknown_type"
-	assert := assert.New(t)
-
-	ociSpec.Annotations = map[string]string{
-		annotations.ContainerType: unknownType,
-	}
-
 		containerType, err := ContainerType(ociSpec)
+		if tt.expectedErr {
 			assert.Error(err)
-	assert.Equal(containerType, expected)
+		} else {
+			assert.NoError(err)
+		}
+		assert.Equal(tt.expectedType, containerType, "test fail: %v", tt.description)
+	}
 }
 
 func TestSandboxIDSuccessful(t *testing.T) {
@@ -241,7 +291,7 @@ func TestSandboxIDSuccessful(t *testing.T) {
 	assert := assert.New(t)
 
 	ociSpec.Annotations = map[string]string{
-		annotations.SandboxID: testSandboxID,
+		crioAnnotations.SandboxID: testSandboxID,
 	}
 
 	sandboxID, err := SandboxID(ociSpec)

src/runtime/vendor/github.com/containerd/containerd/pkg/cri/annotations/annotations.go

@@ -0,0 +1,62 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package annotations
+
+// ContainerType values
+// Following OCI annotations are used by katacontainers now.
+// We'll switch to standard secure pod API after it is defined in CRI.
+const (
+	// ContainerTypeSandbox represents a pod sandbox container
+	ContainerTypeSandbox = "sandbox"
+
+	// ContainerTypeContainer represents a container running within a pod
+	ContainerTypeContainer = "container"
+
+	// ContainerType is the container type (sandbox or container) annotation
+	ContainerType = "io.kubernetes.cri.container-type"
+
+	// SandboxID is the sandbox ID annotation
+	SandboxID = "io.kubernetes.cri.sandbox-id"
+
+	// SandboxLogDir is the pod log directory annotation.
+	// If the sandbox needs to generate any log, it will put it into this directory.
+	// Kubelet will be responsible for:
+	// 1) Monitoring the disk usage of the log, and including it as part of the pod
+	// ephemeral storage usage.
+	// 2) Cleaning up the logs when the pod is deleted.
+	// NOTE: Kubelet is not responsible for rotating the logs.
+	SandboxLogDir = "io.kubernetes.cri.sandbox-log-directory"
+
+	// UntrustedWorkload is the sandbox annotation for untrusted workload. Untrusted
+	// workload can only run on dedicated runtime for untrusted workload.
+	UntrustedWorkload = "io.kubernetes.cri.untrusted-workload"
+
+	// SandboxNamespace is the name of the namespace of the sandbox (pod)
+	SandboxNamespace = "io.kubernetes.cri.sandbox-namespace"
+
+	// SandboxName is the name of the sandbox (pod)
+	SandboxName = "io.kubernetes.cri.sandbox-name"
+
+	// ContainerName is the name of the container in the pod
+	ContainerName = "io.kubernetes.cri.container-name"
+
+	// ImageName is the name of the image used to create the container
+	ImageName = "io.kubernetes.cri.image-name"
+
+	// PodAnnotations are the annotations of the pod
+	PodAnnotations = "io.kubernetes.cri.pod-annotations"
+)

src/runtime/vendor/github.com/containerd/cri-containerd/pkg/annotations/annotations.go

@@ -1,38 +0,0 @@
-/*
-Copyright 2018 The Containerd Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package annotations
-
-// ContainerType values
-// Following OCI annotations are used by katacontainers now.
-// We'll switch to standard secure pod API after it is defined in CRI.
-const (
-	// ContainerTypeSandbox represents a pod sandbox container
-	ContainerTypeSandbox = "sandbox"
-
-	// ContainerTypeContainer represents a container running within a pod
-	ContainerTypeContainer = "container"
-
-	// ContainerType is the container type (sandbox or container) annotation
-	ContainerType = "io.kubernetes.cri.container-type"
-
-	// SandboxID is the sandbox ID annotation
-	SandboxID = "io.kubernetes.cri.sandbox-id"
-
-	// UntrustedWorkload is the sandbox annotation for untrusted workload. Untrusted
-	// workload can only run on dedicated runtime for untrusted workload.
-	UntrustedWorkload = "io.kubernetes.cri.untrusted-workload"
-)

src/runtime/vendor/modules.txt

@@ -71,6 +71,7 @@ github.com/containerd/containerd/identifiers
 github.com/containerd/containerd/log
 github.com/containerd/containerd/mount
 github.com/containerd/containerd/namespaces
+github.com/containerd/containerd/pkg/cri/annotations
 github.com/containerd/containerd/pkg/dialer
 github.com/containerd/containerd/pkg/runtimeoptions/v1
 github.com/containerd/containerd/pkg/ttrpcutil
@@ -85,7 +86,6 @@ github.com/containerd/containerd/sys/reaper
 github.com/containerd/containerd/version
 # github.com/containerd/cri-containerd v1.11.1-0.20190125013620-4dd6735020f5
 ## explicit
-github.com/containerd/cri-containerd/pkg/annotations
 github.com/containerd/cri-containerd/pkg/api/runtimeoptions/v1
 # github.com/containerd/fifo v1.0.0
 ## explicit

src/runtime/virtcontainers/types.go

@@ -10,13 +10,30 @@ type ContainerType string
 
 // List different types of containers
 const (
+	// PodContainer identifies a container that should be associated with an existing pod
 	PodContainer ContainerType = "pod_container"
+	// PodSandbox identifies an infra container that will be used to create the pod
 	PodSandbox ContainerType = "pod_sandbox"
+	// SingleContainer is utilized to describe a container that didn't have a container/sandbox
+	// annotation applied. This is expected when dealing with non-pod container (ie, running
+	// from ctr, podman, etc).
+	SingleContainer ContainerType = "single_container"
+	// UnknownContainerType specifies a container that provides container type annotation, but
+	// it is unknown.
 	UnknownContainerType ContainerType = "unknown_container_type"
 )
 
 // IsSandbox determines if the container type can be considered as a sandbox.
-// We can consider a sandbox in case we have a PodSandbox or a RegularContainer.
+// We can consider a sandbox in case we have a PodSandbox or a "regular" container
 func (cType ContainerType) IsSandbox() bool {
-	return cType == PodSandbox
+	return cType == PodSandbox || cType == SingleContainer
+}
+
+func (t ContainerType) IsCriSandbox() bool {
+	return t == PodSandbox
+}
+
+// "Regular" Container
+func (t ContainerType) IsSingleContainer() bool {
+	return t == SingleContainer
 }