Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for CPU and memory isolators #610

Merged
merged 5 commits into from
Dec 22, 2015
Merged

Add support for CPU and memory isolators #610

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6f3210e
Support CPU and meory isolators for the rkt driver
achanda Dec 21, 2015
19bcf4f
Run gofmt
achanda Dec 21, 2015
c3c8146
Use camelCase for variable names
achanda Dec 21, 2015
f5e2ac0
Do not allow rkt version less than 0.14.0
achanda Dec 21, 2015
84acb25
Move constants to the top
achanda Dec 22, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 35 additions & 14 deletions client/driver/rkt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also put this in a const/variable at the top and put a comment on why we only support 0.14.0 and above.

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
}

Expand All @@ -109,21 +117,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)
Expand All @@ -133,33 +141,46 @@ 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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do you say "infinite" or "don't care at all" ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jippi Drivers that support resource control, shouldn't allow infinite resources in my opinion since from the point of view of the scheduler finite resources were being assigned to a task.

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
cmdArgs = append(cmdArgs, fmt.Sprintf("--memory=%vM", int64(task.Resources.MemoryMB)*1024*1024))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC this was not previously supported in rkt. Does this require a particular version of rkt? If so we should check in the fingerprinter that we have a version that supports this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this was added in 0.14.0, rkt/rkt#1851
I assumed we were targeting to converge after post 1.0, so that it's okay to not be backwards compatible for now. If we think the check is necessary, I can add it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine not to be BC here but we should still check that the version is supported so we can tell the user to upgrade instead of failing later when a task is started.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cbednarski @achanda I almost think we should not make the rkt driver available if the version is less than 0.14

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@diptanu I agree, in favor of uniformity across all the drivers.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move 1024*1024 to a constant at the top of the file


// Add CPU isolator
cmdArgs = append(cmdArgs, fmt.Sprintf("--cpu=%vm", int64(task.Resources.CPU)))

// Add user passed arguments.
if len(driverConfig.Args) != 0 {
parsed := args.ParseAndReplace(driverConfig.Args, envVars.Map())

// 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))
}
}

Expand All @@ -177,7 +198,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

Expand Down
16 changes: 16 additions & 0 deletions client/driver/rkt_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down