From 7e9abbf5296c77975dd98668e07835a9b39d7170 Mon Sep 17 00:00:00 2001 From: Simone Gotti Date: Mon, 22 Apr 2019 18:17:55 +0200 Subject: [PATCH] runservice executor: add driver Setup method Remote custom `copytoolbox` hack and use a generic `Setup` function in the driver interface --- .../runservice/executor/driver/docker.go | 26 ++++++++++++------- .../runservice/executor/driver/docker_test.go | 6 ++++- .../runservice/executor/driver/driver.go | 1 + .../services/runservice/executor/executor.go | 4 +-- 4 files changed, 24 insertions(+), 13 deletions(-) diff --git a/internal/services/runservice/executor/driver/docker.go b/internal/services/runservice/executor/driver/docker.go index 0ea0fb4..8e290a6 100644 --- a/internal/services/runservice/executor/driver/docker.go +++ b/internal/services/runservice/executor/driver/docker.go @@ -42,9 +42,10 @@ type DockerDriver struct { logger *zap.Logger client *client.Client initVolumeHostDir string + toolboxPath string } -func NewDockerDriver(logger *zap.Logger, initVolumeHostDir string) (*DockerDriver, error) { +func NewDockerDriver(logger *zap.Logger, initVolumeHostDir, toolboxPath string) (*DockerDriver, error) { cli, err := client.NewEnvClient() if err != nil { return nil, err @@ -53,13 +54,18 @@ func NewDockerDriver(logger *zap.Logger, initVolumeHostDir string) (*DockerDrive logger: logger, client: cli, initVolumeHostDir: initVolumeHostDir, + toolboxPath: toolboxPath, }, nil } +func (d *DockerDriver) Setup(ctx context.Context) error { + return d.CopyToolbox(ctx) +} + // CopyToolbox is an hack needed when running the executor inside a docker // container. It copies the agola-toolbox binaries from the container to an // host path so it can be bind mounted to the other containers -func (d *DockerDriver) CopyToolbox(ctx context.Context, toolboxPath string) error { +func (d *DockerDriver) CopyToolbox(ctx context.Context) error { // by default always try to pull the image so we are sure only authorized users can fetch them // see https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#alwayspullimages reader, err := d.client.ImagePull(ctx, "busybox", types.ImagePullOptions{}) @@ -85,7 +91,7 @@ func (d *DockerDriver) CopyToolbox(ctx context.Context, toolboxPath string) erro return err } - srcInfo, err := archive.CopyInfoSourcePath(toolboxPath, false) + srcInfo, err := archive.CopyInfoSourcePath(d.toolboxPath, false) if err != nil { return err } @@ -288,9 +294,9 @@ func podLabelsFromContainer(containerLabels map[string]string) map[string]string return labels } -func (d *DockerDriver) GetPodByID(ctx context.Context, containerID string) (Pod, error) { +func (d *DockerDriver) GetPodByID(ctx context.Context, podID string) (Pod, error) { args := filters.NewArgs() - args.Add(podIDKey, containerID) + args.Add(podIDKey, podID) containers, err := d.client.ContainerList(ctx, types.ContainerListOptions{ @@ -300,7 +306,7 @@ func (d *DockerDriver) GetPodByID(ctx context.Context, containerID string) (Pod, return nil, err } if len(containers) == 0 { - return nil, errors.Errorf("no container with id %s", containerID) + return nil, errors.Errorf("no pod with id %s", podID) } return &DockerPod{ @@ -375,7 +381,7 @@ func (s *Stdin) Close() error { return s.hresp.CloseWrite() } -func (dc *DockerPod) Exec(ctx context.Context, execConfig *ExecConfig) (ContainerExec, error) { +func (dp *DockerPod) Exec(ctx context.Context, execConfig *ExecConfig) (ContainerExec, error) { endCh := make(chan error) dockerExecConfig := types.ExecConfig{ @@ -389,7 +395,7 @@ func (dc *DockerPod) Exec(ctx context.Context, execConfig *ExecConfig) (Containe User: execConfig.User, } - response, err := dc.client.ContainerExecCreate(ctx, dc.containers[0].ID, dockerExecConfig) + response, err := dp.client.ContainerExecCreate(ctx, dp.containers[0].ID, dockerExecConfig) if err != nil { return nil, err } @@ -397,7 +403,7 @@ func (dc *DockerPod) Exec(ctx context.Context, execConfig *ExecConfig) (Containe Detach: dockerExecConfig.Detach, Tty: dockerExecConfig.Tty, } - hresp, err := dc.client.ContainerExecAttach(ctx, response.ID, execStartCheck) + hresp, err := dp.client.ContainerExecAttach(ctx, response.ID, execStartCheck) if err != nil { return nil, err } @@ -429,7 +435,7 @@ func (dc *DockerPod) Exec(ctx context.Context, execConfig *ExecConfig) (Containe return &DockerContainerExec{ execID: response.ID, hresp: &hresp, - client: dc.client, + client: dp.client, stdin: stdin, endCh: endCh, }, nil diff --git a/internal/services/runservice/executor/driver/docker_test.go b/internal/services/runservice/executor/driver/docker_test.go index f1402a2..4cd55db 100644 --- a/internal/services/runservice/executor/driver/docker_test.go +++ b/internal/services/runservice/executor/driver/docker_test.go @@ -76,6 +76,10 @@ func TestPod(t *testing.T) { if os.Getenv("SKIP_DOCKER_TESTS") == "1" { t.Skip("skipping since env var SKIP_DOCKER_TESTS is 1") } + toolboxPath := os.Getenv("AGOLA_TOOLBOX_PATH") + if toolboxPath == "" { + t.Fatalf("env var AGOLA_TOOLBOX_PATH is undefined") + } dir, err := ioutil.TempDir("", "agola") if err != nil { @@ -83,7 +87,7 @@ func TestPod(t *testing.T) { } defer os.RemoveAll(dir) - d, err := NewDockerDriver(logger, dir) + d, err := NewDockerDriver(logger, dir, toolboxPath) if err != nil { t.Fatalf("unexpected err: %v", err) } diff --git a/internal/services/runservice/executor/driver/driver.go b/internal/services/runservice/executor/driver/driver.go index 5add210..9550e05 100644 --- a/internal/services/runservice/executor/driver/driver.go +++ b/internal/services/runservice/executor/driver/driver.go @@ -40,6 +40,7 @@ const ( // * Kubernetes pods // * A Virtual Machine on which we execute multiple processes type Driver interface { + Setup(ctx context.Context) error NewPod(ctx context.Context, podConfig *PodConfig, out io.Writer) (Pod, error) GetPodsByLabels(ctx context.Context, labels map[string]string, all bool) ([]Pod, error) GetPodByID(ctx context.Context, containerID string) (Pod, error) diff --git a/internal/services/runservice/executor/executor.go b/internal/services/runservice/executor/executor.go index f3f4d22..de4f3ac 100644 --- a/internal/services/runservice/executor/executor.go +++ b/internal/services/runservice/executor/executor.go @@ -1216,7 +1216,7 @@ func NewExecutor(c *config.RunServiceExecutor) (*Executor, error) { c.ToolboxPath = path } - dockerDriver, err := driver.NewDockerDriver(logger, "/tmp/agola/bin") + dockerDriver, err := driver.NewDockerDriver(logger, "/tmp/agola/bin", c.ToolboxPath) if err != nil { return nil, errors.Wrapf(err, "failed to create docker client") } @@ -1269,7 +1269,7 @@ func NewExecutor(c *config.RunServiceExecutor) (*Executor, error) { } func (e *Executor) Run(ctx context.Context) error { - if err := e.driver.(*driver.DockerDriver).CopyToolbox(context.TODO(), e.c.ToolboxPath); err != nil { + if err := e.driver.Setup(ctx); err != nil { return err }