Skip to content

Commit

Permalink
Reset Swap when memory is set to unlimited (-1)
Browse files Browse the repository at this point in the history 
Kernel validation fails if memory set to -1 which is unlimited but
swap is not set so.

Signed-off-by: Mohammad Arab <[email protected]>
  • Loading branch information
@boynux
boynux committed Feb 15, 2017
1 parent 1e4ca86 commit 18ebc51
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 59 deletions.
33 changes: 23 additions & 10 deletions libcontainer/cgroups/fs/memory.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ import (
"github.com/opencontainers/runc/libcontainer/configs"
)

const cgroupKernelMemoryLimit = "memory.kmem.limit_in_bytes"
const (
cgroupKernelMemoryLimit = "memory.kmem.limit_in_bytes"
cgroupMemorySwapLimit = "memory.memsw.limit_in_bytes"
cgroupMemoryLimit = "memory.limit_in_bytes"
)

type MemoryGroup struct {
}
Expand Down Expand Up @@ -96,9 +100,18 @@ func setKernelMemory(path string, kernelMemoryLimit int64) error {
}

func setMemoryAndSwap(path string, cgroup *configs.Cgroup) error {
// If the memory update is set to -1 we should also set swap to -1
// -1 means unlimited memory
if cgroup.Resources.Memory == -1 {
// Only set swap if it's enbled in kernel
if cgroups.PathExists(filepath.Join(path, cgroupMemorySwapLimit)) {
cgroup.Resources.MemorySwap = -1
}
}

// When memory and swap memory are both set, we need to handle the cases
// for updating container.
if cgroup.Resources.Memory != 0 && cgroup.Resources.MemorySwap > 0 {
if cgroup.Resources.Memory != 0 && cgroup.Resources.MemorySwap != 0 {
memoryUsage, err := getMemoryData(path, "")
if err != nil {
return err
Expand All @@ -107,29 +120,29 @@ func setMemoryAndSwap(path string, cgroup *configs.Cgroup) error {
// When update memory limit, we should adapt the write sequence
// for memory and swap memory, so it won't fail because the new
// value and the old value don't fit kernel's validation.
if memoryUsage.Limit < uint64(cgroup.Resources.MemorySwap) {
if err := writeFile(path, "memory.memsw.limit_in_bytes", strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil {
if cgroup.Resources.MemorySwap == -1 || memoryUsage.Limit < uint64(cgroup.Resources.MemorySwap) {
if err := writeFile(path, cgroupMemorySwapLimit, strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil {
return err
}
if err := writeFile(path, "memory.limit_in_bytes", strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil {
if err := writeFile(path, cgroupMemoryLimit, strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil {
return err
}
} else {
if err := writeFile(path, "memory.limit_in_bytes", strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil {
if err := writeFile(path, cgroupMemoryLimit, strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil {
return err
}
if err := writeFile(path, "memory.memsw.limit_in_bytes", strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil {
if err := writeFile(path, cgroupMemorySwapLimit, strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil {
return err
}
}
} else {
if cgroup.Resources.Memory != 0 {
if err := writeFile(path, "memory.limit_in_bytes", strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil {
if err := writeFile(path, cgroupMemoryLimit, strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil {
return err
}
}
if cgroup.Resources.MemorySwap > 0 {
if err := writeFile(path, "memory.memsw.limit_in_bytes", strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil {
if cgroup.Resources.MemorySwap != 0 {
if err := writeFile(path, cgroupMemorySwapLimit, strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil {
return err
}
}
Expand Down
41 changes: 0 additions & 41 deletions libcontainer/cgroups/fs/memory_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,47 +86,6 @@ func TestMemorySetMemoryswap(t *testing.T) {
}
}

func TestMemorySetNegativeMemoryswap(t *testing.T) {
helper := NewCgroupTestUtil("memory", t)
defer helper.cleanup()

const (
memoryBefore = 314572800 // 300M
memoryAfter = 524288000 // 500M
memoryswapBefore = 629145600 // 600M
memoryswapAfter = 629145600 // 600M
)

helper.writeFileContents(map[string]string{
"memory.limit_in_bytes": strconv.Itoa(memoryBefore),
"memory.memsw.limit_in_bytes": strconv.Itoa(memoryswapBefore),
})

helper.CgroupData.config.Resources.Memory = memoryAfter
// Negative value means not change
helper.CgroupData.config.Resources.MemorySwap = -1
memory := &MemoryGroup{}
if err := memory.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {
t.Fatal(err)
}

value, err := getCgroupParamUint(helper.CgroupPath, "memory.limit_in_bytes")
if err != nil {
t.Fatalf("Failed to parse memory.limit_in_bytes - %s", err)
}
if value != memoryAfter {
t.Fatal("Got the wrong value, set memory.limit_in_bytes failed.")
}

value, err = getCgroupParamUint(helper.CgroupPath, "memory.memsw.limit_in_bytes")
if err != nil {
t.Fatalf("Failed to parse memory.memsw.limit_in_bytes - %s", err)
}
if value != memoryswapAfter {
t.Fatal("Got the wrong value, set memory.memsw.limit_in_bytes failed.")
}
}

func TestMemorySetMemoryLargerThanSwap(t *testing.T) {
helper := NewCgroupTestUtil("memory", t)
defer helper.cleanup()
Expand Down
29 changes: 26 additions & 3 deletions tests/integration/update.bats
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function check_cgroup_value() {
expected=$3

current=$(cat $cgroup/$source)
[ "$current" -eq "$expected" ]
[ "$current" == "$expected" ]
}

# TODO: test rt cgroup updating
Expand All @@ -62,6 +62,8 @@ function check_cgroup_value() {
eval CGROUP_${g}="${base_path}/runc-update-integration-test"
done

CGROUP_SYSTEM_MEMORY=$(grep "cgroup" /proc/self/mountinfo | gawk 'toupper($NF) ~ /\<'MEMORY'\>/ { print $5; exit }')

# check that initial values were properly set
check_cgroup_value $CGROUP_BLKIO "blkio.weight" 1000
check_cgroup_value $CGROUP_CPU "cpu.cfs_period_us" 1000000
Expand Down Expand Up @@ -110,17 +112,38 @@ function check_cgroup_value() {
[ "$status" -eq 0 ]
check_cgroup_value $CGROUP_MEMORY "memory.limit_in_bytes" 52428800


# update memory soft limit
runc update test_update --memory-reservation 33554432
[ "$status" -eq 0 ]
check_cgroup_value $CGROUP_MEMORY "memory.soft_limit_in_bytes" 33554432

# update memory swap (if available)
# Run swap memory tests if swap is avaialble
if [ -f "$CGROUP_MEMORY/memory.memsw.limit_in_bytes" ]; then
# try to remove memory swap limit
runc update test_update --memory-swap -1
[ "$status" -eq 0 ]
# Get System memory swap limit
SYSTEM_MEMORY_SW=$(cat "${CGROUP_SYSTEM_MEMORY}/memory.memsw.limit_in_bytes")
check_cgroup_value $CGROUP_MEMORY "memory.memsw.limit_in_bytes" ${SYSTEM_MEMORY_SW}

# update memory swap
runc update test_update --memory-swap 96468992
[ "$status" -eq 0 ]
check_cgroup_value $CGROUP_MEMORY "memory.memsw.limit_in_bytes" 96468992
fi;

# try to remove memory limit
runc update test_update --memory -1
[ "$status" -eq 0 ]

# Get System memory limit
SYSTEM_MEMORY=$(cat "${CGROUP_SYSTEM_MEMORY}/memory.limit_in_bytes")
# check memory limited is gone
check_cgroup_value $CGROUP_MEMORY "memory.limit_in_bytes" ${SYSTEM_MEMORY}

# check swap memory limited is gone
if [ -f "$CGROUP_MEMORY/memory.memsw.limit_in_bytes" ]; then
check_cgroup_value $CGROUP_MEMORY "memory.memsw.limit_in_bytes" ${SYSTEM_MEMORY}
fi

# update kernel memory limit
Expand Down
16 changes: 11 additions & 5 deletions update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,16 +193,22 @@ other options are ignored.
opt string
dest *uint64
}{
{"memory", r.Memory.Limit},
{"memory-swap", r.Memory.Swap},
{"kernel-memory", r.Memory.Kernel},
{"kernel-memory-tcp", r.Memory.KernelTCP},
{"memory", r.Memory.Limit},
{"memory-reservation", r.Memory.Reservation},
{"memory-swap", r.Memory.Swap},
} {
if val := context.String(pair.opt); val != "" {
v, err := units.RAMInBytes(val)
if err != nil {
return fmt.Errorf("invalid value for %s: %s", pair.opt, err)
var v int64

if val != "-1" {
v, err = units.RAMInBytes(val)
if err != nil {
return fmt.Errorf("invalid value for %s: %s", pair.opt, err)
}
} else {
v = -1
}
*pair.dest = uint64(v)
}
Expand Down

0 comments on commit 18ebc51

Please sign in to comment.