Skip to content

Commit

Permalink
containerupdate: use the exec API instead of the cp API for copying f…
Browse files Browse the repository at this point in the history
…iles

Fixes #3708
  • Loading branch information
nicks committed Jun 8, 2021
1 parent da60503 commit c8c15dc
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 32 deletions.
12 changes: 6 additions & 6 deletions internal/containerupdate/docker_container_updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,16 @@ func (cu *DockerUpdater) UpdateContainer(ctx context.Context, cInfo store.Contai
return errors.Wrap(err, "rmPathsFromContainer")
}

// TODO(maia): catch errors -- CopyToContainer doesn't return errors if e.g. it
// fails to write a file b/c of permissions =(
err = cu.dCli.CopyToContainerRoot(ctx, cInfo.ContainerID.String(), archiveToCopy)
err = cu.dCli.ExecInContainer(ctx, cInfo.ContainerID, model.Cmd{
Argv: []string{"tar", "-C", "/", "-x", "-f", "-"},
}, archiveToCopy, l.Writer(logger.InfoLvl))
if err != nil {
return err
return errors.Wrap(err, "copying files")
}

// Exec run's on container
for _, s := range cmds {
err = cu.dCli.ExecInContainer(ctx, cInfo.ContainerID, s, l.Writer(logger.InfoLvl))
err = cu.dCli.ExecInContainer(ctx, cInfo.ContainerID, s, nil, l.Writer(logger.InfoLvl))
if err != nil {
return build.WrapContainerExecError(err, cInfo.ContainerID, s)
}
Expand All @@ -75,7 +75,7 @@ func (cu *DockerUpdater) rmPathsFromContainer(ctx context.Context, cID container
}

out := bytes.NewBuffer(nil)
err := cu.dCli.ExecInContainer(ctx, cID, model.Cmd{Argv: makeRmCmd(paths)}, out)
err := cu.dCli.ExecInContainer(ctx, cID, model.Cmd{Argv: makeRmCmd(paths)}, nil, out)
if err != nil {
if docker.IsExitError(err) {
return fmt.Errorf("Error deleting files from container: %s", out.String())
Expand Down
33 changes: 24 additions & 9 deletions internal/docker/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,10 @@ type Client interface {
ContainerInspect(ctx context.Context, contianerID string) (types.ContainerJSON, error)
ContainerList(ctx context.Context, options types.ContainerListOptions) ([]types.Container, error)
ContainerRestartNoWait(ctx context.Context, containerID string) error
CopyToContainerRoot(ctx context.Context, container string, content io.Reader) error

// Execute a command in a container, streaming the command output to `out`.
// Returns an ExitError if the command exits with a non-zero exit code.
ExecInContainer(ctx context.Context, cID container.ID, cmd model.Cmd, out io.Writer) error
ExecInContainer(ctx context.Context, cID container.ID, cmd model.Cmd, in io.Reader, out io.Writer) error

ImagePush(ctx context.Context, image reference.NamedTagged) (io.ReadCloser, error)
ImageBuild(ctx context.Context, buildContext io.Reader, options BuildOptions) (types.ImageBuildResponse, error)
Expand Down Expand Up @@ -487,10 +486,6 @@ func (c *Cli) ImageBuild(ctx context.Context, buildContext io.Reader, options Bu
return response, err
}

func (c *Cli) CopyToContainerRoot(ctx context.Context, container string, content io.Reader) error {
return c.CopyToContainer(ctx, container, "/", content, types.CopyToContainerOptions{})
}

func (c *Cli) ContainerRestartNoWait(ctx context.Context, containerID string) error {

// Don't wait on the container to fully start.
Expand All @@ -499,13 +494,14 @@ func (c *Cli) ContainerRestartNoWait(ctx context.Context, containerID string) er
return c.ContainerRestart(ctx, containerID, &dur)
}

func (c *Cli) ExecInContainer(ctx context.Context, cID container.ID, cmd model.Cmd, out io.Writer) error {

func (c *Cli) ExecInContainer(ctx context.Context, cID container.ID, cmd model.Cmd, in io.Reader, out io.Writer) error {
attachStdin := in != nil
cfg := types.ExecConfig{
Cmd: cmd.Argv,
AttachStdout: true,
AttachStderr: true,
Tty: true,
AttachStdin: attachStdin,
Tty: !attachStdin,
}

// ContainerExecCreate error-handling is awful, so before we Create
Expand Down Expand Up @@ -536,11 +532,30 @@ func (c *Cli) ExecInContainer(ctx context.Context, cID container.ID, cmd model.C
return errors.Wrap(err, "ExecInContainer#print")
}

inputDone := make(chan struct{})
if attachStdin {
go func() {
_, err := io.Copy(connection.Conn, in)
if err != nil {
logger.Get(ctx).Debugf("copy error: %v", err)
}
err = connection.CloseWrite()
if err != nil {
logger.Get(ctx).Debugf("close write error: %v", err)
}
close(inputDone)
}()
} else {
close(inputDone)
}

_, err = io.Copy(out, connection.Reader)
if err != nil {
return errors.Wrap(err, "ExecInContainer#copy")
}

<-inputDone

for {
inspected, err := c.ContainerExecInspect(ctx, execId.ID)
if err != nil {
Expand Down
5 changes: 1 addition & 4 deletions internal/docker/exploding.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,7 @@ func (c explodingClient) ContainerList(ctx context.Context, options types.Contai
func (c explodingClient) ContainerRestartNoWait(ctx context.Context, containerID string) error {
return c.err
}
func (c explodingClient) CopyToContainerRoot(ctx context.Context, container string, content io.Reader) error {
return c.err
}
func (c explodingClient) ExecInContainer(ctx context.Context, cID container.ID, cmd model.Cmd, out io.Writer) error {
func (c explodingClient) ExecInContainer(ctx context.Context, cID container.ID, cmd model.Cmd, in io.Reader, out io.Writer) error {
return c.err
}
func (c explodingClient) ImagePush(ctx context.Context, ref reference.NamedTagged) (io.ReadCloser, error) {
Expand Down
16 changes: 8 additions & 8 deletions internal/docker/fake_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,14 @@ func (c *FakeClient) ContainerRestartNoWait(ctx context.Context, containerID str
return nil
}

func (c *FakeClient) ExecInContainer(ctx context.Context, cID container.ID, cmd model.Cmd, out io.Writer) error {
func (c *FakeClient) ExecInContainer(ctx context.Context, cID container.ID, cmd model.Cmd, in io.Reader, out io.Writer) error {
if cmd.Argv[0] == "tar" {
c.CopyCount++
c.CopyContainer = string(cID)
c.CopyContent = in
return nil
}

execCall := ExecCall{
Container: cID.String(),
Cmd: cmd,
Expand All @@ -221,13 +228,6 @@ func (c *FakeClient) ExecInContainer(ctx context.Context, cID container.ID, cmd
return err
}

func (c *FakeClient) CopyToContainerRoot(ctx context.Context, container string, content io.Reader) error {
c.CopyCount++
c.CopyContainer = container
c.CopyContent = content
return nil
}

func (c *FakeClient) ImagePush(ctx context.Context, ref reference.NamedTagged) (io.ReadCloser, error) {
c.PushCount++
c.PushImage = ref.String()
Expand Down
7 changes: 2 additions & 5 deletions internal/docker/switch.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,8 @@ func (c *switchCli) ContainerList(ctx context.Context, options types.ContainerLi
func (c *switchCli) ContainerRestartNoWait(ctx context.Context, containerID string) error {
return c.client().ContainerRestartNoWait(ctx, containerID)
}
func (c *switchCli) CopyToContainerRoot(ctx context.Context, container string, content io.Reader) error {
return c.client().CopyToContainerRoot(ctx, container, content)
}
func (c *switchCli) ExecInContainer(ctx context.Context, cID container.ID, cmd model.Cmd, out io.Writer) error {
return c.client().ExecInContainer(ctx, cID, cmd, out)
func (c *switchCli) ExecInContainer(ctx context.Context, cID container.ID, cmd model.Cmd, in io.Reader, out io.Writer) error {
return c.client().ExecInContainer(ctx, cID, cmd, in, out)
}
func (c *switchCli) ImagePush(ctx context.Context, ref reference.NamedTagged) (io.ReadCloser, error) {
return c.client().ImagePush(ctx, ref)
Expand Down

0 comments on commit c8c15dc

Please sign in to comment.