fix: make cpuquota, MemorySwappiness validation compatible with docker

refactor: logCreateOption before validation, it enable developer to see what's wrong. Signed-off-by: zhuangqh <zhuangqhc@gmail.com>
AliyunContainerService · Nov 19, 2018 · 335c90c · 335c90c
1 parent 8acae57
commit 335c90c
Show file tree

Hide file tree

Showing 8 changed files with 19 additions and 35 deletions.
diff --git a/apis/server/container_bridge.go b/apis/server/container_bridge.go
@@ -38,13 +38,14 @@ func (s *Server) createContainer(ctx context.Context, rw http.ResponseWriter, re
 	if err := json.NewDecoder(reader).Decode(config); err != nil {
 		return httputils.NewHTTPError(err, http.StatusBadRequest)
 	}
+
+	logCreateOptions("container", config)
+
 	// validate request body
 	if err := config.Validate(strfmt.NewFormats()); err != nil {
 		return httputils.NewHTTPError(err, http.StatusBadRequest)
 	}
 
-	logCreateOptions("container", config)
-
 	name := req.FormValue("name")
 	//consider set specific id by url params
 	specificID := req.FormValue("specificId")

diff --git a/apis/server/exec_bridge.go b/apis/server/exec_bridge.go
@@ -24,15 +24,15 @@ func (s *Server) createContainerExec(ctx context.Context, rw http.ResponseWriter
 	if err := json.NewDecoder(req.Body).Decode(config); err != nil {
 		return httputils.NewHTTPError(err, http.StatusBadRequest)
 	}
+	name := mux.Vars(req)["name"]
+
+	logCreateOptions("container exec for "+name, config)
+
 	// validate request body
 	if err := config.Validate(strfmt.NewFormats()); err != nil {
 		return httputils.NewHTTPError(err, http.StatusBadRequest)
 	}
 
-	name := mux.Vars(req)["name"]
-
-	logCreateOptions("container exec for "+name, config)
-
 	id, err := s.ContainerMgr.CreateExec(ctx, name, config)
 	if err != nil {
 		return err

diff --git a/apis/server/network_bridge.go b/apis/server/network_bridge.go
@@ -20,13 +20,14 @@ func (s *Server) createNetwork(ctx context.Context, rw http.ResponseWriter, req
 	if err := json.NewDecoder(req.Body).Decode(config); err != nil {
 		return httputils.NewHTTPError(err, http.StatusBadRequest)
 	}
+
+	logCreateOptions("network", config)
+
 	// validate request body
 	if err := config.Validate(strfmt.NewFormats()); err != nil {
 		return httputils.NewHTTPError(err, http.StatusBadRequest)
 	}
 
-	logCreateOptions("network", config)
-
 	network, err := s.NetworkMgr.Create(ctx, *config)
 	if err != nil {
 		return err

diff --git a/apis/server/volume_bridge.go b/apis/server/volume_bridge.go
@@ -20,13 +20,14 @@ func (s *Server) createVolume(ctx context.Context, rw http.ResponseWriter, req *
 	if err := json.NewDecoder(req.Body).Decode(config); err != nil {
 		return httputils.NewHTTPError(err, http.StatusBadRequest)
 	}
+
+	logCreateOptions("volume", config)
+
 	// validate request body
 	if err := config.Validate(strfmt.NewFormats()); err != nil {
 		return httputils.NewHTTPError(err, http.StatusBadRequest)
 	}
 
-	logCreateOptions("volume", config)
-
 	name := config.Name
 	driver := config.Driver
 	options := config.DriverOpts

diff --git a/apis/swagger.yml b/apis/swagger.yml
@@ -2598,7 +2598,6 @@ definitions:
           Microseconds of CPU time that the container can get in a CPU period."
         type: "integer"
         format: "int64"
-        minimum: 1000
         x-nullable: false
         x-omitempty: false
       CpuRealtimePeriod:
@@ -2660,10 +2659,10 @@ definitions:
         x-nullable: false
         x-omitempty: false
       MemorySwappiness:
-        description: "Tune a container's memory swappiness behavior. Accepts an integer between 0 and 100."
+        description: "Tune a container's memory swappiness behavior. Accepts an integer between 0 and 100. -1 is also accepted, as a legacy alias of 0."
         type: "integer"
         format: "int64"
-        minimum: 0
+        minimum: -1
         maximum: 100
         x-omitempty: false
       NanoCpus:

diff --git a/apis/types/resources.go b/apis/types/resources.go
diff --git a/daemon/mgr/container_validation.go b/daemon/mgr/container_validation.go
@@ -118,7 +118,7 @@ func validateResource(r *types.Resources, update bool) ([]string, error) {
 			r.MemorySwappiness = nil
 		}
 		if r.MemorySwappiness != nil && *r.MemorySwappiness != -1 && (*r.MemorySwappiness < 0 || *r.MemorySwappiness > 100) {
-			return warnings, fmt.Errorf("MemorySwappiness should in range [0, 100]")
+			return warnings, fmt.Errorf("MemorySwappiness should in range [0, 100] or -1 as a legacy alias of 0")
 		}
 		if r.OomKillDisable != nil && !cgroupInfo.Memory.OOMKillDisable {
 			logrus.Warn(OOMKillWarn)

diff --git a/test/cli_run_memory_test.go b/test/cli_run_memory_test.go
@@ -115,7 +115,7 @@ func (suite *PouchRunMemorySuite) TestRunWithMemoryswappiness(c *check.C) {
 		"--memory-swappiness", "-1",
 		"--name", cname, busyboxImage, "top")
 	DelContainerForceMultyTime(c, cname)
-	c.Assert(res.ExitCode, check.Equals, 1)
+	c.Assert(res.ExitCode, check.Equals, 0)
 
 	cname = "TestRunWithMemoryswappiness"
 	memory := "100m"