From 6f3210e3d786f0bcf8230c877a924c273df7aea5 Mon Sep 17 00:00:00 2001 From: Abhishek Chanda Date: Mon, 21 Dec 2015 06:06:45 +0000 Subject: [PATCH 1/5] Support CPU and meory isolators for the rkt driver The rkt community added supprt for these isolators recently --- client/driver/rkt.go | 13 +++++++++++++ client/driver/rkt_test.go | 16 ++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/client/driver/rkt.go b/client/driver/rkt.go index 1d90ba71e45..403cfb53764 100644 --- a/client/driver/rkt.go +++ b/client/driver/rkt.go @@ -149,6 +149,19 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e cmd_args = append(cmd_args, fmt.Sprintf("--exec=%v", exec_cmd)) } + if task.Resources.MemoryMB == 0 { + return nil, fmt.Errorf("Memory limit cannot be zero") + } + if task.Resources.CPU == 0 { + return nil, fmt.Errorf("CPU limit cannot be zero") + } + + // Add memory isolator + cmd_args = append(cmd_args, fmt.Sprintf("--memory=%vM", int64(task.Resources.MemoryMB) * 1024 * 1024)) + + // Add CPU isolator + cmd_args = append(cmd_args, fmt.Sprintf("--cpu=%vm", int64(task.Resources.CPU))) + // Add user passed arguments. if len(driverConfig.Args) != 0 { parsed := args.ParseAndReplace(driverConfig.Args, envVars.Map()) diff --git a/client/driver/rkt_test.go b/client/driver/rkt_test.go index 15db27b2754..d63ca2d9114 100644 --- a/client/driver/rkt_test.go +++ b/client/driver/rkt_test.go @@ -81,6 +81,10 @@ func TestRktDriver_Start(t *testing.T) { "image": "coreos.com/etcd:v2.0.4", "command": "/etcd", }, + Resources: &structs.Resources{ + MemoryMB: 256, + CPU: 512, + }, } driverCtx := testDriverContext(task.Name) @@ -121,6 +125,10 @@ func TestRktDriver_Start_Wait(t *testing.T) { "command": "/etcd", "args": []string{"--version"}, }, + Resources: &structs.Resources{ + MemoryMB: 256, + CPU: 512, + }, } driverCtx := testDriverContext(task.Name) @@ -162,6 +170,10 @@ func TestRktDriver_Start_Wait_Skip_Trust(t *testing.T) { "command": "/etcd", "args": []string{"--version"}, }, + Resources: &structs.Resources{ + MemoryMB: 256, + CPU: 512, + }, } driverCtx := testDriverContext(task.Name) @@ -204,6 +216,10 @@ func TestRktDriver_Start_Wait_Logs(t *testing.T) { "command": "/etcd", "args": []string{"--version"}, }, + Resources: &structs.Resources{ + MemoryMB: 256, + CPU: 512, + }, } driverCtx := testDriverContext(task.Name) From 19bcf4fce3bf905d306532e20cb5b4b70b6ab17c Mon Sep 17 00:00:00 2001 From: Abhishek Chanda Date: Mon, 21 Dec 2015 06:09:11 +0000 Subject: [PATCH 2/5] Run gofmt --- client/driver/rkt.go | 12 ++++++------ client/driver/rkt_test.go | 24 ++++++++++++------------ 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/client/driver/rkt.go b/client/driver/rkt.go index 403cfb53764..b205a480f38 100644 --- a/client/driver/rkt.go +++ b/client/driver/rkt.go @@ -150,14 +150,14 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e } if task.Resources.MemoryMB == 0 { - return nil, fmt.Errorf("Memory limit cannot be zero") - } - if task.Resources.CPU == 0 { - return nil, fmt.Errorf("CPU limit cannot be zero") - } + return nil, fmt.Errorf("Memory limit cannot be zero") + } + if task.Resources.CPU == 0 { + return nil, fmt.Errorf("CPU limit cannot be zero") + } // Add memory isolator - cmd_args = append(cmd_args, fmt.Sprintf("--memory=%vM", int64(task.Resources.MemoryMB) * 1024 * 1024)) + cmd_args = append(cmd_args, fmt.Sprintf("--memory=%vM", int64(task.Resources.MemoryMB)*1024*1024)) // Add CPU isolator cmd_args = append(cmd_args, fmt.Sprintf("--cpu=%vm", int64(task.Resources.CPU))) diff --git a/client/driver/rkt_test.go b/client/driver/rkt_test.go index d63ca2d9114..8ee4425c212 100644 --- a/client/driver/rkt_test.go +++ b/client/driver/rkt_test.go @@ -125,10 +125,10 @@ func TestRktDriver_Start_Wait(t *testing.T) { "command": "/etcd", "args": []string{"--version"}, }, - Resources: &structs.Resources{ - MemoryMB: 256, - CPU: 512, - }, + Resources: &structs.Resources{ + MemoryMB: 256, + CPU: 512, + }, } driverCtx := testDriverContext(task.Name) @@ -170,10 +170,10 @@ func TestRktDriver_Start_Wait_Skip_Trust(t *testing.T) { "command": "/etcd", "args": []string{"--version"}, }, - Resources: &structs.Resources{ - MemoryMB: 256, - CPU: 512, - }, + Resources: &structs.Resources{ + MemoryMB: 256, + CPU: 512, + }, } driverCtx := testDriverContext(task.Name) @@ -216,10 +216,10 @@ func TestRktDriver_Start_Wait_Logs(t *testing.T) { "command": "/etcd", "args": []string{"--version"}, }, - Resources: &structs.Resources{ - MemoryMB: 256, - CPU: 512, - }, + Resources: &structs.Resources{ + MemoryMB: 256, + CPU: 512, + }, } driverCtx := testDriverContext(task.Name) From c3c8146042e11c31072053cd57d1fd3a2519c46e Mon Sep 17 00:00:00 2001 From: Abhishek Chanda Date: Mon, 21 Dec 2015 10:10:37 +0000 Subject: [PATCH 3/5] Use camelCase for variable names --- client/driver/rkt.go | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/client/driver/rkt.go b/client/driver/rkt.go index b205a480f38..c5e1f76dafb 100644 --- a/client/driver/rkt.go +++ b/client/driver/rkt.go @@ -109,21 +109,21 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e taskLocal := filepath.Join(taskDir, allocdir.TaskLocal) // Add the given trust prefix - trust_prefix, trust_cmd := task.Config["trust_prefix"] - if trust_cmd { + trustPrefix, trustCmd := task.Config["trust_prefix"] + if trustCmd { var outBuf, errBuf bytes.Buffer - cmd := exec.Command("rkt", "trust", fmt.Sprintf("--prefix=%s", trust_prefix)) + cmd := exec.Command("rkt", "trust", fmt.Sprintf("--prefix=%s", trustPrefix)) cmd.Stdout = &outBuf cmd.Stderr = &errBuf if err := cmd.Run(); err != nil { return nil, fmt.Errorf("Error running rkt trust: %s\n\nOutput: %s\n\nError: %s", err, outBuf.String(), errBuf.String()) } - d.logger.Printf("[DEBUG] driver.rkt: added trust prefix: %q", trust_prefix) + d.logger.Printf("[DEBUG] driver.rkt: added trust prefix: %q", trustPrefix) } // Build the command. - var cmd_args []string + var cmdArgs []string // Inject the environment variables. envVars := TaskEnvironmentVariables(ctx, task) @@ -133,20 +133,20 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e envVars.ClearAllocDir() for k, v := range envVars.Map() { - cmd_args = append(cmd_args, fmt.Sprintf("--set-env=%v=%v", k, v)) + cmdArgs = append(cmdArgs, fmt.Sprintf("--set-env=%v=%v", k, v)) } // Disble signature verification if the trust command was not run. - if !trust_cmd { - cmd_args = append(cmd_args, "--insecure-skip-verify") + if !trustCmd { + cmdArgs = append(cmdArgs, "--insecure-skip-verify") } // Append the run command. - cmd_args = append(cmd_args, "run", "--mds-register=false", img) + cmdArgs = append(cmdArgs, "run", "--mds-register=false", img) // Check if the user has overriden the exec command. - if exec_cmd, ok := task.Config["command"]; ok { - cmd_args = append(cmd_args, fmt.Sprintf("--exec=%v", exec_cmd)) + if execCmd, ok := task.Config["command"]; ok { + cmdArgs = append(cmdArgs, fmt.Sprintf("--exec=%v", execCmd)) } if task.Resources.MemoryMB == 0 { @@ -157,10 +157,10 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e } // Add memory isolator - cmd_args = append(cmd_args, fmt.Sprintf("--memory=%vM", int64(task.Resources.MemoryMB)*1024*1024)) + cmdArgs = append(cmdArgs, fmt.Sprintf("--memory=%vM", int64(task.Resources.MemoryMB)*1024*1024)) // Add CPU isolator - cmd_args = append(cmd_args, fmt.Sprintf("--cpu=%vm", int64(task.Resources.CPU))) + cmdArgs = append(cmdArgs, fmt.Sprintf("--cpu=%vm", int64(task.Resources.CPU))) // Add user passed arguments. if len(driverConfig.Args) != 0 { @@ -168,11 +168,11 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e // Need to start arguments with "--" if len(parsed) > 0 { - cmd_args = append(cmd_args, "--") + cmdArgs = append(cmdArgs, "--") } for _, arg := range parsed { - cmd_args = append(cmd_args, fmt.Sprintf("%v", arg)) + cmdArgs = append(cmdArgs, fmt.Sprintf("%v", arg)) } } @@ -190,7 +190,7 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e return nil, fmt.Errorf("Error opening file to redirect stderr: %v", err) } - cmd := exec.Command("rkt", cmd_args...) + cmd := exec.Command("rkt", cmdArgs...) cmd.Stdout = stdo cmd.Stderr = stde From f5e2ac06aa7033f25a7a2faff91a2a120ed75c80 Mon Sep 17 00:00:00 2001 From: Abhishek Chanda Date: Mon, 21 Dec 2015 17:48:21 +0000 Subject: [PATCH 4/5] Do not allow rkt version less than 0.14.0 --- client/driver/rkt.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/client/driver/rkt.go b/client/driver/rkt.go index c5e1f76dafb..871b20bda7a 100644 --- a/client/driver/rkt.go +++ b/client/driver/rkt.go @@ -14,6 +14,7 @@ import ( "syscall" "time" + "github.com/hashicorp/go-version" "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/config" cstructs "github.com/hashicorp/nomad/client/driver/structs" @@ -85,6 +86,13 @@ func (d *RktDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, e node.Attributes["driver.rkt.version"] = rktMatches[1] node.Attributes["driver.rkt.appc.version"] = appcMatches[1] + minVersion, _ := version.NewVersion("0.14.0") + currentVersion, _ := version.NewVersion(node.Attributes["driver.rkt.version"]) + if currentVersion.LessThan(minVersion) { + // Do not allow rkt < 0.14.0 + d.logger.Printf("[WARN] driver.rkt: please upgrade rkt to a version >= %s", minVersion) + node.Attributes["driver.rkt"] = "0" + } return true, nil } From 84acb2590b30b2988f2ff66bcd32a651a30385f3 Mon Sep 17 00:00:00 2001 From: Abhishek Chanda Date: Tue, 22 Dec 2015 05:15:37 +0000 Subject: [PATCH 5/5] Move constants to the top --- client/driver/rkt.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/client/driver/rkt.go b/client/driver/rkt.go index 871b20bda7a..90e33cbc492 100644 --- a/client/driver/rkt.go +++ b/client/driver/rkt.go @@ -29,6 +29,13 @@ var ( reAppcVersion = regexp.MustCompile(`appc version (\d[.\d]+)`) ) +const ( + // rkt added support for CPU and memory isolators in 0.14.0. We cannot support + // an earlier version to maintain an uniform interface across all drivers + minRktVersion = "0.14.0" + conversionFactor = 1024 * 1024 +) + // RktDriver is a driver for running images via Rkt // We attempt to chose sane defaults for now, with more configuration available // planned in the future @@ -86,7 +93,7 @@ func (d *RktDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, e node.Attributes["driver.rkt.version"] = rktMatches[1] node.Attributes["driver.rkt.appc.version"] = appcMatches[1] - minVersion, _ := version.NewVersion("0.14.0") + minVersion, _ := version.NewVersion(minRktVersion) currentVersion, _ := version.NewVersion(node.Attributes["driver.rkt.version"]) if currentVersion.LessThan(minVersion) { // Do not allow rkt < 0.14.0 @@ -165,7 +172,7 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e } // Add memory isolator - cmdArgs = append(cmdArgs, fmt.Sprintf("--memory=%vM", int64(task.Resources.MemoryMB)*1024*1024)) + cmdArgs = append(cmdArgs, fmt.Sprintf("--memory=%vM", int64(task.Resources.MemoryMB)*conversionFactor)) // Add CPU isolator cmdArgs = append(cmdArgs, fmt.Sprintf("--cpu=%vm", int64(task.Resources.CPU)))