fix remote start --filter

Fix a number of bugs wrt. filtering remote containers and how to
process specified names or IDs.  I _really_ do not like the duplication
between remote and local Podman but want to focus on fixing containers#18153
for now.

What I desire in the future is to consolidate all functionality of
looking up containers (all, latest, filters, specified names/IDs, etc.)
and for remote clients to just call containers/list etc.

Fixes: containers#18153
Signed-off-by: Valentin Rothberg <[email protected]>

pkg/domain/infra/abi/containers.go

@@ -128,9 +128,28 @@ func getContainers(runtime *libpod.Runtime, options getContainersOptions) ([]con
 		return containers, nil
 	}
 
-	containers := make([]containerWrapper, len(libpodContainers))
-	for i := range libpodContainers {
-		containers[i] = containerWrapper{Container: libpodContainers[i], rawInput: libpodContainers[i].ID()}
+	// If no names or IDs are specified, we can return the result as is.
+	// Otherwise, we need to do some further lookups.
+	if len(options.names) == 0 || (len(options.names) == 1 && options.names[0] == "") {
+		containers := make([]containerWrapper, len(libpodContainers))
+		for i, c := range libpodContainers {
+			containers[i] = containerWrapper{Container: libpodContainers[i], rawInput: c.ID()}
+		}
+		return containers, nil
+	}
+
+	containers := []containerWrapper{}
+	for _, n := range options.names {
+		c, err := runtime.LookupContainer(n)
+		if err != nil {
+			return nil, err
+		}
+		for i, lc := range libpodContainers {
+			if c.ID() == lc.ID() {
+				containers = append(containers, containerWrapper{Container: libpodContainers[i], rawInput: n})
+				break
+			}
+		}
 	}
 
 	return containers, nil

pkg/domain/infra/tunnel/containers.go

@@ -682,7 +682,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
-	ctrs, rawInputs, err := getContainersAndInputByContext(ic.ClientCtx, options.All, false, namesOrIds, options.Filters)
+	ctrs, rawInputs, err := getContainersAndInputByContext(ic.ClientCtx, options.All, len(options.Filters) > 0, namesOrIds, options.Filters)
 	if err != nil {
 		return nil, err
 	}

pkg/domain/infra/tunnel/helpers.go

@@ -29,31 +29,22 @@ func getContainersAndInputByContext(contextWithConnection context.Context, all,
 	if err != nil {
 		return nil, nil, err
 	}
+
 	rawInputs := []string{}
-	switch {
-	case len(filters) > 0:
-		namesOrIDs = nil
-		for i := range allContainers {
-			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:
+
+	// If no names or IDs are specified, we can return the result as is.
+	// Otherwise, we need to do some further lookups.
+	if len(namesOrIDs) == 0 {
 		for i := range allContainers {
 			rawInputs = append(rawInputs, allContainers[i].ID)
 		}
 		return allContainers, rawInputs, err
 	}
 
-	// Note: it would be nicer if the lists endpoint would support that as
-	// we could use the libpod backend for looking up containers rather
-	// than risking diverging the local and remote lookups.
+	// Note: it would be nicer if the lists endpoint would support batch
+	// name/ID lookups as we could use the libpod backend for looking up
+	// containers rather than risking diverging the local and remote
+	// lookups.
 	//
 	// A `--filter nameOrId=abc` that can be specified multiple times would
 	// be awesome to have.

test/system/045-start.bats

@@ -38,15 +38,30 @@ load helpers
 }
 
 @test "podman start --filter - start only containers that match the filter" {
-    run_podman run -d $IMAGE /bin/true
-    cid="$output"
-    run_podman wait $cid
-
-    run_podman start --filter restart-policy=always $cid
-    is "$output" "" "CID of restart-policy=always container"
-
-    run_podman start --filter restart-policy=none $cid
-    is "$output" "$cid" "CID of restart-policy=none container"
+    c1="c1_always_$(random_string 15)"
+    c2="c2_on_failure_$(random_string 15)"
+    c3="c3_always_$(random_string 15)"
+
+    run_podman create --name=$c1 --restart=always $IMAGE /bin/true
+    c1_id="$output"
+    run_podman create --name=$c2 --restart=on-failure $IMAGE /bin/true
+    c2_id="$output"
+    run_podman create --name=$c3 --restart=always $IMAGE /bin/true
+    c3_id="$output"
+
+    # Start via --filter
+    run_podman start --filter restart-policy=always
+    # Output order not sorted wrt creation time, so we need two regexes
+    is "$output" ".*$c1_id.*" "--filter finds container 1"
+    is "$output" ".*$c3_id.*" "--filter finds container 3"
+
+    # Start via filtered names
+    run_podman start --filter restart-policy=on-failure $c2 $c3
+    is "$output" "$c2" "--filter finds container 2"
+
+    # Nothing on match
+    run_podman start --filter restart-policy=none --all
+    is "$output" ""
 }
 
 @test "podman start --filter invalid-restart-policy - return error" {