From 5127e75569132bb8568b4e566b091d46944a2811 Mon Sep 17 00:00:00 2001 From: Matt Mercer Date: Tue, 17 Oct 2017 17:54:22 -0700 Subject: [PATCH 01/12] Qemu driver: add graceful shutdown feature --- client/driver/qemu.go | 99 ++++++++++++++++++---- client/driver/qemu_test.go | 100 ++++++++++++++++------- website/source/docs/drivers/qemu.html.md | 9 +- 3 files changed, 159 insertions(+), 49 deletions(-) diff --git a/client/driver/qemu.go b/client/driver/qemu.go index 1d1f11695..17b137257 100644 --- a/client/driver/qemu.go +++ b/client/driver/qemu.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "log" + "net" "os" "os/exec" "path/filepath" @@ -13,7 +14,8 @@ import ( "strings" "time" - "github.com/hashicorp/go-plugin" + "github.com/coreos/go-semver/semver" + plugin "github.com/hashicorp/go-plugin" "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/driver/executor" dstructs "github.com/hashicorp/nomad/client/driver/structs" @@ -29,9 +31,14 @@ var ( ) const ( - // The key populated in Node Attributes to indicate presence of the Qemu - // driver - qemuDriverAttr = "driver.qemu" + // The key populated in Node Attributes to indicate presence of the Qemu driver + qemuDriverAttr = "driver.qemu" + qemuDriverVersionAttr = "driver.qemu.version" + qemuDriverLongMonitorPathAttr = "driver.qemu.longsocketpaths" + // Represents an ACPI shutdown request to the VM (emulates pressing a physical power button) + // Reference: https://en.wikibooks.org/wiki/QEMU/Monitor + qemuGracefulShutdownMsg = "system_powerdown\n" + legacyMaxMonitorPathLen = 108 ) // QemuDriver is a driver for running images via Qemu @@ -45,10 +52,11 @@ type QemuDriver struct { } type QemuDriverConfig struct { - ImagePath string `mapstructure:"image_path"` - Accelerator string `mapstructure:"accelerator"` - PortMap []map[string]int `mapstructure:"port_map"` // A map of host port labels and to guest ports. - Args []string `mapstructure:"args"` // extra arguments to qemu executable + ImagePath string `mapstructure:"image_path"` + Accelerator string `mapstructure:"accelerator"` + GracefulShutdown bool `mapstructure:"graceful_shutdown"` + PortMap []map[string]int `mapstructure:"port_map"` // A map of host port labels and to guest ports. + Args []string `mapstructure:"args"` // extra arguments to qemu executable } // qemuHandle is returned from Start/Open as a handle to the PID @@ -56,6 +64,7 @@ type qemuHandle struct { pluginClient *plugin.Client userPid int executor executor.Executor + monitorPath string killTimeout time.Duration maxKillTimeout time.Duration logger *log.Logger @@ -64,6 +73,13 @@ type qemuHandle struct { doneCh chan struct{} } +func getMonitorPath(dir string, longPathSupport string) (string, error) { + if len(dir) > legacyMaxMonitorPathLen && longPathSupport != "1" { + return "", fmt.Errorf("monitor path is too long") + } + return fmt.Sprintf("%s/qemu-monitor.sock", dir), nil +} + // NewQemuDriver is used to create a new exec driver func NewQemuDriver(ctx *DriverContext) Driver { return &QemuDriver{DriverContext: *ctx} @@ -81,6 +97,10 @@ func (d *QemuDriver) Validate(config map[string]interface{}) error { "accelerator": { Type: fields.TypeString, }, + "graceful_shutdown": { + Type: fields.TypeBool, + Required: false, + }, "port_map": { Type: fields.TypeArray, }, @@ -129,7 +149,23 @@ func (d *QemuDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, } node.Attributes[qemuDriverAttr] = "1" - node.Attributes["driver.qemu.version"] = matches[1] + node.Attributes[qemuDriverVersionAttr] = matches[1] + + // Prior to qemu 2.10.1, monitor socket paths are truncated to 108 bytes. + // We should consider this if driver.qemu.version is < 2.10.1 and the + // generated monitor path is too long. + // + // Relevant fix is here: + // https://github.com/qemu/qemu/commit/ad9579aaa16d5b385922d49edac2c96c79bcfb6 + currentVer := semver.New(matches[1]) + fixedSocketPathLenVer := semver.New("2.10.1") + if currentVer.LessThan(*fixedSocketPathLenVer) { + node.Attributes[qemuDriverLongMonitorPathAttr] = "0" + d.logger.Printf("[DEBUG] driver.qemu - long socket paths are not available in this version of QEMU") + } else { + d.logger.Printf("[DEBUG] driver.qemu - long socket paths available in this version of QEMU") + node.Attributes[qemuDriverLongMonitorPathAttr] = "1" + } return true, nil } @@ -190,6 +226,19 @@ func (d *QemuDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse "-nographic", } + var monitorPath string + if d.driverConfig.GracefulShutdown { + // This socket will be used to manage the virtual machine (for example, + // to perform graceful shutdowns) + monitorPath, err = getMonitorPath(ctx.TaskDir.Dir, ctx.TaskEnv.NodeAttrs[qemuDriverLongMonitorPathAttr]) + if err == nil { + d.logger.Printf("[DEBUG] driver.qemu - got monitor path OK: %s", monitorPath) + args = append(args, "-monitor", fmt.Sprintf("unix:%s,server,nowait", monitorPath)) + } else { + d.logger.Printf("[WARN] driver.qemu - %s", err) + } + } + // Add pass through arguments to qemu executable. A user can specify // these arguments in driver task configuration. These arguments are // passed directly to the qemu driver as command line options. @@ -239,7 +288,7 @@ func (d *QemuDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse ) } - d.logger.Printf("[DEBUG] Starting QemuVM command: %q", strings.Join(args, " ")) + d.logger.Printf("[DEBUG] driver.qemu - starting QemuVM command: %q", strings.Join(args, " ")) pluginLogFile := filepath.Join(ctx.TaskDir.Dir, "executor.out") executorConfig := &dstructs.ExecutorConfig{ LogFile: pluginLogFile, @@ -272,7 +321,7 @@ func (d *QemuDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse pluginClient.Kill() return nil, err } - d.logger.Printf("[INFO] Started new QemuVM: %s", vmID) + d.logger.Printf("[INFO] driver.qemu - started new QemuVM: %s", vmID) // Create and Return Handle maxKill := d.DriverContext.config.MaxKillTimeout @@ -282,6 +331,7 @@ func (d *QemuDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse userPid: ps.Pid, killTimeout: GetKillTimeout(task.KillTimeout, maxKill), maxKillTimeout: maxKill, + monitorPath: monitorPath, version: d.config.Version.VersionNumber(), logger: d.logger, doneCh: make(chan struct{}), @@ -317,7 +367,7 @@ func (d *QemuDriver) Open(ctx *ExecContext, handleID string) (DriverHandle, erro exec, pluginClient, err := createExecutorWithConfig(pluginConfig, d.config.LogOutput) if err != nil { - d.logger.Println("[ERR] driver.qemu: error connecting to plugin so destroying plugin pid and user pid") + d.logger.Println("[ERR] driver.qemu - error connecting to plugin so destroying plugin pid and user pid") if e := destroyPlugin(id.PluginConfig.Pid, id.UserPid); e != nil { d.logger.Printf("[ERR] driver.qemu: error destroying plugin and userpid: %v", e) } @@ -381,9 +431,26 @@ func (h *qemuHandle) Signal(s os.Signal) error { return fmt.Errorf("Qemu driver can't send signals") } -// TODO: allow a 'shutdown_command' that can be executed over a ssh connection -// to the VM func (h *qemuHandle) Kill() error { + // If a graceful shutdown was requested at task start time, + // monitorPath will not be empty + if h.monitorPath != "" { + monitorSocket, err := net.Dial("unix", h.monitorPath) + if err == nil { + defer monitorSocket.Close() + h.logger.Printf("[DEBUG] driver.qemu - sending graceful shutdown command to qemu monitor socket at %s", h.monitorPath) + _, err = monitorSocket.Write([]byte(qemuGracefulShutdownMsg)) + if err == nil { + return nil + } + h.logger.Printf("[WARN] driver.qemu - failed to send '%s' to monitor socket '%s': %s", qemuGracefulShutdownMsg, h.monitorPath, err) + } else { + // OK, that didn't work - we'll continue on and + // attempt to kill the process as a last resort + h.logger.Printf("[WARN] driver.qemu - could not connect to qemu monitor at %s: %s", h.monitorPath, err) + } + } + if err := h.executor.ShutDown(); err != nil { if h.pluginClient.Exited() { return nil @@ -395,13 +462,13 @@ func (h *qemuHandle) Kill() error { case <-h.doneCh: return nil case <-time.After(h.killTimeout): + h.logger.Printf("[DEBUG] driver.qemu - kill timeout exceeded") if h.pluginClient.Exited() { return nil } if err := h.executor.Exit(); err != nil { return fmt.Errorf("executor Exit failed: %v", err) } - return nil } } @@ -414,7 +481,7 @@ func (h *qemuHandle) run() { ps, werr := h.executor.Wait() if ps.ExitCode == 0 && werr != nil { if e := killProcess(h.userPid); e != nil { - h.logger.Printf("[ERR] driver.qemu: error killing user process: %v", e) + h.logger.Printf("[ERR] driver.qemu - error killing user process: %v", e) } } close(h.doneCh) diff --git a/client/driver/qemu_test.go b/client/driver/qemu_test.go index 98d491d3d..646fd3d71 100644 --- a/client/driver/qemu_test.go +++ b/client/driver/qemu_test.go @@ -14,6 +14,14 @@ import ( ctestutils "github.com/hashicorp/nomad/client/testutil" ) +func generateString(length int) string { + var newString string + for i := 0; i < length; i++ { + newString = newString + "x" + } + return string(newString) +} + // The fingerprinter test should always pass, even if QEMU is not installed. func TestQemuDriver_Fingerprint(t *testing.T) { if !testutil.IsTravis() { @@ -39,12 +47,15 @@ func TestQemuDriver_Fingerprint(t *testing.T) { if !apply { t.Fatalf("should apply") } - if node.Attributes["driver.qemu"] == "" { + if node.Attributes[qemuDriverAttr] == "" { t.Fatalf("Missing Qemu driver") } - if node.Attributes["driver.qemu.version"] == "" { + if node.Attributes[qemuDriverVersionAttr] == "" { t.Fatalf("Missing Qemu driver version") } + if node.Attributes[qemuDriverLongMonitorPathAttr] == "" { + t.Fatalf("Missing Qemu long monitor socket path support flag") + } } func TestQemuDriver_StartOpen_Wait(t *testing.T) { @@ -56,8 +67,9 @@ func TestQemuDriver_StartOpen_Wait(t *testing.T) { Name: "linux", Driver: "qemu", Config: map[string]interface{}{ - "image_path": "linux-0.2.img", - "accelerator": "tcg", + "image_path": "linux-0.2.img", + "accelerator": "tcg", + "graceful_shutdown": true, "port_map": []map[string]int{{ "main": 22, "web": 8080, @@ -85,10 +97,11 @@ func TestQemuDriver_StartOpen_Wait(t *testing.T) { // Copy the test image into the task's directory dst := ctx.ExecCtx.TaskDir.Dir + copyFile("./test-resources/qemu/linux-0.2.img", filepath.Join(dst, "linux-0.2.img"), t) if _, err := d.Prestart(ctx.ExecCtx, task); err != nil { - t.Fatalf("Prestart faild: %v", err) + t.Fatalf("Prestart failed: %v", err) } resp, err := d.Start(ctx.ExecCtx, task) @@ -121,34 +134,36 @@ func TestQemuDriverUser(t *testing.T) { t.Parallel() } ctestutils.QemuCompatible(t) - tasks := []*structs.Task{{ - Name: "linux", - Driver: "qemu", - User: "alice", - Config: map[string]interface{}{ - "image_path": "linux-0.2.img", - "accelerator": "tcg", - "port_map": []map[string]int{{ - "main": 22, - "web": 8080, - }}, - "args": []string{"-nodefconfig", "-nodefaults"}, - "msg": "unknown user alice", - }, - LogConfig: &structs.LogConfig{ - MaxFiles: 10, - MaxFileSizeMB: 10, - }, - Resources: &structs.Resources{ - CPU: 500, - MemoryMB: 512, - Networks: []*structs.NetworkResource{ - { - ReservedPorts: []structs.Port{{Label: "main", Value: 22000}, {Label: "web", Value: 80}}, + tasks := []*structs.Task{ + { + Name: "linux", + Driver: "qemu", + User: "alice", + Config: map[string]interface{}{ + "image_path": "linux-0.2.img", + "accelerator": "tcg", + "graceful_shutdown": false, + "port_map": []map[string]int{{ + "main": 22, + "web": 8080, + }}, + "args": []string{"-nodefconfig", "-nodefaults"}, + "msg": "unknown user alice", + }, + LogConfig: &structs.LogConfig{ + MaxFiles: 10, + MaxFileSizeMB: 10, + }, + Resources: &structs.Resources{ + CPU: 500, + MemoryMB: 512, + Networks: []*structs.NetworkResource{ + { + ReservedPorts: []structs.Port{{Label: "main", Value: 22000}, {Label: "web", Value: 80}}, + }, }, }, }, - }, { Name: "linux", Driver: "qemu", @@ -193,9 +208,34 @@ func TestQemuDriverUser(t *testing.T) { resp.Handle.Kill() t.Fatalf("Should've failed") } + msg := task.Config["msg"].(string) if !strings.Contains(err.Error(), msg) { t.Fatalf("Expecting '%v' in '%v'", msg, err) } } } + +func TestQemuDriverGetMonitorPath(t *testing.T) { + shortPath := generateString(10) + _, err := getMonitorPath(shortPath, "0") + if err != nil { + t.Fatal("Should not have returned an error") + } + + longPath := generateString(legacyMaxMonitorPathLen + 100) + _, err = getMonitorPath(longPath, "0") + if err == nil { + t.Fatal("Should have returned an error") + } + _, err = getMonitorPath(longPath, "1") + if err != nil { + t.Fatal("Should not have returned an error") + } + + maxLengthPath := generateString(legacyMaxMonitorPathLen) + _, err = getMonitorPath(maxLengthPath, "0") + if err != nil { + t.Fatal("Should not have returned an error") + } +} diff --git a/website/source/docs/drivers/qemu.html.md b/website/source/docs/drivers/qemu.html.md index 14490de06..c26d6e82f 100644 --- a/website/source/docs/drivers/qemu.html.md +++ b/website/source/docs/drivers/qemu.html.md @@ -29,9 +29,10 @@ task "webservice" { driver = "qemu" config { - image_path = "/path/to/my/linux.img" - accelerator = "kvm" - args = ["-nodefaults", "-nodefconfig"] + image_path = "/path/to/my/linux.img" + accelerator = "kvm" + graceful_shutdown = true + args = ["-nodefaults", "-nodefconfig"] } } ``` @@ -47,6 +48,8 @@ The `qemu` driver supports the following configuration in the job spec: If the host machine has `qemu` installed with KVM support, users can specify `kvm` for the `accelerator`. Default is `tcg`. +* `graceful_shutdown` `(bool: false)` - Using the [qemu monitor](https://en.wikibooks.org/wiki/QEMU/Monitor), send an ACPI shutdown signal to virtual machines rather than simply terminating them. This emulates a physical power button press, and gives instances a chance to shut down cleanly. If the VM is still running after ``kill_timeout``, it will be forcefully terminated. (Note that [prior to qemu 2.10.1](https://github.com/qemu/qemu/commit/ad9579aaa16d5b385922d49edac2c96c79bcfb6), the monitor socket path is limited to 108 characters. Graceful shutdown will be disabled if qemu is < 2.10.1 and the generated monitor path exceeds this length. You may encounter this issue if you set long [data_dir](https://www.nomadproject.io/docs/agent/configuration/index.html#data_dir) or [alloc_dir](https://www.nomadproject.io/docs/agent/configuration/client.html#alloc_dir) paths.) + * `port_map` - (Optional) A key-value map of port labels. ```hcl From 4afb9dfa2d191762696d2556e532a57372dda907 Mon Sep 17 00:00:00 2001 From: Matt Mercer Date: Mon, 30 Oct 2017 20:10:18 -0700 Subject: [PATCH 02/12] Standardize driver.qemu logging prefix --- client/driver/qemu.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/client/driver/qemu.go b/client/driver/qemu.go index 17b137257..7212a1e93 100644 --- a/client/driver/qemu.go +++ b/client/driver/qemu.go @@ -161,9 +161,9 @@ func (d *QemuDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, fixedSocketPathLenVer := semver.New("2.10.1") if currentVer.LessThan(*fixedSocketPathLenVer) { node.Attributes[qemuDriverLongMonitorPathAttr] = "0" - d.logger.Printf("[DEBUG] driver.qemu - long socket paths are not available in this version of QEMU") + d.logger.Printf("[DEBUG] driver.qemu: long socket paths are not available in this version of QEMU") } else { - d.logger.Printf("[DEBUG] driver.qemu - long socket paths available in this version of QEMU") + d.logger.Printf("[DEBUG] driver.qemu: long socket paths available in this version of QEMU") node.Attributes[qemuDriverLongMonitorPathAttr] = "1" } return true, nil @@ -232,10 +232,10 @@ func (d *QemuDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse // to perform graceful shutdowns) monitorPath, err = getMonitorPath(ctx.TaskDir.Dir, ctx.TaskEnv.NodeAttrs[qemuDriverLongMonitorPathAttr]) if err == nil { - d.logger.Printf("[DEBUG] driver.qemu - got monitor path OK: %s", monitorPath) + d.logger.Printf("[DEBUG] driver.qemu: got monitor path OK: %s", monitorPath) args = append(args, "-monitor", fmt.Sprintf("unix:%s,server,nowait", monitorPath)) } else { - d.logger.Printf("[WARN] driver.qemu - %s", err) + d.logger.Printf("[WARN] driver.qemu: %s", err) } } @@ -288,7 +288,7 @@ func (d *QemuDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse ) } - d.logger.Printf("[DEBUG] driver.qemu - starting QemuVM command: %q", strings.Join(args, " ")) + d.logger.Printf("[DEBUG] driver.qemu: starting QemuVM command: %q", strings.Join(args, " ")) pluginLogFile := filepath.Join(ctx.TaskDir.Dir, "executor.out") executorConfig := &dstructs.ExecutorConfig{ LogFile: pluginLogFile, @@ -321,7 +321,7 @@ func (d *QemuDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse pluginClient.Kill() return nil, err } - d.logger.Printf("[INFO] driver.qemu - started new QemuVM: %s", vmID) + d.logger.Printf("[INFO] driver.qemu: started new QemuVM: %s", vmID) // Create and Return Handle maxKill := d.DriverContext.config.MaxKillTimeout @@ -367,7 +367,7 @@ func (d *QemuDriver) Open(ctx *ExecContext, handleID string) (DriverHandle, erro exec, pluginClient, err := createExecutorWithConfig(pluginConfig, d.config.LogOutput) if err != nil { - d.logger.Println("[ERR] driver.qemu - error connecting to plugin so destroying plugin pid and user pid") + d.logger.Println("[ERR] driver.qemu: error connecting to plugin so destroying plugin pid and user pid") if e := destroyPlugin(id.PluginConfig.Pid, id.UserPid); e != nil { d.logger.Printf("[ERR] driver.qemu: error destroying plugin and userpid: %v", e) } @@ -438,16 +438,16 @@ func (h *qemuHandle) Kill() error { monitorSocket, err := net.Dial("unix", h.monitorPath) if err == nil { defer monitorSocket.Close() - h.logger.Printf("[DEBUG] driver.qemu - sending graceful shutdown command to qemu monitor socket at %s", h.monitorPath) + h.logger.Printf("[DEBUG] driver.qemu: sending graceful shutdown command to qemu monitor socket at %s", h.monitorPath) _, err = monitorSocket.Write([]byte(qemuGracefulShutdownMsg)) if err == nil { return nil } - h.logger.Printf("[WARN] driver.qemu - failed to send '%s' to monitor socket '%s': %s", qemuGracefulShutdownMsg, h.monitorPath, err) + h.logger.Printf("[WARN] driver.qemu: failed to send '%s' to monitor socket '%s': %s", qemuGracefulShutdownMsg, h.monitorPath, err) } else { // OK, that didn't work - we'll continue on and // attempt to kill the process as a last resort - h.logger.Printf("[WARN] driver.qemu - could not connect to qemu monitor at %s: %s", h.monitorPath, err) + h.logger.Printf("[WARN] driver.qemu: could not connect to qemu monitor at %s: %s", h.monitorPath, err) } } @@ -462,7 +462,7 @@ func (h *qemuHandle) Kill() error { case <-h.doneCh: return nil case <-time.After(h.killTimeout): - h.logger.Printf("[DEBUG] driver.qemu - kill timeout exceeded") + h.logger.Printf("[DEBUG] driver.qemu: kill timeout exceeded") if h.pluginClient.Exited() { return nil } @@ -481,7 +481,7 @@ func (h *qemuHandle) run() { ps, werr := h.executor.Wait() if ps.ExitCode == 0 && werr != nil { if e := killProcess(h.userPid); e != nil { - h.logger.Printf("[ERR] driver.qemu - error killing user process: %v", e) + h.logger.Printf("[ERR] driver.qemu: error killing user process: %v", e) } } close(h.doneCh) From 47c832972e3740bf7be58d1a63ae2fd77cae5bfe Mon Sep 17 00:00:00 2001 From: Matt Mercer Date: Mon, 30 Oct 2017 20:19:25 -0700 Subject: [PATCH 03/12] Wrap qemu docs at 80 characters --- website/source/docs/drivers/qemu.html.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/website/source/docs/drivers/qemu.html.md b/website/source/docs/drivers/qemu.html.md index c26d6e82f..3f78c202e 100644 --- a/website/source/docs/drivers/qemu.html.md +++ b/website/source/docs/drivers/qemu.html.md @@ -48,7 +48,20 @@ The `qemu` driver supports the following configuration in the job spec: If the host machine has `qemu` installed with KVM support, users can specify `kvm` for the `accelerator`. Default is `tcg`. -* `graceful_shutdown` `(bool: false)` - Using the [qemu monitor](https://en.wikibooks.org/wiki/QEMU/Monitor), send an ACPI shutdown signal to virtual machines rather than simply terminating them. This emulates a physical power button press, and gives instances a chance to shut down cleanly. If the VM is still running after ``kill_timeout``, it will be forcefully terminated. (Note that [prior to qemu 2.10.1](https://github.com/qemu/qemu/commit/ad9579aaa16d5b385922d49edac2c96c79bcfb6), the monitor socket path is limited to 108 characters. Graceful shutdown will be disabled if qemu is < 2.10.1 and the generated monitor path exceeds this length. You may encounter this issue if you set long [data_dir](https://www.nomadproject.io/docs/agent/configuration/index.html#data_dir) or [alloc_dir](https://www.nomadproject.io/docs/agent/configuration/client.html#alloc_dir) paths.) +* `graceful_shutdown` `(bool: false)` - Using the [qemu + monitor](https://en.wikibooks.org/wiki/QEMU/Monitor), send an ACPI shutdown + signal to virtual machines rather than simply terminating them. This emulates + a physical power button press, and gives instances a chance to shut down + cleanly. If the VM is still running after ``kill_timeout``, it will be + forcefully terminated. (Note that + [prior to qemu 2.10.1](https://github.com/qemu/qemu/commit/ad9579aaa16d5b385922d49edac2c96c79bcfb6), + the monitor socket path is limited to 108 characters. Graceful shutdown will + be disabled if qemu is < 2.10.1 and the generated monitor path exceeds this + length. You may encounter this issue if you set long + [data_dir](https://www.nomadproject.io/docs/agent/configuration/index.html#data_dir) + or + [alloc_dir](https://www.nomadproject.io/docs/agent/configuration/client.html#alloc_dir) + paths.) * `port_map` - (Optional) A key-value map of port labels. From 46f7e2fa4c7d258224b9379bebc9838f0c06e99b Mon Sep 17 00:00:00 2001 From: Matt Mercer Date: Mon, 30 Oct 2017 20:20:31 -0700 Subject: [PATCH 04/12] Qemu driver: minor logging fixes --- client/driver/qemu.go | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/client/driver/qemu.go b/client/driver/qemu.go index 7212a1e93..e77f68189 100644 --- a/client/driver/qemu.go +++ b/client/driver/qemu.go @@ -147,9 +147,10 @@ func (d *QemuDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, delete(node.Attributes, qemuDriverAttr) return false, fmt.Errorf("Unable to parse Qemu version string: %#v", matches) } + currentQemuVersion := matches[1] node.Attributes[qemuDriverAttr] = "1" - node.Attributes[qemuDriverVersionAttr] = matches[1] + node.Attributes[qemuDriverVersionAttr] = currentQemuVersion // Prior to qemu 2.10.1, monitor socket paths are truncated to 108 bytes. // We should consider this if driver.qemu.version is < 2.10.1 and the @@ -157,13 +158,13 @@ func (d *QemuDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, // // Relevant fix is here: // https://github.com/qemu/qemu/commit/ad9579aaa16d5b385922d49edac2c96c79bcfb6 - currentVer := semver.New(matches[1]) + currentQemuSemver := semver.New(currentQemuVersion) fixedSocketPathLenVer := semver.New("2.10.1") - if currentVer.LessThan(*fixedSocketPathLenVer) { + if currentQemuSemver.LessThan(*fixedSocketPathLenVer) { node.Attributes[qemuDriverLongMonitorPathAttr] = "0" - d.logger.Printf("[DEBUG] driver.qemu: long socket paths are not available in this version of QEMU") + d.logger.Printf("[DEBUG] driver.qemu: long socket paths are not available in this version of QEMU (%s)", currentQemuVersion) } else { - d.logger.Printf("[DEBUG] driver.qemu: long socket paths available in this version of QEMU") + d.logger.Printf("[DEBUG] driver.qemu: long socket paths available in this version of QEMU (%s)", currentQemuVersion) node.Attributes[qemuDriverLongMonitorPathAttr] = "1" } return true, nil @@ -358,7 +359,7 @@ type qemuId struct { func (d *QemuDriver) Open(ctx *ExecContext, handleID string) (DriverHandle, error) { id := &qemuId{} if err := json.Unmarshal([]byte(handleID), id); err != nil { - return nil, fmt.Errorf("Failed to parse handle '%s': %v", handleID, err) + return nil, fmt.Errorf("Failed to parse handle %q: %v", handleID, err) } pluginConfig := &plugin.ClientConfig{ @@ -367,9 +368,9 @@ func (d *QemuDriver) Open(ctx *ExecContext, handleID string) (DriverHandle, erro exec, pluginClient, err := createExecutorWithConfig(pluginConfig, d.config.LogOutput) if err != nil { - d.logger.Println("[ERR] driver.qemu: error connecting to plugin so destroying plugin pid and user pid") + d.logger.Printf("[ERR] driver.qemu: error connecting to plugin so destroying plugin pid %d and user pid %d", id.PluginConfig.Pid, id.UserPid) if e := destroyPlugin(id.PluginConfig.Pid, id.UserPid); e != nil { - d.logger.Printf("[ERR] driver.qemu: error destroying plugin and userpid: %v", e) + d.logger.Printf("[ERR] driver.qemu: error destroying plugin pid %d and userpid %d: %v", id.PluginConfig.Pid, id.UserPid, e) } return nil, fmt.Errorf("error connecting to plugin: %v", err) } @@ -438,16 +439,16 @@ func (h *qemuHandle) Kill() error { monitorSocket, err := net.Dial("unix", h.monitorPath) if err == nil { defer monitorSocket.Close() - h.logger.Printf("[DEBUG] driver.qemu: sending graceful shutdown command to qemu monitor socket at %s", h.monitorPath) + h.logger.Printf("[DEBUG] driver.qemu: sending graceful shutdown command to qemu monitor socket %q", h.monitorPath) _, err = monitorSocket.Write([]byte(qemuGracefulShutdownMsg)) if err == nil { return nil } - h.logger.Printf("[WARN] driver.qemu: failed to send '%s' to monitor socket '%s': %s", qemuGracefulShutdownMsg, h.monitorPath, err) + h.logger.Printf("[WARN] driver.qemu: failed to send %q to monitor socket %q: %s", qemuGracefulShutdownMsg, h.monitorPath, err) } else { // OK, that didn't work - we'll continue on and // attempt to kill the process as a last resort - h.logger.Printf("[WARN] driver.qemu: could not connect to qemu monitor at %s: %s", h.monitorPath, err) + h.logger.Printf("[WARN] driver.qemu: could not connect to qemu monitor %q: %s", h.monitorPath, err) } } From 38d9a391aaa2961a9c7cbe71766087b44bcac8ec Mon Sep 17 00:00:00 2001 From: Matt Mercer Date: Mon, 30 Oct 2017 23:03:36 -0700 Subject: [PATCH 05/12] Qemu driver: ensure proper cleanup of resources --- client/driver/qemu.go | 54 ++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/client/driver/qemu.go b/client/driver/qemu.go index e77f68189..12867cdf4 100644 --- a/client/driver/qemu.go +++ b/client/driver/qemu.go @@ -3,6 +3,7 @@ package driver import ( "context" "encoding/json" + "errors" "fmt" "log" "net" @@ -433,32 +434,24 @@ func (h *qemuHandle) Signal(s os.Signal) error { } func (h *qemuHandle) Kill() error { - // If a graceful shutdown was requested at task start time, - // monitorPath will not be empty - if h.monitorPath != "" { - monitorSocket, err := net.Dial("unix", h.monitorPath) - if err == nil { - defer monitorSocket.Close() - h.logger.Printf("[DEBUG] driver.qemu: sending graceful shutdown command to qemu monitor socket %q", h.monitorPath) - _, err = monitorSocket.Write([]byte(qemuGracefulShutdownMsg)) - if err == nil { + // First, try sending a graceful shutdown command via the qemu monitor + err := h.sendQemuShutdown() + + // If we couldn't send a graceful shutdown via the monitor socket, we'll + // issue an interrupt to the qemu process as a last resort + if err != nil { + if err := h.executor.ShutDown(); err != nil { + if h.pluginClient.Exited() { return nil } - h.logger.Printf("[WARN] driver.qemu: failed to send %q to monitor socket %q: %s", qemuGracefulShutdownMsg, h.monitorPath, err) - } else { - // OK, that didn't work - we'll continue on and - // attempt to kill the process as a last resort - h.logger.Printf("[WARN] driver.qemu: could not connect to qemu monitor %q: %s", h.monitorPath, err) + return fmt.Errorf("executor Shutdown failed: %v", err) } } - if err := h.executor.ShutDown(); err != nil { - if h.pluginClient.Exited() { - return nil - } - return fmt.Errorf("executor Shutdown failed: %v", err) - } - + // At this point, we're waiting for the qemu process to exit. If we've + // attempted a graceful shutdown and the guest shuts down in time, doneChan + // will close and we'll exit without an error. If it takes too long, the + // timer will fire and we'll attempt to kill the process. select { case <-h.doneCh: return nil @@ -495,3 +488,22 @@ func (h *qemuHandle) run() { h.waitCh <- &dstructs.WaitResult{ExitCode: ps.ExitCode, Signal: ps.Signal, Err: werr} close(h.waitCh) } + +func (h *qemuHandle) sendQemuShutdown() error { + var err error + if h.monitorPath == "" { + err = errors.New("monitorPath not set") + } else { + monitorSocket, err := net.Dial("unix", h.monitorPath) + if err == nil { + defer monitorSocket.Close() + h.logger.Printf("[DEBUG] driver.qemu: sending graceful shutdown command to qemu monitor socket %q", h.monitorPath) + _, err = monitorSocket.Write([]byte(qemuGracefulShutdownMsg)) + if err != nil { + h.logger.Printf("[WARN] driver.qemu: failed to send shutdown message %q to monitor socket %q: %s", qemuGracefulShutdownMsg, h.monitorPath, err) + } + } + h.logger.Printf("[WARN] driver.qemu: could not connect to qemu monitor %q: %s", h.monitorPath, err) + } + return err +} From c26013ea0b08c3912867b3ef54dddc5a67eab4eb Mon Sep 17 00:00:00 2001 From: Matt Mercer Date: Wed, 1 Nov 2017 10:41:48 -0700 Subject: [PATCH 06/12] Qemu driver: include PIDs in log output --- client/driver/qemu.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/client/driver/qemu.go b/client/driver/qemu.go index 12867cdf4..e053ec094 100644 --- a/client/driver/qemu.go +++ b/client/driver/qemu.go @@ -456,7 +456,7 @@ func (h *qemuHandle) Kill() error { case <-h.doneCh: return nil case <-time.After(h.killTimeout): - h.logger.Printf("[DEBUG] driver.qemu: kill timeout exceeded") + h.logger.Printf("[DEBUG] driver.qemu: kill timeout exceeded for user process pid %d", h.userPid) if h.pluginClient.Exited() { return nil } @@ -475,7 +475,7 @@ func (h *qemuHandle) run() { ps, werr := h.executor.Wait() if ps.ExitCode == 0 && werr != nil { if e := killProcess(h.userPid); e != nil { - h.logger.Printf("[ERR] driver.qemu: error killing user process: %v", e) + h.logger.Printf("[ERR] driver.qemu: error killing user process pid %d: %v", h.userPid, e) } } close(h.doneCh) @@ -497,13 +497,13 @@ func (h *qemuHandle) sendQemuShutdown() error { monitorSocket, err := net.Dial("unix", h.monitorPath) if err == nil { defer monitorSocket.Close() - h.logger.Printf("[DEBUG] driver.qemu: sending graceful shutdown command to qemu monitor socket %q", h.monitorPath) + h.logger.Printf("[DEBUG] driver.qemu: sending graceful shutdown command to qemu monitor socket %q for user process pid %d", h.monitorPath, h.userPid) _, err = monitorSocket.Write([]byte(qemuGracefulShutdownMsg)) if err != nil { - h.logger.Printf("[WARN] driver.qemu: failed to send shutdown message %q to monitor socket %q: %s", qemuGracefulShutdownMsg, h.monitorPath, err) + h.logger.Printf("[WARN] driver.qemu: failed to send shutdown message %q to monitor socket %q for user process pid %d: %s", qemuGracefulShutdownMsg, h.monitorPath, h.userPid, err) } } - h.logger.Printf("[WARN] driver.qemu: could not connect to qemu monitor %q: %s", h.monitorPath, err) + h.logger.Printf("[WARN] driver.qemu: could not connect to qemu monitor %q for user process pid %d: %s", h.monitorPath, h.userPid, err) } return err } From d51d174fa0fad3a8066282b8c7c379bca8764a58 Mon Sep 17 00:00:00 2001 From: Matt Mercer Date: Wed, 1 Nov 2017 14:47:14 -0700 Subject: [PATCH 07/12] Qemu driver: basic testing of graceful shutdown feature --- client/driver/qemu.go | 28 +++++++----- client/driver/qemu_test.go | 94 +++++++++++++++++++++++++++++++++++++- 2 files changed, 109 insertions(+), 13 deletions(-) diff --git a/client/driver/qemu.go b/client/driver/qemu.go index e053ec094..5ad11b9f2 100644 --- a/client/driver/qemu.go +++ b/client/driver/qemu.go @@ -40,6 +40,7 @@ const ( // Reference: https://en.wikibooks.org/wiki/QEMU/Monitor qemuGracefulShutdownMsg = "system_powerdown\n" legacyMaxMonitorPathLen = 108 + qemuMonitorSocketName = "qemu-monitor.sock" ) // QemuDriver is a driver for running images via Qemu @@ -78,7 +79,7 @@ func getMonitorPath(dir string, longPathSupport string) (string, error) { if len(dir) > legacyMaxMonitorPathLen && longPathSupport != "1" { return "", fmt.Errorf("monitor path is too long") } - return fmt.Sprintf("%s/qemu-monitor.sock", dir), nil + return fmt.Sprintf("%s/%s", dir, qemuMonitorSocketName), nil } // NewQemuDriver is used to create a new exec driver @@ -435,9 +436,9 @@ func (h *qemuHandle) Signal(s os.Signal) error { func (h *qemuHandle) Kill() error { // First, try sending a graceful shutdown command via the qemu monitor - err := h.sendQemuShutdown() + err := sendQemuShutdown(h.logger, h.monitorPath, h.userPid) - // If we couldn't send a graceful shutdown via the monitor socket, we'll + // If we did not send a graceful shutdown via the monitor socket, we'll // issue an interrupt to the qemu process as a last resort if err != nil { if err := h.executor.ShutDown(); err != nil { @@ -448,15 +449,15 @@ func (h *qemuHandle) Kill() error { } } - // At this point, we're waiting for the qemu process to exit. If we've - // attempted a graceful shutdown and the guest shuts down in time, doneChan + // If the qemu process exits before the kill timeout is reached, doneChan // will close and we'll exit without an error. If it takes too long, the // timer will fire and we'll attempt to kill the process. select { case <-h.doneCh: return nil case <-time.After(h.killTimeout): - h.logger.Printf("[DEBUG] driver.qemu: kill timeout exceeded for user process pid %d", h.userPid) + h.logger.Printf("[DEBUG] driver.qemu: kill timeout of %s exceeded for user process pid %d", h.killTimeout.String(), h.userPid) + if h.pluginClient.Exited() { return nil } @@ -489,21 +490,24 @@ func (h *qemuHandle) run() { close(h.waitCh) } -func (h *qemuHandle) sendQemuShutdown() error { +func sendQemuShutdown(logger *log.Logger, monitorPath string, userPid int) error { var err error - if h.monitorPath == "" { + if monitorPath == "" { + logger.Printf("[DEBUG] driver.qemu: monitorPath not set; will not attempt graceful shutdown for user process pid %d", userPid) err = errors.New("monitorPath not set") } else { - monitorSocket, err := net.Dial("unix", h.monitorPath) + var monitorSocket net.Conn + monitorSocket, err = net.Dial("unix", monitorPath) if err == nil { defer monitorSocket.Close() - h.logger.Printf("[DEBUG] driver.qemu: sending graceful shutdown command to qemu monitor socket %q for user process pid %d", h.monitorPath, h.userPid) + logger.Printf("[DEBUG] driver.qemu: sending graceful shutdown command to qemu monitor socket %q for user process pid %d", monitorPath, userPid) _, err = monitorSocket.Write([]byte(qemuGracefulShutdownMsg)) if err != nil { - h.logger.Printf("[WARN] driver.qemu: failed to send shutdown message %q to monitor socket %q for user process pid %d: %s", qemuGracefulShutdownMsg, h.monitorPath, h.userPid, err) + logger.Printf("[WARN] driver.qemu: failed to send shutdown message %q to monitor socket %q for user process pid %d: %s", qemuGracefulShutdownMsg, monitorPath, userPid, err) } + } else { + logger.Printf("[WARN] driver.qemu: could not connect to qemu monitor %q for user process pid %d: %s", monitorPath, userPid, err) } - h.logger.Printf("[WARN] driver.qemu: could not connect to qemu monitor %q for user process pid %d: %s", h.monitorPath, h.userPid, err) } return err } diff --git a/client/driver/qemu_test.go b/client/driver/qemu_test.go index 646fd3d71..f770734c7 100644 --- a/client/driver/qemu_test.go +++ b/client/driver/qemu_test.go @@ -2,10 +2,12 @@ package driver import ( "fmt" + "os" "path/filepath" "strings" "syscall" "testing" + "time" "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/nomad/structs" @@ -69,7 +71,7 @@ func TestQemuDriver_StartOpen_Wait(t *testing.T) { Config: map[string]interface{}{ "image_path": "linux-0.2.img", "accelerator": "tcg", - "graceful_shutdown": true, + "graceful_shutdown": false, "port_map": []map[string]int{{ "main": 22, "web": 8080, @@ -129,6 +131,96 @@ func TestQemuDriver_StartOpen_Wait(t *testing.T) { } } +func TestQemuDriver_GracefulShutdown(t *testing.T) { + if !testutil.IsTravis() { + t.Parallel() + } + ctestutils.QemuCompatible(t) + task := &structs.Task{ + Name: "linux", + Driver: "qemu", + Config: map[string]interface{}{ + "image_path": "linux-0.2.img", + "accelerator": "tcg", + "graceful_shutdown": true, + "port_map": []map[string]int{{ + "main": 22, + "web": 8080, + }}, + "args": []string{"-nodefconfig", "-nodefaults"}, + }, + // With the use of tcg acceleration, it's very unlikely a qemu instance + // will boot (and gracefully halt) in a reasonable amount of time, so + // this timeout is kept low to reduce test execution time + KillTimeout: time.Duration(1 * time.Second), + LogConfig: &structs.LogConfig{ + MaxFiles: 10, + MaxFileSizeMB: 10, + }, + Resources: &structs.Resources{ + CPU: 500, + MemoryMB: 512, + Networks: []*structs.NetworkResource{ + { + ReservedPorts: []structs.Port{{Label: "main", Value: 22000}, {Label: "web", Value: 80}}, + }, + }, + }, + } + + ctx := testDriverContexts(t, task) + defer ctx.AllocDir.Destroy() + d := NewQemuDriver(ctx.DriverCtx) + + dst := ctx.ExecCtx.TaskDir.Dir + + copyFile("./test-resources/qemu/linux-0.2.img", filepath.Join(dst, "linux-0.2.img"), t) + + if _, err := d.Prestart(ctx.ExecCtx, task); err != nil { + t.Fatalf("Prestart failed: %v", err) + } + + resp, err := d.Start(ctx.ExecCtx, task) + if err != nil { + t.Fatalf("err: %v", err) + } + + // The monitor socket will not exist immediately, so we'll wait up to + // 5 seconds for it to become available. + monitorPath := fmt.Sprintf("%s/linux/%s", ctx.AllocDir.AllocDir, qemuMonitorSocketName) + monitorPathExists := false + for i := 0; i < 5; i++ { + if _, err := os.Stat(monitorPath); !os.IsNotExist(err) { + fmt.Printf("Monitor socket exists at %q\n", monitorPath) + monitorPathExists = true + break + } + time.Sleep(1 * time.Second) + } + if monitorPathExists == false { + t.Fatalf("monitor socket did not exist after waiting 5 seconds") + } + + // userPid supplied in sendQemuShutdown calls is bogus (it's used only + // for log output) + if err := sendQemuShutdown(ctx.DriverCtx.logger, "", 0); err == nil { + t.Fatalf("sendQemuShutdown should return an error if monitorPath parameter is empty") + } + + if err := sendQemuShutdown(ctx.DriverCtx.logger, "/path/that/does/not/exist", 0); err == nil { + t.Fatalf("sendQemuShutdown should return an error if file does not exist at monitorPath") + } + + if err := sendQemuShutdown(ctx.DriverCtx.logger, monitorPath, 0); err != nil { + t.Fatalf("unexpected error from sendQemuShutdown: %s", err) + } + + // Clean up + if err := resp.Handle.Kill(); err != nil { + fmt.Printf("\nError killing Qemu test: %s", err) + } +} + func TestQemuDriverUser(t *testing.T) { if !testutil.IsTravis() { t.Parallel() From b1145705d30fcc7f76dccfffd23e4f166931de4e Mon Sep 17 00:00:00 2001 From: Matt Mercer Date: Wed, 1 Nov 2017 15:14:56 -0700 Subject: [PATCH 08/12] Use strings.Replace() instead of custom function --- client/driver/qemu_test.go | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/client/driver/qemu_test.go b/client/driver/qemu_test.go index f770734c7..0a7e86af3 100644 --- a/client/driver/qemu_test.go +++ b/client/driver/qemu_test.go @@ -16,14 +16,6 @@ import ( ctestutils "github.com/hashicorp/nomad/client/testutil" ) -func generateString(length int) string { - var newString string - for i := 0; i < length; i++ { - newString = newString + "x" - } - return string(newString) -} - // The fingerprinter test should always pass, even if QEMU is not installed. func TestQemuDriver_Fingerprint(t *testing.T) { if !testutil.IsTravis() { @@ -309,13 +301,13 @@ func TestQemuDriverUser(t *testing.T) { } func TestQemuDriverGetMonitorPath(t *testing.T) { - shortPath := generateString(10) + shortPath := strings.Repeat("x", 10) _, err := getMonitorPath(shortPath, "0") if err != nil { t.Fatal("Should not have returned an error") } - longPath := generateString(legacyMaxMonitorPathLen + 100) + longPath := strings.Repeat("x", legacyMaxMonitorPathLen+100) _, err = getMonitorPath(longPath, "0") if err == nil { t.Fatal("Should have returned an error") @@ -325,7 +317,7 @@ func TestQemuDriverGetMonitorPath(t *testing.T) { t.Fatal("Should not have returned an error") } - maxLengthPath := generateString(legacyMaxMonitorPathLen) + maxLengthPath := strings.Repeat("x", legacyMaxMonitorPathLen) _, err = getMonitorPath(maxLengthPath, "0") if err != nil { t.Fatal("Should not have returned an error") From 43256af5f32b7a86eff097ab15d148405d3a7f5d Mon Sep 17 00:00:00 2001 From: Matt Mercer Date: Wed, 1 Nov 2017 16:52:44 -0700 Subject: [PATCH 09/12] Qemu driver: clean up test logging; retry integration test for longer --- client/driver/qemu.go | 8 +++----- client/driver/qemu_test.go | 20 ++++++++++++-------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/client/driver/qemu.go b/client/driver/qemu.go index 5ad11b9f2..2cab2bfef 100644 --- a/client/driver/qemu.go +++ b/client/driver/qemu.go @@ -491,13 +491,11 @@ func (h *qemuHandle) run() { } func sendQemuShutdown(logger *log.Logger, monitorPath string, userPid int) error { - var err error if monitorPath == "" { logger.Printf("[DEBUG] driver.qemu: monitorPath not set; will not attempt graceful shutdown for user process pid %d", userPid) - err = errors.New("monitorPath not set") + return errors.New("monitorPath not set") } else { - var monitorSocket net.Conn - monitorSocket, err = net.Dial("unix", monitorPath) + monitorSocket, err := net.Dial("unix", monitorPath) if err == nil { defer monitorSocket.Close() logger.Printf("[DEBUG] driver.qemu: sending graceful shutdown command to qemu monitor socket %q for user process pid %d", monitorPath, userPid) @@ -508,6 +506,6 @@ func sendQemuShutdown(logger *log.Logger, monitorPath string, userPid int) error } else { logger.Printf("[WARN] driver.qemu: could not connect to qemu monitor %q for user process pid %d: %s", monitorPath, userPid, err) } + return err } - return err } diff --git a/client/driver/qemu_test.go b/client/driver/qemu_test.go index 0a7e86af3..9a74ca48b 100644 --- a/client/driver/qemu_test.go +++ b/client/driver/qemu_test.go @@ -53,6 +53,7 @@ func TestQemuDriver_Fingerprint(t *testing.T) { } func TestQemuDriver_StartOpen_Wait(t *testing.T) { + logger := testLogger() if !testutil.IsTravis() { t.Parallel() } @@ -119,11 +120,12 @@ func TestQemuDriver_StartOpen_Wait(t *testing.T) { // Clean up if err := resp.Handle.Kill(); err != nil { - fmt.Printf("\nError killing Qemu test: %s", err) + logger.Printf("Error killing Qemu test: %s", err) } } func TestQemuDriver_GracefulShutdown(t *testing.T) { + logger := testLogger() if !testutil.IsTravis() { t.Parallel() } @@ -181,16 +183,16 @@ func TestQemuDriver_GracefulShutdown(t *testing.T) { // 5 seconds for it to become available. monitorPath := fmt.Sprintf("%s/linux/%s", ctx.AllocDir.AllocDir, qemuMonitorSocketName) monitorPathExists := false - for i := 0; i < 5; i++ { + for i := 0; i < 100; i++ { if _, err := os.Stat(monitorPath); !os.IsNotExist(err) { - fmt.Printf("Monitor socket exists at %q\n", monitorPath) + logger.Printf("monitor socket exists at %q\n", monitorPath) monitorPathExists = true break } - time.Sleep(1 * time.Second) + time.Sleep(200 * time.Millisecond) } if monitorPathExists == false { - t.Fatalf("monitor socket did not exist after waiting 5 seconds") + t.Fatalf("monitor socket did not exist after waiting 20 seconds") } // userPid supplied in sendQemuShutdown calls is bogus (it's used only @@ -208,9 +210,11 @@ func TestQemuDriver_GracefulShutdown(t *testing.T) { } // Clean up - if err := resp.Handle.Kill(); err != nil { - fmt.Printf("\nError killing Qemu test: %s", err) - } + defer func() { + if err := resp.Handle.Kill(); err != nil { + logger.Printf("Error killing Qemu test: %s", err) + } + }() } func TestQemuDriverUser(t *testing.T) { From 00f90323c246ada399b2463ecc67abf02720f8c6 Mon Sep 17 00:00:00 2001 From: Matt Mercer Date: Wed, 1 Nov 2017 17:37:43 -0700 Subject: [PATCH 10/12] Qemu driver: defer cleanup sooner --- client/driver/qemu_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/client/driver/qemu_test.go b/client/driver/qemu_test.go index 9a74ca48b..98a5de2c0 100644 --- a/client/driver/qemu_test.go +++ b/client/driver/qemu_test.go @@ -179,6 +179,13 @@ func TestQemuDriver_GracefulShutdown(t *testing.T) { t.Fatalf("err: %v", err) } + // Clean up + defer func() { + if err := resp.Handle.Kill(); err != nil { + logger.Printf("Error killing Qemu test: %s", err) + } + }() + // The monitor socket will not exist immediately, so we'll wait up to // 5 seconds for it to become available. monitorPath := fmt.Sprintf("%s/linux/%s", ctx.AllocDir.AllocDir, qemuMonitorSocketName) @@ -208,13 +215,6 @@ func TestQemuDriver_GracefulShutdown(t *testing.T) { if err := sendQemuShutdown(ctx.DriverCtx.logger, monitorPath, 0); err != nil { t.Fatalf("unexpected error from sendQemuShutdown: %s", err) } - - // Clean up - defer func() { - if err := resp.Handle.Kill(); err != nil { - logger.Printf("Error killing Qemu test: %s", err) - } - }() } func TestQemuDriverUser(t *testing.T) { From cef9ba9770d9ddcd291f1baadc908f930633c65b Mon Sep 17 00:00:00 2001 From: Matt Mercer Date: Thu, 2 Nov 2017 17:37:49 -0700 Subject: [PATCH 11/12] Qemu driver: tweaks in response to PR feedback Remove attribute for long qemu monitor path; misc cleanup; update tests --- client/driver/qemu.go | 100 +++++++++++++++++-------------- client/driver/qemu_test.go | 120 +++++++++++++++++++++++++++++++++---- 2 files changed, 163 insertions(+), 57 deletions(-) diff --git a/client/driver/qemu.go b/client/driver/qemu.go index 2cab2bfef..f26de4b2a 100644 --- a/client/driver/qemu.go +++ b/client/driver/qemu.go @@ -29,18 +29,26 @@ import ( var ( reQemuVersion = regexp.MustCompile(`version (\d[\.\d+]+)`) + + // Prior to qemu 2.10.1, monitor socket paths are truncated to 108 bytes. + // We should consider this if driver.qemu.version is < 2.10.1 and the + // generated monitor path is too long. + // + // Relevant fix is here: + // https://github.com/qemu/qemu/commit/ad9579aaa16d5b385922d49edac2c96c79bcfb6 + qemuVersionLongSocketPathFix = semver.New("2.10.1") ) const ( // The key populated in Node Attributes to indicate presence of the Qemu driver - qemuDriverAttr = "driver.qemu" - qemuDriverVersionAttr = "driver.qemu.version" - qemuDriverLongMonitorPathAttr = "driver.qemu.longsocketpaths" + qemuDriverAttr = "driver.qemu" + qemuDriverVersionAttr = "driver.qemu.version" // Represents an ACPI shutdown request to the VM (emulates pressing a physical power button) // Reference: https://en.wikibooks.org/wiki/QEMU/Monitor qemuGracefulShutdownMsg = "system_powerdown\n" - legacyMaxMonitorPathLen = 108 qemuMonitorSocketName = "qemu-monitor.sock" + // Maximum socket path length prior to qemu 2.10.1 + qemuLegacyMaxMonitorPathLen = 108 ) // QemuDriver is a driver for running images via Qemu @@ -75,11 +83,27 @@ type qemuHandle struct { doneCh chan struct{} } -func getMonitorPath(dir string, longPathSupport string) (string, error) { - if len(dir) > legacyMaxMonitorPathLen && longPathSupport != "1" { - return "", fmt.Errorf("monitor path is too long") +// getMonitorPath is used to determine whether a qemu monitor socket can be +// safely created and accessed in the task directory by the version of qemu +// present on the host. If it is safe to use, the socket's full path is +// returned along with a nil error. Otherwise, an empty string is returned +// along with a descriptive error. +func (d *QemuDriver) getMonitorPath(dir string) (string, error) { + var longPathSupport bool + currentQemuVer := d.DriverContext.node.Attributes[qemuDriverVersionAttr] + currentQemuSemver := semver.New(currentQemuVer) + if currentQemuSemver.LessThan(*qemuVersionLongSocketPathFix) { + longPathSupport = false + d.logger.Printf("[DEBUG] driver.qemu: long socket paths are not available in this version of QEMU (%s)", currentQemuVer) + } else { + longPathSupport = true + d.logger.Printf("[DEBUG] driver.qemu: long socket paths available in this version of QEMU (%s)", currentQemuVer) } - return fmt.Sprintf("%s/%s", dir, qemuMonitorSocketName), nil + fullSocketPath := fmt.Sprintf("%s/%s", dir, qemuMonitorSocketName) + if len(fullSocketPath) > qemuLegacyMaxMonitorPathLen && longPathSupport == false { + return "", fmt.Errorf("monitor path is too long for this version of qemu") + } + return fullSocketPath, nil } // NewQemuDriver is used to create a new exec driver @@ -154,21 +178,6 @@ func (d *QemuDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, node.Attributes[qemuDriverAttr] = "1" node.Attributes[qemuDriverVersionAttr] = currentQemuVersion - // Prior to qemu 2.10.1, monitor socket paths are truncated to 108 bytes. - // We should consider this if driver.qemu.version is < 2.10.1 and the - // generated monitor path is too long. - // - // Relevant fix is here: - // https://github.com/qemu/qemu/commit/ad9579aaa16d5b385922d49edac2c96c79bcfb6 - currentQemuSemver := semver.New(currentQemuVersion) - fixedSocketPathLenVer := semver.New("2.10.1") - if currentQemuSemver.LessThan(*fixedSocketPathLenVer) { - node.Attributes[qemuDriverLongMonitorPathAttr] = "0" - d.logger.Printf("[DEBUG] driver.qemu: long socket paths are not available in this version of QEMU (%s)", currentQemuVersion) - } else { - d.logger.Printf("[DEBUG] driver.qemu: long socket paths available in this version of QEMU (%s)", currentQemuVersion) - node.Attributes[qemuDriverLongMonitorPathAttr] = "1" - } return true, nil } @@ -233,13 +242,13 @@ func (d *QemuDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse if d.driverConfig.GracefulShutdown { // This socket will be used to manage the virtual machine (for example, // to perform graceful shutdowns) - monitorPath, err = getMonitorPath(ctx.TaskDir.Dir, ctx.TaskEnv.NodeAttrs[qemuDriverLongMonitorPathAttr]) - if err == nil { - d.logger.Printf("[DEBUG] driver.qemu: got monitor path OK: %s", monitorPath) - args = append(args, "-monitor", fmt.Sprintf("unix:%s,server,nowait", monitorPath)) - } else { - d.logger.Printf("[WARN] driver.qemu: %s", err) + monitorPath, err := d.getMonitorPath(ctx.TaskDir.Dir) + if err != nil { + d.logger.Printf("[ERR] driver.qemu: could not get qemu monitor path - error: %s", err) + return nil, err } + d.logger.Printf("[DEBUG] driver.qemu: got monitor path OK: %s", monitorPath) + args = append(args, "-monitor", fmt.Sprintf("unix:%s,server,nowait", monitorPath)) } // Add pass through arguments to qemu executable. A user can specify @@ -436,11 +445,9 @@ func (h *qemuHandle) Signal(s os.Signal) error { func (h *qemuHandle) Kill() error { // First, try sending a graceful shutdown command via the qemu monitor - err := sendQemuShutdown(h.logger, h.monitorPath, h.userPid) - - // If we did not send a graceful shutdown via the monitor socket, we'll - // issue an interrupt to the qemu process as a last resort - if err != nil { + if err := sendQemuShutdown(h.logger, h.monitorPath, h.userPid); err != nil { + h.logger.Printf("[DEBUG] driver.qemu: error sending graceful shutdown for user process pid %d: %s", h.userPid, err) + // Issue an interrupt to the qemu process as a last resort if err := h.executor.ShutDown(); err != nil { if h.pluginClient.Exited() { return nil @@ -490,22 +497,23 @@ func (h *qemuHandle) run() { close(h.waitCh) } +// sendQemuShutdown attempts to issue an ACPI power-off command via the qemu +// monitor func sendQemuShutdown(logger *log.Logger, monitorPath string, userPid int) error { if monitorPath == "" { logger.Printf("[DEBUG] driver.qemu: monitorPath not set; will not attempt graceful shutdown for user process pid %d", userPid) return errors.New("monitorPath not set") - } else { - monitorSocket, err := net.Dial("unix", monitorPath) - if err == nil { - defer monitorSocket.Close() - logger.Printf("[DEBUG] driver.qemu: sending graceful shutdown command to qemu monitor socket %q for user process pid %d", monitorPath, userPid) - _, err = monitorSocket.Write([]byte(qemuGracefulShutdownMsg)) - if err != nil { - logger.Printf("[WARN] driver.qemu: failed to send shutdown message %q to monitor socket %q for user process pid %d: %s", qemuGracefulShutdownMsg, monitorPath, userPid, err) - } - } else { - logger.Printf("[WARN] driver.qemu: could not connect to qemu monitor %q for user process pid %d: %s", monitorPath, userPid, err) - } + } + monitorSocket, err := net.Dial("unix", monitorPath) + if err != nil { + logger.Printf("[WARN] driver.qemu: could not connect to qemu monitor %q for user process pid %d: %s", monitorPath, userPid, err) return err } + defer monitorSocket.Close() + logger.Printf("[DEBUG] driver.qemu: sending graceful shutdown command to qemu monitor socket %q for user process pid %d", monitorPath, userPid) + _, err = monitorSocket.Write([]byte(qemuGracefulShutdownMsg)) + if err != nil { + logger.Printf("[WARN] driver.qemu: failed to send shutdown message %q to monitor socket %q for user process pid %d: %s", qemuGracefulShutdownMsg, monitorPath, userPid, err) + } + return err } diff --git a/client/driver/qemu_test.go b/client/driver/qemu_test.go index 98a5de2c0..b0978ec9c 100644 --- a/client/driver/qemu_test.go +++ b/client/driver/qemu_test.go @@ -47,9 +47,6 @@ func TestQemuDriver_Fingerprint(t *testing.T) { if node.Attributes[qemuDriverVersionAttr] == "" { t.Fatalf("Missing Qemu driver version") } - if node.Attributes[qemuDriverLongMonitorPathAttr] == "" { - t.Fatalf("Missing Qemu long monitor socket path support flag") - } } func TestQemuDriver_StartOpen_Wait(t *testing.T) { @@ -145,7 +142,7 @@ func TestQemuDriver_GracefulShutdown(t *testing.T) { }, // With the use of tcg acceleration, it's very unlikely a qemu instance // will boot (and gracefully halt) in a reasonable amount of time, so - // this timeout is kept low to reduce test execution time + // this timeout is kept low to reduce test execution time. KillTimeout: time.Duration(1 * time.Second), LogConfig: &structs.LogConfig{ MaxFiles: 10, @@ -166,6 +163,14 @@ func TestQemuDriver_GracefulShutdown(t *testing.T) { defer ctx.AllocDir.Destroy() d := NewQemuDriver(ctx.DriverCtx) + apply, err := d.Fingerprint(&config.Config{}, ctx.DriverCtx.node) + if err != nil { + t.Fatalf("err: %v", err) + } + if !apply { + t.Fatalf("should apply") + } + dst := ctx.ExecCtx.TaskDir.Dir copyFile("./test-resources/qemu/linux-0.2.img", filepath.Join(dst, "linux-0.2.img"), t) @@ -304,25 +309,118 @@ func TestQemuDriverUser(t *testing.T) { } } -func TestQemuDriverGetMonitorPath(t *testing.T) { +func TestQemuDriverGetMonitorPathOldQemu(t *testing.T) { + task := &structs.Task{ + Name: "linux", + Driver: "qemu", + Config: map[string]interface{}{ + "image_path": "linux-0.2.img", + "accelerator": "tcg", + "graceful_shutdown": true, + "port_map": []map[string]int{{ + "main": 22, + "web": 8080, + }}, + "args": []string{"-nodefconfig", "-nodefaults"}, + }, + KillTimeout: time.Duration(1 * time.Second), + LogConfig: &structs.LogConfig{ + MaxFiles: 10, + MaxFileSizeMB: 10, + }, + Resources: &structs.Resources{ + CPU: 500, + MemoryMB: 512, + Networks: []*structs.NetworkResource{ + { + ReservedPorts: []structs.Port{{Label: "main", Value: 22000}, {Label: "web", Value: 80}}, + }, + }, + }, + } + + ctx := testDriverContexts(t, task) + defer ctx.AllocDir.Destroy() + + // Simulate an older version of qemu which does not support long monitor socket paths + ctx.DriverCtx.node.Attributes[qemuDriverVersionAttr] = "2.0.0" + + d := &QemuDriver{DriverContext: *ctx.DriverCtx} + shortPath := strings.Repeat("x", 10) - _, err := getMonitorPath(shortPath, "0") + _, err := d.getMonitorPath(shortPath) if err != nil { t.Fatal("Should not have returned an error") } - longPath := strings.Repeat("x", legacyMaxMonitorPathLen+100) - _, err = getMonitorPath(longPath, "0") + longPath := strings.Repeat("x", qemuLegacyMaxMonitorPathLen+100) + _, err = d.getMonitorPath(longPath) if err == nil { t.Fatal("Should have returned an error") } - _, err = getMonitorPath(longPath, "1") + + // Max length includes the '/' separator and socket name + maxLengthCount := qemuLegacyMaxMonitorPathLen - len(qemuMonitorSocketName) - 1 + maxLengthLegacyPath := strings.Repeat("x", maxLengthCount) + _, err = d.getMonitorPath(maxLengthLegacyPath) + if err != nil { + t.Fatalf("Should not have returned an error: %s", err) + } +} + +func TestQemuDriverGetMonitorPathNewQemu(t *testing.T) { + task := &structs.Task{ + Name: "linux", + Driver: "qemu", + Config: map[string]interface{}{ + "image_path": "linux-0.2.img", + "accelerator": "tcg", + "graceful_shutdown": true, + "port_map": []map[string]int{{ + "main": 22, + "web": 8080, + }}, + "args": []string{"-nodefconfig", "-nodefaults"}, + }, + KillTimeout: time.Duration(1 * time.Second), + LogConfig: &structs.LogConfig{ + MaxFiles: 10, + MaxFileSizeMB: 10, + }, + Resources: &structs.Resources{ + CPU: 500, + MemoryMB: 512, + Networks: []*structs.NetworkResource{ + { + ReservedPorts: []structs.Port{{Label: "main", Value: 22000}, {Label: "web", Value: 80}}, + }, + }, + }, + } + + ctx := testDriverContexts(t, task) + defer ctx.AllocDir.Destroy() + + // Simulate a version of qemu which supports long monitor socket paths + ctx.DriverCtx.node.Attributes[qemuDriverVersionAttr] = "2.99.99" + + d := &QemuDriver{DriverContext: *ctx.DriverCtx} + + shortPath := strings.Repeat("x", 10) + _, err := d.getMonitorPath(shortPath) if err != nil { t.Fatal("Should not have returned an error") } - maxLengthPath := strings.Repeat("x", legacyMaxMonitorPathLen) - _, err = getMonitorPath(maxLengthPath, "0") + longPath := strings.Repeat("x", qemuLegacyMaxMonitorPathLen+100) + _, err = d.getMonitorPath(longPath) + if err != nil { + t.Fatal("Should not have returned an error") + } + + maxLengthCount := qemuLegacyMaxMonitorPathLen - len(qemuMonitorSocketName) - 1 + maxLengthLegacyPath := strings.Repeat("x", maxLengthCount) + _, err = d.getMonitorPath(maxLengthLegacyPath) if err != nil { t.Fatal("Should not have returned an error") } From 11e28708751291d96c50b2df3ca9c6d2c0234705 Mon Sep 17 00:00:00 2001 From: Matt Mercer Date: Fri, 3 Nov 2017 14:45:09 -0700 Subject: [PATCH 12/12] Qemu driver: clean up logging; fail unsupported features on Windows --- client/driver/qemu.go | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/client/driver/qemu.go b/client/driver/qemu.go index f26de4b2a..f256c829c 100644 --- a/client/driver/qemu.go +++ b/client/driver/qemu.go @@ -33,6 +33,7 @@ var ( // Prior to qemu 2.10.1, monitor socket paths are truncated to 108 bytes. // We should consider this if driver.qemu.version is < 2.10.1 and the // generated monitor path is too long. + // // Relevant fix is here: // https://github.com/qemu/qemu/commit/ad9579aaa16d5b385922d49edac2c96c79bcfb6 @@ -240,11 +241,14 @@ func (d *QemuDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse var monitorPath string if d.driverConfig.GracefulShutdown { + if runtime.GOOS == "windows" { + return nil, errors.New("QEMU graceful shutdown is unsupported on the Windows platform") + } // This socket will be used to manage the virtual machine (for example, // to perform graceful shutdowns) monitorPath, err := d.getMonitorPath(ctx.TaskDir.Dir) if err != nil { - d.logger.Printf("[ERR] driver.qemu: could not get qemu monitor path - error: %s", err) + d.logger.Printf("[ERR] driver.qemu: could not get qemu monitor path: %s", err) return nil, err } d.logger.Printf("[DEBUG] driver.qemu: got monitor path OK: %s", monitorPath) @@ -292,6 +296,9 @@ func (d *QemuDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse // If using KVM, add optimization args if accelerator == "kvm" { + if runtime.GOOS == "windows" { + return nil, errors.New("KVM accelerator is unsupported on the Windows platform") + } args = append(args, "-enable-kvm", "-cpu", "host", @@ -444,10 +451,19 @@ func (h *qemuHandle) Signal(s os.Signal) error { } func (h *qemuHandle) Kill() error { - // First, try sending a graceful shutdown command via the qemu monitor - if err := sendQemuShutdown(h.logger, h.monitorPath, h.userPid); err != nil { - h.logger.Printf("[DEBUG] driver.qemu: error sending graceful shutdown for user process pid %d: %s", h.userPid, err) - // Issue an interrupt to the qemu process as a last resort + gracefulShutdownSent := false + // Attempt a graceful shutdown only if it was configured in the job + if h.monitorPath != "" { + if err := sendQemuShutdown(h.logger, h.monitorPath, h.userPid); err == nil { + gracefulShutdownSent = true + } else { + h.logger.Printf("[DEBUG] driver.qemu: error sending graceful shutdown for user process pid %d: %s", h.userPid, err) + } + } + + // If Nomad did not send a graceful shutdown signal, issue an interrupt to + // the qemu process as a last resort + if gracefulShutdownSent == false { if err := h.executor.ShutDown(); err != nil { if h.pluginClient.Exited() { return nil @@ -501,7 +517,6 @@ func (h *qemuHandle) run() { // monitor func sendQemuShutdown(logger *log.Logger, monitorPath string, userPid int) error { if monitorPath == "" { - logger.Printf("[DEBUG] driver.qemu: monitorPath not set; will not attempt graceful shutdown for user process pid %d", userPid) return errors.New("monitorPath not set") } monitorSocket, err := net.Dial("unix", monitorPath)