Add timeouts for docker queries

As these can otherwise block indefinitely due to docker issues. This is to fix kubernetes/kubernetes#53207, where kubelet relies on cadvisor for gathering docker information as part of its periodic node status update.
jsravn · Dec 14, 2017 · 83adb46 · 83adb46
1 parent b69b9b6
commit 83adb46
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 20 deletions.
diff --git a/container/docker/docker.go b/container/docker/docker.go
@@ -24,26 +24,33 @@ import (
 	dockertypes "github.com/docker/engine-api/types"
 	"golang.org/x/net/context"
 
+	"time"
+
 	"github.com/google/cadvisor/info/v1"
 	"github.com/google/cadvisor/machine"
 )
 
+const defaultTimeout = time.Second * 5
+
+func defaultContext() context.Context {
+	ctx, _ := context.WithTimeout(context.Background(), defaultTimeout)
+	return ctx
+}
+
 func Status() (v1.DockerStatus, error) {
 	client, err := Client()
 	if err != nil {
 		return v1.DockerStatus{}, fmt.Errorf("unable to communicate with docker daemon: %v", err)
 	}
-	dockerInfo, err := client.Info(context.Background())
+	dockerInfo, err := client.Info(defaultContext())
 	if err != nil {
 		return v1.DockerStatus{}, err
 	}
-	return StatusFromDockerInfo(dockerInfo), nil
+	return StatusFromDockerInfo(dockerInfo)
 }
 
-func StatusFromDockerInfo(dockerInfo dockertypes.Info) v1.DockerStatus {
+func StatusFromDockerInfo(dockerInfo dockertypes.Info) (v1.DockerStatus, error) {
 	out := v1.DockerStatus{}
-	out.Version = VersionString()
-	out.APIVersion = APIVersionString()
 	out.KernelVersion = machine.KernelVersion()
 	out.OS = dockerInfo.OperatingSystem
 	out.Hostname = dockerInfo.Name
@@ -56,15 +63,26 @@ func StatusFromDockerInfo(dockerInfo dockertypes.Info) v1.DockerStatus {
 	for _, v := range dockerInfo.DriverStatus {
 		out.DriverStatus[v[0]] = v[1]
 	}
-	return out
+	var err error
+	ver, err := VersionString()
+	if err != nil {
+		return out, err
+	}
+	out.Version = ver
+	ver, err = APIVersionString()
+	if err != nil {
+		return out, err
+	}
+	out.APIVersion = ver
+	return out, nil
 }
 
 func Images() ([]v1.DockerImage, error) {
 	client, err := Client()
 	if err != nil {
 		return nil, fmt.Errorf("unable to communicate with docker daemon: %v", err)
 	}
-	images, err := client.ImageList(context.Background(), dockertypes.ImageListOptions{All: false})
+	images, err := client.ImageList(defaultContext(), dockertypes.ImageListOptions{All: false})
 	if err != nil {
 		return nil, err
 	}
@@ -97,14 +115,14 @@ func ValidateInfo() (*dockertypes.Info, error) {
 		return nil, fmt.Errorf("unable to communicate with docker daemon: %v", err)
 	}
 
-	dockerInfo, err := client.Info(context.Background())
+	dockerInfo, err := client.Info(defaultContext())
 	if err != nil {
 		return nil, fmt.Errorf("failed to detect Docker info: %v", err)
 	}
 
 	// Fall back to version API if ServerVersion is not set in info.
 	if dockerInfo.ServerVersion == "" {
-		version, err := client.ServerVersion(context.Background())
+		version, err := client.ServerVersion(defaultContext())
 		if err != nil {
 			return nil, fmt.Errorf("unable to get docker version: %v", err)
 		}
@@ -134,35 +152,43 @@ func ValidateInfo() (*dockertypes.Info, error) {
 }
 
 func Version() ([]int, error) {
-	return parseVersion(VersionString(), version_re, 3)
+	ver, err := VersionString()
+	if err != nil {
+		return nil, err
+	}
+	return parseVersion(ver, version_re, 3)
 }
 
 func APIVersion() ([]int, error) {
-	return parseVersion(APIVersionString(), apiversion_re, 2)
+	ver, err := APIVersionString()
+	if err != nil {
+		return nil, err
+	}
+	return parseVersion(ver, apiversion_re, 2)
 }
 
-func VersionString() string {
+func VersionString() (string, error) {
 	docker_version := "Unknown"
 	client, err := Client()
 	if err == nil {
-		version, err := client.ServerVersion(context.Background())
+		version, err := client.ServerVersion(defaultContext())
 		if err == nil {
 			docker_version = version.Version
 		}
 	}
-	return docker_version
+	return docker_version, err
 }
 
-func APIVersionString() string {
+func APIVersionString() (string, error) {
 	docker_api_version := "Unknown"
 	client, err := Client()
 	if err == nil {
-		version, err := client.ServerVersion(context.Background())
+		version, err := client.ServerVersion(defaultContext())
 		if err == nil {
 			docker_api_version = version.APIVersion
 		}
 	}
-	return docker_api_version
+	return docker_api_version, err
 }
 
 func parseVersion(version_string string, regex *regexp.Regexp, length int) ([]int, error) {

diff --git a/container/docker/factory.go b/container/docker/factory.go
@@ -340,7 +340,8 @@ func Register(factory info.MachineInfoFactory, fsInfo fs.FsInfo, ignoreMetrics c
 			glog.Errorf("devicemapper filesystem stats will not be reported: %v", err)
 		}
 
-		status := StatusFromDockerInfo(*dockerInfo)
+		// Safe to ignore error - driver status should always be populated.
+		status, _ := StatusFromDockerInfo(*dockerInfo)
 		thinPoolName = status.DriverStatus[dockerutil.DriverStatusPoolName]
 	}
 

diff --git a/manager/manager.go b/manager/manager.go
@@ -1305,8 +1305,14 @@ func getVersionInfo() (*info.VersionInfo, error) {
 
 	kernel_version := machine.KernelVersion()
 	container_os := machine.ContainerOsVersion()
-	docker_version := docker.VersionString()
-	docker_api_version := docker.APIVersionString()
+	docker_version, err := docker.VersionString()
+	if err != nil {
+		return nil, err
+	}
+	docker_api_version, err := docker.APIVersionString()
+	if err != nil {
+		return nil, err
+	}
 
 	return &info.VersionInfo{
 		KernelVersion:      kernel_version,