start filter flag changes

Tying filtering logic for podman stop and start to same place in getContainersAndInputByContext() to reduce code redundancy

Signed-off-by: Karthik Elango <kelango@redhat.com>

cmd/podman/containers/start.go

@@ -59,8 +59,10 @@ func startFlags(cmd *cobra.Command) {
 
 	flags.BoolVarP(&startOptions.Interactive, "interactive", "i", false, "Keep STDIN open even if not attached")
 	flags.BoolVar(&startOptions.SigProxy, "sig-proxy", false, "Proxy received signals to the process (default true if attaching, false otherwise)")
-	flags.StringSliceVarP(&filters, "filter", "f", []string{}, "Filter output based on conditions given")
-	_ = cmd.RegisterFlagCompletionFunc("filter", common.AutocompletePsFilters)
+
+	filterFlagName := "filter"
+	flags.StringSliceVarP(&filters, filterFlagName, "f", []string{}, "Filter output based on conditions given")
+	_ = cmd.RegisterFlagCompletionFunc(filterFlagName, common.AutocompletePsFilters)
 
 	flags.BoolVar(&startOptions.All, "all", false, "Start all containers regardless of their state or configuration")
 
@@ -84,7 +86,7 @@ func init() {
 }
 
 func validateStart(cmd *cobra.Command, args []string) error {
-	if len(args) == 0 && !startOptions.Latest && !startOptions.All {
+	if len(args) == 0 && !startOptions.Latest && !startOptions.All && len(filters) < 1 {
 		return errors.New("start requires at least one argument")
 	}
 	if startOptions.All && startOptions.Latest {
@@ -123,14 +125,12 @@ func start(cmd *cobra.Command, args []string) error {
 	}
 
 	containers := args
-	if len(filters) > 0 {
-		for _, f := range filters {
-			split := strings.SplitN(f, "=", 2)
-			if len(split) == 1 {
-				return fmt.Errorf("invalid filter %q", f)
-			}
-			startOptions.Filters[split[0]] = append(startOptions.Filters[split[0]], split[1])
+	for _, f := range filters {
+		split := strings.SplitN(f, "=", 2)
+		if len(split) < 2 {
+			return fmt.Errorf("invalid filter %q", f)
 		}
+		startOptions.Filters[split[0]] = append(startOptions.Filters[split[0]], split[1])
 	}
 
 	responses, err := registry.ContainerEngine().ContainerStart(registry.GetContext(), containers, startOptions)

pkg/domain/infra/abi/containers.go

@@ -40,6 +40,7 @@ import (
 // is specified.  It also returns a list of the corresponding input name used to lookup each container.
 func getContainersAndInputByContext(all, latest bool, names []string, filters map[string][]string, runtime *libpod.Runtime) (ctrs []*libpod.Container, rawInput []string, err error) {
 	var ctr *libpod.Container
+	var filteredCtrs []*libpod.Container
 	ctrs = []*libpod.Container{}
 	filterFuncs := make([]libpod.ContainerFilter, 0, len(filters))
 
@@ -58,7 +59,17 @@ func getContainersAndInputByContext(all, latest bool, names []string, filters ma
 		}
 		rawInput = []string{}
 		for _, candidate := range ctrs {
-			rawInput = append(rawInput, candidate.ID())
+			if len(names) > 0 {
+				for _, name := range names {
+					if candidate.ID() == name || candidate.Name() == name {
+						rawInput = append(rawInput, candidate.ID())
+						filteredCtrs = append(filteredCtrs, candidate)
+					}
+				}
+				ctrs = filteredCtrs
+			} else {
+				rawInput = append(rawInput, candidate.ID())
+			}
 		}
 	case all:
 		ctrs, err = runtime.GetAllContainers()
@@ -886,38 +897,7 @@ func (ic *ContainerEngine) ContainerExecDetached(ctx context.Context, nameOrID s
 func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []string, options entities.ContainerStartOptions) ([]*entities.ContainerStartReport, error) {
 	reports := []*entities.ContainerStartReport{}
 	var exitCode = define.ExecErrorCodeGeneric
-	containersNamesOrIds := namesOrIds
-	all := options.All
-	if len(options.Filters) > 0 {
-		all = false
-		filterFuncs := make([]libpod.ContainerFilter, 0, len(options.Filters))
-		if len(options.Filters) > 0 {
-			for k, v := range options.Filters {
-				generatedFunc, err := dfilters.GenerateContainerFilterFuncs(k, v, ic.Libpod)
-				if err != nil {
-					return nil, err
-				}
-				filterFuncs = append(filterFuncs, generatedFunc)
-			}
-		}
-		candidates, err := ic.Libpod.GetContainers(filterFuncs...)
-		if err != nil {
-			return nil, err
-		}
-		containersNamesOrIds = []string{}
-		for _, candidate := range candidates {
-			if options.All {
-				containersNamesOrIds = append(containersNamesOrIds, candidate.ID())
-				continue
-			}
-			for _, nameOrID := range namesOrIds {
-				if nameOrID == candidate.ID() || nameOrID == candidate.Name() {
-					containersNamesOrIds = append(containersNamesOrIds, nameOrID)
-				}
-			}
-		}
-	}
-	ctrs, rawInputs, err := getContainersAndInputByContext(all, options.Latest, containersNamesOrIds, options.Filters, ic.Libpod)
+	ctrs, rawInputs, err := getContainersAndInputByContext(options.All, options.Latest, namesOrIds, options.Filters, ic.Libpod)
 	if err != nil {
 		return nil, err
 	}

pkg/domain/infra/tunnel/containers.go

@@ -639,36 +639,7 @@ func logIfRmError(id string, err error, reports []*reports.RmReport) {
 func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []string, options entities.ContainerStartOptions) ([]*entities.ContainerStartReport, error) {
 	reports := []*entities.ContainerStartReport{}
 	var exitCode = define.ExecErrorCodeGeneric
-	containersNamesOrIds := namesOrIds
-	all := options.All
-	if len(options.Filters) > 0 {
-		all = false
-		containersNamesOrIds = []string{}
-		opts := new(containers.ListOptions).WithFilters(options.Filters).WithAll(true)
-		candidates, listErr := containers.List(ic.ClientCtx, opts)
-		if listErr != nil {
-			return nil, listErr
-		}
-		for _, candidate := range candidates {
-			if options.All {
-				containersNamesOrIds = append(containersNamesOrIds, candidate.ID)
-				continue
-			}
-			for _, nameOrID := range namesOrIds {
-				if nameOrID == candidate.ID {
-					containersNamesOrIds = append(containersNamesOrIds, nameOrID)
-					continue
-				}
-				for _, containerName := range candidate.Names {
-					if containerName == nameOrID {
-						containersNamesOrIds = append(containersNamesOrIds, nameOrID)
-						continue
-					}
-				}
-			}
-		}
-	}
-	ctrs, err := getContainersByContext(ic.ClientCtx, all, false, containersNamesOrIds)
+	ctrs, namesOrIds, err := getContainersAndInputByContext(ic.ClientCtx, options.All, false, namesOrIds, options.Filters)
 	if err != nil {
 		return nil, err
 	}

pkg/domain/infra/tunnel/helpers.go

@@ -31,8 +31,17 @@ func getContainersAndInputByContext(contextWithConnection context.Context, all,
 	rawInputs := []string{}
 	switch {
 	case len(filters) > 0:
+		namesOrIDs = nil
 		for i := range allContainers {
-			namesOrIDs = append(namesOrIDs, allContainers[i].ID)
+			if len(namesOrIDs) > 0 {
+				for _, name := range namesOrIDs {
+					if name == allContainers[i].ID {
+						namesOrIDs = append(namesOrIDs, allContainers[i].ID)
+					}
+				}
+			} else {
+				namesOrIDs = append(namesOrIDs, allContainers[i].ID)
+			}
 		}
 	case all:
 		for i := range allContainers {

test/e2e/start_test.go

@@ -1,6 +1,7 @@
 package integration
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"strconv"
@@ -231,4 +232,42 @@ var _ = Describe("Podman start", func() {
 		_, err = strconv.Atoi(containerPID) // Make sure it's a proper integer
 		Expect(err).To(BeNil())
 	})
+
+	It("podman start container --filter", func() {
+		session1 := podmanTest.Podman([]string{"container", "create", ALPINE})
+		session1.WaitWithDefaultTimeout()
+		Expect(session1).Should(Exit(0))
+		cid1 := session1.OutputToString()
+
+		session1 = podmanTest.Podman([]string{"container", "create", ALPINE})
+		session1.WaitWithDefaultTimeout()
+		Expect(session1).Should(Exit(0))
+		cid2 := session1.OutputToString()
+
+		session1 = podmanTest.Podman([]string{"container", "create", ALPINE})
+		session1.WaitWithDefaultTimeout()
+		Expect(session1).Should(Exit(0))
+		cid3 := session1.OutputToString()
+		shortCid3 := cid3[0:5]
+
+		session1 = podmanTest.Podman([]string{"start", cid1, "-f", "status=running"})
+		session1.WaitWithDefaultTimeout()
+		Expect(session1).Should(Exit(0))
+		Expect(session1.OutputToString()).To(HaveLen(0))
+
+		session1 = podmanTest.Podman([]string{"start", "--all", "--filter", fmt.Sprintf("id=%swrongid", shortCid3)})
+		session1.WaitWithDefaultTimeout()
+		Expect(session1).Should(Exit(0))
+		Expect(session1.OutputToString()).To(HaveLen(0))
+
+		session1 = podmanTest.Podman([]string{"start", "--all", "--filter", fmt.Sprintf("id=%s", shortCid3)})
+		session1.WaitWithDefaultTimeout()
+		Expect(session1).Should(Exit(0))
+		Expect(session1.OutputToString()).To(BeEquivalentTo(cid3))
+
+		session1 = podmanTest.Podman([]string{"start", "-f", fmt.Sprintf("id=%s", cid2)})
+		session1.WaitWithDefaultTimeout()
+		Expect(session1).Should(Exit(0))
+		Expect(session1.OutputToString()).To(BeEquivalentTo(cid2))
+	})
 })