Skip to content

Commit

Permalink
Refactor build/parse dockername. kubernetes#3511
Browse files Browse the repository at this point in the history 
Functions Build/ParseDockerName now work with struct instead of the long
list of arguments. This new struct also was reused in the kubelet.go
instead of auxilary podContainer struct.
  • Loading branch information
@AAzza
AAzza committed Mar 21, 2015
1 parent ef75888 commit 73c2338
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 60 deletions.
7 changes: 4 additions & 3 deletions pkg/kubelet/container_gc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,13 +213,14 @@ func (self *realContainerGC) evictableContainers() (containersByEvictUnit, []con
createTime: data.Created,
}

_, uid, name, _, err := dockertools.ParseDockerName(container.Names[0])
containerName, _, err := dockertools.ParseDockerName(container.Names[0])

if err != nil {
unidentifiedContainers = append(unidentifiedContainers, containerInfo)
} else {
key := evictUnit{
uid: uid,
name: name,
uid: containerName.PodUID,
name: containerName.ContainerName,
}
evictUnits[key] = append(evictUnits[key], containerInfo)
}
Expand Down
60 changes: 32 additions & 28 deletions pkg/kubelet/dockertools/docker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ type DockerInterface interface {
// DockerID is an ID of docker container. It is a type to make it clear when we're working with docker container Ids
type DockerID string

type KubeletContainerName struct {
PodFullName string
PodUID types.UID
ContainerName string
}

// DockerPuller is an abstract interface for testability. It abstracts image pull operations.
type DockerPuller interface {
Pull(image string) error
Expand Down Expand Up @@ -396,13 +402,13 @@ func (c DockerContainers) FindPodContainer(podFullName string, uid types.UID, co
continue
}
// TODO(proppy): build the docker container name and do a map lookup instead?
dockerManifestID, dockerUUID, dockerContainerName, hash, err := ParseDockerName(dockerContainer.Names[0])
dockerName, hash, err := ParseDockerName(dockerContainer.Names[0])
if err != nil {
continue
}
if dockerManifestID == podFullName &&
(uid == "" || dockerUUID == uid) &&
dockerContainerName == containerName {
if dockerName.PodFullName == podFullName &&
(uid == "" || dockerName.PodUID == uid) &&
dockerName.ContainerName == containerName {
return dockerContainer, true, hash
}
}
Expand All @@ -421,12 +427,12 @@ func (c DockerContainers) FindContainersByPod(podUID types.UID, podFullName stri
if len(dockerContainer.Names) == 0 {
continue
}
dockerPodName, uuid, _, _, err := ParseDockerName(dockerContainer.Names[0])
dockerName, _, err := ParseDockerName(dockerContainer.Names[0])
if err != nil {
continue
}
if podUID == uuid ||
(podUID == "" && podFullName == dockerPodName) {
if podUID == dockerName.PodUID ||
(podUID == "" && podFullName == dockerName.PodFullName) {
containers[DockerID(dockerContainer.ID)] = dockerContainer
}
}
Expand Down Expand Up @@ -471,17 +477,17 @@ func GetRecentDockerContainersWithNameAndUUID(client DockerInterface, podFullNam
if len(dockerContainer.Names) == 0 {
continue
}
dockerPodName, dockerUUID, dockerContainerName, _, err := ParseDockerName(dockerContainer.Names[0])
dockerName, _, err := ParseDockerName(dockerContainer.Names[0])
if err != nil {
continue
}
if dockerPodName != podFullName {
if dockerName.PodFullName != podFullName {
continue
}
if uid != "" && dockerUUID != uid {
if uid != "" && dockerName.PodUID != uid {
continue
}
if dockerContainerName != containerName {
if dockerName.ContainerName != containerName {
continue
}
inspectResult, _ := client.InspectContainer(dockerContainer.ID)
Expand Down Expand Up @@ -628,16 +634,17 @@ func GetDockerPodStatus(client DockerInterface, manifest api.PodSpec, podFullNam
if len(value.Names) == 0 {
continue
}
dockerManifestID, dockerUUID, dockerContainerName, _, err := ParseDockerName(value.Names[0])
dockerName, _, err := ParseDockerName(value.Names[0])
if err != nil {
continue
}
if dockerManifestID != podFullName {
if dockerName.PodFullName != podFullName {
continue
}
if uid != "" && dockerUUID != uid {
if uid != "" && dockerName.PodUID != uid {
continue
}
dockerContainerName := dockerName.ContainerName
c, found := expectedContainers[dockerContainerName]
terminationMessagePath := ""
if !found {
Expand Down Expand Up @@ -713,52 +720,49 @@ func HashContainer(container *api.Container) uint64 {
}

// Creates a name which can be reversed to identify both full pod name and container name.
func BuildDockerName(podUID types.UID, podFullName string, container *api.Container) string {
containerName := container.Name + "." + strconv.FormatUint(HashContainer(container), 16)
func BuildDockerName(dockerName KubeletContainerName, container *api.Container) string {
containerName := dockerName.ContainerName + "." + strconv.FormatUint(HashContainer(container), 16)
return fmt.Sprintf("%s_%s_%s_%s_%08x",
containerNamePrefix,
containerName,
podFullName,
podUID,
dockerName.PodFullName,
dockerName.PodUID,
rand.Uint32())
}

// Unpacks a container name, returning the pod full name and container name we would have used to
// construct the docker name. If we are unable to parse the name, an error is returned.
func ParseDockerName(name string) (podFullName string, podUID types.UID, containerName string, hash uint64, err error) {
func ParseDockerName(name string) (dockerName *KubeletContainerName, hash uint64, err error) {
// For some reason docker appears to be appending '/' to names.
// If it's there, strip it.
name = strings.TrimPrefix(name, "/")
parts := strings.Split(name, "_")
if len(parts) == 0 || parts[0] != containerNamePrefix {
err = fmt.Errorf("failed to parse Docker container name %q into parts", name)
return
return nil, 0, err
}
if len(parts) < 6 {
// We have at least 5 fields. We may have more in the future.
// Anything with less fields than this is not something we can
// manage.
glog.Warningf("found a container with the %q prefix, but too few fields (%d): %q", containerNamePrefix, len(parts), name)
err = fmt.Errorf("Docker container name %q has less parts than expected %v", name, parts)
return
return nil, 0, err
}

// Container name.
nameParts := strings.Split(parts[1], ".")
containerName = nameParts[0]
containerName := nameParts[0]
if len(nameParts) > 1 {
hash, err = strconv.ParseUint(nameParts[1], 16, 32)
if err != nil {
glog.Warningf("invalid container hash %q in container %q", nameParts[1], name)
}
}

// Pod fullname.
podFullName = parts[2] + "_" + parts[3]
podFullName := parts[2] + "_" + parts[3]
podUID := types.UID(parts[4])

// Pod UID.
podUID = types.UID(parts[4])
return
return &KubeletContainerName{podFullName, podUID, containerName}, hash, nil
}

func GetRunningContainers(client DockerInterface, ids []string) ([]*docker.Container, error) {
Expand Down
14 changes: 7 additions & 7 deletions pkg/kubelet/dockertools/docker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,13 @@ func verifyPackUnpack(t *testing.T, podNamespace, podUID, podName, containerName
util.DeepHashObject(hasher, *container)
computedHash := uint64(hasher.Sum32())
podFullName := fmt.Sprintf("%s_%s", podName, podNamespace)
name := BuildDockerName(types.UID(podUID), podFullName, container)
returnedPodFullName, returnedUID, returnedContainerName, hash, err := ParseDockerName(name)
name := BuildDockerName(KubeletContainerName{podFullName, types.UID(podUID), container.Name}, container)
returned, hash, err := ParseDockerName(name)
if err != nil {
t.Errorf("Failed to parse Docker container name %q: %v", name, err)
}
if podFullName != returnedPodFullName || podUID != string(returnedUID) || containerName != returnedContainerName || computedHash != hash {
t.Errorf("For (%s, %s, %s, %d), unpacked (%s, %s, %s, %d)", podFullName, podUID, containerName, computedHash, returnedPodFullName, returnedUID, returnedContainerName, hash)
if podFullName != returned.PodFullName || podUID != string(returned.PodUID) || containerName != returned.ContainerName || computedHash != hash {
t.Errorf("For (%s, %s, %s, %d), unpacked (%s, %s, %s, %d)", podFullName, podUID, containerName, computedHash, returned.PodFullName, returned.PodUID, returned.ContainerName, hash)
}
}

Expand All @@ -117,12 +117,12 @@ func TestContainerManifestNaming(t *testing.T) {
name := fmt.Sprintf("k8s_%s_%s_%s_%s_42", container.Name, podName, podNamespace, podUID)
podFullName := fmt.Sprintf("%s_%s", podName, podNamespace)

returnedPodFullName, returnedPodUID, returnedContainerName, hash, err := ParseDockerName(name)
returned, hash, err := ParseDockerName(name)
if err != nil {
t.Errorf("Failed to parse Docker container name %q: %v", name, err)
}
if returnedPodFullName != podFullName || string(returnedPodUID) != podUID || returnedContainerName != container.Name || hash != 0 {
t.Errorf("unexpected parse: %s %s %s %d", returnedPodFullName, returnedPodUID, returnedContainerName, hash)
if returned.PodFullName != podFullName || string(returned.PodUID) != podUID || returned.ContainerName != container.Name || hash != 0 {
t.Errorf("unexpected parse: %s %s %s %d", returned.PodFullName, returned.PodUID, returned.ContainerName, hash)
}
}

Expand Down
33 changes: 13 additions & 20 deletions pkg/kubelet/kubelet.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,7 @@ func (kl *Kubelet) runContainer(pod *api.Pod, container *api.Container, podVolum
exposedPorts, portBindings := makePortsAndBindings(container)

opts := docker.CreateContainerOptions{
Name: dockertools.BuildDockerName(pod.UID, GetPodFullName(pod), container),
Name: dockertools.BuildDockerName(dockertools.KubeletContainerName{GetPodFullName(pod), pod.UID, container.Name}, container),
Config: &docker.Config{
Cmd: container.Command,
Env: envVariables,
Expand Down Expand Up @@ -1411,12 +1411,6 @@ func (kl *Kubelet) syncPod(pod *api.Pod, hasMirrorPod bool, containersInPod dock
return nil
}

type podContainer struct {
podFullName string
uid types.UID
containerName string
}

// Stores all volumes defined by the set of pods into a map.
// Keys for each entry are in the format (POD_ID)/(VOLUME_NAME)
func getDesiredVolumes(pods []api.Pod) map[string]api.Volume {
Expand Down Expand Up @@ -1461,11 +1455,11 @@ func (kl *Kubelet) cleanupOrphanedVolumes(pods []api.Pod, running []*docker.Cont
if len(running[ix].Name) == 0 {
glog.V(2).Infof("Found running container ix=%d with info: %+v", ix, running[ix])
}
_, uid, _, _, err := dockertools.ParseDockerName(running[ix].Name)
containerName, _, err := dockertools.ParseDockerName(running[ix].Name)
if err != nil {
continue
}
runningSet.Insert(string(uid))
runningSet.Insert(string(containerName.PodUID))
}
for name, vol := range currentVolumes {
if _, ok := desiredVolumes[name]; !ok {
Expand Down Expand Up @@ -1513,7 +1507,7 @@ func (kl *Kubelet) SyncPods(allPods []api.Pod, podSyncTypes map[types.UID]metric

glog.V(4).Infof("Desired: %#v", pods)
var err error
desiredContainers := make(map[podContainer]empty)
desiredContainers := make(map[dockertools.KubeletContainerName]empty)
desiredPods := make(map[types.UID]empty)

dockerContainers, err := kl.dockerCache.RunningContainers()
Expand All @@ -1530,9 +1524,9 @@ func (kl *Kubelet) SyncPods(allPods []api.Pod, podSyncTypes map[types.UID]metric
desiredPods[uid] = empty{}

// Add all containers (including net) to the map.
desiredContainers[podContainer{podFullName, uid, dockertools.PodInfraContainerName}] = empty{}
desiredContainers[dockertools.KubeletContainerName{podFullName, uid, dockertools.PodInfraContainerName}] = empty{}
for _, cont := range pod.Spec.Containers {
desiredContainers[podContainer{podFullName, uid, cont.Name}] = empty{}
desiredContainers[dockertools.KubeletContainerName{podFullName, uid, cont.Name}] = empty{}
}

// Run the sync in an async manifest worker.
Expand All @@ -1559,28 +1553,27 @@ func (kl *Kubelet) SyncPods(allPods []api.Pod, podSyncTypes map[types.UID]metric
killed := []string{}
for ix := range dockerContainers {
// Don't kill containers that are in the desired pods.
podFullName, uid, containerName, _, err := dockertools.ParseDockerName(dockerContainers[ix].Names[0])
_, found := desiredPods[uid]
dockerName, _, err := dockertools.ParseDockerName(dockerContainers[ix].Names[0])
_, found := desiredPods[dockerName.PodUID]
if err == nil && found {
// syncPod() will handle this one.
continue
}

pc := podContainer{podFullName, uid, containerName}
_, ok := desiredContainers[pc]
_, ok := desiredContainers[*dockerName]
if err != nil || !ok {
// call the networking plugin for teardown
if containerName == dockertools.PodInfraContainerName {
name, namespace, _ := ParsePodFullName(podFullName)
if dockerName.ContainerName == dockertools.PodInfraContainerName {
name, namespace, _ := ParsePodFullName(dockerName.PodFullName)
err := kl.networkPlugin.TearDownPod(namespace, name, dockertools.DockerID(dockerContainers[ix].ID))
if err != nil {
glog.Errorf("Network plugin pre-delete method returned an error: %v", err)
}
}
glog.V(1).Infof("Killing unwanted container %+v", pc)
glog.V(1).Infof("Killing unwanted container %+v", *dockerName)
err = kl.killContainer(dockerContainers[ix])
if err != nil {
glog.Errorf("Error killing container %+v: %v", pc, err)
glog.Errorf("Error killing container %+v: %v", *dockerName, err)
} else {
killed = append(killed, dockerContainers[ix].ID)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/kubelet/metrics/metrics.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,11 @@ func (self *podAndContainerCollector) Collect(ch chan<- prometheus.Metric) {
// Get a set of running pods.
runningPods := make(map[types.UID]struct{})
for _, cont := range runningContainers {
_, uid, _, _, err := dockertools.ParseDockerName(cont.Names[0])
containerName, _, err := dockertools.ParseDockerName(cont.Names[0])
if err != nil {
continue
}
runningPods[uid] = struct{}{}
runningPods[containerName.PodUID] = struct{}{}
}

ch <- prometheus.MustNewConstMetric(
Expand Down

0 comments on commit 73c2338

Please sign in to comment.