Make ParseDockerName() return an error.

This forces callers to handle cases where the container name could not
be parsed.
This commit is contained in:
Victor Marmol 2015-03-12 16:31:57 -07:00
parent 7d72b64f60
commit 4675fcc57d
4 changed files with 49 additions and 25 deletions

View File

@ -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 {
@ -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
@ -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
}
@ -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
}
@ -705,17 +717,15 @@ 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 {
@ -723,6 +733,7 @@ func ParseDockerName(name string) (podFullName string, podUID types.UID, contain
// 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
}
@ -730,10 +741,9 @@ func ParseDockerName(name string) (podFullName string, podUID types.UID, contain
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)
}
}

View File

@ -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)
}
@ -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)
}

View File

@ -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],
@ -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 {
@ -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 {

View File

@ -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,