From a6ea3325a08378c1b39a446647aa697b3bd31619 Mon Sep 17 00:00:00 2001 From: Karthik Elango Date: Wed, 27 Jul 2022 10:43:06 -0400 Subject: 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 --- cmd/podman/containers/start.go | 20 +++++++-------- pkg/domain/infra/abi/containers.go | 46 ++++++++++------------------------- pkg/domain/infra/tunnel/containers.go | 31 +---------------------- pkg/domain/infra/tunnel/helpers.go | 11 ++++++++- test/e2e/start_test.go | 39 +++++++++++++++++++++++++++++ 5 files changed, 73 insertions(+), 74 deletions(-) diff --git a/cmd/podman/containers/start.go b/cmd/podman/containers/start.go index cd4fa17b8..fc3488e0c 100644 --- a/cmd/podman/containers/start.go +++ b/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) diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 1b6c1617c..f12e17c09 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/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() @@ -860,38 +871,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 } diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index c8371baf9..0bf792879 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -623,36 +623,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 } diff --git a/pkg/domain/infra/tunnel/helpers.go b/pkg/domain/infra/tunnel/helpers.go index 9ff1641f0..20ea07948 100644 --- a/pkg/domain/infra/tunnel/helpers.go +++ b/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 { diff --git a/test/e2e/start_test.go b/test/e2e/start_test.go index 73af9d12c..736008ed3 100644 --- a/test/e2e/start_test.go +++ b/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)) + }) }) -- cgit v1.2.3-54-g00ecf