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
This commit is contained in:
Kostiantyn Kalynovskyi 2020-10-27 15:21:42 -05:00
parent 19360953b0
commit ac6e8d1194
5 changed files with 20 additions and 20 deletions

View File

@ -99,7 +99,7 @@ func (c *Executor) Run(evtCh chan events.Event, opts ifc.RunOptions) {
if c.builder == nil { if c.builder == nil {
ctx := context.Background() ctx := context.Background()
builder, err := container.NewContainer( builder, err := container.NewContainer(
&ctx, ctx,
c.imgConf.Container.ContainerRuntime, c.imgConf.Container.ContainerRuntime,
c.imgConf.Container.Image) c.imgConf.Container.Image)
c.builder = builder c.builder = builder

View File

@ -36,7 +36,7 @@ type Container interface {
// arguments (e.g. "docker"). // arguments (e.g. "docker").
// Supported drivers: // Supported drivers:
// * docker // * 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 { switch driver {
case "": case "":
return nil, ErrNoContainerDriver{} return nil, ErrNoContainerDriver{}

View File

@ -95,17 +95,17 @@ type DockerContainer struct {
imageURL string imageURL string
id string id string
dockerClient DockerClient dockerClient DockerClient
ctx *context.Context ctx context.Context
} }
// NewDockerClient returns instance of DockerClient. // NewDockerClient returns instance of DockerClient.
// Function essentially returns new Docker API client with default values // 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) cli, err := client.NewClientWithOpts(client.FromEnv)
if err != nil { if err != nil {
return nil, err return nil, err
} }
cli.NegotiateAPIVersion(*ctx) cli.NegotiateAPIVersion(ctx)
return cli, nil return cli, nil
} }
@ -115,7 +115,7 @@ func NewDockerClient(ctx *context.Context) (DockerClient, error) {
// //
// url format: <image_path>:<tag>. If tag is not specified "latest" is used // url format: <image_path>:<tag>. If tag is not specified "latest" is used
// as default value // 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" t := "latest"
nameTag := strings.Split(url, ":") nameTag := strings.Split(url, ":")
if len(nameTag) == 2 { if len(nameTag) == 2 {
@ -155,7 +155,7 @@ func (c *DockerContainer) getCmd(cmd []string) ([]string, error) {
return nil, err return nil, err
} }
insp, _, err := c.dockerClient.ImageInspectWithRaw(*c.ctx, id) insp, _, err := c.dockerClient.ImageInspectWithRaw(c.ctx, id)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -195,7 +195,7 @@ func (c *DockerContainer) getImageID(url string) (string, error) {
All: false, All: false,
Filters: filter, Filters: filter,
} }
img, err := c.dockerClient.ImageList(*c.ctx, opts) img, err := c.dockerClient.ImageList(c.ctx, opts)
if err != nil { if err != nil {
return "", err return "", err
} }
@ -217,12 +217,12 @@ func (c *DockerContainer) ImagePull() error {
// skip image download if already downloaded // skip image download if already downloaded
// ImageInspectWithRaw returns err when image not found local and // ImageInspectWithRaw returns err when image not found local and
// in this case it will proceed for ImagePull. // 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 { if err == nil {
log.Debug("Image Already exists, skip download") log.Debug("Image Already exists, skip download")
return nil 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 { if err != nil {
return err return err
} }
@ -249,7 +249,7 @@ func (c *DockerContainer) RunCommand(
containerConfig, hostConfig := c.getConfig(realCmd, volumeMounts, envVars) containerConfig, hostConfig := c.getConfig(realCmd, volumeMounts, envVars)
resp, err := c.dockerClient.ContainerCreate( resp, err := c.dockerClient.ContainerCreate(
*c.ctx, c.ctx,
&containerConfig, &containerConfig,
&hostConfig, &hostConfig,
nil, nil,
@ -262,7 +262,7 @@ func (c *DockerContainer) RunCommand(
c.id = resp.ID c.id = resp.ID
if containerInput != nil { 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, Stream: true,
Stdin: 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 return err
} }
@ -284,13 +284,13 @@ func (c *DockerContainer) RunCommand(
// GetContainerLogs returns logs from the container as io.ReadCloser // GetContainerLogs returns logs from the container as io.ReadCloser
func (c *DockerContainer) GetContainerLogs() (io.ReadCloser, error) { 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. // RmContainer kills and removes a container from the docker host.
func (c *DockerContainer) RmContainer() error { func (c *DockerContainer) RmContainer() error {
return c.dockerClient.ContainerRemove( return c.dockerClient.ContainerRemove(
*c.ctx, c.ctx,
c.id, c.id,
types.ContainerRemoveOptions{ types.ContainerRemoveOptions{
Force: true, Force: true,
@ -300,7 +300,7 @@ func (c *DockerContainer) RmContainer() error {
// WaitUntilFinished waits unit container command is finished, return an error if failed // WaitUntilFinished waits unit container command is finished, return an error if failed
func (c *DockerContainer) WaitUntilFinished() error { 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...") log.Debugf("waiting until command is finished...")
select { select {
case err := <-errCh: case err := <-errCh:

View File

@ -123,7 +123,7 @@ func getDockerContainerMock(mdc mockDockerClient) *DockerContainer {
ctx := context.Background() ctx := context.Background()
cnt := &DockerContainer{ cnt := &DockerContainer{
dockerClient: &mdc, dockerClient: &mdc,
ctx: &ctx, ctx: ctx,
} }
return cnt return cnt
} }
@ -520,7 +520,7 @@ func TestNewDockerContainer(t *testing.T) {
}, },
} }
for _, tt := range tests { 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) assert.Equal(t, tt.expectedErr, actualErr)

View File

@ -27,13 +27,13 @@ func TestNewContainer(t *testing.T) {
ctx := context.Background() ctx := context.Background()
t.Run("not-supported-container", func(t *testing.T) { 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(nil, cnt)
assert.Equal(ErrContainerDrvNotSupported{Driver: "test_drv"}, err) assert.Equal(ErrContainerDrvNotSupported{Driver: "test_drv"}, err)
}) })
t.Run("empty-container", func(t *testing.T) { t.Run("empty-container", func(t *testing.T) {
cnt, err := NewContainer(&ctx, "", "") cnt, err := NewContainer(ctx, "", "")
assert.Equal(nil, cnt) assert.Equal(nil, cnt)
assert.Equal(ErrNoContainerDriver{}, err) assert.Equal(ErrNoContainerDriver{}, err)
}) })