diff --git a/client/driver/lxc.go b/client/driver/lxc.go index 3c5c19fff..e95063d91 100644 --- a/client/driver/lxc.go +++ b/client/driver/lxc.go @@ -210,9 +210,20 @@ func (d *LxcDriver) Prestart(*ExecContext, *structs.Task) (*PrestartResponse, er // Start starts the LXC Driver func (d *LxcDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse, error) { + sresp, err, errCleanup := d.startWithCleanup(ctx, task) + if err != nil { + if cleanupErr := errCleanup(); cleanupErr != nil { + d.logger.Printf("[ERR] error occurred while cleaning up from error in Start: %v", cleanupErr) + } + } + return sresp, err +} + +func (d *LxcDriver) startWithCleanup(ctx *ExecContext, task *structs.Task) (*StartResponse, error, func() error) { + noCleanup := func() error { return nil } var driverConfig LxcDriverConfig if err := mapstructure.WeakDecode(task.Config, &driverConfig); err != nil { - return nil, err + return nil, err, noCleanup } lxcPath := lxc.DefaultConfigPath() if path := d.config.Read("driver.lxc.path"); path != "" { @@ -222,7 +233,7 @@ func (d *LxcDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse, containerName := fmt.Sprintf("%s-%s", task.Name, d.DriverContext.allocID) c, err := lxc.NewContainer(containerName, lxcPath) if err != nil { - return nil, fmt.Errorf("unable to initialize container: %v", err) + return nil, fmt.Errorf("unable to initialize container: %v", err), noCleanup } var verbosity lxc.Verbosity @@ -232,7 +243,7 @@ func (d *LxcDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse, case "", "quiet": verbosity = lxc.Quiet default: - return nil, fmt.Errorf("lxc driver config 'verbosity' can only be either quiet or verbose") + return nil, fmt.Errorf("lxc driver config 'verbosity' can only be either quiet or verbose"), noCleanup } c.SetVerbosity(verbosity) @@ -249,7 +260,7 @@ func (d *LxcDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse, case "", "error": logLevel = lxc.ERROR default: - return nil, fmt.Errorf("lxc driver config 'log_level' can only be trace, debug, info, warn or error") + return nil, fmt.Errorf("lxc driver config 'log_level' can only be trace, debug, info, warn or error"), noCleanup } c.SetLogLevel(logLevel) @@ -267,12 +278,12 @@ func (d *LxcDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse, } if err := c.Create(options); err != nil { - return nil, fmt.Errorf("unable to create container: %v", err) + return nil, fmt.Errorf("unable to create container: %v", err), noCleanup } // Set the network type to none if err := c.SetConfigItem("lxc.network.type", "none"); err != nil { - return nil, fmt.Errorf("error setting network type configuration: %v", err) + return nil, fmt.Errorf("error setting network type configuration: %v", err), c.Destroy } // Bind mount the shared alloc dir and task local dir in the container @@ -290,7 +301,7 @@ func (d *LxcDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse, if filepath.IsAbs(paths[0]) { if !volumesEnabled { - return nil, fmt.Errorf("absolute bind-mount volume in config but '%v' is false", lxcVolumesConfigOption) + return nil, fmt.Errorf("absolute bind-mount volume in config but '%v' is false", lxcVolumesConfigOption), c.Destroy } } else { // Relative source paths are treated as relative to alloc dir @@ -302,21 +313,28 @@ func (d *LxcDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse, for _, mnt := range mounts { if err := c.SetConfigItem("lxc.mount.entry", mnt); err != nil { - return nil, fmt.Errorf("error setting bind mount %q error: %v", mnt, err) + return nil, fmt.Errorf("error setting bind mount %q error: %v", mnt, err), c.Destroy } } // Start the container if err := c.Start(); err != nil { - return nil, fmt.Errorf("unable to start container: %v", err) + return nil, fmt.Errorf("unable to start container: %v", err), c.Destroy + } + + stopAndDestroyCleanup := func() error { + if err := c.Stop(); err != nil { + return err + } + return c.Destroy() } // Set the resource limits if err := c.SetMemoryLimit(lxc.ByteSize(task.Resources.MemoryMB) * lxc.MB); err != nil { - return nil, fmt.Errorf("unable to set memory limits: %v", err) + return nil, fmt.Errorf("unable to set memory limits: %v", err), stopAndDestroyCleanup } if err := c.SetCgroupItem("cpu.shares", strconv.Itoa(task.Resources.CPU)); err != nil { - return nil, fmt.Errorf("unable to set cpu shares: %v", err) + return nil, fmt.Errorf("unable to set cpu shares: %v", err), stopAndDestroyCleanup } h := lxcDriverHandle{ @@ -335,7 +353,7 @@ func (d *LxcDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse, go h.run() - return &StartResponse{Handle: &h}, nil + return &StartResponse{Handle: &h}, nil, noCleanup } func (d *LxcDriver) Cleanup(*ExecContext, *CreatedResources) error { return nil } diff --git a/client/driver/lxc_test.go b/client/driver/lxc_test.go index ddc78193c..b1d94cf20 100644 --- a/client/driver/lxc_test.go +++ b/client/driver/lxc_test.go @@ -310,6 +310,7 @@ func TestLxcDriver_Start_NoVolumes(t *testing.T) { ctx := testDriverContexts(t, task) defer ctx.AllocDir.Destroy() + // set lxcVolumesConfigOption to false to disallow absolute paths as the source for the bind mount ctx.DriverCtx.config.Options = map[string]string{lxcVolumesConfigOption: "false"} d := NewLxcDriver(ctx.DriverCtx) @@ -317,8 +318,19 @@ func TestLxcDriver_Start_NoVolumes(t *testing.T) { if _, err := d.Prestart(ctx.ExecCtx, task); err != nil { t.Fatalf("prestart err: %v", err) } + + // expect the "absolute bind-mount volume in config.. " error _, err := d.Start(ctx.ExecCtx, task) if err == nil { t.Fatalf("expected error in start, got nil.") } + + // Because the container was created but not started before + // the expected error, we can test that the destroy-only + // cleanup is done here. + containerName := fmt.Sprintf("%s-%s", task.Name, ctx.DriverCtx.allocID) + if err := exec.Command("bash", "-c", fmt.Sprintf("lxc-ls -1 | grep -q %s", containerName)).Run(); err == nil { + t.Fatalf("error, container '%s' is still around", containerName) + } + }