Augment logs in runContainer path

Generating errors that are useful is hard.

pkg/kubelet/dockertools/manager.go

@@ -1473,12 +1473,12 @@ func (dm *DockerManager) runContainerInPod(pod *api.Pod, container *api.Containe
 
 	ref, err := kubecontainer.GenerateContainerRef(pod, container)
 	if err != nil {
-		glog.Errorf("Couldn't make a ref to pod %v, container %v: '%v'", pod.Name, container.Name, err)
+		glog.Errorf("Can't make a ref to pod %v, container %v: '%v'", pod.Name, container.Name, err)
 	}
 
 	opts, err := dm.generator.GenerateRunContainerOptions(pod, container)
 	if err != nil {
-		return kubecontainer.ContainerID{}, err
+		return kubecontainer.ContainerID{}, fmt.Errorf("GenerateRunContainerOptions: %v", err)
 	}
 
 	utsMode := ""
@@ -1487,7 +1487,7 @@ func (dm *DockerManager) runContainerInPod(pod *api.Pod, container *api.Containe
 	}
 	id, err := dm.runContainer(pod, container, opts, ref, netMode, ipcMode, utsMode, pidMode, restartCount)
 	if err != nil {
-		return kubecontainer.ContainerID{}, err
+		return kubecontainer.ContainerID{}, fmt.Errorf("runContainer: %v", err)
 	}
 
 	// Remember this reference so we can report events about this container
@@ -1498,7 +1498,7 @@ func (dm *DockerManager) runContainerInPod(pod *api.Pod, container *api.Containe
 	if container.Lifecycle != nil && container.Lifecycle.PostStart != nil {
 		handlerErr := dm.runner.Run(id, pod, container, container.Lifecycle.PostStart)
 		if handlerErr != nil {
-			err := fmt.Errorf("failed to call event handler: %v", handlerErr)
+			err := fmt.Errorf("PostStart handler: %v", handlerErr)
 			dm.KillContainerInPod(id, container, pod, err.Error())
 			return kubecontainer.ContainerID{}, err
 		}
@@ -1517,11 +1517,11 @@ func (dm *DockerManager) runContainerInPod(pod *api.Pod, container *api.Containe
 	// Container information is used in adjusting OOM scores and adding ndots.
 	containerInfo, err := dm.client.InspectContainer(id.ID)
 	if err != nil {
-		return kubecontainer.ContainerID{}, err
+		return kubecontainer.ContainerID{}, fmt.Errorf("InspectContainer: %v", err)
 	}
 	// Ensure the PID actually exists, else we'll move ourselves.
 	if containerInfo.State.Pid == 0 {
-		return kubecontainer.ContainerID{}, fmt.Errorf("failed to get init PID for Docker container %q", id)
+		return kubecontainer.ContainerID{}, fmt.Errorf("can't get init PID for container %q", id)
 	}
 
 	// Set OOM score of the container based on the priority of the container.
@@ -1536,20 +1536,21 @@ func (dm *DockerManager) runContainerInPod(pod *api.Pod, container *api.Containe
 	}
 	cgroupName, err := dm.procFs.GetFullContainerName(containerInfo.State.Pid)
 	if err != nil {
-		return kubecontainer.ContainerID{}, err
+		return kubecontainer.ContainerID{}, fmt.Errorf("GetFullContainerName: %v", err)
 	}
 	if err = dm.oomAdjuster.ApplyOOMScoreAdjContainer(cgroupName, oomScoreAdj, 5); err != nil {
-		return kubecontainer.ContainerID{}, err
+		return kubecontainer.ContainerID{}, fmt.Errorf("ApplyOOMScoreAdjContainer: %v", err)
 	}
 
-	// currently, Docker does not have a flag by which the ndots option can be passed.
-	// (A separate issue has been filed with Docker to add a ndots flag)
 	// The addNDotsOption call appends the ndots option to the resolv.conf file generated by docker.
 	// This resolv.conf file is shared by all containers of the same pod, and needs to be modified only once per pod.
 	// we modify it when the pause container is created since it is the first container created in the pod since it holds
 	// the networking namespace.
 	if container.Name == PodInfraContainerName && utsMode != "host" {
 		err = addNDotsOption(containerInfo.ResolvConfPath)
+		if err != nil {
+			return kubecontainer.ContainerID{}, fmt.Errorf("addNDotsOption: %v", err)
+		}
 	}
 
 	return id, err
@@ -1557,18 +1558,18 @@ func (dm *DockerManager) runContainerInPod(pod *api.Pod, container *api.Containe
 
 func addNDotsOption(resolvFilePath string) error {
 	if len(resolvFilePath) == 0 {
-		glog.Errorf("DNS ResolvConfPath is empty.")
+		glog.Errorf("ResolvConfPath is empty.")
 		return nil
 	}
 
 	if _, err := os.Stat(resolvFilePath); os.IsNotExist(err) {
-		return fmt.Errorf("DNS ResolvConfPath specified but does not exist. It could not be updated: %s", resolvFilePath)
+		return fmt.Errorf("ResolvConfPath %q does not exist", resolvFilePath)
 	}
 
 	glog.V(4).Infof("DNS ResolvConfPath exists: %s. Will attempt to add ndots option: %s", resolvFilePath, ndotsDNSOption)
 
 	if err := appendToFile(resolvFilePath, ndotsDNSOption); err != nil {
-		glog.Errorf("resolv.conf could not be updated. err:%v", err)
+		glog.Errorf("resolv.conf could not be updated: %v", err)
 		return err
 	}
 	return nil
@@ -1968,7 +1969,7 @@ func (dm *DockerManager) verifyNonRoot(container *api.Container) error {
 
 	imgRoot, err := dm.isImageRoot(container.Image)
 	if err != nil {
-		return err
+		return fmt.Errorf("can't tell if image runs as root: %v", err)
 	}
 	if imgRoot {
 		return fmt.Errorf("container has no runAsUser and image will run as root")
@@ -1997,7 +1998,7 @@ func (dm *DockerManager) isImageRoot(image string) (bool, error) {
 	// do not allow non-numeric user directives
 	uid, err := strconv.Atoi(user)
 	if err != nil {
-		return false, fmt.Errorf("unable to validate image is non-root, non-numeric user (%s) is not allowed", user)
+		return false, fmt.Errorf("non-numeric user (%s) is not allowed", user)
 	}
 	// user is numeric, check for 0
 	return uid == 0, nil

pkg/kubelet/dockertools/manager_test.go

@@ -1777,7 +1777,7 @@ func TestVerifyNonRoot(t *testing.T) {
 					User: "foo",
 				},
 			},
-			expectedError: "unable to validate image is non-root, non-numeric user",
+			expectedError: "non-numeric user",
 		},
 		"numeric root image user": {
 			container: &api.Container{},
@@ -1812,10 +1812,10 @@ func TestVerifyNonRoot(t *testing.T) {
 		fakeDocker.Image = v.inspectImage
 		err := dm.verifyNonRoot(v.container)
 		if v.expectedError == "" && err != nil {
-			t.Errorf("%s had unexpected error %v", k, err)
+			t.Errorf("case[%q]: unexpected error: %v", k, err)
 		}
 		if v.expectedError != "" && !strings.Contains(err.Error(), v.expectedError) {
-			t.Errorf("%s expected error %s but received %s", k, v.expectedError, err.Error())
+			t.Errorf("case[%q]: expected: %q, got: %q", k, v.expectedError, err.Error())
 		}
 	}
 }