Skip to content

Commit

Permalink
Fix docker stdin write.
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@teoyaomiqui
teoyaomiqui committed Apr 7, 2021
1 parent c17246b commit 7c9dd85
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 12 deletions.
6 changes: 4 additions & 2 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -26,15 +26,17 @@ 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
github.com/stretchr/testify v1.6.1
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
Expand Down
13 changes: 11 additions & 2 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand All @@ -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=
Expand Down Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
20 changes: 13 additions & 7 deletions pkg/container/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down
47 changes: 47 additions & 0 deletions pkg/container/api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package container
import (
"bytes"
"context"
"fmt"
"io"
"io/ioutil"
"path/filepath"
Expand Down Expand Up @@ -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{
Expand Down
19 changes: 18 additions & 1 deletion pkg/container/container_docker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -271,6 +273,7 @@ func (c *DockerContainer) RunCommand(opts RunCommandOptions) (err error) {
&containerConfig,
&hostConfig,
nil,
nil,
"",
)
if err != nil {
Expand All @@ -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 {
Expand Down
31 changes: 31 additions & 0 deletions pkg/container/container_docker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package container

import (
"bytes"
"context"
"fmt"
"io"
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 7c9dd85

Please sign in to comment.