Merge pull request #15845 from calavera/refactor_daemon_list

Refactor daemon container list.

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)
 	}
 
 }