From ededb4c3c466801b8cbfea9bfe1870f50e4f9271 Mon Sep 17 00:00:00 2001 From: Arthur Sengileyev Date: Sat, 31 Aug 2024 20:56:16 +0300 Subject: [PATCH] Improve platform specific URL handling in `podman compose` for machines Use filepath utility instead of generic string replace to convert path on Windows. This also separates OS specific implementations to separate compilation sources and removes redundant check for virtualization provider on Windows platform. Signed-off-by: Arthur Sengileyev --- cmd/podman/compose_machine.go | 13 +------ cmd/podman/compose_machine_unix.go | 16 ++++++++ cmd/podman/compose_machine_windows.go | 15 ++++++++ pkg/machine/e2e/compose_test.go | 48 ++++++++++++++++++++++++ pkg/machine/e2e/config_compose_test.go | 12 ++++++ pkg/machine/e2e/machine_test.go | 16 ++++++++ pkg/machine/e2e/scripts/fake_compose | 6 +++ pkg/machine/e2e/scripts/fake_compose.bat | 18 +++++++++ 8 files changed, 132 insertions(+), 12 deletions(-) create mode 100644 cmd/podman/compose_machine_unix.go create mode 100644 cmd/podman/compose_machine_windows.go create mode 100644 pkg/machine/e2e/compose_test.go create mode 100644 pkg/machine/e2e/config_compose_test.go create mode 100755 pkg/machine/e2e/scripts/fake_compose create mode 100644 pkg/machine/e2e/scripts/fake_compose.bat diff --git a/cmd/podman/compose_machine.go b/cmd/podman/compose_machine.go index 7d70cc84c9..aadbfe42fc 100644 --- a/cmd/podman/compose_machine.go +++ b/cmd/podman/compose_machine.go @@ -3,11 +3,9 @@ package main import ( - "errors" "fmt" "net/url" "strconv" - "strings" "github.com/containers/podman/v5/pkg/machine/define" "github.com/containers/podman/v5/pkg/machine/env" @@ -55,16 +53,7 @@ func getMachineConn(connectionURI string, parsedConnection *url.URL) (string, er if err != nil { return "", err } - if machineProvider.VMType() == define.WSLVirt || machineProvider.VMType() == define.HyperVVirt { - if podmanPipe == nil { - return "", errors.New("pipe of machine is not set") - } - return strings.Replace(podmanPipe.Path, `\\.\pipe\`, "npipe:////./pipe/", 1), nil - } - if podmanSocket == nil { - return "", errors.New("socket of machine is not set") - } - return "unix://" + podmanSocket.Path, nil + return extractConnectionString(podmanSocket, podmanPipe) } return "", fmt.Errorf("could not find a matching machine for connection %q", connectionURI) } diff --git a/cmd/podman/compose_machine_unix.go b/cmd/podman/compose_machine_unix.go new file mode 100644 index 0000000000..de2fb2934c --- /dev/null +++ b/cmd/podman/compose_machine_unix.go @@ -0,0 +1,16 @@ +//go:build (amd64 || arm64) && !windows + +package main + +import ( + "errors" + + "github.com/containers/podman/v5/pkg/machine/define" +) + +func extractConnectionString(podmanSocket *define.VMFile, podmanPipe *define.VMFile) (string, error) { + if podmanSocket == nil { + return "", errors.New("socket of machine is not set") + } + return "unix://" + podmanSocket.Path, nil +} diff --git a/cmd/podman/compose_machine_windows.go b/cmd/podman/compose_machine_windows.go new file mode 100644 index 0000000000..49b8be172a --- /dev/null +++ b/cmd/podman/compose_machine_windows.go @@ -0,0 +1,15 @@ +package main + +import ( + "errors" + "path/filepath" + + "github.com/containers/podman/v5/pkg/machine/define" +) + +func extractConnectionString(podmanSocket *define.VMFile, podmanPipe *define.VMFile) (string, error) { + if podmanPipe == nil { + return "", errors.New("pipe of machine is not set") + } + return "npipe://" + filepath.ToSlash(podmanPipe.Path), nil +} diff --git a/pkg/machine/e2e/compose_test.go b/pkg/machine/e2e/compose_test.go new file mode 100644 index 0000000000..3c1f8d7982 --- /dev/null +++ b/pkg/machine/e2e/compose_test.go @@ -0,0 +1,48 @@ +package e2e_test + +import ( + "path/filepath" + "runtime" + "strings" + + "github.com/containers/podman/v5/pkg/machine" + jsoniter "github.com/json-iterator/go" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gexec" +) + +var _ = Describe("podman machine compose", func() { + + It("compose test environment variable setup", func() { + name := randomString() + i := new(initMachine) + session, err := mb.setName(name).setCmd(i.withImage(mb.imagePath).withNow()).run() + Expect(err).ToNot(HaveOccurred()) + Expect(session).To(Exit(0)) + + inspectJSON := new(inspectMachine) + inspectSession, err := mb.setName(name).setCmd(inspectJSON).run() + Expect(err).ToNot(HaveOccurred()) + Expect(inspectSession).To(Exit(0)) + + var inspectInfo []machine.InspectInfo + err = jsoniter.Unmarshal(inspectSession.Bytes(), &inspectInfo) + Expect(err).ToNot(HaveOccurred()) + + compose := new(fakeCompose) + composeSession, err := mb.setName(name).setCmd(compose).run() + Expect(err).ToNot(HaveOccurred()) + Expect(composeSession).To(Exit(0)) + + lines := composeSession.outputToStringSlice() + if runtime.GOOS != "windows" { + Expect(lines[0]).To(Equal("unix://" + inspectInfo[0].ConnectionInfo.PodmanSocket.GetPath())) + } else { + Expect(strings.TrimSuffix(lines[0], "\r")).To(Equal("npipe://" + filepath.ToSlash(inspectInfo[0].ConnectionInfo.PodmanPipe.GetPath()))) + } + Expect(strings.TrimSuffix(lines[1], "\r")).To(Equal("0")) + }) + +}) diff --git a/pkg/machine/e2e/config_compose_test.go b/pkg/machine/e2e/config_compose_test.go new file mode 100644 index 0000000000..9f16f04708 --- /dev/null +++ b/pkg/machine/e2e/config_compose_test.go @@ -0,0 +1,12 @@ +package e2e_test + +type fakeCompose struct { + cmd []string +} + +func (f *fakeCompose) buildCmd(m *machineTestBuilder) []string { + cmd := []string{"compose"} + cmd = append(cmd, "env") + f.cmd = cmd + return cmd +} diff --git a/pkg/machine/e2e/machine_test.go b/pkg/machine/e2e/machine_test.go index 7ef0c60549..a4c3084a1a 100644 --- a/pkg/machine/e2e/machine_test.go +++ b/pkg/machine/e2e/machine_test.go @@ -154,6 +154,22 @@ func setup() (string, *machineTestBuilder) { if err := os.Setenv("PODMAN_CONNECTIONS_CONF", filepath.Join(homeDir, "connections.json")); err != nil { Fail("failed to set PODMAN_CONNECTIONS_CONF") } + if err := os.Setenv("PODMAN_COMPOSE_WARNING_LOGS", "false"); err != nil { + Fail("failed to set PODMAN_COMPOSE_WARNING_LOGS") + } + cwd, err := os.Getwd() + if err != nil { + Fail("unable to get working directory") + } + var fakeComposeBin string + if runtime.GOOS != "windows" { + fakeComposeBin = "fake_compose" + } else { + fakeComposeBin = "fake_compose.bat" + } + if err := os.Setenv("PODMAN_COMPOSE_PROVIDER", filepath.Join(cwd, "scripts", fakeComposeBin)); err != nil { + Fail("failed to set PODMAN_COMPOSE_PROVIDER") + } mb, err := newMB() if err != nil { Fail(fmt.Sprintf("failed to create machine test: %q", err)) diff --git a/pkg/machine/e2e/scripts/fake_compose b/pkg/machine/e2e/scripts/fake_compose new file mode 100755 index 0000000000..b0261541c2 --- /dev/null +++ b/pkg/machine/e2e/scripts/fake_compose @@ -0,0 +1,6 @@ +#!/bin/bash +if [[ "$@" == "env" ]]; then + printenv DOCKER_HOST DOCKER_BUILDKIT + exit 0 +fi +echo "arguments: $@" diff --git a/pkg/machine/e2e/scripts/fake_compose.bat b/pkg/machine/e2e/scripts/fake_compose.bat new file mode 100644 index 0000000000..e9ae5d99ad --- /dev/null +++ b/pkg/machine/e2e/scripts/fake_compose.bat @@ -0,0 +1,18 @@ +@ECHO off + +IF "%1"=="env" ( + goto procenv +) + +echo arguments: %* +exit + +:procenv + +if NOT "%DOCKER_HOST%" == "" ( + echo %DOCKER_HOST% +) + +if NOT "%DOCKER_BUILDKIT%" == "" ( + echo %DOCKER_BUILDKIT% +)