Browse code

Validate --cpuset-cpus, --cpuset-mems

Before this patch libcontainer badly errored out with `invalid
argument` or `numerical result out of range` while trying to write
to cpuset.cpus or cpuset.mems with an invalid value provided.
This patch adds validation to --cpuset-cpus and --cpuset-mems flag along with
validation based on system's available cpus/mems before starting a container.

Signed-off-by: Antonio Murdaca <runcom@linux.com>

Antonio Murdaca authored on 2015/09/09 03:40:55
Showing 10 changed files
... ...
@@ -15,6 +15,7 @@ import (
15 15
 	"github.com/docker/docker/autogen/dockerversion"
16 16
 	"github.com/docker/docker/context"
17 17
 	"github.com/docker/docker/daemon/graphdriver"
18
+	derr "github.com/docker/docker/errors"
18 19
 	"github.com/docker/docker/pkg/fileutils"
19 20
 	"github.com/docker/docker/pkg/parsers"
20 21
 	"github.com/docker/docker/pkg/parsers/kernel"
... ...
@@ -197,6 +198,20 @@ func verifyPlatformContainerSettings(ctx context.Context, daemon *Daemon, hostCo
197 197
 		hostConfig.CpusetCpus = ""
198 198
 		hostConfig.CpusetMems = ""
199 199
 	}
200
+	cpusAvailable, err := sysInfo.IsCpusetCpusAvailable(hostConfig.CpusetCpus)
201
+	if err != nil {
202
+		return warnings, derr.ErrorCodeInvalidCpusetCpus.WithArgs(hostConfig.CpusetCpus)
203
+	}
204
+	if !cpusAvailable {
205
+		return warnings, derr.ErrorCodeNotAvailableCpusetCpus.WithArgs(hostConfig.CpusetCpus, sysInfo.Cpus)
206
+	}
207
+	memsAvailable, err := sysInfo.IsCpusetMemsAvailable(hostConfig.CpusetMems)
208
+	if err != nil {
209
+		return warnings, derr.ErrorCodeInvalidCpusetMems.WithArgs(hostConfig.CpusetMems)
210
+	}
211
+	if !memsAvailable {
212
+		return warnings, derr.ErrorCodeNotAvailableCpusetMems.WithArgs(hostConfig.CpusetMems, sysInfo.Mems)
213
+	}
200 214
 	if hostConfig.BlkioWeight > 0 && !sysInfo.BlkioWeight {
201 215
 		warnings = append(warnings, "Your kernel does not support Block I/O weight. Weight discarded.")
202 216
 		logrus.Warnf("Your kernel does not support Block I/O weight. Weight discarded.")
... ...
@@ -237,10 +252,7 @@ func checkSystem() error {
237 237
 	if os.Geteuid() != 0 {
238 238
 		return fmt.Errorf("The Docker daemon needs to be run as root")
239 239
 	}
240
-	if err := checkKernel(); err != nil {
241
-		return err
242
-	}
243
-	return nil
240
+	return checkKernel()
244 241
 }
245 242
 
246 243
 // configureKernelSecuritySupport configures and validate security support for the kernel
... ...
@@ -827,4 +827,40 @@ var (
827 827
 		Description:    "While trying to delete a container, there was an error trying to delete one of its volumes",
828 828
 		HTTPStatusCode: http.StatusInternalServerError,
829 829
 	})
830
+
831
+	// ErrorCodeInvalidCpusetCpus is generated when user provided cpuset CPUs
832
+	// are invalid.
833
+	ErrorCodeInvalidCpusetCpus = errcode.Register(errGroup, errcode.ErrorDescriptor{
834
+		Value:          "INVALIDCPUSETCPUS",
835
+		Message:        "Invalid value %s for cpuset cpus.",
836
+		Description:    "While verifying the container's 'HostConfig', CpusetCpus value was in an incorrect format",
837
+		HTTPStatusCode: http.StatusInternalServerError,
838
+	})
839
+
840
+	// ErrorCodeInvalidCpusetMems is generated when user provided cpuset mems
841
+	// are invalid.
842
+	ErrorCodeInvalidCpusetMems = errcode.Register(errGroup, errcode.ErrorDescriptor{
843
+		Value:          "INVALIDCPUSETMEMS",
844
+		Message:        "Invalid value %s for cpuset mems.",
845
+		Description:    "While verifying the container's 'HostConfig', CpusetMems value was in an incorrect format",
846
+		HTTPStatusCode: http.StatusInternalServerError,
847
+	})
848
+
849
+	// ErrorCodeNotAvailableCpusetCpus is generated when user provided cpuset
850
+	// CPUs aren't available in the container's cgroup.
851
+	ErrorCodeNotAvailableCpusetCpus = errcode.Register(errGroup, errcode.ErrorDescriptor{
852
+		Value:          "NOTAVAILABLECPUSETCPUS",
853
+		Message:        "Requested CPUs are not available - requested %s, available: %s.",
854
+		Description:    "While verifying the container's 'HostConfig', cpuset CPUs provided aren't available in the container's cgroup available set",
855
+		HTTPStatusCode: http.StatusInternalServerError,
856
+	})
857
+
858
+	// ErrorCodeNotAvailableCpusetMems is generated when user provided cpuset
859
+	// memory nodes aren't available in the container's cgroup.
860
+	ErrorCodeNotAvailableCpusetMems = errcode.Register(errGroup, errcode.ErrorDescriptor{
861
+		Value:          "NOTAVAILABLECPUSETMEMS",
862
+		Message:        "Requested memory nodes are not available - requested %s, available: %s.",
863
+		Description:    "While verifying the container's 'HostConfig', cpuset memory nodes provided aren't available in the container's cgroup available set",
864
+		HTTPStatusCode: http.StatusInternalServerError,
865
+	})
830 866
 )
... ...
@@ -1525,3 +1525,29 @@ func (s *DockerSuite) TestContainersApiGetContainersJSONEmpty(c *check.C) {
1525 1525
 		c.Fatalf("Expected empty response to be `[]`, got %q", string(body))
1526 1526
 	}
1527 1527
 }
1528
+
1529
+func (s *DockerSuite) TestPostContainersCreateWithWrongCpusetValues(c *check.C) {
1530
+	testRequires(c, DaemonIsLinux)
1531
+
1532
+	c1 := struct {
1533
+		Image      string
1534
+		CpusetCpus string
1535
+	}{"busybox", "1-42,,"}
1536
+	name := "wrong-cpuset-cpus"
1537
+	status, body, err := sockRequest("POST", "/containers/create?name="+name, c1)
1538
+	c.Assert(err, check.IsNil)
1539
+	c.Assert(status, check.Equals, http.StatusInternalServerError)
1540
+	expected := "Invalid value 1-42,, for cpuset cpus.\n"
1541
+	c.Assert(string(body), check.Equals, expected, check.Commentf("Expected output to contain %q, got %q", expected, string(body)))
1542
+
1543
+	c2 := struct {
1544
+		Image      string
1545
+		CpusetMems string
1546
+	}{"busybox", "42-3,1--"}
1547
+	name = "wrong-cpuset-mems"
1548
+	status, body, err = sockRequest("POST", "/containers/create?name="+name, c2)
1549
+	c.Assert(err, check.IsNil)
1550
+	c.Assert(status, check.Equals, http.StatusInternalServerError)
1551
+	expected = "Invalid value 42-3,1-- for cpuset mems.\n"
1552
+	c.Assert(string(body), check.Equals, expected, check.Commentf("Expected output to contain %q, got %q", expected, string(body)))
1553
+}
... ...
@@ -3407,3 +3407,17 @@ func (s *DockerSuite) TestRunStdinBlockedAfterContainerExit(c *check.C) {
3407 3407
 		c.Fatal("timeout waiting for command to exit")
3408 3408
 	}
3409 3409
 }
3410
+
3411
+func (s *DockerSuite) TestRunWrongCpusetCpusFlagValue(c *check.C) {
3412
+	out, _, err := dockerCmdWithError("run", "--cpuset-cpus", "1-10,11--", "busybox", "true")
3413
+	c.Assert(err, check.NotNil)
3414
+	expected := "Error response from daemon: Invalid value 1-10,11-- for cpuset cpus.\n"
3415
+	c.Assert(out, check.Equals, expected, check.Commentf("Expected output to contain %q, got %q", expected, out))
3416
+}
3417
+
3418
+func (s *DockerSuite) TestRunWrongCpusetMemsFlagValue(c *check.C) {
3419
+	out, _, err := dockerCmdWithError("run", "--cpuset-mems", "1-42--", "busybox", "true")
3420
+	c.Assert(err, check.NotNil)
3421
+	expected := "Error response from daemon: Invalid value 1-42-- for cpuset mems.\n"
3422
+	c.Assert(out, check.Equals, expected, check.Commentf("Expected output to contain %q, got %q", expected, out))
3423
+}
... ...
@@ -9,11 +9,14 @@ import (
9 9
 	"os"
10 10
 	"os/exec"
11 11
 	"path/filepath"
12
+	"strconv"
12 13
 	"strings"
13 14
 	"time"
14 15
 
15 16
 	"github.com/docker/docker/pkg/integration/checker"
16 17
 	"github.com/docker/docker/pkg/mount"
18
+	"github.com/docker/docker/pkg/parsers"
19
+	"github.com/docker/docker/pkg/sysinfo"
17 20
 	"github.com/go-check/check"
18 21
 	"github.com/kr/pty"
19 22
 )
... ...
@@ -370,3 +373,41 @@ func (s *DockerSuite) TestRunSwapLessThanMemoryLimit(c *check.C) {
370 370
 		c.Fatalf("Expected output to contain %q, not %q", out, expected)
371 371
 	}
372 372
 }
373
+
374
+func (s *DockerSuite) TestRunInvalidCpusetCpusFlagValue(c *check.C) {
375
+	testRequires(c, cgroupCpuset)
376
+
377
+	sysInfo := sysinfo.New(true)
378
+	cpus, err := parsers.ParseUintList(sysInfo.Cpus)
379
+	c.Assert(err, check.IsNil)
380
+	var invalid int
381
+	for i := 0; i <= len(cpus)+1; i++ {
382
+		if !cpus[i] {
383
+			invalid = i
384
+			break
385
+		}
386
+	}
387
+	out, _, err := dockerCmdWithError("run", "--cpuset-cpus", strconv.Itoa(invalid), "busybox", "true")
388
+	c.Assert(err, check.NotNil)
389
+	expected := fmt.Sprintf("Error response from daemon: Requested CPUs are not available - requested %s, available: %s.\n", strconv.Itoa(invalid), sysInfo.Cpus)
390
+	c.Assert(out, check.Equals, expected, check.Commentf("Expected output to contain %q, got %q", expected, out))
391
+}
392
+
393
+func (s *DockerSuite) TestRunInvalidCpusetMemsFlagValue(c *check.C) {
394
+	testRequires(c, cgroupCpuset)
395
+
396
+	sysInfo := sysinfo.New(true)
397
+	mems, err := parsers.ParseUintList(sysInfo.Mems)
398
+	c.Assert(err, check.IsNil)
399
+	var invalid int
400
+	for i := 0; i <= len(mems)+1; i++ {
401
+		if !mems[i] {
402
+			invalid = i
403
+			break
404
+		}
405
+	}
406
+	out, _, err := dockerCmdWithError("run", "--cpuset-mems", strconv.Itoa(invalid), "busybox", "true")
407
+	c.Assert(err, check.NotNil)
408
+	expected := fmt.Sprintf("Error response from daemon: Requested memory nodes are not available - requested %s, available: %s.\n", strconv.Itoa(invalid), sysInfo.Mems)
409
+	c.Assert(out, check.Equals, expected, check.Commentf("Expected output to contain %q, got %q", expected, out))
410
+}
... ...
@@ -127,7 +127,7 @@ func PartParser(template, data string) (map[string]string, error) {
127 127
 		out           = make(map[string]string, len(templateParts))
128 128
 	)
129 129
 	if len(parts) != len(templateParts) {
130
-		return nil, fmt.Errorf("Invalid format to parse.  %s should match template %s", data, template)
130
+		return nil, fmt.Errorf("Invalid format to parse. %s should match template %s", data, template)
131 131
 	}
132 132
 
133 133
 	for i, t := range templateParts {
... ...
@@ -196,3 +196,53 @@ func ParseLink(val string) (string, string, error) {
196 196
 	}
197 197
 	return arr[0], arr[1], nil
198 198
 }
199
+
200
+// ParseUintList parses and validates the specified string as the value
201
+// found in some cgroup file (e.g. `cpuset.cpus`, `cpuset.mems`), which could be
202
+// one of the formats below. Note that duplicates are actually allowed in the
203
+// input string. It returns a `map[int]bool` with available elements from `val`
204
+// set to `true`.
205
+// Supported formats:
206
+//     7
207
+//     1-6
208
+//     0,3-4,7,8-10
209
+//     0-0,0,1-7
210
+//     03,1-3      <- this is gonna get parsed as [1,2,3]
211
+//     3,2,1
212
+//     0-2,3,1
213
+func ParseUintList(val string) (map[int]bool, error) {
214
+	if val == "" {
215
+		return map[int]bool{}, nil
216
+	}
217
+
218
+	availableInts := make(map[int]bool)
219
+	split := strings.Split(val, ",")
220
+	errInvalidFormat := fmt.Errorf("invalid format: %s", val)
221
+
222
+	for _, r := range split {
223
+		if !strings.Contains(r, "-") {
224
+			v, err := strconv.Atoi(r)
225
+			if err != nil {
226
+				return nil, errInvalidFormat
227
+			}
228
+			availableInts[v] = true
229
+		} else {
230
+			split := strings.SplitN(r, "-", 2)
231
+			min, err := strconv.Atoi(split[0])
232
+			if err != nil {
233
+				return nil, errInvalidFormat
234
+			}
235
+			max, err := strconv.Atoi(split[1])
236
+			if err != nil {
237
+				return nil, errInvalidFormat
238
+			}
239
+			if max < min {
240
+				return nil, errInvalidFormat
241
+			}
242
+			for i := min; i <= max; i++ {
243
+				availableInts[i] = true
244
+			}
245
+		}
246
+	}
247
+	return availableInts, nil
248
+}
... ...
@@ -1,6 +1,7 @@
1 1
 package parsers
2 2
 
3 3
 import (
4
+	"reflect"
4 5
 	"runtime"
5 6
 	"strings"
6 7
 	"testing"
... ...
@@ -238,3 +239,40 @@ func TestParseLink(t *testing.T) {
238 238
 		t.Fatalf("Expected error 'bad format for links: link:alias:wrong' but got: %v", err)
239 239
 	}
240 240
 }
241
+
242
+func TestParseUintList(t *testing.T) {
243
+	valids := map[string]map[int]bool{
244
+		"":             {},
245
+		"7":            {7: true},
246
+		"1-6":          {1: true, 2: true, 3: true, 4: true, 5: true, 6: true},
247
+		"0-7":          {0: true, 1: true, 2: true, 3: true, 4: true, 5: true, 6: true, 7: true},
248
+		"0,3-4,7,8-10": {0: true, 3: true, 4: true, 7: true, 8: true, 9: true, 10: true},
249
+		"0-0,0,1-4":    {0: true, 1: true, 2: true, 3: true, 4: true},
250
+		"03,1-3":       {1: true, 2: true, 3: true},
251
+		"3,2,1":        {1: true, 2: true, 3: true},
252
+		"0-2,3,1":      {0: true, 1: true, 2: true, 3: true},
253
+	}
254
+	for k, v := range valids {
255
+		out, err := ParseUintList(k)
256
+		if err != nil {
257
+			t.Fatalf("Expected not to fail, got %v", err)
258
+		}
259
+		if !reflect.DeepEqual(out, v) {
260
+			t.Fatalf("Expected %v, got %v", v, out)
261
+		}
262
+	}
263
+
264
+	invalids := []string{
265
+		"this",
266
+		"1--",
267
+		"1-10,,10",
268
+		"10-1",
269
+		"-1",
270
+		"-1,0",
271
+	}
272
+	for _, v := range invalids {
273
+		if out, err := ParseUintList(v); err == nil {
274
+			t.Fatalf("Expected failure with %s but got %v", v, out)
275
+		}
276
+	}
277
+}
... ...
@@ -1,5 +1,7 @@
1 1
 package sysinfo
2 2
 
3
+import "github.com/docker/docker/pkg/parsers"
4
+
3 5
 // SysInfo stores information about which features a kernel supports.
4 6
 // TODO Windows: Factor out platform specific capabilities.
5 7
 type SysInfo struct {
... ...
@@ -63,4 +65,41 @@ type cgroupBlkioInfo struct {
63 63
 type cgroupCpusetInfo struct {
64 64
 	// Whether Cpuset is supported or not
65 65
 	Cpuset bool
66
+
67
+	// Available Cpuset's cpus
68
+	Cpus string
69
+
70
+	// Available Cpuset's memory nodes
71
+	Mems string
72
+}
73
+
74
+// IsCpusetCpusAvailable returns `true` if the provided string set is contained
75
+// in cgroup's cpuset.cpus set, `false` otherwise.
76
+// If error is not nil a parsing error occurred.
77
+func (c cgroupCpusetInfo) IsCpusetCpusAvailable(provided string) (bool, error) {
78
+	return isCpusetListAvailable(provided, c.Cpus)
79
+}
80
+
81
+// IsCpusetMemsAvailable returns `true` if the provided string set is contained
82
+// in cgroup's cpuset.mems set, `false` otherwise.
83
+// If error is not nil a parsing error occurred.
84
+func (c cgroupCpusetInfo) IsCpusetMemsAvailable(provided string) (bool, error) {
85
+	return isCpusetListAvailable(provided, c.Mems)
86
+}
87
+
88
+func isCpusetListAvailable(provided, available string) (bool, error) {
89
+	parsedProvided, err := parsers.ParseUintList(provided)
90
+	if err != nil {
91
+		return false, err
92
+	}
93
+	parsedAvailable, err := parsers.ParseUintList(available)
94
+	if err != nil {
95
+		return false, err
96
+	}
97
+	for k := range parsedProvided {
98
+		if !parsedAvailable[k] {
99
+			return false, nil
100
+		}
101
+	}
102
+	return true, nil
66 103
 }
... ...
@@ -126,7 +126,7 @@ func checkCgroupBlkioInfo(quiet bool) cgroupBlkioInfo {
126 126
 
127 127
 // checkCgroupCpusetInfo reads the cpuset information from the cpuset cgroup mount point.
128 128
 func checkCgroupCpusetInfo(quiet bool) cgroupCpusetInfo {
129
-	_, err := cgroups.FindCgroupMountpoint("cpuset")
129
+	mountPoint, err := cgroups.FindCgroupMountpoint("cpuset")
130 130
 	if err != nil {
131 131
 		if !quiet {
132 132
 			logrus.Warn(err)
... ...
@@ -134,7 +134,21 @@ func checkCgroupCpusetInfo(quiet bool) cgroupCpusetInfo {
134 134
 		return cgroupCpusetInfo{}
135 135
 	}
136 136
 
137
-	return cgroupCpusetInfo{Cpuset: true}
137
+	cpus, err := ioutil.ReadFile(path.Join(mountPoint, "cpuset.cpus"))
138
+	if err != nil {
139
+		return cgroupCpusetInfo{}
140
+	}
141
+
142
+	mems, err := ioutil.ReadFile(path.Join(mountPoint, "cpuset.mems"))
143
+	if err != nil {
144
+		return cgroupCpusetInfo{}
145
+	}
146
+
147
+	return cgroupCpusetInfo{
148
+		Cpuset: true,
149
+		Cpus:   strings.TrimSpace(string(cpus)),
150
+		Mems:   strings.TrimSpace(string(mems)),
151
+	}
138 152
 }
139 153
 
140 154
 func cgroupEnabled(mountPoint, name string) bool {
141 155
new file mode 100644
... ...
@@ -0,0 +1,26 @@
0
+package sysinfo
1
+
2
+import "testing"
3
+
4
+func TestIsCpusetListAvailable(t *testing.T) {
5
+	cases := []struct {
6
+		provided  string
7
+		available string
8
+		res       bool
9
+		err       bool
10
+	}{
11
+		{"1", "0-4", true, false},
12
+		{"01,3", "0-4", true, false},
13
+		{"", "0-7", true, false},
14
+		{"1--42", "0-7", false, true},
15
+		{"1-42", "00-1,8,,9", false, true},
16
+		{"1,41-42", "43,45", false, false},
17
+		{"0-3", "", false, false},
18
+	}
19
+	for _, c := range cases {
20
+		r, err := isCpusetListAvailable(c.provided, c.available)
21
+		if (c.err && err == nil) && r != c.res {
22
+			t.Fatalf("Expected pair: %v, %v for %s, %s. Got %v, %v instead", c.res, c.err, c.provided, c.available, (c.err && err == nil), r)
23
+		}
24
+	}
25
+}