Merge pull request #43762 from sjenning/docker-pid-fail

Automatic merge from submit-queue refactor getPidsForProcess and change error handling xref https://github.com/openshift/origin/issues/13262 Right now, failure to read the docker pid from the pid file results in some premature nasty logging. There is still a chance we can get the docker pid from `procfs.PidOf()`. If that fails we should just log at `V(4)` rather than `runtime.HanldeError()`. This PR refactors `getPidsForProcess()` to wait until both methods for determining the pid fail before logging anything. @smarterclayton @ncdc @derekwaynecarr

Merge pull request #43762 from sjenning/docker-pid-fail
d42d630d · Kubernetes Submit Queue · GitHub · 63872a09 · ebb1243a · d42d630d
Commit d42d630d authored Mar 31, 2017 by Kubernetes Submit Queue Committed by GitHub Mar 31, 2017
Show whitespace changes
Inline Side-by-side

Showing with 16 additions and 8 deletions

BUILD pkg/kubelet/cm/BUILD +0 -1

container_manager_linux.go pkg/kubelet/cm/container_manager_linux.go +16 -7

No files found.
--- a/pkg/kubelet/cm/BUILD
+++ b/pkg/kubelet/cm/BUILD
@@ -46,7 +46,6 @@ go_library(
        "//vendor:k8s.io/apimachinery/pkg/api/resource",
        "//vendor:k8s.io/apimachinery/pkg/types",
        "//vendor:k8s.io/apimachinery/pkg/util/errors",
-        "//vendor:k8s.io/apimachinery/pkg/util/runtime",
        "//vendor:k8s.io/apimachinery/pkg/util/sets",
        "//vendor:k8s.io/apimachinery/pkg/util/wait",
        "//vendor:k8s.io/client-go/tools/record",

--- a/pkg/kubelet/cm/container_manager_linux.go
+++ b/pkg/kubelet/cm/container_manager_linux.go
@@ -35,7 +35,6 @@ import (
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"k8s.io/apimachinery/pkg/api/resource"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
-	"k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/tools/record"
@@ -630,15 +629,25 @@ func getPidFromPidFile(pidFile string) (int, error) {
 }
 func getPidsForProcess(name, pidFile string) ([]int, error) {
-	if len(pidFile) > 0 {
+	if len(pidFile) == 0 {
-		if pid, err := getPidFromPidFile(pidFile); err == nil {
+		return procfs.PidOf(name)
+	}
+	pid, err := getPidFromPidFile(pidFile)
+	if err == nil {
 		return []int{pid}, nil
-		} else {
-			// log the error and fall back to pidof
-			runtime.HandleError(err)
 	}
+	// Try to lookup pid by process name
+	pids, err2 := procfs.PidOf(name)
+	if err2 == nil {
+		return pids, nil
 	}
-	return procfs.PidOf(name)
+	// Return error from getPidFromPidFile since that should have worked
+	// and is the real source of the problem.
+	glog.V(4).Infof("unable to get pid from %s: %v", pidFile, err)
+	return []int{}, err
 }
 // Ensures that the Docker daemon is in the desired container.