Skip to content

Commit

Permalink
api: Windows MemorySwappiness Workaround
Browse files Browse the repository at this point in the history
Abstracted platformHostConfigOverride in task_windows.go to another
method to clarify the intention of the override. Updated related
comments regarding injection point for HostConfig override values.

Commented on task_unix.go to explain the use of memorySwappinessDefault
constant.

Cleaned up extra lines in task_windows_test.go. Removed unneeded
properties in task object use in TestWindowsPlatformHostConfigOverride.
  • Loading branch information
cabbruzzese committed Aug 15, 2017
1 parent 10ad0af commit 6dd6f70
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 19 deletions.
3 changes: 3 additions & 0 deletions agent/api/task_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ import (

const (
portBindingHostIP = "0.0.0.0"

//memorySwappinessDefault is the expected default value for this platform. This is used in task_windows.go
//and is maintained here for unix default. Also used for testing
memorySwappinessDefault = 0
)

Expand Down
15 changes: 11 additions & 4 deletions agent/api/task_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (

const (
portBindingHostIP = ""

//memorySwappinessDefault is the expected default value for this platform
memorySwappinessDefault = -1
)

Expand Down Expand Up @@ -54,13 +56,18 @@ func getCanonicalPath(path string) string {
return filepath.Clean(strings.ToLower(path))
}

// platformHostConfigOverride Sets up default HostConfig options to be passed to Docker API.
// Docker for Windows treats some options different than Unix and some are not supported at all.
// platformHostConfigOverride provides an entry point to set up default HostConfig options to be
// passed to Docker API.
func (task *Task) platformHostConfigOverride(hostConfig *docker.HostConfig) {
task.overrideDefaultMemorySwappiness(hostConfig);
}

// overrideDefaultMemorySwappiness Overrides the value of MemorySwappiness to -1
// Version 1.12.x of Docker for Windows would ignore the unsupported option MemorySwappiness.
// Version 17.03.x will cause an error if any value other than -1 is passed in for MemorySwappiness.
// This bug is not noticed when no value is passed in. However, the go-dockerclient client version
// we are using removed the json option omitempty causing this parameter to default to 0 if empty.
// https://github.com/fsouza/go-dockerclient/commit/72342f96fabfa614a94b6ca57d987eccb8a836bf
func (task *Task) platformHostConfigOverride(hostConfig *docker.HostConfig) {
func (task *Task) overrideDefaultMemorySwappiness(hostConfig *docker.HostConfig) {
hostConfig.MemorySwappiness = memorySwappinessDefault
}
}
18 changes: 3 additions & 15 deletions agent/api/task_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,20 +108,9 @@ func TestWindowsPlatformHostConfigOverride(t *testing.T) {
// Testing Windows platform override for HostConfig.
// Expects MemorySwappiness option to be set to -1

task := &Task{
Arn: "myArn",
DesiredStatusUnsafe: TaskRunning,
Family: "myFamily",
Version: "1",
Containers: []*Container{
{
Name: "myName",
},
},
}
task := &Task{}

hostConfig := &docker.HostConfig{
}
hostConfig := &docker.HostConfig{}

task.platformHostConfigOverride(hostConfig)

Expand All @@ -131,8 +120,7 @@ func TestWindowsPlatformHostConfigOverride(t *testing.T) {

func TestWindowsMemorySwappinessOption(t *testing.T) {
// Testing sending a task to windows overriding MemorySwappiness value
rawHostConfigInput := docker.HostConfig{
}
rawHostConfigInput := docker.HostConfig{}

rawHostConfig, err := json.Marshal(&rawHostConfigInput)
if err != nil {
Expand Down

0 comments on commit 6dd6f70

Please sign in to comment.