Reset Swap when memory is set to unlimited (-1)
Kernel validation fails if memory set to -1 which is unlimited but swap is not set so. Signed-off-by: Mohammad Arab <boynux@gmail.com>
This commit is contained in:
parent
1e4ca86a72
commit
18ebc51b3c
|
@ -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 {
|
||||
}
|
||||
|
@ -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
|
||||
|
@ -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
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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()
|
||||
|
|
|
@ -45,7 +45,7 @@ function check_cgroup_value() {
|
|||
expected=$3
|
||||
|
||||
current=$(cat $cgroup/$source)
|
||||
[ "$current" -eq "$expected" ]
|
||||
[ "$current" == "$expected" ]
|
||||
}
|
||||
|
||||
# TODO: test rt cgroup updating
|
||||
|
@ -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
|
||||
|
@ -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
|
||||
|
|
16
update.go
16
update.go
|
@ -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)
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue