Skip to content
This repository has been archived by the owner on Jan 10, 2023. It is now read-only.

Commit

Permalink
Fix CPU bursting settings
Browse files Browse the repository at this point in the history
  • Loading branch information
sargun committed Jun 25, 2018
1 parent 463e779 commit 1f6caa5
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 20 deletions.
29 changes: 9 additions & 20 deletions executor/runtime/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
runtimeTypes "github.com/Netflix/titus-executor/executor/runtime/types"
"github.com/Netflix/titus-executor/nvidia"
vpcTypes "github.com/Netflix/titus-executor/vpc/types"
"github.com/aws/aws-sdk-go/aws/arn"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/events"
Expand Down Expand Up @@ -192,11 +191,6 @@ var (
envFileTemplate = template.Must(template.New("").Parse(envFileTemplateStr))
)

// ErrMissingIAMRole indicates that the Titus job was submitted without an IAM role
// This is a transition because previously the protobuf had this marked as an optional field
// and it's a temporary measure during protocol evolution.
var ErrMissingIAMRole = errors.New("IAM Role Missing")

// NoEntrypointError indicates that the Titus job does not have an entrypoint, or command
var NoEntrypointError = &runtimeTypes.BadEntryPointError{Reason: errors.New("Image, and job have no entrypoint, or command")}

Expand Down Expand Up @@ -335,7 +329,7 @@ func setupLoggingInfra(dockerRuntime *DockerRuntime) error {
}

func maybeSetCFSBandwidth(cfsBandwidthMode string, cfsBandwidthPeriod uint64, c *runtimeTypes.Container, hostCfg *container.HostConfig) {
cpuBurst := c.TitusInfo.GetAllowNetworkBursting()
cpuBurst := c.TitusInfo.GetAllowCpuBursting()
logEntry := log.WithField("taskID", c.TaskID).WithField("bandwidthMode", cfsBandwidthMode).WithField("cpuBurst", cpuBurst)

if cpuBurst {
Expand All @@ -344,6 +338,11 @@ func maybeSetCFSBandwidth(cfsBandwidthMode string, cfsBandwidthPeriod uint64, c
return
}

if c.Resources.CPU == 0 {
logEntry.WithField("shares", 1).Info("Setting shares as this is an opportunistic workload")
hostCfg.CPUShares = 1
}

switch cfsBandwidthMode {
case bandwidthMode:
setCFSBandwidth(logEntry, cfsBandwidthPeriod, c, hostCfg)
Expand Down Expand Up @@ -398,17 +397,11 @@ func (r *DockerRuntime) dockerConfig(c *runtimeTypes.Container, binds []string,
return nil, nil, err
}

if c.TitusInfo.IamProfile == nil || c.TitusInfo.GetIamProfile() == "" {
return nil, nil, ErrMissingIAMRole
}

_, err = arn.Parse(c.TitusInfo.GetIamProfile())
c.Env["TITUS_IAM_ROLE"], err = c.GetIamProfile()
if err != nil {
return nil, nil, err
}

c.Env["TITUS_IAM_ROLE"] = c.TitusInfo.GetIamProfile()

hostname := strings.ToLower(c.TaskID)
containerCfg := &container.Config{
Image: c.QualifiedImageName(),
Expand Down Expand Up @@ -496,12 +489,8 @@ func (r *DockerRuntime) dockerConfig(c *runtimeTypes.Container, binds []string,
hostCfg.NetworkMode = container.NetworkMode("none")
}

if c.TitusInfo.GetBatch() {
if c.TitusInfo.GetPassthroughAttributes()["titusParameter.agent.batchPriority"] == "idle" {
c.Env["TITUS_BATCH"] = "idle"
} else {
c.Env["TITUS_BATCH"] = "true"
}
if batch := c.GetBatch(); batch != nil {
c.Env["TITUS_BATCH"] = *batch
}

// This must got after all setup
Expand Down
41 changes: 41 additions & 0 deletions executor/runtime/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,18 @@ import (
"github.com/Netflix/titus-executor/executor/metatron"
vpcTypes "github.com/Netflix/titus-executor/vpc/types"
// The purpose of this is to tell gometalinter to keep vendoring this package
"errors"

_ "github.com/Netflix/titus-api-definitions/src/main/proto/netflix/titus"
"github.com/Netflix/titus-executor/executor/dockershellparser"
"github.com/aws/aws-sdk-go/aws/arn"
)

// ErrMissingIAMRole indicates that the Titus job was submitted without an IAM role
// This is a transition because previously the protobuf had this marked as an optional field
// and it's a temporary measure during protocol evolution.
var ErrMissingIAMRole = errors.New("IAM Role Missing")

// RegistryImageNotFoundError represents an error where an image
// did not exist in the registry
type RegistryImageNotFoundError struct {
Expand Down Expand Up @@ -178,6 +186,39 @@ func (c *Container) GetSortedEnvArray() []string {

}

// GetIamProfile retrieves, and validates the format of the IAM profile
func (c *Container) GetIamProfile() (string, error) {
if c.TitusInfo.IamProfile == nil || c.TitusInfo.GetIamProfile() == "" {
return "", ErrMissingIAMRole
}
if _, err := arn.Parse(c.TitusInfo.GetIamProfile()); err != nil {
return "", err
}

return c.TitusInfo.GetIamProfile(), nil
}

// GetBatch returns what the environment variable TITUS_BATCH should be set to.
// if it returns nil, TITUS_BATCH should be unset
func (c *Container) GetBatch() *string {
idleStr := "idle"
trueStr := "true"

if c.Resources.CPU == 0 {
return &idleStr
}

if !c.TitusInfo.GetBatch() {
return nil
}

if c.TitusInfo.GetPassthroughAttributes()["titusParameter.agent.batchPriority"] == "idle" {
return &idleStr
}

return &trueStr
}

// Resources specify constraints to be applied to a Container
type Resources struct {
Mem int64 // in MiB
Expand Down

0 comments on commit 1f6caa5

Please sign in to comment.