Skip to content
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.

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

bugfix: fix wrongly update cpu-quota to 0 #2684

Merged
merged 1 commit into from
Jan 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion daemon/mgr/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -1328,7 +1328,7 @@ func (mgr *ContainerManager) updateContainerResources(c *Container, resources ty
if resources.CPUPeriod != 0 {
cResources.CPUPeriod = resources.CPUPeriod
}
if resources.CPUQuota > -1 {
if resources.CPUQuota == -1 || resources.CPUQuota >= 1000 {
cResources.CPUQuota = resources.CPUQuota
}
if resources.CPUShares != 0 {
Expand Down
2 changes: 1 addition & 1 deletion test/cli_run_cgroup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func testRunWithCgroupParent(c *check.C, cgroupParent, name string) {

file := filepath.Join(cgroupMount, cgroupPaths["memory"], "memory.limit_in_bytes")
if _, err := os.Stat(file); err != nil {
c.Fatalf("container %s cgroup mountpoint not exists", name)
c.Fatalf("failed to Stat container %s cgroup mountpoint %s: %v", name, file, err)
}

out, err := exec.Command("cat", file).Output()
Expand Down
92 changes: 92 additions & 0 deletions test/cli_update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -403,3 +403,95 @@ func (suite *PouchUpdateSuite) TestUpdateContainerDiskQuota(c *check.C) {
}
c.Assert(found, check.Equals, true)
}

func checkContainerCPUQuota(c *check.C, cName, cpuQuota string) {
var (
containerID string
cgroupCPUQuota = cpuQuota
)

output := command.PouchRun("inspect", cName).Stdout()
result := []types.ContainerJSON{}
if err := json.Unmarshal([]byte(output), &result); err != nil {
c.Errorf("failed to decode inspect output: %v", err)
}
containerID = result[0].ID

if string(result[0].HostConfig.CPUQuota) == cpuQuota {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it should be "!=" rather than "=="

c.Errorf("expect CPUQuota %s, but got: %v", cpuQuota, result[0].HostConfig.CPUQuota)
}

// container's cpu-quota default is 0 that means not limit cpu quota in cgroup, and
// cpu.cfs_quota_us value is -1 when not limit cpu quota in cgroup.
if cgroupCPUQuota == "0" {
cgroupCPUQuota = "-1"
}
path := fmt.Sprintf("/sys/fs/cgroup/cpu/default/%s/cpu.cfs_quota_us", containerID)
checkFileContains(c, path, cgroupCPUQuota)
}

// TestUpdateContainerCPUQuota is to verify the correctness of update cpuquota by update interface
func (suite *PouchUpdateSuite) TestUpdateContainerCPUQuota(c *check.C) {
name := "TestUpdateContainerCPUQuota"

command.PouchRun("run", "-d",
"--name", name,
busyboxImage, "top").Assert(c, icmd.Success)
defer DelContainerForceMultyTime(c, name)

// default cpuquota should be 0
checkContainerCPUQuota(c, name, "0")

// update cpuquota to 0, should not take effect
command.PouchRun("update", "--cpu-quota", "0", name).Assert(c, icmd.Success)
// 0 is a meaningless value
checkContainerCPUQuota(c, name, "0")

// update not specified any parameters, cpuquota should still be 0
command.PouchRun("update", name).Assert(c, icmd.Success)
checkContainerCPUQuota(c, name, "0")

// update cpuquota to [1, 1000), should return error
res := command.PouchRun("update", "--cpu-quota", "20", name)
c.Assert(res.Stderr(), check.NotNil, check.Commentf("CPU cfs quota should be greater than 1ms(1000)"))

// update cpuquota to 1100, should take effect
command.PouchRun("update", "--cpu-quota", "1100", name).Assert(c, icmd.Success)
checkContainerCPUQuota(c, name, "1100")

// update cpuquota to -1, should take effect
command.PouchRun("update", "--cpu-quota", "-1", name).Assert(c, icmd.Success)
checkContainerCPUQuota(c, name, "-1")

}
HusterWan marked this conversation as resolved.
Show resolved Hide resolved

// TestUpdateStoppedContainerCPUQuota is to verify the correctness of update the cpuquota
// of a stopped container by update interface
func (suite *PouchUpdateSuite) TestUpdateStoppedContainerCPUQuota(c *check.C) {
name := "TestUpdateContainerCPUQuota"

command.PouchRun("create",
"--cpu-quota", "1100",
"--name", name,
busyboxImage, "top").Assert(c, icmd.Success)
defer DelContainerForceMultyTime(c, name)

// update cpuquota to 1200, should take effect
command.PouchRun("update", "--cpu-quota", "1200", name).Assert(c, icmd.Success)

// start container
command.PouchRun("start", name).Assert(c, icmd.Success)

// then check the cpu-quota value
checkContainerCPUQuota(c, name, "1200")

// update cpuquota to 0, should not take effect
command.PouchRun("update", "--cpu-quota", "0", name).Assert(c, icmd.Success)
// 0 is a meaningless value
checkContainerCPUQuota(c, name, "1200")

// update not specified any parameters, cpuquota should still be 1200
command.PouchRun("update", name).Assert(c, icmd.Success)
checkContainerCPUQuota(c, name, "1200")

}