Fix the since and before filter behavior

vdemeester · vdemeester · commit b41dba58a018 · 2016-02-09T09:26:20.000+01:00
Filters should not include stopped container if `-a` is not specified.
Right now, before and since filter are acting as --before and --since
deprecated flags. This commit is fixing that.

Signed-off-by: Vincent Demeester &lt;vincent@sbr.pm&gt;
diff --git a/daemon/list.go b/daemon/list.go
@@ -58,6 +58,13 @@ type listContext struct {
 	filters filters.Args
 	// exitAllowed is a list of exit codes allowed to filter with
 	exitAllowed []int
+
+	// FIXME Remove this for 1.12 as --since and --before are deprecated
+	// beforeContainer is a filter to ignore containers that appear before the one given
+	beforeContainer *container.Container
+	// sinceContainer is a filter to stop the filtering when the iterator arrive to the given container
+	sinceContainer *container.Container
+
 	// beforeFilter is a filter to ignore containers that appear before the one given
 	// this is used for --filter=before= and --before=, the latter is deprecated.
 	beforeFilter *container.Container
@@ -146,6 +153,9 @@ func (daemon *Daemon) foldFilter(config *types.ContainerListOptions) (*listConte
 	}
 
 	var beforeContFilter, sinceContFilter *container.Container
+	// FIXME remove this for 1.12 as --since and --before are deprecated
+	var beforeContainer, sinceContainer *container.Container
+
 	err = psFilters.WalkValues("before", func(value string) error {
 		beforeContFilter, err = daemon.GetContainer(value)
 		return err
@@ -182,15 +192,17 @@ func (daemon *Daemon) foldFilter(config *types.ContainerListOptions) (*listConte
 		})
 	}
 
-	if config.Before != "" && beforeContFilter == nil {
-		beforeContFilter, err = daemon.GetContainer(config.Before)
+	// FIXME remove this for 1.12 as --since and --before are deprecated
+	if config.Before != "" {
+		beforeContainer, err = daemon.GetContainer(config.Before)
 		if err != nil {
 			return nil, err
 		}
 	}
 
-	if config.Since != "" && sinceContFilter == nil {
-		sinceContFilter, err = daemon.GetContainer(config.Since)
+	// FIXME remove this for 1.12 as --since and --before are deprecated
+	if config.Since != "" {
+		sinceContainer, err = daemon.GetContainer(config.Since)
 		if err != nil {
 			return nil, err
 		}
@@ -201,6 +213,8 @@ func (daemon *Daemon) foldFilter(config *types.ContainerListOptions) (*listConte
 		ancestorFilter:       ancestorFilter,
 		images:               imagesFilter,
 		exitAllowed:          filtExited,
+		beforeContainer:      beforeContainer,
+		sinceContainer:       sinceContainer,
 		beforeFilter:         beforeContFilter,
 		sinceFilter:          sinceContFilter,
 		ContainerListOptions: config,
@@ -212,7 +226,8 @@ func (daemon *Daemon) foldFilter(config *types.ContainerListOptions) (*listConte
 // It also decides if the iteration should be stopped or not.
 func includeContainerInList(container *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.beforeFilter == nil && ctx.sinceFilter == nil {
+	// FIXME remove the ctx.beforContainer part of the condition for 1.12 as --since and --before are deprecated
+	if !container.Running && !ctx.All && ctx.Limit <= 0 && ctx.beforeContainer == nil && ctx.sinceContainer == nil {
 		return excludeContainer
 	}
 
@@ -236,6 +251,21 @@ func includeContainerInList(container *container.Container, ctx *listContext) it
 		return excludeContainer
 	}
 
+	// FIXME remove this for 1.12 as --since and --before are deprecated
+	if ctx.beforeContainer != nil {
+		if container.ID == ctx.beforeContainer.ID {
+			ctx.beforeContainer = nil
+		}
+		return excludeContainer
+	}
+
+	// FIXME remove this for 1.12 as --since and --before are deprecated
+	if ctx.sinceContainer != nil {
+		if container.ID == ctx.sinceContainer.ID {
+			return stopIteration
+		}
+	}
+
 	// 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.beforeFilter != nil {
diff --git a/integration-cli/docker_cli_ps_test.go b/integration-cli/docker_cli_ps_test.go
@@ -47,8 +47,6 @@ func (s *DockerSuite) TestPsListContainersBase(c *check.C) {
 	out, _ = dockerCmd(c, "ps")
 	c.Assert(assertContainerList(out, []string{fourthID, secondID, firstID}), checker.Equals, true, check.Commentf("RUNNING: Container list is not in the correct order: \n%s", out))
 
-	// from here all flag '-a' is ignored
-
 	// limit
 	out, _ = dockerCmd(c, "ps", "-n=2", "-a")
 	expected := []string{fourthID, thirdID}
@@ -60,56 +58,138 @@ func (s *DockerSuite) TestPsListContainersBase(c *check.C) {
 	// filter since
 	out, _ = dockerCmd(c, "ps", "-f", "since="+firstID, "-a")
 	expected = []string{fourthID, thirdID, secondID}
-	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE & ALL: Container list is not in the correct order: \n%s", out))
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE filter & ALL: Container list is not in the correct order: \n%s", out))
 
 	out, _ = dockerCmd(c, "ps", "-f", "since="+firstID)
-	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE: Container list is not in the correct order: \n%s", out))
+	expected = []string{fourthID, secondID}
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE filter: Container list is not in the correct order: \n%s", out))
 
 	// filter before
-	out, _ = dockerCmd(c, "ps", "-f", "before="+thirdID, "-a")
-	expected = []string{secondID, firstID}
-	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("BEFORE & ALL: Container list is not in the correct order: \n%s", out))
+	out, _ = dockerCmd(c, "ps", "-f", "before="+fourthID, "-a")
+	expected = []string{thirdID, secondID, firstID}
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("BEFORE filter & ALL: Container list is not in the correct order: \n%s", out))
 
-	out, _ = dockerCmd(c, "ps", "-f", "before="+thirdID)
-	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("BEFORE: Container list is not in the correct order: \n%s", out))
+	out, _ = dockerCmd(c, "ps", "-f", "before="+fourthID)
+	expected = []string{secondID, firstID}
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("BEFORE filter: Container list is not in the correct order: \n%s", out))
 
 	// filter since & before
 	out, _ = dockerCmd(c, "ps", "-f", "since="+firstID, "-f", "before="+fourthID, "-a")
 	expected = []string{thirdID, secondID}
-	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE, BEFORE & ALL: Container list is not in the correct order: \n%s", out))
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE filter, BEFORE filter & ALL: Container list is not in the correct order: \n%s", out))
 
 	out, _ = dockerCmd(c, "ps", "-f", "since="+firstID, "-f", "before="+fourthID)
-	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE, BEFORE: Container list is not in the correct order: \n%s", out))
+	expected = []string{secondID}
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE filter, BEFORE filter: Container list is not in the correct order: \n%s", out))
 
 	// filter since & limit
 	out, _ = dockerCmd(c, "ps", "-f", "since="+firstID, "-n=2", "-a")
 	expected = []string{fourthID, thirdID}
 
-	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE, LIMIT & ALL: Container list is not in the correct order: \n%s", out))
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE filter, LIMIT & ALL: Container list is not in the correct order: \n%s", out))
 
 	out, _ = dockerCmd(c, "ps", "-f", "since="+firstID, "-n=2")
-	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE, LIMIT: Container list is not in the correct order: \n%s", out))
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE filter, LIMIT: Container list is not in the correct order: \n%s", out))
 
 	// filter before & limit
 	out, _ = dockerCmd(c, "ps", "-f", "before="+fourthID, "-n=1", "-a")
 	expected = []string{thirdID}
-	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("BEFORE, LIMIT & ALL: Container list is not in the correct order: \n%s", out))
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("BEFORE filter, LIMIT & ALL: Container list is not in the correct order: \n%s", out))
 
 	out, _ = dockerCmd(c, "ps", "-f", "before="+fourthID, "-n=1")
-	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("BEFORE, LIMIT: Container list is not in the correct order: \n%s", out))
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("BEFORE filter, LIMIT: Container list is not in the correct order: \n%s", out))
 
 	// filter since & filter before & limit
 	out, _ = dockerCmd(c, "ps", "-f", "since="+firstID, "-f", "before="+fourthID, "-n=1", "-a")
 	expected = []string{thirdID}
-	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE, BEFORE, LIMIT & ALL: Container list is not in the correct order: \n%s", out))
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE filter, BEFORE filter, LIMIT & ALL: Container list is not in the correct order: \n%s", out))
 
 	out, _ = dockerCmd(c, "ps", "-f", "since="+firstID, "-f", "before="+fourthID, "-n=1")
-	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE, BEFORE, LIMIT: Container list is not in the correct order: \n%s", out))
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE filter, BEFORE filter, LIMIT: Container list is not in the correct order: \n%s", out))
+
+}
+
+// FIXME remove this for 1.12 as --since and --before are deprecated
+func (s *DockerSuite) TestPsListContainersDeprecatedSinceAndBefore(c *check.C) {
+	out, _ := runSleepingContainer(c, "-d")
+	firstID := strings.TrimSpace(out)
+
+	out, _ = runSleepingContainer(c, "-d")
+	secondID := strings.TrimSpace(out)
+
+	// not long running
+	out, _ = dockerCmd(c, "run", "-d", "busybox", "true")
+	thirdID := strings.TrimSpace(out)
+
+	out, _ = runSleepingContainer(c, "-d")
+	fourthID := strings.TrimSpace(out)
+
+	// make sure the second is running
+	c.Assert(waitRun(secondID), checker.IsNil)
+
+	// make sure third one is not running
+	dockerCmd(c, "wait", thirdID)
+
+	// make sure the forth is running
+	c.Assert(waitRun(fourthID), checker.IsNil)
+
+	// since
+	out, _ = dockerCmd(c, "ps", "--since="+firstID, "-a")
+	expected := []string{fourthID, thirdID, secondID}
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE & ALL: Container list is not in the correct order: %v \n%s", expected, out))
+
+	out, _ = dockerCmd(c, "ps", "--since="+firstID)
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE: Container list is not in the correct order: %v \n%s", expected, out))
+
+	// before
+	out, _ = dockerCmd(c, "ps", "--before="+thirdID, "-a")
+	expected = []string{secondID, firstID}
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("BEFORE & ALL: Container list is not in the correct order: %v \n%s", expected, out))
+
+	out, _ = dockerCmd(c, "ps", "--before="+thirdID)
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("BEFORE: Container list is not in the correct order: %v \n%s", expected, out))
+
+	// since & before
+	out, _ = dockerCmd(c, "ps", "--since="+firstID, "--before="+fourthID, "-a")
+	expected = []string{thirdID, secondID}
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE, BEFORE & ALL: Container list is not in the correct order: %v \n%s", expected, out))
+
+	out, _ = dockerCmd(c, "ps", "--since="+firstID, "--before="+fourthID)
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE, BEFORE: Container list is not in the correct order: %v \n%s", expected, out))
+
+	// since & limit
+	out, _ = dockerCmd(c, "ps", "--since="+firstID, "-n=2", "-a")
+	expected = []string{fourthID, thirdID}
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE, LIMIT & ALL: Container list is not in the correct order: %v \n%s", expected, out))
+
+	out, _ = dockerCmd(c, "ps", "--since="+firstID, "-n=2")
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE, LIMIT: Container list is not in the correct order: %v \n%s", expected, out))
+
+	// before & limit
+	out, _ = dockerCmd(c, "ps", "--before="+fourthID, "-n=1", "-a")
+	expected = []string{thirdID}
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("BEFORE, LIMIT & ALL: Container list is not in the correct order: %v \n%s", expected, out))
+
+	out, _ = dockerCmd(c, "ps", "--before="+fourthID, "-n=1")
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("BEFORE, LIMIT: Container list is not in the correct order: %v \n%s", expected, out))
+
+	// since & before & limit
+	out, _ = dockerCmd(c, "ps", "--since="+firstID, "--before="+fourthID, "-n=1", "-a")
+	expected = []string{thirdID}
+	c.Assert(assertContainerList(out, expected), checker.Equals, true, check.Commentf("SINCE, BEFORE, LIMIT & ALL: Container list is not in the correct order: %v \n%s", expected, out))
 
 }
 
 func assertContainerList(out string, expected []string) bool {
 	lines := strings.Split(strings.Trim(out, "\n "), "\n")
+	// FIXME remove this for 1.12 as --since and --before are deprecated
+	// This is here to remove potential Warning: lines (printed out with deprecated flags)
+	for i := 0; i < 2; i++ {
+		if strings.Contains(lines[0], "Warning:") {
+			lines = lines[1:]
+		}
+	}
+
 	if len(lines)-1 != len(expected) {
 		return false
 	}
