From 7c9dd85eeb59b29c2bad77c29cbc4effdce0cbcd Mon Sep 17 00:00:00 2001 From: Kostiantyn Kalynovskyi Date: Sat, 3 Apr 2021 20:54:11 +0000 Subject: [PATCH] Fix docker stdin write. Without this commit airship can hang endlessly waiting for stdin to be open. Apparently it depends on the containerd and docker server version. This commit adds asnyc writing to stdin, this way we don't have to wait for write to complete before starting docker container. The code uses similar approach to upstream docker cli implementation. Related-To: #513 Change-Id: I2e6d4cbe37df1f8cba356af79c1c2cf18438e86c --- go.mod | 6 ++-- go.sum | 13 +++++-- pkg/container/api.go | 20 +++++++---- pkg/container/api_test.go | 47 ++++++++++++++++++++++++++ pkg/container/container_docker.go | 19 ++++++++++- pkg/container/container_docker_test.go | 31 +++++++++++++++++ 6 files changed, 124 insertions(+), 12 deletions(-) diff --git a/go.mod b/go.mod index df4f1a37a..0b1ce3371 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/ahmetb/dlog v0.0.0-20170105205344-4fb5f8204f26 github.com/chai2010/gettext-go v0.0.0-20170215093142-bf70f2a70fb1 // indirect github.com/containerd/containerd v1.4.1 // indirect - github.com/docker/docker v1.4.2-0.20200203170920-46ec8731fbce + github.com/docker/docker v20.10.5+incompatible github.com/docker/go-connections v0.4.0 // indirect github.com/docker/spdystream v0.0.0-20181023171402-6480d4af844c // indirect github.com/elazarl/goproxy v0.0.0-20190421051319-9d40249d3c2f // indirect @@ -26,8 +26,9 @@ require ( github.com/gregjones/httpcache v0.0.0-20190212212710-3befbb6ad0cc // indirect github.com/hashicorp/go-cleanhttp v0.5.1 // indirect github.com/lucasjones/reggen v0.0.0-20200904144131-37ba4fa293bb + github.com/moby/term v0.0.0-20201216013528-df9cb8a40635 // indirect github.com/morikuni/aec v1.0.0 // indirect - github.com/opencontainers/image-spec v1.0.1 // indirect + github.com/opencontainers/image-spec v1.0.1 github.com/pkg/errors v0.9.1 github.com/spf13/cobra v1.0.0 github.com/spf13/pflag v1.0.5 @@ -35,6 +36,7 @@ require ( golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 golang.org/x/net v0.0.0-20201021035429-f5854403a974 // indirect golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 // indirect + gotest.tools/v3 v3.0.3 // indirect k8s.io/api v0.17.9 k8s.io/apiextensions-apiserver v0.17.9 k8s.io/apimachinery v0.17.9 diff --git a/go.sum b/go.sum index 22ff14d13..999466176 100644 --- a/go.sum +++ b/go.sum @@ -131,6 +131,8 @@ github.com/cpuguy83/go-md2man/v2 v2.0.0 h1:EoUDS0afbrsXAZ9YQ9jdu/mZ2sXgT1/2yyNng github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= +github.com/creack/pty v1.1.11 h1:07n33Z8lZxZ2qwegKbObQohDhXDQxiMMz1NOUGYlesw= +github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/davecgh/go-spew v0.0.0-20151105211317-5215b55f46b2/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= @@ -142,8 +144,8 @@ github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8 github.com/docker/distribution v2.7.1+incompatible h1:a5mlkVzth6W5A4fOsS3D2EO5BUmsJpcB+cRlLU7cSug= github.com/docker/distribution v2.7.1+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w= github.com/docker/docker v0.7.3-0.20190327010347-be7ac8be2ae0/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= -github.com/docker/docker v1.4.2-0.20200203170920-46ec8731fbce h1:KXS1Jg+ddGcWA8e1N7cupxaHHZhit5rB9tfDU+mfjyY= -github.com/docker/docker v1.4.2-0.20200203170920-46ec8731fbce/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= +github.com/docker/docker v20.10.5+incompatible h1:o5WL5onN4awYGwrW7+oTn5x9AF2prw7V0Ox8ZEkoCdg= +github.com/docker/docker v20.10.5+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= github.com/docker/go-connections v0.4.0 h1:El9xVISelRB7BuFusrZozjnkIM5YnzCViNKohAFqRJQ= github.com/docker/go-connections v0.4.0/go.mod h1:Gbd7IOopHjR8Iph03tsViu4nIes5XhDvyHbTtUxmeec= github.com/docker/go-units v0.3.3/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk= @@ -501,6 +503,8 @@ github.com/mitchellh/mapstructure v1.1.2 h1:fmNYVwqnSfB9mZU6OS2O6GsXM+wcskZDuKQz github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= github.com/mitchellh/reflectwalk v1.0.0 h1:9D+8oIskB4VJBN5SFlmc27fSlIBZaov1Wpk/IfikLNY= github.com/mitchellh/reflectwalk v1.0.0/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw= +github.com/moby/term v0.0.0-20201216013528-df9cb8a40635 h1:rzf0wL0CHVc8CEsgyygG0Mn9CNCCPZqOPaz8RiiHYQk= +github.com/moby/term v0.0.0-20201216013528-df9cb8a40635/go.mod h1:FBS0z0QWA44HXygs7VXDUOGoN/1TV3RuWkLO04am3wc= github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= @@ -827,6 +831,7 @@ golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200519105757-fe76b779f299/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200602225109-6fdc65e7d980/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20200831180312-196b9ba8737a/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k= golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -867,6 +872,7 @@ golang.org/x/tools v0.0.0-20190606124116-d0a3d012864b/go.mod h1:/rFqwRUd4F7ZHNgw golang.org/x/tools v0.0.0-20190614205625-5aca471b1d59/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= golang.org/x/tools v0.0.0-20190617190820-da514acc4774/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= golang.org/x/tools v0.0.0-20190621195816-6e04913cbbac/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= +golang.org/x/tools v0.0.0-20190624222133-a101b041ded4/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= golang.org/x/tools v0.0.0-20190628153133-6cdbf07be9d0/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= golang.org/x/tools v0.0.0-20190719005602-e377ae9d6386/go.mod h1:jcCCGcm9btYwXyDqrUWc6MKQKKGJCWEQ3AfLSRIbEuI= golang.org/x/tools v0.0.0-20190816200558-6889da9d5479/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= @@ -957,6 +963,9 @@ gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo= gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw= +gotest.tools/v3 v3.0.2/go.mod h1:3SzNCllyD9/Y+b5r9JIKQ474KzkZyqLqEfYqMsX94Bk= +gotest.tools/v3 v3.0.3 h1:4AuOwCGf4lLR9u3YOe2awrHygurzhO/HeQ6laiA6Sx0= +gotest.tools/v3 v3.0.3/go.mod h1:Z7Lb0S5l+klDB31fvDQX8ss/FlKDxtlFlw3Oa8Ymbl8= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/pkg/container/api.go b/pkg/container/api.go index 1319b16da..050679771 100644 --- a/pkg/container/api.go +++ b/pkg/container/api.go @@ -167,10 +167,19 @@ func (c *clientV1Alpha1) runAirship() error { c.conf.Spec.Airship.Cmd) // write logs asynchronously while waiting for for container to finish - go writeLogs(cont) + cErr := make(chan error, 1) + go func() { + cErr <- writeLogs(cont) + }() err = cont.WaitUntilFinished() if err != nil { + <-cErr + return err + } + + // check writeLogs error after container is done waiting + if err = <-cErr; err != nil { return err } @@ -224,20 +233,17 @@ func (c *clientV1Alpha1) runKRM() error { return fns.Execute() } -func writeLogs(cont Container) { +func writeLogs(cont Container) error { stderr, err := cont.GetContainerLogs(GetLogOptions{ Stderr: true, Follow: true}) if err != nil { - log.Fatalf("received an error trying to attach to container to retrieve logs %e", err) - return + return err } defer stderr.Close() parsedStdErr := dlog.NewReader(stderr) _, err = io.Copy(log.Writer(), parsedStdErr) - if err != nil { - log.Fatalf("received an error while copying logs from container %e", err) - } + return err } // writeSink output to directory on filesystem sink diff --git a/pkg/container/api_test.go b/pkg/container/api_test.go index a7ee0c7a7..206694bf7 100644 --- a/pkg/container/api_test.go +++ b/pkg/container/api_test.go @@ -17,6 +17,7 @@ package container import ( "bytes" "context" + "fmt" "io" "io/ioutil" "path/filepath" @@ -135,6 +136,52 @@ func TestGenericContainer(t *testing.T) { }), nil }, }, + { + name: "error airship container writeLogs", + expectedErr: "container logs error", + containerAPI: &v1alpha1.GenericContainer{ + Spec: v1alpha1.GenericContainerSpec{ + Type: v1alpha1.GenericContainerTypeAirship, + Image: "some image", + StorageMounts: []v1alpha1.StorageMount{ + { + MountType: "bind", + Src: "test", + DstPath: "/mount", + }, + { + MountType: "bind", + Src: "~/test", + DstPath: "/mnt", + }, + }, + }, + Config: `kind: ConfigMap`, + }, + execFunc: func(ctx context.Context, driver, url string) (Container, error) { + return getDockerContainerMock(mockDockerClient{ + containerAttach: func() (types.HijackedResponse, error) { + conn := types.HijackedResponse{ + Conn: mockConn{WData: make([]byte, len([]byte("foo: bar")))}, + } + return conn, nil + }, + imageList: func() ([]types.ImageSummary, error) { + return []types.ImageSummary{{ID: "imgid"}}, nil + }, + imageInspectWithRaw: func() (types.ImageInspect, []byte, error) { + return types.ImageInspect{ + Config: &container.Config{ + Cmd: []string{"testCmd"}, + }, + }, nil, nil + }, + containerLogs: func() (io.ReadCloser, error) { + return nil, fmt.Errorf("container logs error") + }, + }), nil + }, + }, { name: "basic success airship container", containerAPI: &v1alpha1.GenericContainer{ diff --git a/pkg/container/container_docker.go b/pkg/container/container_docker.go index 32f09d295..729c64651 100644 --- a/pkg/container/container_docker.go +++ b/pkg/container/container_docker.go @@ -27,6 +27,7 @@ import ( "github.com/docker/docker/api/types/mount" "github.com/docker/docker/api/types/network" "github.com/docker/docker/client" + specs "github.com/opencontainers/image-spec/specs-go/v1" "opendev.org/airship/airshipctl/pkg/log" ) @@ -57,6 +58,7 @@ type DockerClient interface { *container.Config, *container.HostConfig, *network.NetworkingConfig, + *specs.Platform, string, ) (container.ContainerCreateCreatedBody, error) // ContainerAttach attaches a connection to a container in the server. @@ -271,6 +273,7 @@ func (c *DockerContainer) RunCommand(opts RunCommandOptions) (err error) { &containerConfig, &hostConfig, nil, + nil, "", ) if err != nil { @@ -284,14 +287,28 @@ func (c *DockerContainer) RunCommand(opts RunCommandOptions) (err error) { Stream: true, Stdin: true, }) + if attachErr != nil { return attachErr } + defer conn.Close() - if _, err = io.Copy(conn.Conn, opts.Input); err != nil { + // This code is smiplified version of docker cli code + cErr := make(chan error, 1) + + // Write to stdin asynchronously + go func() { + _, copyErr := io.Copy(conn.Conn, opts.Input) + cErr <- copyErr + }() + + if err = c.dockerClient.ContainerStart(c.ctx, c.id, types.ContainerStartOptions{}); err != nil { + <-cErr return err } + // lock until error is returned from the write channel + return <-cErr } if err = c.dockerClient.ContainerStart(c.ctx, c.id, types.ContainerStartOptions{}); err != nil { diff --git a/pkg/container/container_docker_test.go b/pkg/container/container_docker_test.go index b28eab930..2c2f66839 100644 --- a/pkg/container/container_docker_test.go +++ b/pkg/container/container_docker_test.go @@ -15,6 +15,7 @@ package container import ( + "bytes" "context" "fmt" "io" @@ -30,6 +31,7 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/network" + specs "github.com/opencontainers/image-spec/specs-go/v1" ) type mockConn struct { @@ -77,6 +79,7 @@ func (mdc *mockDockerClient) ContainerCreate( *container.Config, *container.HostConfig, *network.NetworkingConfig, + *specs.Platform, string, ) (container.ContainerCreateCreatedBody, error) { return container.ContainerCreateCreatedBody{ID: "testID"}, nil @@ -370,6 +373,34 @@ func TestRunCommand(t *testing.T) { expectedWaitErr: nil, assertF: func(t *testing.T) {}, }, + { + // pass empty buffer to make sure we cover error when input isn't nil + containerInput: bytes.NewBuffer([]byte{}), + volumeMounts: nil, + debug: false, + mockDockerClient: mockDockerClient{ + containerStart: func() error { + return containerStartError + }, + imageList: func() ([]types.ImageSummary, error) { + return []types.ImageSummary{{ID: "imgid"}}, nil + }, + imageInspectWithRaw: func() (types.ImageInspect, []byte, error) { + return types.ImageInspect{ + Config: &container.Config{}, + }, nil, nil + }, + containerAttach: func() (types.HijackedResponse, error) { + conn := types.HijackedResponse{ + Conn: mockConn{WData: make([]byte, len([]byte("testInput")))}, + } + return conn, nil + }, + }, + expectedRunErr: containerStartError, + expectedWaitErr: nil, + assertF: func(t *testing.T) {}, + }, { cmd: []string{"testCmd"}, containerInput: nil,