Merge pull request #537 from hqhq/hq_cleanup_cpushares_check

cleanup cpushares check

cgroups/fs/apply_raw.go

@@ -5,7 +5,6 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
-	"path"
 	"path/filepath"
 	"strconv"
 	"sync"
@@ -115,25 +114,9 @@ func (m *Manager) Apply(pid int) error {
 	}
 	m.Paths = paths
 
-	var cpuShares int64
-
-	fd, err := os.Open(path.Join(m.Paths["cpu"], "cpu.shares"))
-	if err != nil {
-		return err
-	}
-
-	_, err = fmt.Fscanf(fd, "%d", &cpuShares)
-	if err != nil && err != io.EOF {
-		return err
-	}
-
-	fd.Close()
-
-	if c.CpuShares != 0 {
-		if c.CpuShares > cpuShares {
-			return fmt.Errorf("The maximum allowed cpu-shares is %d", cpuShares)
-		} else if c.CpuShares < cpuShares {
-			return fmt.Errorf("The minimum allowed cpu-shares is %d", cpuShares)
+	if paths["cpu"] != "" {
+		if err := CheckCpushares(paths["cpu"], c.CpuShares); err != nil {
+			return err
 		}
 	}
 
@@ -309,3 +292,27 @@ func removePath(p string, err error) error {
 	}
 	return nil
 }
+
+func CheckCpushares(path string, c int64) error {
+	var cpuShares int64
+
+	fd, err := os.Open(filepath.Join(path, "cpu.shares"))
+	if err != nil {
+		return err
+	}
+	defer fd.Close()
+
+	_, err = fmt.Fscanf(fd, "%d", &cpuShares)
+	if err != nil && err != io.EOF {
+		return err
+	}
+	if c != 0 {
+		if c > cpuShares {
+			return fmt.Errorf("The maximum allowed cpu-shares is %d", cpuShares)
+		} else if c < cpuShares {
+			return fmt.Errorf("The minimum allowed cpu-shares is %d", cpuShares)
+		}
+	}
+
+	return nil
+}

cgroups/systemd/apply_systemd.go

@@ -4,10 +4,8 @@ package systemd
 
 import (
 	"fmt"
-	"io"
 	"io/ioutil"
 	"os"
-	"path"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -238,28 +236,11 @@ func (m *Manager) Apply(pid int) error {
 		}
 		paths[sysname] = subsystemPath
 	}
-
 	m.Paths = paths
 
-	var cpuShares int64
-
-	fd, err := os.Open(path.Join(m.Paths["cpu"], "cpu.shares"))
-	if err != nil {
-		return err
-	}
-
-	_, err = fmt.Fscanf(fd, "%d", &cpuShares)
-	if err != nil && err != io.EOF {
-		return err
-	}
-
-	fd.Close()
-
-	if c.CpuShares != 0 {
-		if c.CpuShares > cpuShares {
-			return fmt.Errorf("The maximum allowed cpu-shares is %d", cpuShares)
-		} else if c.CpuShares < cpuShares {
-			return fmt.Errorf("The minimum allowed cpu-shares is %d", cpuShares)
+	if paths["cpu"] != "" {
+		if err := fs.CheckCpushares(paths["cpu"], c.CpuShares); err != nil {
+			return err
 		}
 	}
 

integration/exec_test.go

@@ -475,6 +475,37 @@ func testFreeze(t *testing.T, systemd bool) {
 	}
 }
 
+func TestCpuShares(t *testing.T) {
+	testCpuShares(t, false)
+}
+
+func TestSystemdCpuShares(t *testing.T) {
+	if !systemd.UseSystemd() {
+		t.Skip("Systemd is unsupported")
+	}
+	testCpuShares(t, true)
+}
+
+func testCpuShares(t *testing.T, systemd bool) {
+	if testing.Short() {
+		return
+	}
+	rootfs, err := newRootfs()
+	ok(t, err)
+	defer remove(rootfs)
+
+	config := newTemplateConfig(rootfs)
+	if systemd {
+		config.Cgroups.Slice = "system.slice"
+	}
+	config.Cgroups.CpuShares = 1
+
+	_, _, err = runContainer(config, "", "ps")
+	if err == nil {
+		t.Fatalf("runContainer should failed with invalid CpuShares")
+	}
+}
+
 func TestContainerState(t *testing.T) {
 	if testing.Short() {
 		return