Merge pull request #32835 from yujuhong/docker_annotations

pkg/kubelet/api/v1alpha1/runtime/api.proto

@@ -301,6 +301,9 @@ message PodSandbox {
     optional int64 created_at = 4;
     // The labels of the PodSandbox
     map<string, string> labels = 5;
+    // Annotations is an unstructured key value map that may be set by external
+    // tools to store and retrieve arbitrary metadata.
+    map<string, string> annotations = 6;
 }
 
 message ListPodSandboxResponse {

pkg/kubelet/dockershim/convert.go

@@ -55,14 +55,15 @@ func toRuntimeAPIContainer(c *dockertypes.Container) (*runtimeApi.Container, err
 	if err != nil {
 		return nil, err
 	}
+	labels, annotations := extractLabels(c.Labels)
 	return &runtimeApi.Container{
-		Id:       &c.ID,
-		Metadata: metadata,
-		Image:    &runtimeApi.ImageSpec{Image: &c.Image},
-		ImageRef: &c.ImageID,
-		State:    &state,
-		// TODO: Extract annotations from labels.
-		Labels: c.Labels,
+		Id:          &c.ID,
+		Metadata:    metadata,
+		Image:       &runtimeApi.ImageSpec{Image: &c.Image},
+		ImageRef:    &c.ImageID,
+		State:       &state,
+		Labels:      labels,
+		Annotations: annotations,
 	}, nil
 }
 
@@ -113,11 +114,13 @@ func toRuntimeAPISandbox(c *dockertypes.Container) (*runtimeApi.PodSandbox, erro
 	if err != nil {
 		return nil, err
 	}
+	labels, annotations := extractLabels(c.Labels)
 	return &runtimeApi.PodSandbox{
-		Id:        &c.ID,
-		Metadata:  metadata,
-		State:     &state,
-		CreatedAt: &c.Created,
-		Labels:    c.Labels, // TODO: Need to disthinguish annotaions and labels.
+		Id:          &c.ID,
+		Metadata:    metadata,
+		State:       &state,
+		CreatedAt:   &c.Created,
+		Labels:      labels,
+		Annotations: annotations,
 	}, nil
 }

pkg/kubelet/dockershim/docker_container.go

@@ -37,6 +37,8 @@ func (ds *dockerService) ListContainers(filter *runtimeApi.ContainerFilter) ([]*
 
 	opts.Filter = dockerfilters.NewArgs()
 	f := newDockerFilter(&opts.Filter)
+	// Add filter to get *only* (non-sandbox) containers.
+	f.AddLabel(containerTypeLabelKey, containerTypeLabelContainer)
 
 	if filter != nil {
 		if filter.Id != nil {
@@ -46,7 +48,7 @@ func (ds *dockerService) ListContainers(filter *runtimeApi.ContainerFilter) ([]*
 			f.Add("status", toDockerContainerStatus(filter.GetState()))
 		}
 		if filter.PodSandboxId != nil {
-			// TODO: implement this after sandbox functions are implemented.
+			f.AddLabel(sandboxIDLabelKey, *filter.PodSandboxId)
 		}
 
 		if filter.LabelSelector != nil {
@@ -54,8 +56,6 @@ func (ds *dockerService) ListContainers(filter *runtimeApi.ContainerFilter) ([]*
 				f.AddLabel(k, v)
 			}
 		}
-		// Filter out sandbox containers.
-		f.AddLabel(containerTypeLabelKey, containerTypeLabelContainer)
 	}
 	containers, err := ds.client.ListContainers(opts)
 	if err != nil {
@@ -88,12 +88,11 @@ func (ds *dockerService) CreateContainer(podSandboxID string, config *runtimeApi
 		return "", fmt.Errorf("sandbox config is nil for container %q", config.Metadata.GetName())
 	}
 
-	// Merge annotations and labels because docker supports only labels.
-	// TODO: add a prefix to annotations so that we can distinguish labels and
-	// annotations when reading back them from the docker container.
 	labels := makeLabels(config.GetLabels(), config.GetAnnotations())
 	// Apply a the container type label.
 	labels[containerTypeLabelKey] = containerTypeLabelContainer
+	// Write the sandbox ID in the labels.
+	labels[sandboxIDLabelKey] = podSandboxID
 
 	image := ""
 	if iSpec := config.GetImage(); iSpec != nil {
@@ -284,21 +283,21 @@ func (ds *dockerService) ContainerStatus(containerID string) (*runtimeApi.Contai
 		return nil, err
 	}
 
+	labels, annotations := extractLabels(r.Config.Labels)
 	return &runtimeApi.ContainerStatus{
-		Id:         &r.ID,
-		Metadata:   metadata,
-		Image:      &runtimeApi.ImageSpec{Image: &r.Config.Image},
-		ImageRef:   &r.Image,
-		Mounts:     mounts,
-		ExitCode:   &exitCode,
-		State:      &state,
-		CreatedAt:  &ct,
-		StartedAt:  &st,
-		FinishedAt: &ft,
-		Reason:     &reason,
-		// TODO: We write annotations as labels on the docker containers. All
-		// these annotations will be read back as labels. Need to fix this.
-		Labels: r.Config.Labels,
+		Id:          &r.ID,
+		Metadata:    metadata,
+		Image:       &runtimeApi.ImageSpec{Image: &r.Config.Image},
+		ImageRef:    &r.Image,
+		Mounts:      mounts,
+		ExitCode:    &exitCode,
+		State:       &state,
+		CreatedAt:   &ct,
+		StartedAt:   &st,
+		FinishedAt:  &ft,
+		Reason:      &reason,
+		Labels:      labels,
+		Annotations: annotations,
 	}, nil
 }
 

pkg/kubelet/dockershim/docker_container_test.go

@@ -27,13 +27,15 @@ import (
 )
 
 // A helper to create a basic config.
-func makeContainerConfig(sConfig *runtimeApi.PodSandboxConfig, name, image string, attempt uint32) *runtimeApi.ContainerConfig {
+func makeContainerConfig(sConfig *runtimeApi.PodSandboxConfig, name, image string, attempt uint32, labels, annotations map[string]string) *runtimeApi.ContainerConfig {
 	return &runtimeApi.ContainerConfig{
 		Metadata: &runtimeApi.ContainerMetadata{
 			Name:    &name,
 			Attempt: &attempt,
 		},
-		Image: &runtimeApi.ImageSpec{Image: &image},
+		Image:       &runtimeApi.ImageSpec{Image: &image},
+		Labels:      labels,
+		Annotations: annotations,
 	}
 }
 
@@ -49,8 +51,10 @@ func TestListContainers(t *testing.T) {
 	for i := 0; i < 3; i++ {
 		s := makeSandboxConfig(fmt.Sprintf("%s%d", podName, i),
 			fmt.Sprintf("%s%d", namespace, i), fmt.Sprintf("%d", i), 0)
+		labels := map[string]string{"abc.xyz": fmt.Sprintf("label%d", i)}
+		annotations := map[string]string{"foo.bar.baz": fmt.Sprintf("annotaion%d", i)}
 		c := makeContainerConfig(s, fmt.Sprintf("%s%d", containerName, i),
-			fmt.Sprintf("%s:v%d", image, i), uint32(i))
+			fmt.Sprintf("%s:v%d", image, i), uint32(i), labels, annotations)
 		sConfigs = append(sConfigs, s)
 		configs = append(configs, c)
 	}
@@ -69,12 +73,13 @@ func TestListContainers(t *testing.T) {
 		// Prepend to the expected list because ListContainers returns
 		// the most recent containers first.
 		expected = append([]*runtimeApi.Container{{
-			Metadata: configs[i].Metadata,
-			Id:       &id,
-			State:    &state,
-			Image:    configs[i].Image,
-			ImageRef: &imageRef,
-			Labels:   map[string]string{containerTypeLabelKey: containerTypeLabelContainer},
+			Metadata:    configs[i].Metadata,
+			Id:          &id,
+			State:       &state,
+			Image:       configs[i].Image,
+			ImageRef:    &imageRef,
+			Labels:      configs[i].Labels,
+			Annotations: configs[i].Annotations,
 		}}, expected...)
 	}
 	containers, err := ds.ListContainers(nil)
@@ -88,7 +93,9 @@ func TestListContainers(t *testing.T) {
 func TestContainerStatus(t *testing.T) {
 	ds, _, fClock := newTestDockerSevice()
 	sConfig := makeSandboxConfig("foo", "bar", "1", 0)
-	config := makeContainerConfig(sConfig, "pause", "iamimage", 0)
+	labels := map[string]string{"abc.xyz": "foo"}
+	annotations := map[string]string{"foo.bar.baz": "abc"}
+	config := makeContainerConfig(sConfig, "pause", "iamimage", 0, labels, annotations)
 
 	var defaultTime time.Time
 	dt := defaultTime.Unix()
@@ -100,17 +107,18 @@ func TestContainerStatus(t *testing.T) {
 	reason := ""
 
 	expected := &runtimeApi.ContainerStatus{
-		State:      &state,
-		CreatedAt:  &ct,
-		StartedAt:  &st,
-		FinishedAt: &ft,
-		Metadata:   config.Metadata,
-		Image:      config.Image,
-		ImageRef:   &imageRef,
-		ExitCode:   &exitCode,
-		Reason:     &reason,
-		Mounts:     []*runtimeApi.Mount{},
-		Labels:     map[string]string{containerTypeLabelKey: containerTypeLabelContainer},
+		State:       &state,
+		CreatedAt:   &ct,
+		StartedAt:   &st,
+		FinishedAt:  &ft,
+		Metadata:    config.Metadata,
+		Image:       config.Image,
+		ImageRef:    &imageRef,
+		ExitCode:    &exitCode,
+		Reason:      &reason,
+		Mounts:      []*runtimeApi.Mount{},
+		Labels:      config.Labels,
+		Annotations: config.Annotations,
 	}
 
 	// Create the container.

pkg/kubelet/dockershim/docker_sandbox.go

@@ -123,17 +123,16 @@ func (ds *dockerService) PodSandboxStatus(podSandboxID string) (*runtimeApi.PodS
 		return nil, err
 	}
 
+	labels, annotations := extractLabels(r.Config.Labels)
 	return &runtimeApi.PodSandboxStatus{
-		Id:        &r.ID,
-		State:     &state,
-		CreatedAt: &ct,
-		Metadata:  metadata,
-		// TODO: We write annotations as labels on the docker containers. All
-		// these annotations will be read back as labels. Need to fix this.
-		// Also filter out labels only relevant to this shim.
-		Labels:  r.Config.Labels,
-		Network: network,
-		Linux:   &runtimeApi.LinuxPodSandboxStatus{Namespaces: &runtimeApi.Namespace{Network: &netNS}},
+		Id:          &r.ID,
+		State:       &state,
+		CreatedAt:   &ct,
+		Metadata:    metadata,
+		Labels:      labels,
+		Annotations: annotations,
+		Network:     network,
+		Linux:       &runtimeApi.LinuxPodSandboxStatus{Namespaces: &runtimeApi.Namespace{Network: &netNS}},
 	}, nil
 }
 
@@ -145,6 +144,9 @@ func (ds *dockerService) ListPodSandbox(filter *runtimeApi.PodSandboxFilter) ([]
 
 	opts.Filter = dockerfilters.NewArgs()
 	f := newDockerFilter(&opts.Filter)
+	// Add filter to select only sandbox containers.
+	f.AddLabel(containerTypeLabelKey, containerTypeLabelSandbox)
+
 	if filter != nil {
 		if filter.Id != nil {
 			f.Add("id", filter.GetId())
@@ -168,8 +170,6 @@ func (ds *dockerService) ListPodSandbox(filter *runtimeApi.PodSandboxFilter) ([]
 				f.AddLabel(k, v)
 			}
 		}
-		// Filter out sandbox containers.
-		f.AddLabel(containerTypeLabelKey, containerTypeLabelSandbox)
 	}
 	containers, err := ds.client.ListContainers(opts)
 	if err != nil {

pkg/kubelet/dockershim/docker_sandbox_test.go

@@ -29,6 +29,10 @@ import (
 
 // A helper to create a basic config.
 func makeSandboxConfig(name, namespace, uid string, attempt uint32) *runtimeApi.PodSandboxConfig {
+	return makeSandboxConfigWithLabelsAndAnnotations(name, namespace, uid, attempt, map[string]string{}, map[string]string{})
+}
+
+func makeSandboxConfigWithLabelsAndAnnotations(name, namespace, uid string, attempt uint32, labels, annotations map[string]string) *runtimeApi.PodSandboxConfig {
 	return &runtimeApi.PodSandboxConfig{
 		Metadata: &runtimeApi.PodSandboxMetadata{
 			Name:      &name,
@@ -36,6 +40,8 @@ func makeSandboxConfig(name, namespace, uid string, attempt uint32) *runtimeApi.
 			Uid:       &uid,
 			Attempt:   &attempt,
 		},
+		Labels:      labels,
+		Annotations: annotations,
 	}
 }
 
@@ -46,8 +52,11 @@ func TestListSandboxes(t *testing.T) {
 	name, namespace := "foo", "bar"
 	configs := []*runtimeApi.PodSandboxConfig{}
 	for i := 0; i < 3; i++ {
-		c := makeSandboxConfig(fmt.Sprintf("%s%d", name, i),
-			fmt.Sprintf("%s%d", namespace, i), fmt.Sprintf("%d", i), 0)
+		c := makeSandboxConfigWithLabelsAndAnnotations(fmt.Sprintf("%s%d", name, i),
+			fmt.Sprintf("%s%d", namespace, i), fmt.Sprintf("%d", i), 0,
+			map[string]string{"label": fmt.Sprintf("foo%d", i)},
+			map[string]string{"annotation": fmt.Sprintf("bar%d", i)},
+		)
 		configs = append(configs, c)
 	}
 
@@ -60,11 +69,12 @@ func TestListSandboxes(t *testing.T) {
 		// Prepend to the expected list because ListPodSandbox returns
 		// the most recent sandbox first.
 		expected = append([]*runtimeApi.PodSandbox{{
-			Metadata:  configs[i].Metadata,
-			Id:        &id,
-			State:     &state,
-			Labels:    map[string]string{containerTypeLabelKey: containerTypeLabelSandbox},
-			CreatedAt: &createdAt,
+			Metadata:    configs[i].Metadata,
+			Id:          &id,
+			State:       &state,
+			CreatedAt:   &createdAt,
+			Labels:      configs[i].Labels,
+			Annotations: configs[i].Annotations,
 		}}, expected...)
 	}
 	sandboxes, err := ds.ListPodSandbox(nil)
@@ -77,7 +87,9 @@ func TestListSandboxes(t *testing.T) {
 // the status returned reflects the operations performed.
 func TestSandboxStatus(t *testing.T) {
 	ds, _, fClock := newTestDockerSevice()
-	config := makeSandboxConfig("foo", "bar", "1", 0)
+	labels := map[string]string{"label": "foobar1"}
+	annotations := map[string]string{"annotation": "abc"}
+	config := makeSandboxConfigWithLabelsAndAnnotations("foo", "bar", "1", 0, labels, annotations)
 
 	// TODO: The following variables depend on the internal
 	// implementation of FakeDockerClient, and should be fixed.
@@ -87,12 +99,13 @@ func TestSandboxStatus(t *testing.T) {
 	state := runtimeApi.PodSandBoxState_READY
 	ct := int64(0)
 	expected := &runtimeApi.PodSandboxStatus{
-		State:     &state,
-		CreatedAt: &ct,
-		Metadata:  config.Metadata,
-		Labels:    map[string]string{containerTypeLabelKey: containerTypeLabelSandbox},
-		Network:   &runtimeApi.PodSandboxNetworkStatus{Ip: &fakeIP},
-		Linux:     &runtimeApi.LinuxPodSandboxStatus{Namespaces: &runtimeApi.Namespace{Network: &fakeNS}},
+		State:       &state,
+		CreatedAt:   &ct,
+		Metadata:    config.Metadata,
+		Network:     &runtimeApi.PodSandboxNetworkStatus{Ip: &fakeIP},
+		Linux:       &runtimeApi.LinuxPodSandboxStatus{Namespaces: &runtimeApi.Namespace{Network: &fakeNS}},
+		Labels:      labels,
+		Annotations: annotations,
 	}
 
 	// Create the sandbox.

pkg/kubelet/dockershim/docker_service.go

@@ -47,8 +47,11 @@ const (
 	containerTypeLabelKey       = "io.kubernetes.docker.type"
 	containerTypeLabelSandbox   = "podsandbox"
 	containerTypeLabelContainer = "container"
+	sandboxIDLabelKey           = "io.kubernetes.sandbox.id"
 )
 
+var internalLabelKeys []string = []string{containerTypeLabelKey, sandboxIDLabelKey}
+
 func NewDockerService(client dockertools.DockerInterface) DockerLegacyService {
 	return &dockerService{
 		client: dockertools.NewInstrumentedDockerInterface(client),

pkg/kubelet/dockershim/helpers.go

@@ -30,6 +30,10 @@ import (
 	runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
 )
 
+const (
+	annotationPrefix = "annotation."
+)
+
 // apiVersion implements kubecontainer.Version interface by implementing
 // Compare() and String(). It uses the compare function of engine-api to
 // compare docker apiversions.
@@ -57,23 +61,52 @@ func generateEnvList(envs []*runtimeApi.KeyValue) (result []string) {
 	return
 }
 
-// Merge annotations and labels because docker supports only labels.
-// TODO: Need to be able to distinguish annotations from labels; otherwise, we
-// couldn't restore the information when reading the labels back from docker.
+// makeLabels converts annotations to labels and merge them with the given
+// labels. This is necessary because docker does not support annotations;
+// we *fake* annotations using labels. Note that docker labels are not
+// updatable.
 func makeLabels(labels, annotations map[string]string) map[string]string {
 	merged := make(map[string]string)
 	for k, v := range labels {
 		merged[k] = v
 	}
 	for k, v := range annotations {
-		if _, ok := merged[k]; !ok {
-			// Don't overwrite the key if it already exists.
-			merged[k] = v
-		}
+		// Assume there won't be conflict.
+		merged[fmt.Sprintf("%s%s", annotationPrefix, k)] = v
 	}
 	return merged
 }
 
+// extractLabels converts raw docker labels to the CRI labels and annotations.
+// It also filters out internal labels used by this shim.
+func extractLabels(input map[string]string) (map[string]string, map[string]string) {
+	labels := make(map[string]string)
+	annotations := make(map[string]string)
+	for k, v := range input {
+		// Check if the key is used internally by the shim.
+		internal := false
+		for _, internalKey := range internalLabelKeys {
+			// TODO: containerTypeLabelKey is the only internal label the shim uses
+			// right now. Expand this to a list later.
+			if k == internalKey {
+				internal = true
+				break
+			}
+		}
+		if internal {
+			continue
+		}
+
+		// Check if the label should be treated as an annotation.
+		if strings.HasPrefix(k, annotationPrefix) {
+			annotations[strings.TrimPrefix(k, annotationPrefix)] = v
+			continue
+		}
+		labels[k] = v
+	}
+	return labels, annotations
+}
+
 // generateMountBindings converts the mount list to a list of strings that
 // can be understood by docker.
 // Each element in the string is in the form of:

pkg/kubelet/dockershim/helpers_test.go

+/*
+Copyright 2016 The Kubernetes 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 dockershim
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestLabelsAndAnnotationsRoundTrip(t *testing.T) {
+	expectedLabels := map[string]string{"foo.123.abc": "baz", "bar.456.xyz": "qwe"}
+	expectedAnnotations := map[string]string{"uio.ert": "dfs", "jkl": "asd"}
+	// Merge labels and annotations into docker labels.
+	dockerLabels := makeLabels(expectedLabels, expectedAnnotations)
+	// Extract labels and annotations from docker labels.
+	actualLabels, actualAnnotations := extractLabels(dockerLabels)
+	assert.Equal(t, expectedLabels, actualLabels)
+	assert.Equal(t, expectedAnnotations, actualAnnotations)
+}