Refactor daemon container list.

Separate container iteration, filtering and reducing.
This will make easier in the future to improve the implementation of
docker ps as we know it.

The end goal is to unify the objects returned by the api for docker ps
and docker inspect, leaving all docker ps transformations to the client.

Signed-off-by: David Calavera <david.calavera@gmail.com>

daemon/list.go

@@ -14,13 +14,31 @@ import (
 	"github.com/docker/docker/pkg/parsers/filters"
 )
 
+// iterationAction represents possible outcomes happening during the container iteration.
+type iterationAction int
+
+// containerReducer represents a reducer for a container.
+// Returns the object to serialize by the api.
+type containerReducer func(*Container, *listContext) (*types.Container, error)
+
+const (
+	// includeContainer is the action to include a container in the reducer.
+	includeContainer iterationAction = iota
+	// excludeContainer is the action to exclude a container in the reducer.
+	excludeContainer
+	// stopIteration is the action to stop iterating over the list of containers.
+	stopIteration
+)
+
+// errStopIteration makes the iterator to stop without returning an error.
+var errStopIteration = errors.New("container list iteration stopped")
+
 // List returns an array of all containers registered in the daemon.
 func (daemon *Daemon) List() []*Container {
 	return daemon.containers.List()
 }
 
-// ContainersConfig is a struct for configuring the command to list
-// containers.
+// ContainersConfig is the filtering specified by the user to iterate over containers.
 type ContainersConfig struct {
 	// if true show all containers, otherwise only running containers.
 	All bool
@@ -36,24 +54,85 @@ type ContainersConfig struct {
 	Filters string
 }
 
-// Containers returns a list of all the containers.
-func (daemon *Daemon) Containers(config *ContainersConfig) ([]*types.Container, error) {
-	var (
-		foundBefore    bool
-		displayed      int
-		ancestorFilter bool
-		all            = config.All
-		n              = config.Limit
-		psFilters      filters.Args
-		filtExited     []int
-	)
-	imagesFilter := map[string]bool{}
-	containers := []*types.Container{}
+// listContext is the daemon generated filtering to iterate over containers.
+// This is created based on the user specification.
+type listContext struct {
+	// idx is the container iteration index for this context
+	idx int
+	// ancestorFilter tells whether it should check ancestors or not
+	ancestorFilter bool
+	// names is a list of container names to filter with
+	names map[string][]string
+	// images is a list of images to filter with
+	images map[string]bool
+	// filters is a collection of arguments to filter with, specified by the user
+	filters filters.Args
+	// exitAllowed is a list of exit codes allowed to filter with
+	exitAllowed []int
+	// beforeContainer is a filter to ignore containers that appear before the one given
+	beforeContainer *Container
+	// sinceContainer is a filter to stop the filtering when the iterator arrive to the given container
+	sinceContainer *Container
+	// ContainersConfig is the filters set by the user
+	*ContainersConfig
+}
 
+// Containers returns the list of containers to show given the user's filtering.
+func (daemon *Daemon) Containers(config *ContainersConfig) ([]*types.Container, error) {
+	return daemon.reduceContainers(config, daemon.transformContainer)
+}
+
+// reduceContainer parses the user filtering and generates the list of containers to return based on a reducer.
+func (daemon *Daemon) reduceContainers(config *ContainersConfig, reducer containerReducer) ([]*types.Container, error) {
+	var containers []*types.Container
+
+	ctx, err := daemon.foldFilter(config)
+	if err != nil {
+		return nil, err
+	}
+
+	for _, container := range daemon.List() {
+		t, err := daemon.reducePsContainer(container, ctx, reducer)
+		if err != nil {
+			if err != errStopIteration {
+				return nil, err
+			}
+			break
+		}
+		if t != nil {
+			containers = append(containers, t)
+			ctx.idx++
+		}
+	}
+	return containers, nil
+}
+
+// reducePsContainer is the basic representation for a container as expected by the ps command.
+func (daemon *Daemon) reducePsContainer(container *Container, ctx *listContext, reducer containerReducer) (*types.Container, error) {
+	container.Lock()
+	defer container.Unlock()
+
+	// filter containers to return
+	action := includeContainerInList(container, ctx)
+	switch action {
+	case excludeContainer:
+		return nil, nil
+	case stopIteration:
+		return nil, errStopIteration
+	}
+
+	// transform internal container struct into api structs
+	return reducer(container, ctx)
+}
+
+// foldFilter generates the container filter based in the user's filtering options.
+func (daemon *Daemon) foldFilter(config *ContainersConfig) (*listContext, error) {
 	psFilters, err := filters.FromParam(config.Filters)
 	if err != nil {
 		return nil, err
 	}
+
+	var filtExited []int
 	if i, ok := psFilters["exited"]; ok {
 		for _, value := range i {
 			code, err := strconv.Atoi(value)
@@ -70,11 +149,13 @@ func (daemon *Daemon) Containers(config *ContainersConfig) ([]*types.Container,
 				return nil, errors.New("Unrecognised filter value for status")
 			}
 			if value == "exited" || value == "created" {
-				all = true
+				config.All = true
 			}
 		}
 	}
 
+	imagesFilter := map[string]bool{}
+	var ancestorFilter bool
 	if ancestors, ok := psFilters["ancestor"]; ok {
 		ancestorFilter = true
 		byParents := daemon.Graph().ByParent()
@@ -116,147 +197,165 @@ func (daemon *Daemon) Containers(config *ContainersConfig) ([]*types.Container,
 		}
 	}
 
-	errLast := errors.New("last container")
-	writeCont := func(container *Container) error {
-		container.Lock()
-		defer container.Unlock()
-		if !container.Running && !all && n <= 0 && config.Since == "" && config.Before == "" {
-			return nil
-		}
-		if !psFilters.Match("name", container.Name) {
-			return nil
-		}
+	return &listContext{
+		filters:          psFilters,
+		ancestorFilter:   ancestorFilter,
+		names:            names,
+		images:           imagesFilter,
+		exitAllowed:      filtExited,
+		beforeContainer:  beforeCont,
+		sinceContainer:   sinceCont,
+		ContainersConfig: config,
+	}, nil
+}
 
-		if !psFilters.Match("id", container.ID) {
-			return nil
-		}
-
-		if !psFilters.MatchKVList("label", container.Config.Labels) {
-			return nil
-		}
-
-		if config.Before != "" && !foundBefore {
-			if container.ID == beforeCont.ID {
-				foundBefore = true
-			}
-			return nil
-		}
-		if n > 0 && displayed == n {
-			return errLast
-		}
-		if config.Since != "" {
-			if container.ID == sinceCont.ID {
-				return errLast
-			}
-		}
-		if len(filtExited) > 0 {
-			shouldSkip := true
-			for _, code := range filtExited {
-				if code == container.ExitCode && !container.Running {
-					shouldSkip = false
-					break
-				}
-			}
-			if shouldSkip {
-				return nil
-			}
-		}
-
-		if !psFilters.Match("status", container.State.StateString()) {
-			return nil
-		}
-
-		if ancestorFilter {
-			if len(imagesFilter) == 0 {
-				return nil
-			}
-			if !imagesFilter[container.ImageID] {
-				return nil
-			}
-		}
-
-		displayed++
-		newC := &types.Container{
-			ID:    container.ID,
-			Names: names[container.ID],
-		}
-
-		img, err := daemon.Repositories().LookupImage(container.Config.Image)
-		if err != nil {
-			// If the image can no longer be found by its original reference,
-			// it makes sense to show the ID instead of a stale reference.
-			newC.Image = container.ImageID
-		} else if container.ImageID == img.ID {
-			newC.Image = container.Config.Image
-		} else {
-			newC.Image = container.ImageID
-		}
-
-		if len(container.Args) > 0 {
-			args := []string{}
-			for _, arg := range container.Args {
-				if strings.Contains(arg, " ") {
-					args = append(args, fmt.Sprintf("'%s'", arg))
-				} else {
-					args = append(args, arg)
-				}
-			}
-			argsAsString := strings.Join(args, " ")
-
-			newC.Command = fmt.Sprintf("%s %s", container.Path, argsAsString)
-		} else {
-			newC.Command = fmt.Sprintf("%s", container.Path)
-		}
-		newC.Created = container.Created.Unix()
-		newC.Status = container.State.String()
-		newC.HostConfig.NetworkMode = string(container.hostConfig.NetworkMode)
-
-		newC.Ports = []types.Port{}
-		for port, bindings := range container.NetworkSettings.Ports {
-			p, err := nat.ParsePort(port.Port())
-			if err != nil {
-				return err
-			}
-			if len(bindings) == 0 {
-				newC.Ports = append(newC.Ports, types.Port{
-					PrivatePort: p,
-					Type:        port.Proto(),
-				})
-				continue
-			}
-			for _, binding := range bindings {
-				h, err := nat.ParsePort(binding.HostPort)
-				if err != nil {
-					return err
-				}
-				newC.Ports = append(newC.Ports, types.Port{
-					PrivatePort: p,
-					PublicPort:  h,
-					Type:        port.Proto(),
-					IP:          binding.HostIP,
-				})
-			}
-		}
-
-		if config.Size {
-			sizeRw, sizeRootFs := container.getSize()
-			newC.SizeRw = sizeRw
-			newC.SizeRootFs = sizeRootFs
-		}
-		newC.Labels = container.Config.Labels
-		containers = append(containers, newC)
-		return nil
+// includeContainerInList decides whether a containers should be include in the output or not based in the filter.
+// It also decides if the iteration should be stopped or not.
+func includeContainerInList(container *Container, ctx *listContext) iterationAction {
+	// Do not include container if it's stopped and we're not filters
+	if !container.Running && !ctx.All && ctx.Limit <= 0 && ctx.beforeContainer == nil && ctx.sinceContainer == nil {
+		return excludeContainer
 	}
 
-	for _, container := range daemon.List() {
-		if err := writeCont(container); err != nil {
-			if err != errLast {
+	// Do not include container if the name doesn't match
+	if !ctx.filters.Match("name", container.Name) {
+		return excludeContainer
+	}
+
+	// Do not include container if the id doesn't match
+	if !ctx.filters.Match("id", container.ID) {
+		return excludeContainer
+	}
+
+	// Do not include container if any of the labels don't match
+	if !ctx.filters.MatchKVList("label", container.Config.Labels) {
+		return excludeContainer
+	}
+
+	// Do not include container if it's in the list before the filter container.
+	// Set the filter container to nil to include the rest of containers after this one.
+	if ctx.beforeContainer != nil {
+		if container.ID == ctx.beforeContainer.ID {
+			ctx.beforeContainer = nil
+		}
+		return excludeContainer
+	}
+
+	// Stop iteration when the index is over the limit
+	if ctx.Limit > 0 && ctx.idx == ctx.Limit {
+		return stopIteration
+	}
+
+	// Stop interation when the container arrives to the filter container
+	if ctx.sinceContainer != nil {
+		if container.ID == ctx.sinceContainer.ID {
+			return stopIteration
+		}
+	}
+
+	// Do not include container if its exit code is not in the filter
+	if len(ctx.exitAllowed) > 0 {
+		shouldSkip := true
+		for _, code := range ctx.exitAllowed {
+			if code == container.ExitCode && !container.Running {
+				shouldSkip = false
+				break
+			}
+		}
+		if shouldSkip {
+			return excludeContainer
+		}
+	}
+
+	// Do not include container if its status doesn't match the filter
+	if !ctx.filters.Match("status", container.State.StateString()) {
+		return excludeContainer
+	}
+
+	if ctx.ancestorFilter {
+		if len(ctx.images) == 0 {
+			return excludeContainer
+		}
+		if !ctx.images[container.ImageID] {
+			return excludeContainer
+		}
+	}
+
+	return includeContainer
+}
+
+// transformContainer generates the container type expected by the docker ps command.
+func (daemon *Daemon) transformContainer(container *Container, ctx *listContext) (*types.Container, error) {
+	newC := &types.Container{
+		ID:    container.ID,
+		Names: ctx.names[container.ID],
+	}
+
+	img, err := daemon.Repositories().LookupImage(container.Config.Image)
+	if err != nil {
+		// If the image can no longer be found by its original reference,
+		// it makes sense to show the ID instead of a stale reference.
+		newC.Image = container.ImageID
+	} else if container.ImageID == img.ID {
+		newC.Image = container.Config.Image
+	} else {
+		newC.Image = container.ImageID
+	}
+
+	if len(container.Args) > 0 {
+		args := []string{}
+		for _, arg := range container.Args {
+			if strings.Contains(arg, " ") {
+				args = append(args, fmt.Sprintf("'%s'", arg))
+			} else {
+				args = append(args, arg)
+			}
+		}
+		argsAsString := strings.Join(args, " ")
+
+		newC.Command = fmt.Sprintf("%s %s", container.Path, argsAsString)
+	} else {
+		newC.Command = container.Path
+	}
+	newC.Created = container.Created.Unix()
+	newC.Status = container.State.String()
+	newC.HostConfig.NetworkMode = string(container.hostConfig.NetworkMode)
+
+	newC.Ports = []types.Port{}
+	for port, bindings := range container.NetworkSettings.Ports {
+		p, err := nat.ParsePort(port.Port())
+		if err != nil {
+			return nil, err
+		}
+		if len(bindings) == 0 {
+			newC.Ports = append(newC.Ports, types.Port{
+				PrivatePort: p,
+				Type:        port.Proto(),
+			})
+			continue
+		}
+		for _, binding := range bindings {
+			h, err := nat.ParsePort(binding.HostPort)
+			if err != nil {
 				return nil, err
 			}
-			break
+			newC.Ports = append(newC.Ports, types.Port{
+				PrivatePort: p,
+				PublicPort:  h,
+				Type:        port.Proto(),
+				IP:          binding.HostIP,
+			})
 		}
 	}
-	return containers, nil
+
+	if ctx.Size {
+		sizeRw, sizeRootFs := container.getSize()
+		newC.SizeRw = sizeRw
+		newC.SizeRootFs = sizeRootFs
+	}
+	newC.Labels = container.Config.Labels
+
+	return newC, nil
 }
 
 // Volumes lists known volumes, using the filter to restrict the range

integration-cli/docker_cli_ps_test.go

@@ -17,8 +17,7 @@ import (
 	"github.com/docker/docker/pkg/stringid"
 )
 
-func (s *DockerSuite) TestPsListContainers(c *check.C) {
-
+func (s *DockerSuite) TestPsListContainersBase(c *check.C) {
 	out, _ := dockerCmd(c, "run", "-d", "busybox", "top")
 	firstID := strings.TrimSpace(out)
 
@@ -44,13 +43,13 @@ func (s *DockerSuite) TestPsListContainers(c *check.C) {
 	// all
 	out, _ = dockerCmd(c, "ps", "-a")
 	if !assertContainerList(out, []string{fourthID, thirdID, secondID, firstID}) {
-		c.Errorf("Container list is not in the correct order: %s", out)
+		c.Errorf("ALL: Container list is not in the correct order: \n%s", out)
 	}
 
 	// running
 	out, _ = dockerCmd(c, "ps")
 	if !assertContainerList(out, []string{fourthID, secondID, firstID}) {
-		c.Errorf("Container list is not in the correct order: %s", out)
+		c.Errorf("RUNNING: Container list is not in the correct order: \n%s", out)
 	}
 
 	// from here all flag '-a' is ignored
@@ -59,48 +58,48 @@ func (s *DockerSuite) TestPsListContainers(c *check.C) {
 	out, _ = dockerCmd(c, "ps", "-n=2", "-a")
 	expected := []string{fourthID, thirdID}
 	if !assertContainerList(out, expected) {
-		c.Errorf("Container list is not in the correct order: %s", out)
+		c.Errorf("LIMIT & ALL: Container list is not in the correct order: \n%s", out)
 	}
 
 	out, _ = dockerCmd(c, "ps", "-n=2")
 	if !assertContainerList(out, expected) {
-		c.Errorf("Container list is not in the correct order: %s", out)
+		c.Errorf("LIMIT: Container list is not in the correct order: \n%s", out)
 	}
 
 	// since
 	out, _ = dockerCmd(c, "ps", "--since", firstID, "-a")
 	expected = []string{fourthID, thirdID, secondID}
 	if !assertContainerList(out, expected) {
-		c.Errorf("Container list is not in the correct order: %s", out)
+		c.Errorf("SINCE & ALL: Container list is not in the correct order: \n%s", out)
 	}
 
 	out, _ = dockerCmd(c, "ps", "--since", firstID)
 	if !assertContainerList(out, expected) {
-		c.Errorf("Container list is not in the correct order: %s", out)
+		c.Errorf("SINCE: Container list is not in the correct order: \n%s", out)
 	}
 
 	// before
 	out, _ = dockerCmd(c, "ps", "--before", thirdID, "-a")
 	expected = []string{secondID, firstID}
 	if !assertContainerList(out, expected) {
-		c.Errorf("Container list is not in the correct order: %s", out)
+		c.Errorf("BEFORE & ALL: Container list is not in the correct order: \n%s", out)
 	}
 
 	out, _ = dockerCmd(c, "ps", "--before", thirdID)
 	if !assertContainerList(out, expected) {
-		c.Errorf("Container list is not in the correct order: %s", out)
+		c.Errorf("BEFORE: Container list is not in the correct order: \n%s", out)
 	}
 
 	// since & before
 	out, _ = dockerCmd(c, "ps", "--since", firstID, "--before", fourthID, "-a")
 	expected = []string{thirdID, secondID}
 	if !assertContainerList(out, expected) {
-		c.Errorf("Container list is not in the correct order: %s", out)
+		c.Errorf("SINCE, BEFORE & ALL: Container list is not in the correct order: \n%s", out)
 	}
 
 	out, _ = dockerCmd(c, "ps", "--since", firstID, "--before", fourthID)
 	if !assertContainerList(out, expected) {
-		c.Errorf("Container list is not in the correct order: %s", out)
+		c.Errorf("SINCE, BEFORE: Container list is not in the correct order: \n%s", out)
 	}
 
 	// since & limit
@@ -108,35 +107,35 @@ func (s *DockerSuite) TestPsListContainers(c *check.C) {
 	expected = []string{fourthID, thirdID}
 
 	if !assertContainerList(out, expected) {
-		c.Errorf("Container list is not in the correct order: %s", out)
+		c.Errorf("SINCE, LIMIT & ALL: Container list is not in the correct order: \n%s", out)
 	}
 
 	out, _ = dockerCmd(c, "ps", "--since", firstID, "-n=2")
 	if !assertContainerList(out, expected) {
-		c.Errorf("Container list is not in the correct order: %s", out)
+		c.Errorf("SINCE, LIMIT: Container list is not in the correct order: \n%s", out)
 	}
 
 	// before & limit
 	out, _ = dockerCmd(c, "ps", "--before", fourthID, "-n=1", "-a")
 	expected = []string{thirdID}
 	if !assertContainerList(out, expected) {
-		c.Errorf("Container list is not in the correct order: %s", out)
+		c.Errorf("BEFORE, LIMIT & ALL: Container list is not in the correct order: \n%s", out)
 	}
 
 	out, _ = dockerCmd(c, "ps", "--before", fourthID, "-n=1")
 	if !assertContainerList(out, expected) {
-		c.Errorf("Container list is not in the correct order: %s", out)
+		c.Errorf("BEFORE, LIMIT: Container list is not in the correct order: \n%s", out)
 	}
 
 	out, _ = dockerCmd(c, "ps", "--since", firstID, "--before", fourthID, "-n=1", "-a")
 	expected = []string{thirdID}
 	if !assertContainerList(out, expected) {
-		c.Errorf("Container list is not in the correct order: %s", out)
+		c.Errorf("SINCE, BEFORE, LIMIT & ALL: Container list is not in the correct order: \n%s", out)
 	}
 
 	out, _ = dockerCmd(c, "ps", "--since", firstID, "--before", fourthID, "-n=1")
 	if !assertContainerList(out, expected) {
-		c.Errorf("Container list is not in the correct order: %s", out)
+		c.Errorf("SINCE, BEFORE, LIMIT: Container list is not in the correct order: \n%s", out)
 	}
 
 }