From 6d3571dcf5009bdd036978054760a59aa8990862 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 26 Feb 2024 14:57:21 +0100 Subject: [PATCH 1/4] podman compose: build for all arches Machine only works on amd64 and arm64 but the compose command can still be used without machine so split out the machine only logic to make it build for all arches. [NO NEW TESTS NEEDED] Fixes #21757 Signed-off-by: Paul Holzinger --- cmd/podman/compose.go | 59 +------------------ cmd/podman/compose_machine.go | 69 +++++++++++++++++++++++ cmd/podman/compose_machine_unsupported.go | 14 +++++ 3 files changed, 86 insertions(+), 56 deletions(-) create mode 100644 cmd/podman/compose_machine.go create mode 100644 cmd/podman/compose_machine_unsupported.go diff --git a/cmd/podman/compose.go b/cmd/podman/compose.go index ddbce2ac13..7afbbba52a 100644 --- a/cmd/podman/compose.go +++ b/cmd/podman/compose.go @@ -1,5 +1,3 @@ -//go:build amd64 || arm64 - package main import ( @@ -16,10 +14,6 @@ import ( "github.com/containers/podman/v5/cmd/podman/registry" "github.com/containers/podman/v5/pkg/errorhandling" - "github.com/containers/podman/v5/pkg/machine" - "github.com/containers/podman/v5/pkg/machine/define" - "github.com/containers/podman/v5/pkg/machine/provider" - "github.com/containers/podman/v5/pkg/machine/vmconfigs" "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -145,58 +139,11 @@ func composeDockerHost() (string, error) { // machine which is the case for podman machines. return strings.TrimSuffix(connection.URI, parsedConnection.Path), nil } - - machineProvider, err := provider.Get() + uri, err := getMachineConn(connection, parsedConnection) if err != nil { - return "", fmt.Errorf("getting machine provider: %w", err) + return "", fmt.Errorf("get machine connection URI: %w", err) } - dirs, err := machine.GetMachineDirs(machineProvider.VMType()) - if err != nil { - return "", err - } - - machineList, err := vmconfigs.LoadMachinesInDir(dirs) - if err != nil { - return "", fmt.Errorf("listing machines: %w", err) - } - - // Now we know that the connection points to a machine and we - // can find the machine by looking for the one with the - // matching port. - connectionPort, err := strconv.Atoi(parsedConnection.Port()) - if err != nil { - return "", fmt.Errorf("parsing connection port: %w", err) - } - for _, item := range machineList { - if connectionPort != item.SSH.Port { - continue - } - - state, err := machineProvider.State(item, false) - if err != nil { - return "", err - } - - if state != define.Running { - return "", fmt.Errorf("machine %s is not running but in state %s", item.Name, state) - } - - // TODO This needs to be wired back in when all providers are complete - // TODO Need someoone to plumb in the connection information below - // if machineProvider.VMType() == define.WSLVirt || machineProvider.VMType() == define.HyperVVirt { - // if info.ConnectionInfo.PodmanPipe == nil { - // return "", errors.New("pipe of machine is not set") - // } - // return strings.Replace(info.ConnectionInfo.PodmanPipe.Path, `\\.\pipe\`, "npipe:////./pipe/", 1), nil - // } - // if info.ConnectionInfo.PodmanSocket == nil { - // return "", errors.New("socket of machine is not set") - // } - // return "unix://" + info.ConnectionInfo.PodmanSocket.Path, nil - return "", nil - } - - return "", fmt.Errorf("could not find a matching machine for connection %q", connection.URI) + return uri, nil } // composeEnv returns the compose-specific environment variables. diff --git a/cmd/podman/compose_machine.go b/cmd/podman/compose_machine.go new file mode 100644 index 0000000000..d1ec369b78 --- /dev/null +++ b/cmd/podman/compose_machine.go @@ -0,0 +1,69 @@ +//go:build amd64 || arm64 + +package main + +import ( + "fmt" + "net/url" + "strconv" + + "github.com/containers/common/pkg/config" + "github.com/containers/podman/v5/pkg/machine" + "github.com/containers/podman/v5/pkg/machine/define" + "github.com/containers/podman/v5/pkg/machine/provider" + "github.com/containers/podman/v5/pkg/machine/vmconfigs" +) + +func getMachineConn(connection *config.Connection, parsedConnection *url.URL) (string, error) { + machineProvider, err := provider.Get() + if err != nil { + return "", fmt.Errorf("getting machine provider: %w", err) + } + dirs, err := machine.GetMachineDirs(machineProvider.VMType()) + if err != nil { + return "", err + } + + machineList, err := vmconfigs.LoadMachinesInDir(dirs) + if err != nil { + return "", fmt.Errorf("listing machines: %w", err) + } + + // Now we know that the connection points to a machine and we + // can find the machine by looking for the one with the + // matching port. + connectionPort, err := strconv.Atoi(parsedConnection.Port()) + if err != nil { + return "", fmt.Errorf("parsing connection port: %w", err) + } + for _, item := range machineList { + if connectionPort != item.SSH.Port { + continue + } + + state, err := machineProvider.State(item, false) + if err != nil { + return "", err + } + + if state != define.Running { + return "", fmt.Errorf("machine %s is not running but in state %s", item.Name, state) + } + + // TODO This needs to be wired back in when all providers are complete + // TODO Need someoone to plumb in the connection information below + // if machineProvider.VMType() == define.WSLVirt || machineProvider.VMType() == define.HyperVVirt { + // if info.ConnectionInfo.PodmanPipe == nil { + // return "", errors.New("pipe of machine is not set") + // } + // return strings.Replace(info.ConnectionInfo.PodmanPipe.Path, `\\.\pipe\`, "npipe:////./pipe/", 1), nil + // } + // if info.ConnectionInfo.PodmanSocket == nil { + // return "", errors.New("socket of machine is not set") + // } + // return "unix://" + info.ConnectionInfo.PodmanSocket.Path, nil + return "", nil + } + + return "", fmt.Errorf("could not find a matching machine for connection %q", connection.URI) +} diff --git a/cmd/podman/compose_machine_unsupported.go b/cmd/podman/compose_machine_unsupported.go new file mode 100644 index 0000000000..219f78d57c --- /dev/null +++ b/cmd/podman/compose_machine_unsupported.go @@ -0,0 +1,14 @@ +//go:build !(amd64 || arm64) + +package main + +import ( + "errors" + "net/url" + + "github.com/containers/common/pkg/config" +) + +func getMachineConn(connection *config.Connection, parsedConnection *url.URL) (string, error) { + return "", errors.New("podman machine not supported on this architecture") +} From 3cada04099ee4c02bf31d86861d4b257eb54bed6 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 26 Feb 2024 15:11:21 +0100 Subject: [PATCH 2/4] podman compose: correctly accept --connection/--url Make the logic here much simpler, we already pass all the conection info before so just use the parsed URL here. Fixes #20943 Signed-off-by: Paul Holzinger --- cmd/podman/compose.go | 14 ++++++-------- cmd/podman/compose_machine.go | 6 ++---- cmd/podman/compose_machine_unsupported.go | 4 +--- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/cmd/podman/compose.go b/cmd/podman/compose.go index 7afbbba52a..f90e5acfe5 100644 --- a/cmd/podman/compose.go +++ b/cmd/podman/compose.go @@ -108,10 +108,8 @@ func composeDockerHost() (string, error) { return registry.DefaultAPIAddress(), nil } - // TODO need to add support for --connection and --url - connection, err := registry.PodmanConfig().ContainersConfDefaultsRO.GetConnection("", true) - if err != nil { - logrus.Info(err) + conf := registry.PodmanConfig() + if conf.URI == "" { switch runtime.GOOS { // If no default connection is set on Linux or FreeBSD, // we just use the local socket by default - just as @@ -126,20 +124,20 @@ func composeDockerHost() (string, error) { } } - parsedConnection, err := url.Parse(connection.URI) + parsedConnection, err := url.Parse(conf.URI) if err != nil { return "", fmt.Errorf("preparing connection to remote machine: %w", err) } // If the default connection does not point to a `podman // machine`, we cannot use a local path and need to use SSH. - if !connection.IsMachine { + if !conf.MachineMode { // Compose doesn't like paths, so we optimistically // assume the presence of a Docker socket on the remote // machine which is the case for podman machines. - return strings.TrimSuffix(connection.URI, parsedConnection.Path), nil + return strings.TrimSuffix(conf.URI, parsedConnection.Path), nil } - uri, err := getMachineConn(connection, parsedConnection) + uri, err := getMachineConn(conf.URI, parsedConnection) if err != nil { return "", fmt.Errorf("get machine connection URI: %w", err) } diff --git a/cmd/podman/compose_machine.go b/cmd/podman/compose_machine.go index d1ec369b78..953b1eae0f 100644 --- a/cmd/podman/compose_machine.go +++ b/cmd/podman/compose_machine.go @@ -7,14 +7,13 @@ import ( "net/url" "strconv" - "github.com/containers/common/pkg/config" "github.com/containers/podman/v5/pkg/machine" "github.com/containers/podman/v5/pkg/machine/define" "github.com/containers/podman/v5/pkg/machine/provider" "github.com/containers/podman/v5/pkg/machine/vmconfigs" ) -func getMachineConn(connection *config.Connection, parsedConnection *url.URL) (string, error) { +func getMachineConn(connectionURI string, parsedConnection *url.URL) (string, error) { machineProvider, err := provider.Get() if err != nil { return "", fmt.Errorf("getting machine provider: %w", err) @@ -64,6 +63,5 @@ func getMachineConn(connection *config.Connection, parsedConnection *url.URL) (s // return "unix://" + info.ConnectionInfo.PodmanSocket.Path, nil return "", nil } - - return "", fmt.Errorf("could not find a matching machine for connection %q", connection.URI) + return "", fmt.Errorf("could not find a matching machine for connection %q", connectionURI) } diff --git a/cmd/podman/compose_machine_unsupported.go b/cmd/podman/compose_machine_unsupported.go index 219f78d57c..f1ec7f67bd 100644 --- a/cmd/podman/compose_machine_unsupported.go +++ b/cmd/podman/compose_machine_unsupported.go @@ -5,10 +5,8 @@ package main import ( "errors" "net/url" - - "github.com/containers/common/pkg/config" ) -func getMachineConn(connection *config.Connection, parsedConnection *url.URL) (string, error) { +func getMachineConn(connection string, parsedConnection *url.URL) (string, error) { return "", errors.New("podman machine not supported on this architecture") } From d9aff9b41e0f9b9096c51b6988577b086a807ed6 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 26 Feb 2024 15:20:49 +0100 Subject: [PATCH 3/4] podman compose: only trim path suffix when ssh protocol For a unix socket we should not trim this at all. The problem exists for ssh only so make sure we only do this when a ssh URL is given. Signed-off-by: Paul Holzinger --- cmd/podman/compose.go | 7 +++++-- test/system/850-compose.bats | 16 +++++++++++++--- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/cmd/podman/compose.go b/cmd/podman/compose.go index f90e5acfe5..cd764e5c61 100644 --- a/cmd/podman/compose.go +++ b/cmd/podman/compose.go @@ -132,10 +132,13 @@ func composeDockerHost() (string, error) { // If the default connection does not point to a `podman // machine`, we cannot use a local path and need to use SSH. if !conf.MachineMode { - // Compose doesn't like paths, so we optimistically + // Docker Compose v1 doesn't like paths for ssh, so we optimistically // assume the presence of a Docker socket on the remote // machine which is the case for podman machines. - return strings.TrimSuffix(conf.URI, parsedConnection.Path), nil + if parsedConnection.Scheme == "ssh" { + return strings.TrimSuffix(conf.URI, parsedConnection.Path), nil + } + return conf.URI, nil } uri, err := getMachineConn(conf.URI, parsedConnection) if err != nil { diff --git a/test/system/850-compose.bats b/test/system/850-compose.bats index 91e6afc267..8d967ad2d8 100644 --- a/test/system/850-compose.bats +++ b/test/system/850-compose.bats @@ -55,10 +55,20 @@ EOF CONTAINERS_CONF_OVERRIDE=$compose_conf run_podman 42 compose fail # Make sure the three env variables are set (and parsed) + op='=~' + url=".*/podman.sock" + # if we run remote with --url check the url arg is honored + if [[ "$PODMAN" =~ "--url" ]]; then + # get the url from the podman string + url="${PODMAN##*--url }" + url="${url%% *}" + op='=' + fi + # podman-remote test might run with --url so unset this because the socket will be used otherwise CONTAINERS_CONF_OVERRIDE=$compose_conf run_podman compose env - is "${lines[0]}" ".*/podman.sock" - is "${lines[1]}" "0" - is "${lines[2]}" "" + assert "${lines[0]}" $op "$url" "line 1 of 3 (DOCKER_HOST)" + assert "${lines[1]}" = "0" "line 2 of 3 (DOCKER_BUILDKIT)" + assert "${lines[2]}" = "" "line 3 of 3 (DOCKER_CONFIG)" DOCKER_HOST="$random_data" DOCKER_CONFIG="$random_data" CONTAINERS_CONF_OVERRIDE=$compose_conf run_podman compose env is "${lines[0]}" "$random_data" From a210a4d7c2891b95a6cbe597243e24b6de6dfc8a Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 26 Feb 2024 15:59:01 +0100 Subject: [PATCH 4/4] test/compose: add test for default connection Make sure that we use the --connection correctly with podman compose. Signed-off-by: Paul Holzinger --- test/compose/test-compose | 35 +++++++++++++++++++++++++++++------ test/compose/uptwice/tests.sh | 2 +- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/test/compose/test-compose b/test/compose/test-compose index a123a8de2a..3275f987f6 100755 --- a/test/compose/test-compose +++ b/test/compose/test-compose @@ -247,6 +247,15 @@ function podman() { return $rc } +# as rootless we want to test the remote connection so we add --connection +function podman_compose() { + if is_rootless; then + $PODMAN_BIN --connection compose-sock compose "$@" + else + podman compose "$@" + fi +} + ################### # random_string # Returns a pseudorandom human-readable string ################### @@ -271,12 +280,26 @@ done # When rootless use a socket path accessible by the rootless user if is_rootless; then + # lets test two cases here, for rootless we try to connect to the connection as this should be respected DOCKER_SOCK="$WORKDIR/docker.sock" + # use PODMAN_CONNECTIONS_CONF so we do not overwrite user settings + PODMAN_CONNECTIONS_CONF="$WORKDIR/connections.json" + export PODMAN_CONNECTIONS_CONF + $PODMAN_BIN system connection add --default notexists "unix:///I/do/not/exist" + $PODMAN_BIN system connection add compose-sock "unix://$DOCKER_SOCK" + +else + # export DOCKER_HOST docker-compose will use it + DOCKER_HOST="unix://$DOCKER_SOCK" + export DOCKER_HOST fi -# export DOCKER_HOST docker-compose will use it -DOCKER_HOST="unix://$DOCKER_SOCK" -export DOCKER_HOST +# hide annoying podman compose warnings, some tests want to check compose stderr and this breaks it. +CONTAINERS_CONF_OVERRIDE="$WORKDIR/containers.conf" +echo '[engine] +compose_warning_logs=false' > "$CONTAINERS_CONF_OVERRIDE" +export CONTAINERS_CONF_OVERRIDE + # Identify the tests to run. If called with args, use those as globs. tests_to_run=() @@ -336,12 +359,12 @@ for t in "${tests_to_run[@]}"; do trap - ERR fi - podman compose up -d &> $logfile + podman_compose up -d &> $logfile docker_compose_rc=$? if [[ $docker_compose_rc -ne 0 ]]; then _show_ok 0 "$testname - up" "[ok]" "status=$docker_compose_rc" sed -e 's/^/# /' <$logfile - podman compose down >>$logfile 2>&1 # No status check here + podman_compose down >>$logfile 2>&1 # No status check here exit 1 fi _show_ok 1 "$testname - up" @@ -361,7 +384,7 @@ for t in "${tests_to_run[@]}"; do fi # Done. Clean up. - podman compose down &>> $logfile + podman_compose down &>> $logfile rc=$? if [[ $rc -eq 0 ]]; then _show_ok 1 "$testname - down" diff --git a/test/compose/uptwice/tests.sh b/test/compose/uptwice/tests.sh index 013b5a29a8..0bf1b12146 100644 --- a/test/compose/uptwice/tests.sh +++ b/test/compose/uptwice/tests.sh @@ -5,7 +5,7 @@ NL=$'\n' cp docker-compose.yml docker-compose.yml.bak sed -i -e 's/10001/10002/' docker-compose.yml -output=$(docker-compose up -d 2>&1) +output=$(podman_compose up -d 2>&1) # Horrible output check here but we really want to make sure that there are # no unexpected warning/errors and the normal messages are send on stderr as