Refactoring and fixes of /docker API endpoint.

This canges the output of the Docker endpoint to be a map so that it is
more consistent from single to multiple returns. It also refactors
internally how we handle both types of requests.

Without this PR the /docker API endpoint is broken completely so this
change in format has no effect anyways.

These changes are tested by the upcoming integration tests.

api/handler.go

@@ -158,6 +158,7 @@ func handleRequest(m manager.Manager, w http.ResponseWriter, r *http.Request) er
 			return fmt.Errorf("request type of %q not supported in API version %q", requestType, version)
 		}
 
+		containerName = strings.TrimLeft(containerName, "/")
 		glog.V(2).Infof("Api - Docker(%s)", containerName)
 
 		// Get the query request.
@@ -166,10 +167,23 @@ func handleRequest(m manager.Manager, w http.ResponseWriter, r *http.Request) er
 			return err
 		}
 
-		// Get the Docker containers.
+		var containers map[string]info.ContainerInfo
-		containers, err := m.DockerContainersInfo(containerName, query)
+		if containerName == "" {
-		if err != nil {
+			// Get all Docker containers.
-			return fmt.Errorf("failed to get docker containers for %q with error: %s", containerName, err)
+			containers, err = m.AllDockerContainers(query)
+			if err != nil {
+				return fmt.Errorf("failed to get all Docker containers with error: %v", err)
+			}
+		} else {
+			// Get one Docker container.
+			var cont info.ContainerInfo
+			cont, err = m.DockerContainer(containerName, query)
+			if err != nil {
+				return fmt.Errorf("failed to get Docker container %q with error: %v", containerName, err)
+			}
+			containers = map[string]info.ContainerInfo{
+				cont.Name: cont,
+			}
 		}
 
 		// Only output the containers as JSON.

client/client.go

@@ -39,7 +39,7 @@ func NewClient(url string) (*Client, error) {
 	}
 
 	return &Client{
-		baseUrl: fmt.Sprintf("%sapi/v1.1/", url),
+		baseUrl: fmt.Sprintf("%sapi/v1.2/", url),
 	}, nil
 }
 
@@ -80,6 +80,38 @@ func (self *Client) SubcontainersInfo(name string, query *info.ContainerInfoRequ
 	return response, nil
 }
 
+// Returns the JSON container information for the specified
+// Docker container and request.
+func (self *Client) DockerContainer(name string, query *info.ContainerInfoRequest) (cinfo info.ContainerInfo, err error) {
+	u := self.dockerInfoUrl(name)
+	ret := make(map[string]info.ContainerInfo)
+	if err = self.httpGetJsonData(&ret, query, u, fmt.Sprintf("Docker container info for %q", name)); err != nil {
+		return
+	}
+	if len(ret) != 1 {
+		err = fmt.Errorf("expected to only receive 1 Docker container: %+v", ret)
+		return
+	}
+	for _, cont := range ret {
+		cinfo = cont
+	}
+	return
+}
+
+// Returns the JSON container information for all Docker containers.
+func (self *Client) AllDockerContainers(query *info.ContainerInfoRequest) (cinfo []info.ContainerInfo, err error) {
+	u := self.dockerInfoUrl("/")
+	ret := make(map[string]info.ContainerInfo)
+	if err = self.httpGetJsonData(&ret, query, u, "all Docker containers info"); err != nil {
+		return
+	}
+	cinfo = make([]info.ContainerInfo, 0, len(ret))
+	for _, cont := range ret {
+		cinfo = append(cinfo, cont)
+	}
+	return
+}
+
 func (self *Client) machineInfoUrl() string {
 	return self.baseUrl + path.Join("machine")
 }
@@ -92,6 +124,10 @@ func (self *Client) subcontainersInfoUrl(name string) string {
 	return self.baseUrl + path.Join("subcontainers", name)
 }
 
+func (self *Client) dockerInfoUrl(name string) string {
+	return self.baseUrl + path.Join("docker", name)
+}
+
 func (self *Client) httpGetJsonData(data, postData interface{}, url, infoName string) error {
 	var resp *http.Response
 	var err error
@@ -106,17 +142,19 @@ func (self *Client) httpGetJsonData(data, postData interface{}, url, infoName st
 		resp, err = http.Get(url)
 	}
 	if err != nil {
-		err = fmt.Errorf("unable to get %v: %v", infoName, err)
+		return fmt.Errorf("unable to get %q: %v", infoName, err)
-		return err
+	}
+	if resp == nil {
+		return fmt.Errorf("received empty response from %q", infoName)
 	}
 	defer resp.Body.Close()
 	body, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
-		err = fmt.Errorf("unable to read all %v: %v", infoName, err)
+		err = fmt.Errorf("unable to read all %q: %v", infoName, err)
 		return err
 	}
 	if err = json.Unmarshal(body, data); err != nil {
-		err = fmt.Errorf("unable to unmarshal %v (%v): %v", infoName, string(body), err)
+		err = fmt.Errorf("unable to unmarshal %q (%v): %v", infoName, string(body), err)
 		return err
 	}
 	return nil

client/client_test.go

@@ -57,7 +57,7 @@ func cadvisorTestClient(path string, expectedPostObj, expectedPostObjEmpty, repl
 			}
 			encoder := json.NewEncoder(w)
 			encoder.Encode(replyObj)
-		} else if r.URL.Path == "/api/v1.1/machine" {
+		} else if r.URL.Path == "/api/v1.2/machine" {
 			fmt.Fprint(w, `{"num_cores":8,"memory_capacity":31625871360}`)
 		} else {
 			w.WriteHeader(http.StatusNotFound)
@@ -79,7 +79,7 @@ func TestGetMachineinfo(t *testing.T) {
 		NumCores:       8,
 		MemoryCapacity: 31625871360,
 	}
-	client, server, err := cadvisorTestClient("/api/v1.1/machine", nil, nil, minfo, t)
+	client, server, err := cadvisorTestClient("/api/v1.2/machine", nil, nil, minfo, t)
 	if err != nil {
 		t.Fatalf("unable to get a client %v", err)
 	}
@@ -101,7 +101,7 @@ func TestGetContainerInfo(t *testing.T) {
 	}
 	containerName := "/some/container"
 	cinfo := itest.GenerateRandomContainerInfo(containerName, 4, query, 1*time.Second)
-	client, server, err := cadvisorTestClient(fmt.Sprintf("/api/v1.1/containers%v", containerName), query, &info.ContainerInfoRequest{}, cinfo, t)
+	client, server, err := cadvisorTestClient(fmt.Sprintf("/api/v1.2/containers%v", containerName), query, &info.ContainerInfoRequest{}, cinfo, t)
 	if err != nil {
 		t.Fatalf("unable to get a client %v", err)
 	}
@@ -129,7 +129,7 @@ func TestGetSubcontainersInfo(t *testing.T) {
 		*cinfo1,
 		*cinfo2,
 	}
-	client, server, err := cadvisorTestClient(fmt.Sprintf("/api/v1.1/subcontainers%v", containerName), query, &info.ContainerInfoRequest{}, response, t)
+	client, server, err := cadvisorTestClient(fmt.Sprintf("/api/v1.2/subcontainers%v", containerName), query, &info.ContainerInfoRequest{}, response, t)
 	if err != nil {
 		t.Fatalf("unable to get a client %v", err)
 	}

manager/manager.go

@@ -50,9 +50,11 @@ type Manager interface {
 	// Get information about all subcontainers of the specified container (includes self).
 	SubcontainersInfo(containerName string, query *info.ContainerInfoRequest) ([]*info.ContainerInfo, error)
 
-	// Get information for the specified Docker container (or "/" for all Docker containers).
+	// Gets all the Docker containers. Return is a map from full container name to ContainerInfo.
-	// Full container names here are interpreted within the Docker namespace (e.g.: /test -> top-level Docker container named 'test').
+	AllDockerContainers(query *info.ContainerInfoRequest) (map[string]info.ContainerInfo, error)
-	DockerContainersInfo(containerName string, query *info.ContainerInfoRequest) ([]*info.ContainerInfo, error)
+
+	// Gets information about a specific Docker container. The specified name is within the Docker namespace.
+	DockerContainer(dockerName string, query *info.ContainerInfoRequest) (info.ContainerInfo, error)
 
 	// Get information about the machine.
 	GetMachineInfo() (*info.MachineInfo, error)
@@ -268,45 +270,56 @@ func (self *manager) SubcontainersInfo(containerName string, query *info.Contain
 	return self.containerDataSliceToContainerInfoSlice(containers, query)
 }
 
-func (self *manager) DockerContainersInfo(containerName string, query *info.ContainerInfoRequest) ([]*info.ContainerInfo, error) {
+func (self *manager) AllDockerContainers(query *info.ContainerInfoRequest) (map[string]info.ContainerInfo, error) {
 	var containers map[string]*containerData
-	err := func() error {
+	func() {
 		self.containersLock.RLock()
 		defer self.containersLock.RUnlock()
 		containers = make(map[string]*containerData, len(self.containers))
 
-		if containerName == "/" {
+		// Get containers in the Docker namespace.
-			// Get all the unique containers in the Docker namespace.
+		for name, cont := range self.containers {
-			for name, cont := range self.containers {
+			if name.Namespace == docker.DockerNamespace {
-				if name.Namespace == docker.DockerNamespace {
-					containers[cont.info.Name] = cont
-				}
-			}
-		} else {
-			// Check for the container in the Docker container namespace.
-			cont, ok := self.containers[namespacedContainerName{
-				Namespace: docker.DockerNamespace,
-				Name:      containerName,
-			}]
-			if ok {
 				containers[cont.info.Name] = cont
-			} else {
-				return fmt.Errorf("unable to find Docker container %q", containerName)
 			}
 		}
-		return nil
 	}()
+
+	output := make(map[string]info.ContainerInfo, len(containers))
+	for name, cont := range containers {
+		inf, err := self.containerDataToContainerInfo(cont, query)
+		if err != nil {
+			return nil, err
+		}
+		output[name] = *inf
+	}
+	return output, nil
+}
+
+func (self *manager) DockerContainer(containerName string, query *info.ContainerInfoRequest) (info.ContainerInfo, error) {
+	var container *containerData = nil
+	func() {
+		self.containersLock.RLock()
+		defer self.containersLock.RUnlock()
+
+		// Check for the container in the Docker container namespace.
+		cont, ok := self.containers[namespacedContainerName{
+			Namespace: docker.DockerNamespace,
+			Name:      containerName,
+		}]
+		if ok {
+			container = cont
+		}
+	}()
+	if container == nil {
+		return info.ContainerInfo{}, fmt.Errorf("unable to find Docker container %q", containerName)
+	}
+
+	inf, err := self.containerDataToContainerInfo(container, query)
 	if err != nil {
-		return nil, err
+		return info.ContainerInfo{}, err
 	}
-
+	return *inf, nil
-	// Convert to a slice.
-	containersSlice := make([]*containerData, 0, len(containers))
-	for _, cont := range containers {
-		containersSlice = append(containersSlice, cont)
-	}
-
-	return self.containerDataSliceToContainerInfoSlice(containersSlice, query)
 }
 
 func (self *manager) containerDataSliceToContainerInfoSlice(containers []*containerData, query *info.ContainerInfoRequest) ([]*info.ContainerInfo, error) {

manager/manager_test.go

@@ -190,15 +190,12 @@ func TestDockerContainersInfo(t *testing.T) {
 
 	m, _, _ := expectManagerWithContainers(containers, query, t)
 
-	result, err := m.DockerContainersInfo("c1", query)
+	result, err := m.DockerContainer("c1", query)
 	if err != nil {
 		t.Fatalf("expected to succeed: %s", err)
 	}
-	if len(result) != len(containers) {
+	if result.Name != containers[0] {
-		t.Errorf("expected to received a containers %v, but received: %v", containers, result)
+		t.Errorf("Unexpected container %q in result. Expected container %q", result.Name, containers[0])
-	}
-	if result[0].Name != containers[0] {
-		t.Errorf("Unexpected container %q in result. Expected container %q", result[0].Name, containers[0])
 	}
 }
 

pages/docker.go

@@ -27,7 +27,7 @@ func serveDockerPage(m manager.Manager, w http.ResponseWriter, u *url.URL) error
 		reqParams := info.ContainerInfoRequest{
 			NumStats: 0,
 		}
-		conts, err := m.DockerContainersInfo("/", &reqParams)
+		conts, err := m.AllDockerContainers(&reqParams)
 		if err != nil {
 			return fmt.Errorf("Failed to get container %q with error: %v", containerName, err)
 		}
@@ -54,14 +54,10 @@ func serveDockerPage(m manager.Manager, w http.ResponseWriter, u *url.URL) error
 		reqParams := info.ContainerInfoRequest{
 			NumStats: 60,
 		}
-		conts, err := m.DockerContainersInfo(containerName, &reqParams)
+		cont, err := m.DockerContainer(containerName, &reqParams)
 		if err != nil {
 			return fmt.Errorf("failed to get container %q with error: %v", containerName, err)
 		}
-		if len(conts) != 1 {
-			return fmt.Errorf("received unexpected container for %q: %v", conts)
-		}
-		cont := conts[0]
 		displayName := getContainerDisplayName(cont.ContainerReference)
 
 		// Make a list of the parent containers and their links