From 966a150c58cad1b961fac21b642b77066e94ff4d Mon Sep 17 00:00:00 2001 From: Matej Vasek Date: Wed, 14 Sep 2022 09:21:49 +0200 Subject: [PATCH] fix: podman auto-svc has own control group (#1239) This keeps the podman process alive while `func` receives SIGKILL or SIGTERM. We must keep podman alive for cleanup (e.g. container removal). Signed-off-by: Matej Vasek Signed-off-by: Matej Vasek --- docker/docker_client.go | 79 ---------------------------- docker/docker_client_linux.go | 89 ++++++++++++++++++++++++++++++++ docker/docker_client_nonlinux.go | 10 ++++ 3 files changed, 99 insertions(+), 79 deletions(-) create mode 100644 docker/docker_client_linux.go create mode 100644 docker/docker_client_nonlinux.go diff --git a/docker/docker_client.go b/docker/docker_client.go index 01042a0b7..d54970a58 100644 --- a/docker/docker_client.go +++ b/docker/docker_client.go @@ -1,20 +1,14 @@ package docker import ( - "bytes" - "context" "encoding/json" "errors" - "fmt" "io" "net/http" "net/url" "os" "os/exec" - "path/filepath" "runtime" - "syscall" - "time" "github.com/docker/docker/client" "golang.org/x/crypto/ssh" @@ -172,79 +166,6 @@ func podmanPresent() bool { return err == nil } -// creates a docker client that has its own podman service associated with it -// the service is shutdown when Close() is called on the client -func newClientWithPodmanService() (dockerClient client.CommonAPIClient, dockerHost string, err error) { - tmpDir, err := os.MkdirTemp("", "func-podman-") - if err != nil { - return - } - - podmanSocket := filepath.Join(tmpDir, "podman.sock") - dockerHost = fmt.Sprintf("unix://%s", podmanSocket) - - cmd := exec.Command("podman", "system", "service", dockerHost, "--time=0") - - outBuff := bytes.Buffer{} - cmd.Stdout = &outBuff - cmd.Stderr = &outBuff - - err = cmd.Start() - if err != nil { - return - } - - waitErrCh := make(chan error) - go func() { waitErrCh <- cmd.Wait() }() - - dockerClient, err = client.NewClientWithOpts(client.FromEnv, client.WithHost(dockerHost), client.WithAPIVersionNegotiation()) - stopPodmanService := func() { - _ = cmd.Process.Signal(syscall.SIGTERM) - _ = os.RemoveAll(tmpDir) - - select { - case <-waitErrCh: - // the podman service has been shutdown, we don't care about error - return - case <-time.After(time.Second * 1): - // failed to gracefully shutdown the podman service, sending SIGKILL - _ = cmd.Process.Signal(syscall.SIGKILL) - } - } - dockerClient = clientWithAdditionalCleanup{ - CommonAPIClient: dockerClient, - cleanUp: stopPodmanService, - } - - svcUpCh := make(chan struct{}) - go func() { - // give a time to podman to start - for i := 0; i < 40; i++ { - if _, e := dockerClient.Ping(context.Background()); e == nil { - svcUpCh <- struct{}{} - } - time.Sleep(time.Millisecond * 250) - } - }() - - select { - case <-svcUpCh: - return - case <-time.After(time.Second * 10): - stopPodmanService() - err = errors.New("the podman service has not come up in time") - case err = <-waitErrCh: - // If this `case` is not selected then the waitErrCh is eventually read by calling stopPodmanService - if err != nil { - err = fmt.Errorf("failed to start the podman service (cmd out: %q): %w", outBuff.String(), err) - } else { - err = fmt.Errorf("the podman process exited before the service come up (cmd out: %q)", outBuff.String()) - } - } - - return -} - type clientWithAdditionalCleanup struct { client.CommonAPIClient cleanUp func() diff --git a/docker/docker_client_linux.go b/docker/docker_client_linux.go new file mode 100644 index 000000000..4927c89ba --- /dev/null +++ b/docker/docker_client_linux.go @@ -0,0 +1,89 @@ +package docker + +import ( + "bytes" + "context" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "syscall" + "time" + + "github.com/docker/docker/client" +) + +// creates a docker client that has its own podman service associated with it +// the service is shutdown when Close() is called on the client +func newClientWithPodmanService() (dockerClient client.CommonAPIClient, dockerHost string, err error) { + tmpDir, err := os.MkdirTemp("", "func-podman-") + if err != nil { + return + } + + podmanSocket := filepath.Join(tmpDir, "podman.sock") + dockerHost = fmt.Sprintf("unix://%s", podmanSocket) + + cmd := exec.Command("podman", "system", "service", dockerHost, "--time=0") + + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true, Pgid: 0} + outBuff := bytes.Buffer{} + cmd.Stdout = &outBuff + cmd.Stderr = &outBuff + + err = cmd.Start() + if err != nil { + return + } + + waitErrCh := make(chan error) + go func() { waitErrCh <- cmd.Wait() }() + + dockerClient, err = client.NewClientWithOpts(client.FromEnv, client.WithHost(dockerHost), client.WithAPIVersionNegotiation()) + stopPodmanService := func() { + _ = cmd.Process.Signal(syscall.SIGTERM) + _ = os.RemoveAll(tmpDir) + + select { + case <-waitErrCh: + // the podman service has been shutdown, we don't care about error + return + case <-time.After(time.Second * 1): + // failed to gracefully shutdown the podman service, sending SIGKILL + _ = cmd.Process.Signal(syscall.SIGKILL) + } + } + dockerClient = clientWithAdditionalCleanup{ + CommonAPIClient: dockerClient, + cleanUp: stopPodmanService, + } + + svcUpCh := make(chan struct{}) + go func() { + // give a time to podman to start + for i := 0; i < 40; i++ { + if _, e := dockerClient.Ping(context.Background()); e == nil { + svcUpCh <- struct{}{} + } + time.Sleep(time.Millisecond * 250) + } + }() + + select { + case <-svcUpCh: + return + case <-time.After(time.Second * 10): + stopPodmanService() + err = errors.New("the podman service has not come up in time") + case err = <-waitErrCh: + // If this `case` is not selected then the waitErrCh is eventually read by calling stopPodmanService + if err != nil { + err = fmt.Errorf("failed to start the podman service (cmd out: %q): %w", outBuff.String(), err) + } else { + err = fmt.Errorf("the podman process exited before the service come up (cmd out: %q)", outBuff.String()) + } + } + + return +} diff --git a/docker/docker_client_nonlinux.go b/docker/docker_client_nonlinux.go new file mode 100644 index 000000000..3a0e83b88 --- /dev/null +++ b/docker/docker_client_nonlinux.go @@ -0,0 +1,10 @@ +//go:build !linux +// +build !linux + +package docker + +import "github.com/docker/docker/client" + +func newClientWithPodmanService() (dockerClient client.CommonAPIClient, dockerHost string, err error) { + panic("only implemented on Linux") +}