Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make ParseDockerName() return an error. #5413

Merged
merged 1 commit into from
Mar 13, 2015
Merged

Make ParseDockerName() return an error. #5413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 22 additions & 12 deletions pkg/kubelet/dockertools/docker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,10 @@ 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 := ParseDockerName(dockerContainer.Names[0])
dockerManifestID, dockerUUID, dockerContainerName, hash, err := ParseDockerName(dockerContainer.Names[0])
if err != nil {
continue
}
if dockerManifestID == podFullName &&
(uid == "" || dockerUUID == uid) &&
dockerContainerName == containerName {
Expand All @@ -425,7 +428,10 @@ func (c DockerContainers) FindContainersByPod(podUID types.UID, podFullName stri
if len(dockerContainer.Names) == 0 {
continue
}
dockerPodName, uuid, _, _ := ParseDockerName(dockerContainer.Names[0])
dockerPodName, uuid, _, _, err := ParseDockerName(dockerContainer.Names[0])
if err != nil {
continue
}
if podUID == uuid ||
(podUID == "" && podFullName == dockerPodName) {
containers[DockerID(dockerContainer.ID)] = dockerContainer
Expand Down Expand Up @@ -472,7 +478,10 @@ func GetRecentDockerContainersWithNameAndUUID(client DockerInterface, podFullNam
if len(dockerContainer.Names) == 0 {
continue
}
dockerPodName, dockerUUID, dockerContainerName, _ := ParseDockerName(dockerContainer.Names[0])
dockerPodName, dockerUUID, dockerContainerName, _, err := ParseDockerName(dockerContainer.Names[0])
if err != nil {
continue
}
if dockerPodName != podFullName {
continue
}
Expand Down Expand Up @@ -614,7 +623,10 @@ func GetDockerPodInfo(client DockerInterface, manifest api.PodSpec, podFullName
if len(value.Names) == 0 {
continue
}
dockerManifestID, dockerUUID, dockerContainerName, _ := ParseDockerName(value.Names[0])
dockerManifestID, dockerUUID, dockerContainerName, _, err := ParseDockerName(value.Names[0])
if err != nil {
continue
}
if dockerManifestID != podFullName {
continue
}
Expand Down Expand Up @@ -705,35 +717,33 @@ func BuildDockerName(podUID types.UID, podFullName string, container *api.Contai
rand.Uint32())
}

// TODO(vmarmol): This should probably return an error.
// Unpacks a container name, returning the pod full name and container name we would have used to
// construct the docker name. If the docker name isn't the one we created, we may return empty strings.
func ParseDockerName(name string) (podFullName string, podUID types.UID, containerName string, hash uint64) {
// 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) {
// For some reason docker appears to be appending '/' to names.
// If it's there, strip it.
if name[0] == '/' {
name = name[1:]
}
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
}
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
}

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

Expand Down
10 changes: 8 additions & 2 deletions pkg/kubelet/dockertools/docker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,10 @@ func verifyPackUnpack(t *testing.T, podNamespace, podUID, podName, containerName
computedHash := uint64(hasher.Sum32())
podFullName := fmt.Sprintf("%s_%s", podName, podNamespace)
name := BuildDockerName(types.UID(podUID), podFullName, container)
returnedPodFullName, returnedUID, returnedContainerName, hash := ParseDockerName(name)
returnedPodFullName, returnedUID, returnedContainerName, 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)
}
Expand All @@ -114,7 +117,10 @@ 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 := ParseDockerName(name)
returnedPodFullName, returnedPodUID, returnedContainerName, 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)
}
Expand Down
17 changes: 11 additions & 6 deletions pkg/kubelet/kubelet.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,8 +478,8 @@ func (kl *Kubelet) GarbageCollectContainers() error {
unidentifiedContainers := make([]unidentifiedContainer, 0)
uidToIDMap := map[string][]string{}
for _, container := range containers {
_, uid, name, _ := dockertools.ParseDockerName(container.Names[0])
if uid == "" && name == "" {
_, uid, name, _, err := dockertools.ParseDockerName(container.Names[0])
if err != nil {
unidentifiedContainers = append(unidentifiedContainers, unidentifiedContainer{
id: container.ID,
name: container.Names[0],
Expand Down Expand Up @@ -1352,7 +1352,10 @@ func (kl *Kubelet) cleanupOrphanedVolumes(pods []api.BoundPod, running []*docker
if len(running[ix].Name) == 0 {
glog.V(2).Infof("Found running container ix=%d with info: %+v", ix, running[ix])
}
_, uid, _, _ := dockertools.ParseDockerName(running[ix].Name)
_, uid, _, _, err := dockertools.ParseDockerName(running[ix].Name)
if err != nil {
continue
}
runningSet.Insert(string(uid))
}
for name, vol := range currentVolumes {
Expand Down Expand Up @@ -1446,14 +1449,16 @@ func (kl *Kubelet) SyncPods(allPods []api.BoundPod, podSyncTypes map[types.UID]m
killed := []string{}
for ix := range dockerContainers {
// Don't kill containers that are in the desired pods.
podFullName, uid, containerName, _ := dockertools.ParseDockerName(dockerContainers[ix].Names[0])
if _, found := desiredPods[uid]; found {
podFullName, uid, containerName, _, err := dockertools.ParseDockerName(dockerContainers[ix].Names[0])
_, found := desiredPods[uid]
if err == nil && found {
// syncPod() will handle this one.
continue
}

pc := podContainer{podFullName, uid, containerName}
if _, ok := desiredContainers[pc]; !ok {
_, ok := desiredContainers[pc]
if err != nil || !ok {
glog.V(1).Infof("Killing unwanted container %+v", pc)
err = kl.killContainer(dockerContainers[ix])
if err != nil {
Expand Down
13 changes: 8 additions & 5 deletions pkg/kubelet/metrics/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,17 +145,20 @@ func (self *podAndContainerCollector) Collect(ch chan<- prometheus.Metric) {
return
}

// Get a mapping of pod to number of containers in that pod.
podToContainerCount := make(map[types.UID]struct{})
// Get a set of running pods.
runningPods := make(map[types.UID]struct{})
for _, cont := range runningContainers {
_, uid, _, _ := dockertools.ParseDockerName(cont.Names[0])
podToContainerCount[uid] = struct{}{}
_, uid, _, _, err := dockertools.ParseDockerName(cont.Names[0])
if err != nil {
continue
}
runningPods[uid] = struct{}{}
}

ch <- prometheus.MustNewConstMetric(
runningPodCountDesc,
prometheus.GaugeValue,
float64(len(podToContainerCount)))
float64(len(runningPods)))
ch <- prometheus.MustNewConstMetric(
runningContainerCountDesc,
prometheus.GaugeValue,
Expand Down