From 84aab8d4a88ea3a3e614c575dbe596419a57fb7b Mon Sep 17 00:00:00 2001 From: Yu-Ju Hong Date: Fri, 26 Aug 2016 19:04:09 -0700 Subject: [PATCH] dockershim: utilize the Metadata in container names This commit changes how the shim constructs and parses docker container names by using the new "Metadata" types. --- pkg/kubelet/dockershim/convert.go | 43 ++++--- pkg/kubelet/dockershim/docker_container.go | 30 ++--- pkg/kubelet/dockershim/docker_sandbox.go | 34 ++--- pkg/kubelet/dockershim/naming.go | 138 +++++++++++++-------- pkg/kubelet/dockershim/naming_test.go | 84 +++++++++++++ 5 files changed, 221 insertions(+), 108 deletions(-) create mode 100644 pkg/kubelet/dockershim/naming_test.go diff --git a/pkg/kubelet/dockershim/convert.go b/pkg/kubelet/dockershim/convert.go index a2ee3bfc77..4dafd7a87b 100644 --- a/pkg/kubelet/dockershim/convert.go +++ b/pkg/kubelet/dockershim/convert.go @@ -49,20 +49,21 @@ func toRuntimeAPIImage(image *dockertypes.Image) (*runtimeApi.Image, error) { }, nil } -func toRuntimeAPIContainer(c *dockertypes.Container) *runtimeApi.Container { +func toRuntimeAPIContainer(c *dockertypes.Container) (*runtimeApi.Container, error) { state := toRuntimeAPIContainerState(c.Status) - _, _, _, containerName, attempt, _ := parseContainerName(c.Names[0]) + metadata, err := parseContainerName(c.Names[0]) + if err != nil { + return nil, err + } return &runtimeApi.Container{ - Id: &c.ID, - Metadata: &runtimeApi.ContainerMetadata{ - Name: &containerName, - Attempt: &attempt, - }, + Id: &c.ID, + Metadata: metadata, Image: &runtimeApi.ImageSpec{Image: &c.Image}, ImageRef: &c.ImageID, State: &state, - Labels: c.Labels, - } + // TODO: Extract annotations from labels. + Labels: c.Labels, + }, nil } func toDockerContainerStatus(state runtimeApi.ContainerState) string { @@ -106,19 +107,17 @@ func toRuntimeAPISandboxState(state string) runtimeApi.PodSandBoxState { } } -func toRuntimeAPISandbox(c *dockertypes.Container) *runtimeApi.PodSandbox { +func toRuntimeAPISandbox(c *dockertypes.Container) (*runtimeApi.PodSandbox, error) { state := toRuntimeAPISandboxState(c.Status) - podName, podNamespace, podUID, attempt, _ := parseSandboxName(c.Names[0]) - return &runtimeApi.PodSandbox{ - Id: &c.ID, - Metadata: &runtimeApi.PodSandboxMetadata{ - Name: &podName, - Namespace: &podNamespace, - Uid: &podUID, - Attempt: &attempt, - }, - State: &state, - CreatedAt: &c.Created, // TODO: Why do we need CreateAt timestamp for sandboxes? - Labels: c.Labels, // TODO: Need to disthinguish annotaions and labels. + metadata, err := parseSandboxName(c.Names[0]) + if err != nil { + return nil, err } + return &runtimeApi.PodSandbox{ + Id: &c.ID, + Metadata: metadata, + State: &state, + CreatedAt: &c.Created, + Labels: c.Labels, // TODO: Need to disthinguish annotaions and labels. + }, nil } diff --git a/pkg/kubelet/dockershim/docker_container.go b/pkg/kubelet/dockershim/docker_container.go index 075785e0a1..5bfbd63ac5 100644 --- a/pkg/kubelet/dockershim/docker_container.go +++ b/pkg/kubelet/dockershim/docker_container.go @@ -25,6 +25,7 @@ import ( dockercontainer "github.com/docker/engine-api/types/container" dockerfilters "github.com/docker/engine-api/types/filters" dockerstrslice "github.com/docker/engine-api/types/strslice" + "github.com/golang/glog" runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime" "k8s.io/kubernetes/pkg/kubelet/dockertools" @@ -64,14 +65,18 @@ func (ds *dockerService) ListContainers(filter *runtimeApi.ContainerFilter) ([]* result := []*runtimeApi.Container{} for i := range containers { c := containers[i] - if len(filter.GetName()) > 0 { - _, _, _, containerName, _, err := parseContainerName(c.Names[0]) - if err != nil || containerName != filter.GetName() { - continue - } - } - result = append(result, toRuntimeAPIContainer(&c)) + converted, err := toRuntimeAPIContainer(&c) + if err != nil { + glog.V(5).Infof("Unable to convert docker to runtime API container: %v", err) + continue + } + if len(filter.GetName()) != 0 && converted.Metadata.GetName() != filter.GetName() { + // TODO: Remove "name" from the ContainerFilter because name can no + // longer be used to identify a container. + continue + } + result = append(result, converted) } return result, nil } @@ -99,7 +104,7 @@ func (ds *dockerService) CreateContainer(podSandboxID string, config *runtimeApi image = iSpec.GetImage() } createConfig := dockertypes.ContainerCreateConfig{ - Name: buildContainerName(sandboxConfig, config), + Name: makeContainerName(sandboxConfig, config), Config: &dockercontainer.Config{ // TODO: set User. Hostname: sandboxConfig.GetHostname(), @@ -279,17 +284,14 @@ func (ds *dockerService) ContainerStatus(containerID string) (*runtimeApi.Contai ct, st, ft := createdAt.Unix(), startedAt.Unix(), finishedAt.Unix() exitCode := int32(r.State.ExitCode) - _, _, _, containerName, attempt, err := parseContainerName(r.Name) + metadata, err := parseContainerName(r.Name) if err != nil { return nil, err } return &runtimeApi.ContainerStatus{ - Id: &r.ID, - Metadata: &runtimeApi.ContainerMetadata{ - Name: &containerName, - Attempt: &attempt, - }, + Id: &r.ID, + Metadata: metadata, Image: &runtimeApi.ImageSpec{Image: &r.Config.Image}, ImageRef: &r.Image, Mounts: mounts, diff --git a/pkg/kubelet/dockershim/docker_sandbox.go b/pkg/kubelet/dockershim/docker_sandbox.go index 986c3b72d9..ffc14aa41c 100644 --- a/pkg/kubelet/dockershim/docker_sandbox.go +++ b/pkg/kubelet/dockershim/docker_sandbox.go @@ -22,6 +22,7 @@ import ( dockertypes "github.com/docker/engine-api/types" dockercontainer "github.com/docker/engine-api/types/container" dockerfilters "github.com/docker/engine-api/types/filters" + "github.com/golang/glog" runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime" ) @@ -117,7 +118,7 @@ func (ds *dockerService) PodSandboxStatus(podSandboxID string) (*runtimeApi.PodS network := &runtimeApi.PodSandboxNetworkStatus{Ip: &IP} netNS := getNetworkNamespace(r) - podName, podNamespace, podUID, attempt, err := parseSandboxName(r.Name) + metadata, err := parseSandboxName(r.Name) if err != nil { return nil, err } @@ -126,12 +127,7 @@ func (ds *dockerService) PodSandboxStatus(podSandboxID string) (*runtimeApi.PodS Id: &r.ID, State: &state, CreatedAt: &ct, - Metadata: &runtimeApi.PodSandboxMetadata{ - Name: &podName, - Namespace: &podNamespace, - Uid: &podUID, - Attempt: &attempt, - }, + 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. @@ -184,18 +180,22 @@ func (ds *dockerService) ListPodSandbox(filter *runtimeApi.PodSandboxFilter) ([] result := []*runtimeApi.PodSandbox{} for i := range containers { c := containers[i] - if len(filter.GetName()) > 0 { - sandboxName, _, _, _, err := parseSandboxName(c.Names[0]) - if err != nil || sandboxName != filter.GetName() { - continue - } - } - s := toRuntimeAPISandbox(&c) - if filterOutReadySandboxes && s.GetState() == runtimeApi.PodSandBoxState_READY { + converted, err := toRuntimeAPISandbox(&c) + if err != nil { + glog.V(5).Infof("Unable to convert docker to runtime API sandbox: %v", err) continue } - result = append(result, s) + if len(filter.GetName()) > 0 && converted.Metadata.GetName() != filter.GetName() { + // TODO: Remove "name" from the SandboxFilter because name can no + // longer be used to identify a container. + continue + } + if filterOutReadySandboxes && converted.GetState() == runtimeApi.PodSandBoxState_READY { + continue + } + + result = append(result, converted) } return result, nil } @@ -208,7 +208,7 @@ func makeSandboxDockerConfig(c *runtimeApi.PodSandboxConfig, image string) *dock hc := &dockercontainer.HostConfig{} createConfig := &dockertypes.ContainerCreateConfig{ - Name: buildSandboxName(c), + Name: makeSandboxName(c), Config: &dockercontainer.Config{ Hostname: c.GetHostname(), // TODO: Handle environment variables. diff --git a/pkg/kubelet/dockershim/naming.go b/pkg/kubelet/dockershim/naming.go index 5c9d67e0cd..a5c1cf0c8c 100644 --- a/pkg/kubelet/dockershim/naming.go +++ b/pkg/kubelet/dockershim/naming.go @@ -18,86 +18,114 @@ package dockershim import ( "fmt" - "math/rand" "strconv" "strings" - "github.com/golang/glog" - runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime" ) +// Container "names" are implementation details that do not concern +// kubelet/CRI. This CRI shim uses names to fulfill the CRI requirement to +// make sandbox/container creation idempotent. CRI states that there can +// only exist one sandbox/container with the given metadata. To enforce this, +// this shim constructs a name using the fields in the metadata so that +// docker will reject the creation request if the name already exists. +// +// Note that changes to naming will likely break the backward compatibility. +// Code must be added to ensure the shim knows how to recognize and extract +// information the older containers. +// +// TODO: Add code to handle backward compatibility, i.e., making sure we can +// recognize older containers and extract information from their names if +// necessary. + const ( // kubePrefix is used to identify the containers/sandboxes on the node managed by kubelet kubePrefix = "k8s" - // kubeSandboxNamePrefix is used to keep sandbox name consistent with old podInfraContainer name - kubeSandboxNamePrefix = "POD" + // sandboxContainerName is a string to include in the docker container so + // that users can easily identify the sandboxes. + sandboxContainerName = "POD" + // Delimiter used to construct docker container names. + nameDelimiter = "_" ) -// buildKubeGenericName creates a name which can be reversed to identify container/sandbox name. -// This function returns the unique name. -func buildKubeGenericName(sandboxConfig *runtimeApi.PodSandboxConfig, containerName string) string { - stableName := fmt.Sprintf("%s_%s_%s_%s_%s", - kubePrefix, - containerName, - sandboxConfig.Metadata.GetName(), - sandboxConfig.Metadata.GetNamespace(), - sandboxConfig.Metadata.GetUid(), - ) - UID := fmt.Sprintf("%08x", rand.Uint32()) - return fmt.Sprintf("%s_%s", stableName, UID) +func makeSandboxName(s *runtimeApi.PodSandboxConfig) string { + return strings.Join([]string{ + kubePrefix, // 0 + sandboxContainerName, // 1 + s.Metadata.GetName(), // 2 + s.Metadata.GetNamespace(), // 3 + s.Metadata.GetUid(), // 4 + fmt.Sprintf("%d", s.Metadata.GetAttempt()), // 5 + }, nameDelimiter) } -// buildSandboxName creates a name which can be reversed to identify sandbox full name. -func buildSandboxName(sandboxConfig *runtimeApi.PodSandboxConfig) string { - sandboxName := fmt.Sprintf("%s.%d", kubeSandboxNamePrefix, sandboxConfig.Metadata.GetAttempt()) - return buildKubeGenericName(sandboxConfig, sandboxName) +func makeContainerName(s *runtimeApi.PodSandboxConfig, c *runtimeApi.ContainerConfig) string { + return strings.Join([]string{ + kubePrefix, // 0 + c.Metadata.GetName(), // 1: + s.Metadata.GetName(), // 2: sandbox name + s.Metadata.GetNamespace(), // 3: sandbox namesapce + s.Metadata.GetUid(), // 4 sandbox uid + fmt.Sprintf("%d", c.Metadata.GetAttempt()), // 5 + }, nameDelimiter) + } -// parseSandboxName unpacks a sandbox full name, returning the pod name, namespace, uid and attempt. -func parseSandboxName(name string) (string, string, string, uint32, error) { - podName, podNamespace, podUID, _, attempt, err := parseContainerName(name) +func parseUint32(s string) (uint32, error) { + n, err := strconv.ParseUint(s, 10, 32) if err != nil { - return "", "", "", 0, err + return 0, err } - - return podName, podNamespace, podUID, attempt, nil + return uint32(n), nil } -// buildContainerName creates a name which can be reversed to identify container name. -// This function returns stable name, unique name and an unique id. -func buildContainerName(sandboxConfig *runtimeApi.PodSandboxConfig, containerConfig *runtimeApi.ContainerConfig) string { - containerName := fmt.Sprintf("%s.%d", containerConfig.Metadata.GetName(), containerConfig.Metadata.GetAttempt()) - return buildKubeGenericName(sandboxConfig, containerName) -} - -// parseContainerName unpacks a container name, returning the pod name, namespace, UID, -// container name and attempt. -func parseContainerName(name string) (podName, podNamespace, podUID, containerName string, attempt uint32, err error) { +// TODO: Evaluate whether we should rely on labels completely. +func parseSandboxName(name string) (*runtimeApi.PodSandboxMetadata, error) { // Docker adds a "/" prefix to names. so trim it. name = strings.TrimPrefix(name, "/") - parts := strings.Split(name, "_") - if len(parts) == 0 || parts[0] != kubePrefix { - err = fmt.Errorf("failed to parse container name %q into parts", name) - return "", "", "", "", 0, err + parts := strings.Split(name, nameDelimiter) + if len(parts) != 6 { + return nil, fmt.Errorf("failed to parse the sandbox name: %q", name) } - if len(parts) < 6 { - glog.Warningf("Found a container with the %q prefix, but too few fields (%d): %q", kubePrefix, len(parts), name) - err = fmt.Errorf("container name %q has fewer parts than expected %v", name, parts) - return "", "", "", "", 0, err + if parts[0] != kubePrefix { + return nil, fmt.Errorf("container is not managed by kubernetes: %q", name) } - nameParts := strings.Split(parts[1], ".") - containerName = nameParts[0] - if len(nameParts) > 1 { - attemptNumber, err := strconv.ParseUint(nameParts[1], 10, 32) - if err != nil { - glog.Warningf("invalid container attempt %q in container %q", nameParts[1], name) - } - - attempt = uint32(attemptNumber) + attempt, err := parseUint32(parts[5]) + if err != nil { + return nil, fmt.Errorf("failed to parse the sandbox name %q: %v", name, err) } - return parts[2], parts[3], parts[4], containerName, attempt, nil + return &runtimeApi.PodSandboxMetadata{ + Name: &parts[2], + Namespace: &parts[3], + Uid: &parts[4], + Attempt: &attempt, + }, nil +} + +// TODO: Evaluate whether we should rely on labels completely. +func parseContainerName(name string) (*runtimeApi.ContainerMetadata, error) { + // Docker adds a "/" prefix to names. so trim it. + name = strings.TrimPrefix(name, "/") + + parts := strings.Split(name, nameDelimiter) + if len(parts) != 6 { + return nil, fmt.Errorf("failed to parse the container name: %q", name) + } + if parts[0] != kubePrefix { + return nil, fmt.Errorf("container is not managed by kubernetes: %q", name) + } + + attempt, err := parseUint32(parts[5]) + if err != nil { + return nil, fmt.Errorf("failed to parse the container name %q: %v", name, err) + } + + return &runtimeApi.ContainerMetadata{ + Name: &parts[1], + Attempt: &attempt, + }, nil } diff --git a/pkg/kubelet/dockershim/naming_test.go b/pkg/kubelet/dockershim/naming_test.go new file mode 100644 index 0000000000..62096b8ec6 --- /dev/null +++ b/pkg/kubelet/dockershim/naming_test.go @@ -0,0 +1,84 @@ +/* +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" + + runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime" +) + +func TestSandboxNameRoundTrip(t *testing.T) { + config := makeSandboxConfig("foo", "bar", "iamuid", 3) + actualName := makeSandboxName(config) + assert.Equal(t, "k8s_POD_foo_bar_iamuid_3", actualName) + + actualMetadata, err := parseSandboxName(actualName) + assert.NoError(t, err) + assert.Equal(t, config.Metadata, actualMetadata) +} + +func TestNonParsableSandboxNames(t *testing.T) { + // All names must start with the kubernetes prefix "k8s". + _, err := parseSandboxName("owner_POD_foo_bar_iamuid_4") + assert.Error(t, err) + + // All names must contain exactly 6 parts. + _, err = parseSandboxName("k8s_POD_dummy_foo_bar_iamuid_4") + assert.Error(t, err) + _, err = parseSandboxName("k8s_foo_bar_iamuid_4") + assert.Error(t, err) + + // Should be able to parse attempt number. + _, err = parseSandboxName("k8s_POD_foo_bar_iamuid_notanumber") + assert.Error(t, err) +} + +func TestContainerNameRoundTrip(t *testing.T) { + sConfig := makeSandboxConfig("foo", "bar", "iamuid", 3) + name, attempt := "pause", uint32(5) + config := &runtimeApi.ContainerConfig{ + Metadata: &runtimeApi.ContainerMetadata{ + Name: &name, + Attempt: &attempt, + }, + } + actualName := makeContainerName(sConfig, config) + assert.Equal(t, "k8s_pause_foo_bar_iamuid_5", actualName) + + actualMetadata, err := parseContainerName(actualName) + assert.NoError(t, err) + assert.Equal(t, config.Metadata, actualMetadata) +} + +func TestNonParsableContainerNames(t *testing.T) { + // All names must start with the kubernetes prefix "k8s". + _, err := parseContainerName("owner_frontend_foo_bar_iamuid_4") + assert.Error(t, err) + + // All names must contain exactly 6 parts. + _, err = parseContainerName("k8s_frontend_dummy_foo_bar_iamuid_4") + assert.Error(t, err) + _, err = parseContainerName("k8s_foo_bar_iamuid_4") + assert.Error(t, err) + + // Should be able to parse attempt number. + _, err = parseContainerName("k8s_frontend_foo_bar_iamuid_notanumber") + assert.Error(t, err) +}