Merge pull request #25933 from timstclair/subcontainers

Automatic merge from submit-queue

Handle cAdvisor partial failures

Kubernetes side of https://github.com/google/cadvisor/issues/1286

Partially fixes https://github.com/kubernetes/kubernetes/issues/25131

(Depends on cAdvisor Godeps bump https://github.com/kubernetes/kubernetes/pull/25914)

/cc @kubernetes/sig-node

pkg/kubelet/cadvisor/cadvisor_linux.go

@@ -150,7 +150,7 @@ func (cc *cadvisorClient) VersionInfo() (*cadvisorapi.VersionInfo, error) {
 
 func (cc *cadvisorClient) SubcontainerInfo(name string, req *cadvisorapi.ContainerInfoRequest) (map[string]*cadvisorapi.ContainerInfo, error) {
 	infos, err := cc.SubcontainersInfo(name, req)
-	if err != nil {
+	if err != nil && len(infos) == 0 {
 		return nil, err
 	}
 
@@ -158,7 +158,7 @@ func (cc *cadvisorClient) SubcontainerInfo(name string, req *cadvisorapi.Contain
 	for _, info := range infos {
 		result[info.Name] = info
 	}
-	return result, nil
+	return result, err
 }
 
 func (cc *cadvisorClient) MachineInfo() (*cadvisorapi.MachineInfo, error) {

pkg/kubelet/server/stats/handler.go

@@ -24,11 +24,11 @@ import (
 	"path"
 	"time"
 
-	"github.com/emicklei/go-restful"
 	"github.com/golang/glog"
 	cadvisorapi "github.com/google/cadvisor/info/v1"
 	cadvisorapiv2 "github.com/google/cadvisor/info/v2"
 
+	"github.com/emicklei/go-restful"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/kubelet/cm"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
@@ -133,14 +133,14 @@ func parseStatsRequest(request *restful.Request) (StatsRequest, error) {
 func (h *handler) handleStats(request *restful.Request, response *restful.Response) {
 	query, err := parseStatsRequest(request)
 	if err != nil {
-		handleError(response, err)
+		handleError(response, "/stats", err)
 		return
 	}
 
 	// Root container stats.
 	statsMap, err := h.provider.GetRawContainerInfo("/", query.cadvisorRequest(), false)
 	if err != nil {
-		handleError(response, err)
+		handleError(response, fmt.Sprintf("/stats %v", query), err)
 		return
 	}
 	writeResponse(response, statsMap["/"])
@@ -150,7 +150,7 @@ func (h *handler) handleStats(request *restful.Request, response *restful.Respon
 func (h *handler) handleSummary(request *restful.Request, response *restful.Response) {
 	summary, err := h.summaryProvider.Get()
 	if err != nil {
-		handleError(response, err)
+		handleError(response, "/stats/summary", err)
 	} else {
 		writeResponse(response, summary)
 	}
@@ -160,7 +160,7 @@ func (h *handler) handleSummary(request *restful.Request, response *restful.Resp
 func (h *handler) handleSystemContainer(request *restful.Request, response *restful.Response) {
 	query, err := parseStatsRequest(request)
 	if err != nil {
-		handleError(response, err)
+		handleError(response, "/stats/container", err)
 		return
 	}
 
@@ -169,8 +169,13 @@ func (h *handler) handleSystemContainer(request *restful.Request, response *rest
 	stats, err := h.provider.GetRawContainerInfo(
 		containerName, query.cadvisorRequest(), query.Subcontainers)
 	if err != nil {
-		handleError(response, err)
+		if _, ok := stats[containerName]; ok {
-		return
+			// If the failure is partial, log it and return a best-effort response.
+			glog.Errorf("Partial failure issuing GetRawContainerInfo(%v): %v", query, err)
+		} else {
+			handleError(response, fmt.Sprintf("/stats/container %v", query), err)
+			return
+		}
 	}
 	writeResponse(response, stats)
 }
@@ -181,7 +186,7 @@ func (h *handler) handleSystemContainer(request *restful.Request, response *rest
 func (h *handler) handlePodContainer(request *restful.Request, response *restful.Response) {
 	query, err := parseStatsRequest(request)
 	if err != nil {
-		handleError(response, err)
+		handleError(response, request.Request.URL.String(), err)
 		return
 	}
 
@@ -203,7 +208,7 @@ func (h *handler) handlePodContainer(request *restful.Request, response *restful
 	pod, ok := h.provider.GetPodByName(params["namespace"], params["podName"])
 	if !ok {
 		glog.V(4).Infof("Container not found: %v", params)
-		handleError(response, kubecontainer.ErrContainerNotFound)
+		response.WriteError(http.StatusNotFound, kubecontainer.ErrContainerNotFound)
 		return
 	}
 	stats, err := h.provider.GetContainerInfo(
@@ -213,7 +218,7 @@ func (h *handler) handlePodContainer(request *restful.Request, response *restful
 		query.cadvisorRequest())
 
 	if err != nil {
-		handleError(response, err)
+		handleError(response, fmt.Sprintf("%s %v", request.Request.URL.String(), query), err)
 		return
 	}
 	writeResponse(response, stats)
@@ -226,13 +231,14 @@ func writeResponse(response *restful.Response, stats interface{}) {
 }
 
 // handleError serializes an error object into an HTTP response.
-func handleError(response *restful.Response, err error) {
+// request is provided for logging.
+func handleError(response *restful.Response, request string, err error) {
 	switch err {
 	case kubecontainer.ErrContainerNotFound:
 		response.WriteError(http.StatusNotFound, err)
 	default:
 		msg := fmt.Sprintf("Internal Error: %v", err)
-		glog.Errorf("HTTP InternalServerError: %s", msg)
+		glog.Errorf("HTTP InternalServerError serving %s: %s", request, msg)
 		response.WriteErrorString(http.StatusInternalServerError, msg)
 	}
 }

pkg/kubelet/server/stats/summary.go

@@ -68,26 +68,33 @@ func (sp *summaryProviderImpl) Get() (*stats.Summary, error) {
 	}
 	infos, err := sp.provider.GetContainerInfoV2("/", options)
 	if err != nil {
-		return nil, err
+		if _, ok := infos["/"]; ok {
+			// If the failure is partial, log it and return a best-effort response.
+			glog.Errorf("Partial failure issuing GetContainerInfoV2: %v", err)
+		} else {
+			return nil, fmt.Errorf("failed GetContainerInfoV2: %v", err)
+		}
 	}
 
+	// TODO(timstclair): Consider returning a best-effort response if any of the following errors
+	// occur.
 	node, err := sp.provider.GetNode()
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed GetNode: %v", err)
 	}
 
 	nodeConfig := sp.provider.GetNodeConfig()
 	rootFsInfo, err := sp.provider.RootFsInfo()
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed RootFsInfo: %v", err)
 	}
 	imageFsInfo, err := sp.provider.ImagesFsInfo()
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed DockerImagesFsInfo: %v", err)
 	}
 	imageStats, err := sp.runtime.ImageStats()
 	if err != nil || imageStats == nil {
-		return nil, err
+		return nil, fmt.Errorf("failed ImageStats: %v", err)
 	}
 	sb := &summaryBuilder{sp.fsResourceAnalyzer, node, nodeConfig, rootFsInfo, imageFsInfo, *imageStats, infos}
 	return sb.build()