From ac6e8d1194885381d5ee3aaa9b86a4bf1f8308a4 Mon Sep 17 00:00:00 2001 From: Kostiantyn Kalynovskyi Date: Tue, 27 Oct 2020 15:21:42 -0500 Subject: [PATCH] Remove not needed context pointers in container interface Container interface is being extended right now, because we build new things on top of it, such as bootstrap containers. Current version of it, was delivered as MVP and is used only in ISOGEN. During MVP stage we didn't bother too much about the pointers and readability. However now when we built something new on top of it, we want to make sure that it we dont make matters worse, and building on solid foundation. The pointers are not needed in any way, and they are dereferenced on top of that, context.Context is an interface, and there is very limited theoretical use of pointers to interfaces. Change-Id: Iee1eeb89f058aa8e994cba685b49085707362ee1 --- pkg/bootstrap/isogen/executor.go | 2 +- pkg/container/container.go | 2 +- pkg/container/container_docker.go | 28 +++++++++++++------------- pkg/container/container_docker_test.go | 4 ++-- pkg/container/container_test.go | 4 ++-- 5 files changed, 20 insertions(+), 20 deletions(-) diff --git a/pkg/bootstrap/isogen/executor.go b/pkg/bootstrap/isogen/executor.go index 0126a30e1..bede44c53 100644 --- a/pkg/bootstrap/isogen/executor.go +++ b/pkg/bootstrap/isogen/executor.go @@ -99,7 +99,7 @@ func (c *Executor) Run(evtCh chan events.Event, opts ifc.RunOptions) { if c.builder == nil { ctx := context.Background() builder, err := container.NewContainer( - &ctx, + ctx, c.imgConf.Container.ContainerRuntime, c.imgConf.Container.Image) c.builder = builder diff --git a/pkg/container/container.go b/pkg/container/container.go index a6a92a755..af8851dc1 100644 --- a/pkg/container/container.go +++ b/pkg/container/container.go @@ -36,7 +36,7 @@ type Container interface { // arguments (e.g. "docker"). // Supported drivers: // * docker -func NewContainer(ctx *context.Context, driver string, url string) (Container, error) { +func NewContainer(ctx context.Context, driver string, url string) (Container, error) { switch driver { case "": return nil, ErrNoContainerDriver{} diff --git a/pkg/container/container_docker.go b/pkg/container/container_docker.go index 1816b702f..db5a51bfe 100644 --- a/pkg/container/container_docker.go +++ b/pkg/container/container_docker.go @@ -95,17 +95,17 @@ type DockerContainer struct { imageURL string id string dockerClient DockerClient - ctx *context.Context + ctx context.Context } // NewDockerClient returns instance of DockerClient. // Function essentially returns new Docker API client with default values -func NewDockerClient(ctx *context.Context) (DockerClient, error) { +func NewDockerClient(ctx context.Context) (DockerClient, error) { cli, err := client.NewClientWithOpts(client.FromEnv) if err != nil { return nil, err } - cli.NegotiateAPIVersion(*ctx) + cli.NegotiateAPIVersion(ctx) return cli, nil } @@ -115,7 +115,7 @@ func NewDockerClient(ctx *context.Context) (DockerClient, error) { // // url format: :. If tag is not specified "latest" is used // as default value -func NewDockerContainer(ctx *context.Context, url string, cli DockerClient) (*DockerContainer, error) { +func NewDockerContainer(ctx context.Context, url string, cli DockerClient) (*DockerContainer, error) { t := "latest" nameTag := strings.Split(url, ":") if len(nameTag) == 2 { @@ -155,7 +155,7 @@ func (c *DockerContainer) getCmd(cmd []string) ([]string, error) { return nil, err } - insp, _, err := c.dockerClient.ImageInspectWithRaw(*c.ctx, id) + insp, _, err := c.dockerClient.ImageInspectWithRaw(c.ctx, id) if err != nil { return nil, err } @@ -195,7 +195,7 @@ func (c *DockerContainer) getImageID(url string) (string, error) { All: false, Filters: filter, } - img, err := c.dockerClient.ImageList(*c.ctx, opts) + img, err := c.dockerClient.ImageList(c.ctx, opts) if err != nil { return "", err } @@ -217,12 +217,12 @@ func (c *DockerContainer) ImagePull() error { // skip image download if already downloaded // ImageInspectWithRaw returns err when image not found local and // in this case it will proceed for ImagePull. - _, _, err := c.dockerClient.ImageInspectWithRaw(*c.ctx, c.imageURL) + _, _, err := c.dockerClient.ImageInspectWithRaw(c.ctx, c.imageURL) if err == nil { log.Debug("Image Already exists, skip download") return nil } - resp, err := c.dockerClient.ImagePull(*c.ctx, c.imageURL, types.ImagePullOptions{}) + resp, err := c.dockerClient.ImagePull(c.ctx, c.imageURL, types.ImagePullOptions{}) if err != nil { return err } @@ -249,7 +249,7 @@ func (c *DockerContainer) RunCommand( containerConfig, hostConfig := c.getConfig(realCmd, volumeMounts, envVars) resp, err := c.dockerClient.ContainerCreate( - *c.ctx, + c.ctx, &containerConfig, &hostConfig, nil, @@ -262,7 +262,7 @@ func (c *DockerContainer) RunCommand( c.id = resp.ID if containerInput != nil { - conn, attachErr := c.dockerClient.ContainerAttach(*c.ctx, c.id, types.ContainerAttachOptions{ + conn, attachErr := c.dockerClient.ContainerAttach(c.ctx, c.id, types.ContainerAttachOptions{ Stream: true, Stdin: true, }) @@ -274,7 +274,7 @@ func (c *DockerContainer) RunCommand( } } - if err = c.dockerClient.ContainerStart(*c.ctx, c.id, types.ContainerStartOptions{}); err != nil { + if err = c.dockerClient.ContainerStart(c.ctx, c.id, types.ContainerStartOptions{}); err != nil { return err } @@ -284,13 +284,13 @@ func (c *DockerContainer) RunCommand( // GetContainerLogs returns logs from the container as io.ReadCloser func (c *DockerContainer) GetContainerLogs() (io.ReadCloser, error) { - return c.dockerClient.ContainerLogs(*c.ctx, c.id, types.ContainerLogsOptions{ShowStdout: true, Follow: true}) + return c.dockerClient.ContainerLogs(c.ctx, c.id, types.ContainerLogsOptions{ShowStdout: true, Follow: true}) } // RmContainer kills and removes a container from the docker host. func (c *DockerContainer) RmContainer() error { return c.dockerClient.ContainerRemove( - *c.ctx, + c.ctx, c.id, types.ContainerRemoveOptions{ Force: true, @@ -300,7 +300,7 @@ func (c *DockerContainer) RmContainer() error { // WaitUntilFinished waits unit container command is finished, return an error if failed func (c *DockerContainer) WaitUntilFinished() error { - statusCh, errCh := c.dockerClient.ContainerWait(*c.ctx, c.id, container.WaitConditionNotRunning) + statusCh, errCh := c.dockerClient.ContainerWait(c.ctx, c.id, container.WaitConditionNotRunning) log.Debugf("waiting until command is finished...") select { case err := <-errCh: diff --git a/pkg/container/container_docker_test.go b/pkg/container/container_docker_test.go index 6df1ec858..d50db1c45 100644 --- a/pkg/container/container_docker_test.go +++ b/pkg/container/container_docker_test.go @@ -123,7 +123,7 @@ func getDockerContainerMock(mdc mockDockerClient) *DockerContainer { ctx := context.Background() cnt := &DockerContainer{ dockerClient: &mdc, - ctx: &ctx, + ctx: ctx, } return cnt } @@ -520,7 +520,7 @@ func TestNewDockerContainer(t *testing.T) { }, } for _, tt := range tests { - actualRes, actualErr := NewDockerContainer(&(tt.ctx), tt.url, &(tt.cli)) + actualRes, actualErr := NewDockerContainer((tt.ctx), tt.url, &(tt.cli)) assert.Equal(t, tt.expectedErr, actualErr) diff --git a/pkg/container/container_test.go b/pkg/container/container_test.go index 3026e7549..e4aab4add 100644 --- a/pkg/container/container_test.go +++ b/pkg/container/container_test.go @@ -27,13 +27,13 @@ func TestNewContainer(t *testing.T) { ctx := context.Background() t.Run("not-supported-container", func(t *testing.T) { - cnt, err := NewContainer(&ctx, "test_drv", "") + cnt, err := NewContainer(ctx, "test_drv", "") assert.Equal(nil, cnt) assert.Equal(ErrContainerDrvNotSupported{Driver: "test_drv"}, err) }) t.Run("empty-container", func(t *testing.T) { - cnt, err := NewContainer(&ctx, "", "") + cnt, err := NewContainer(ctx, "", "") assert.Equal(nil, cnt) assert.Equal(ErrNoContainerDriver{}, err) })