Skip to content

Fixes set memory to unlimited #1127

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

Merged
hqhq merged 1 commit into from
Feb 16, 2017
Merged

Fixes set memory to unlimited #1127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 23 additions & 10 deletions libcontainer/cgroups/fs/memory.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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