From 3ae387e9f2a5cf235081e3af70f840a254ad3a8a Mon Sep 17 00:00:00 2001 From: Kostiantyn Kalynovskyi Date: Thu, 4 Feb 2021 20:50:35 +0000 Subject: [PATCH] Change container RunMethod to allow extending This will make sure that when we add new features to how the docker container is run, we don't need to change interface method signature everywhere we use it. Relates-To: #458 Change-Id: I12273264c1a8061300017246a1a4a17125ca8ae2 --- pkg/bootstrap/ephemeral/command.go | 2 +- pkg/bootstrap/isogen/command.go | 2 +- pkg/container/container.go | 10 +++++++++- pkg/container/container_docker.go | 15 +++++---------- pkg/container/container_docker_test.go | 16 +++++++++++++--- testutil/container/container.go | 2 +- 6 files changed, 30 insertions(+), 17 deletions(-) diff --git a/pkg/bootstrap/ephemeral/command.go b/pkg/bootstrap/ephemeral/command.go index bb3d0a1f8..ef569a126 100644 --- a/pkg/bootstrap/ephemeral/command.go +++ b/pkg/bootstrap/ephemeral/command.go @@ -197,7 +197,7 @@ func (options *BootstrapContainerOptions) CreateBootstrapContainer() error { fmt.Sprintf("%s=%s", envBootstrapVolume, containerVolMount), } - err := options.Container.RunCommand([]string{}, nil, vols, envVars) + err := options.Container.RunCommand(container.RunCommandOptions{EnvVars: envVars, VolumeMounts: vols}) if err != nil { return err } diff --git a/pkg/bootstrap/isogen/command.go b/pkg/bootstrap/isogen/command.go index 7de20c2b3..b10306395 100644 --- a/pkg/bootstrap/isogen/command.go +++ b/pkg/bootstrap/isogen/command.go @@ -134,7 +134,7 @@ func (opts BootstrapIsoOptions) CreateBootstrapIso() error { fmt.Sprintf("NO_PROXY=%s", os.Getenv("NO_PROXY")), } - err = opts.Builder.RunCommand([]string{}, nil, vols, envVars) + err = opts.Builder.RunCommand(container.RunCommandOptions{EnvVars: envVars, VolumeMounts: vols}) if err != nil { return err } diff --git a/pkg/container/container.go b/pkg/container/container.go index 36df0c483..64837ee40 100644 --- a/pkg/container/container.go +++ b/pkg/container/container.go @@ -35,7 +35,7 @@ type State struct { // defines methods that must be implemented for CRE (e.g. docker, containerd or CRI-O) type Container interface { ImagePull() error - RunCommand([]string, io.Reader, []string, []string) error + RunCommand(RunCommandOptions) error GetContainerLogs() (io.ReadCloser, error) InspectContainer() (State, error) WaitUntilFinished() error @@ -43,6 +43,14 @@ type Container interface { GetID() string } +// RunCommandOptions options for RunCommand +type RunCommandOptions struct { + Cmd []string + EnvVars []string + VolumeMounts []string + Input io.Reader +} + // NewContainer returns instance of Container interface implemented by particular driver // Returned instance type (i.e. implementation) depends on driver specified via function // arguments (e.g. "docker"). diff --git a/pkg/container/container_docker.go b/pkg/container/container_docker.go index 0505ea4a9..11406bd97 100644 --- a/pkg/container/container_docker.go +++ b/pkg/container/container_docker.go @@ -241,18 +241,13 @@ func (c *DockerContainer) ImagePull() error { // RunCommand executes specified command in Docker container. Method handles // container STDIN and volume binds -func (c *DockerContainer) RunCommand( - cmd []string, - containerInput io.Reader, - volumeMounts []string, - envVars []string, -) error { - realCmd, err := c.getCmd(cmd) +func (c *DockerContainer) RunCommand(opts RunCommandOptions) error { + realCmd, err := c.getCmd(opts.Cmd) if err != nil { return err } - containerConfig, hostConfig := c.getConfig(realCmd, volumeMounts, envVars) + containerConfig, hostConfig := c.getConfig(realCmd, opts.VolumeMounts, opts.EnvVars) resp, err := c.dockerClient.ContainerCreate( c.ctx, &containerConfig, @@ -266,7 +261,7 @@ func (c *DockerContainer) RunCommand( c.id = resp.ID - if containerInput != nil { + if opts.Input != nil { conn, attachErr := c.dockerClient.ContainerAttach(c.ctx, c.id, types.ContainerAttachOptions{ Stream: true, Stdin: true, @@ -274,7 +269,7 @@ func (c *DockerContainer) RunCommand( if attachErr != nil { return attachErr } - if _, err = io.Copy(conn.Conn, containerInput); err != nil { + if _, err = io.Copy(conn.Conn, opts.Input); err != nil { return err } } diff --git a/pkg/container/container_docker_test.go b/pkg/container/container_docker_test.go index 632b7963a..e3d53c75b 100644 --- a/pkg/container/container_docker_test.go +++ b/pkg/container/container_docker_test.go @@ -268,7 +268,9 @@ func TestImagePull(t *testing.T) { func TestGetId(t *testing.T) { cnt := getDockerContainerMock(mockDockerClient{}) - err := cnt.RunCommand([]string{"testCmd"}, nil, nil, []string{}) + err := cnt.RunCommand(RunCommandOptions{ + Cmd: []string{"testCmd"}, + }) require.NoError(t, err) actualID := cnt.GetID() @@ -419,7 +421,11 @@ func TestRunCommand(t *testing.T) { } for _, tt := range tests { cnt := getDockerContainerMock(tt.mockDockerClient) - actualErr := cnt.RunCommand(tt.cmd, tt.containerInput, tt.volumeMounts, []string{}) + actualErr := cnt.RunCommand(RunCommandOptions{ + Input: tt.containerInput, + Cmd: tt.cmd, + VolumeMounts: tt.volumeMounts, + }) assert.Equal(t, tt.expectedRunErr, actualErr) actualErr = cnt.WaitUntilFinished() assert.Equal(t, tt.expectedWaitErr, actualErr) @@ -461,7 +467,11 @@ func TestRunCommandOutput(t *testing.T) { } for _, tt := range tests { cnt := getDockerContainerMock(tt.mockDockerClient) - actualErr := cnt.RunCommand(tt.cmd, tt.containerInput, tt.volumeMounts, []string{}) + actualErr := cnt.RunCommand(RunCommandOptions{ + Input: tt.containerInput, + Cmd: tt.cmd, + VolumeMounts: tt.volumeMounts, + }) assert.Equal(t, tt.expectedErr, actualErr) actualRes, actualErr := cnt.GetContainerLogs() require.NoError(t, actualErr) diff --git a/testutil/container/container.go b/testutil/container/container.go index a973309c8..dd01b8077 100755 --- a/testutil/container/container.go +++ b/testutil/container/container.go @@ -37,7 +37,7 @@ func (mc *MockContainer) ImagePull() error { } // RunCommand Container interface implementation for unit test purposes -func (mc *MockContainer) RunCommand([]string, io.Reader, []string, []string) error { +func (mc *MockContainer) RunCommand(container.RunCommandOptions) error { return mc.MockRunCommand() }