From 45e5912c3d4e9498c1bac1b20e00c7820816b7fc Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Mon, 25 Jan 2016 16:31:32 -0800 Subject: [PATCH] Augment logs in runContainer path Generating errors that are useful is hard. --- pkg/kubelet/dockertools/manager.go | 31 +++++++++++++------------ pkg/kubelet/dockertools/manager_test.go | 6 ++--- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index 973f78facef..67d1a1df4dd 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/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 diff --git a/pkg/kubelet/dockertools/manager_test.go b/pkg/kubelet/dockertools/manager_test.go index ede3251da17..9243d498c13 100644 --- a/pkg/kubelet/dockertools/manager_test.go +++ b/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()) } } }