From 9cd17c65a4400e01a6a91db55a920b1a35cfffce Mon Sep 17 00:00:00 2001 From: YaoZengzeng Date: Mon, 2 Apr 2018 11:19:57 +0800 Subject: [PATCH 1/4] bugfix: specify both cmd and its args when create a container Signed-off-by: YaoZengzeng --- daemon/mgr/cri.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/daemon/mgr/cri.go b/daemon/mgr/cri.go index 91bec5d2c4..58a9469324 100644 --- a/daemon/mgr/cri.go +++ b/daemon/mgr/cri.go @@ -424,8 +424,7 @@ func (c *CriManager) CreateContainer(ctx context.Context, r *runtime.CreateConta } createConfig := &apitypes.ContainerCreateConfig{ ContainerConfig: apitypes.ContainerConfig{ - // TODO: maybe we should ditinguish cmd and entrypoint more clearly. - Cmd: config.Command, + Cmd: append(config.Command, config.Args...), Env: generateEnvList(config.GetEnvs()), Image: image, WorkingDir: config.WorkingDir, From 26dc5931070ca143116a482460165a89252740a2 Mon Sep 17 00:00:00 2001 From: Zou Rui <21751189@zju.edu.cn> Date: Mon, 2 Apr 2018 15:10:52 +0800 Subject: [PATCH 2/4] test: add mock test for image operations on client side Signed-off-by: Zou Rui <21751189@zju.edu.cn> --- client/image.go | 69 --------------------------------- client/image_inspect.go | 21 ++++++++++ client/image_inspect_test.go | 74 ++++++++++++++++++++++++++++++++++++ client/image_list.go | 22 +++++++++++ client/image_list_test.go | 70 ++++++++++++++++++++++++++++++++++ client/image_pull.go | 24 ++++++++++++ client/image_pull_test.go | 60 +++++++++++++++++++++++++++++ client/image_remove.go | 19 +++++++++ client/image_remove_test.go | 48 +++++++++++++++++++++++ 9 files changed, 338 insertions(+), 69 deletions(-) delete mode 100644 client/image.go create mode 100644 client/image_inspect.go create mode 100644 client/image_inspect_test.go create mode 100644 client/image_list.go create mode 100644 client/image_list_test.go create mode 100644 client/image_pull.go create mode 100644 client/image_pull_test.go create mode 100644 client/image_remove.go create mode 100644 client/image_remove_test.go diff --git a/client/image.go b/client/image.go deleted file mode 100644 index 3fc1c55e4a..0000000000 --- a/client/image.go +++ /dev/null @@ -1,69 +0,0 @@ -package client - -import ( - "context" - "io" - "net/url" - - "github.com/alibaba/pouch/apis/types" -) - -// ImageInspect requests daemon to inspect an image. -func (client *APIClient) ImageInspect(ctx context.Context, name string) (types.ImageInfo, error) { - image := types.ImageInfo{} - - resp, err := client.get(ctx, "/images/"+name+"/json", nil, nil) - if err != nil { - return image, err - } - - defer ensureCloseReader(resp) - err = decodeBody(&image, resp.Body) - return image, err -} - -// ImagePull requests daemon to pull an image from registry. -func (client *APIClient) ImagePull(ctx context.Context, name, tag, encodedAuth string) (io.ReadCloser, error) { - q := url.Values{} - q.Set("fromImage", name) - q.Set("tag", tag) - - headers := map[string][]string{} - if encodedAuth != "" { - headers["X-Registry-Auth"] = []string{encodedAuth} - } - resp, err := client.post(ctx, "/images/create", q, nil, headers) - if err != nil { - return nil, err - } - return resp.Body, nil -} - -// ImageList requests daemon to list all images -func (client *APIClient) ImageList(ctx context.Context) ([]types.ImageInfo, error) { - resp, err := client.get(ctx, "/images/json", nil, nil) - if err != nil { - return nil, err - } - - imageList := []types.ImageInfo{} - - err = decodeBody(&imageList, resp.Body) - ensureCloseReader(resp) - - return imageList, err - -} - -// ImageRemove deletes an image. -func (client *APIClient) ImageRemove(ctx context.Context, name string, force bool) error { - q := url.Values{} - if force { - q.Set("force", "true") - } - - resp, err := client.delete(ctx, "/images/"+name, q, nil) - ensureCloseReader(resp) - - return err -} diff --git a/client/image_inspect.go b/client/image_inspect.go new file mode 100644 index 0000000000..e7520b518c --- /dev/null +++ b/client/image_inspect.go @@ -0,0 +1,21 @@ +package client + +import ( + "context" + + "github.com/alibaba/pouch/apis/types" +) + +// ImageInspect requests daemon to inspect an image. +func (client *APIClient) ImageInspect(ctx context.Context, name string) (types.ImageInfo, error) { + image := types.ImageInfo{} + + resp, err := client.get(ctx, "/images/"+name+"/json", nil, nil) + if err != nil { + return image, err + } + + defer ensureCloseReader(resp) + err = decodeBody(&image, resp.Body) + return image, err +} diff --git a/client/image_inspect_test.go b/client/image_inspect_test.go new file mode 100644 index 0000000000..ce174a2359 --- /dev/null +++ b/client/image_inspect_test.go @@ -0,0 +1,74 @@ +package client + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + "strings" + "testing" + + "github.com/alibaba/pouch/apis/types" + + "github.com/stretchr/testify/assert" +) + +func TestImageInspectServerError(t *testing.T) { + client := &APIClient{ + HTTPCli: newMockClient(errorMockResponse(http.StatusInternalServerError, "Server error")), + } + _, err := client.ImageInspect(context.Background(), "image_id") + if err == nil || !strings.Contains(err.Error(), "Server error") { + t.Fatalf("expected a Server Error, got %v", err) + } +} + +func TestImageInspectNotFoundError(t *testing.T) { + client := &APIClient{ + HTTPCli: newMockClient(errorMockResponse(http.StatusConflict, "Not Found")), + } + _, err := client.ImageInspect(context.Background(), "no image") + if err == nil || !strings.Contains(err.Error(), "Not Found") { + t.Fatalf("expected a Server Error, got %v", err) + } +} + +func TestImageInspect(t *testing.T) { + expectedURL := "/images/image_id" + + httpClient := newMockClient(func(req *http.Request) (*http.Response, error) { + if !strings.HasPrefix(req.URL.Path, expectedURL) { + return nil, fmt.Errorf("Expected URL '%s', got '%s'", expectedURL, req.URL) + } + if req.Method != "GET" { + return nil, fmt.Errorf("expected GET method, got %s", req.Method) + } + + imageInspectResp, err := json.Marshal(types.ImageInfo{ + ID: "1", + Size: int64(94), + }) + if err != nil { + return nil, err + } + + return &http.Response{ + StatusCode: http.StatusOK, + Body: ioutil.NopCloser(bytes.NewReader([]byte(imageInspectResp))), + }, nil + }) + + client := &APIClient{ + HTTPCli: httpClient, + } + + image, err := client.ImageInspect(context.Background(), "image_id") + if err != nil { + t.Fatal(err) + } + assert.Equal(t, image.ID, "1") + assert.Equal(t, image.Size, int64(94)) + +} diff --git a/client/image_list.go b/client/image_list.go new file mode 100644 index 0000000000..bbf34516e7 --- /dev/null +++ b/client/image_list.go @@ -0,0 +1,22 @@ +package client + +import ( + "context" + "github.com/alibaba/pouch/apis/types" +) + +// ImageList requests daemon to list all images +func (client *APIClient) ImageList(ctx context.Context) ([]types.ImageInfo, error) { + resp, err := client.get(ctx, "/images/json", nil, nil) + if err != nil { + return nil, err + } + + imageList := []types.ImageInfo{} + + err = decodeBody(&imageList, resp.Body) + ensureCloseReader(resp) + + return imageList, err + +} diff --git a/client/image_list_test.go b/client/image_list_test.go new file mode 100644 index 0000000000..1cfa6fc35c --- /dev/null +++ b/client/image_list_test.go @@ -0,0 +1,70 @@ +package client + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + "strings" + "testing" + + "github.com/alibaba/pouch/apis/types" + + "github.com/stretchr/testify/assert" +) + +func TestImageListServerError(t *testing.T) { + client := &APIClient{ + HTTPCli: newMockClient(errorMockResponse(http.StatusInternalServerError, "Server error")), + } + _, err := client.ImageList(context.Background()) + if err == nil || !strings.Contains(err.Error(), "Server error") { + t.Fatalf("expected a Server Error, got %v", err) + } +} + +func TestImageList(t *testing.T) { + expectedURL := "/images" + + httpClient := newMockClient(func(req *http.Request) (*http.Response, error) { + if !strings.HasPrefix(req.URL.Path, expectedURL) { + return nil, fmt.Errorf("Expected URL '%s', got '%s'", expectedURL, req.URL) + } + if req.Method != "GET" { + return nil, fmt.Errorf("expected GET method, got %s", req.Method) + } + + imageListResp, err := json.Marshal([]types.ImageInfo{ + { + ID: "1", + Size: 703, + Os: "CentOS", + }, + { + ID: "2", + Size: 44, + Os: "Ubuntu TLS 16.04", + }, + }) + if err != nil { + return nil, err + } + + return &http.Response{ + StatusCode: http.StatusOK, + Body: ioutil.NopCloser(bytes.NewReader([]byte(imageListResp))), + }, nil + }) + + client := &APIClient{ + HTTPCli: httpClient, + } + + image, err := client.ImageList(context.Background()) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, len(image), 2) +} diff --git a/client/image_pull.go b/client/image_pull.go new file mode 100644 index 0000000000..de33c49c08 --- /dev/null +++ b/client/image_pull.go @@ -0,0 +1,24 @@ +package client + +import ( + "context" + "io" + "net/url" +) + +// ImagePull requests daemon to pull an image from registry. +func (client *APIClient) ImagePull(ctx context.Context, name, tag, encodedAuth string) (io.ReadCloser, error) { + q := url.Values{} + q.Set("fromImage", name) + q.Set("tag", tag) + + headers := map[string][]string{} + if encodedAuth != "" { + headers["X-Registry-Auth"] = []string{encodedAuth} + } + resp, err := client.post(ctx, "/images/create", q, nil, headers) + if err != nil { + return nil, err + } + return resp.Body, nil +} diff --git a/client/image_pull_test.go b/client/image_pull_test.go new file mode 100644 index 0000000000..011c03f70b --- /dev/null +++ b/client/image_pull_test.go @@ -0,0 +1,60 @@ +package client + +import ( + "bytes" + "context" + "fmt" + "io/ioutil" + "net/http" + "strings" + "testing" +) + +func TestImagePullServerError(t *testing.T) { + client := &APIClient{ + HTTPCli: newMockClient(errorMockResponse(http.StatusInternalServerError, "Server error")), + } + _, err := client.ImagePull(context.Background(), "image_name", "image_tag", "auth") + if err == nil || !strings.Contains(err.Error(), "Server error") { + t.Fatalf("expected a Server Error, got %v", err) + } +} + +func TestImagePullWrongError(t *testing.T) { + client := &APIClient{ + HTTPCli: newMockClient(errorMockResponse(http.StatusNotFound, "Image not found")), + } + _, err := client.ImagePull(context.Background(), "image_name", "image_tag", "auth") + if err == nil || !strings.Contains(err.Error(), "Image not found") { + t.Fatalf("expected an Image Not Found Error, got %v", err) + } +} + +func TestImagePull(t *testing.T) { + expectedURL := "/images/create" + + httpClient := newMockClient(func(req *http.Request) (*http.Response, error) { + if !strings.HasPrefix(req.URL.Path, expectedURL) { + return nil, fmt.Errorf("Expected URL '%s', got '%s'", expectedURL, req.URL) + } + + if req.Method != "POST" { + return nil, fmt.Errorf("Expected POST method, got %s", req.Method) + } + + return &http.Response{ + StatusCode: http.StatusOK, + Body: ioutil.NopCloser(bytes.NewReader([]byte(""))), + }, nil + }) + + client := &APIClient{ + HTTPCli: httpClient, + } + + _, err := client.ImagePull(context.Background(), "image_name", "image_tag", "auth") + if err != nil { + t.Fatal(err) + } + +} diff --git a/client/image_remove.go b/client/image_remove.go new file mode 100644 index 0000000000..612bb832a5 --- /dev/null +++ b/client/image_remove.go @@ -0,0 +1,19 @@ +package client + +import ( + "context" + "net/url" +) + +// ImageRemove deletes an image. +func (client *APIClient) ImageRemove(ctx context.Context, name string, force bool) error { + q := url.Values{} + if force { + q.Set("force", "true") + } + + resp, err := client.delete(ctx, "/images/"+name, q, nil) + ensureCloseReader(resp) + + return err +} diff --git a/client/image_remove_test.go b/client/image_remove_test.go new file mode 100644 index 0000000000..77302928b0 --- /dev/null +++ b/client/image_remove_test.go @@ -0,0 +1,48 @@ +package client + +import ( + "bytes" + "context" + "fmt" + "io/ioutil" + "net/http" + "strings" + "testing" +) + +func TestImageRemoveNotFoundError(t *testing.T) { + client := &APIClient{ + HTTPCli: newMockClient(errorMockResponse(http.StatusNotFound, "Not Found")), + } + err := client.ImageRemove(context.Background(), "no network", true) + if err == nil || !strings.Contains(err.Error(), "Not Found") { + t.Fatalf("expected a Not Found Error, got %v", err) + } +} + +func TestImageRemove(t *testing.T) { + expectedURL := "/images/image_id" + + httpClient := newMockClient(func(req *http.Request) (*http.Response, error) { + if !strings.HasPrefix(req.URL.Path, expectedURL) { + return nil, fmt.Errorf("Expected URL '%s', got '%s'", expectedURL, req.URL) + } + if req.Method != "DELETE" { + return nil, fmt.Errorf("expected DELETE method, got %s", req.Method) + } + + return &http.Response{ + StatusCode: http.StatusNoContent, + Body: ioutil.NopCloser(bytes.NewReader([]byte(""))), + }, nil + }) + + client := &APIClient{ + HTTPCli: httpClient, + } + + err := client.ImageRemove(context.Background(), "image_id", false) + if err != nil { + t.Fatal(err) + } +} From 4690a0082a8046fc3ede9a85bc12c702c74c5ac7 Mon Sep 17 00:00:00 2001 From: Michael Wan Date: Mon, 2 Apr 2018 08:02:28 -0400 Subject: [PATCH 3/4] bugfix: restful api url should support both with version info and without version info Signed-off-by: Michael Wan --- apis/server/router.go | 2 ++ test/api_version_parameter_test.go | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/apis/server/router.go b/apis/server/router.go index 8b4e990b8a..f80e8a8612 100644 --- a/apis/server/router.go +++ b/apis/server/router.go @@ -74,6 +74,7 @@ func initRoute(s *Server) http.Handler { // metrics r.Path(versionMatcher + "/metrics").Methods(http.MethodGet).Handler(prometheus.Handler()) + r.Path("/metrics").Methods(http.MethodGet).Handler(prometheus.Handler()) if s.Config.Debug { profilerSetup(r) @@ -83,6 +84,7 @@ func initRoute(s *Server) http.Handler { func addRoute(r *mux.Router, mothod string, path string, f func(ctx context.Context, rw http.ResponseWriter, req *http.Request) error) { r.Path(versionMatcher + path).Methods(mothod).Handler(filter(f)) + r.Path(path).Methods(mothod).Handler(filter(f)) } func profilerSetup(mainRouter *mux.Router) { diff --git a/test/api_version_parameter_test.go b/test/api_version_parameter_test.go index 49a97550e8..66decb375f 100644 --- a/test/api_version_parameter_test.go +++ b/test/api_version_parameter_test.go @@ -25,6 +25,7 @@ func (suite *APIVersionSuite) SetUpTest(c *check.C) { } // TestNoVersionParamsInURL test api url not contains version info. +// Pouch api url support with or without version info. func (suite *APIVersionSuite) TestNoVersionParamsInURL(c *check.C) { cname := "TestCreateURLNoVersionInfo" @@ -55,5 +56,5 @@ func (suite *APIVersionSuite) TestNoVersionParamsInURL(c *check.C) { resp, err := apiClient.HTTPCli.Do(req) c.Assert(err, check.IsNil) - CheckRespStatus(c, resp, 404) + CheckRespStatus(c, resp, 201) } From 0dfe9eef28961dc430b466a281c41ecf3517b41c Mon Sep 17 00:00:00 2001 From: Dewey-Ding Date: Mon, 2 Apr 2018 22:50:04 +0800 Subject: [PATCH 4/4] test:add container get test Signed-off-by: Dewey-Ding --- client/container.go | 14 ---------- client/container_get.go | 21 +++++++++++++++ client/container_get_test.go | 52 ++++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 14 deletions(-) create mode 100644 client/container_get.go create mode 100644 client/container_get_test.go diff --git a/client/container.go b/client/container.go index bbc00f853f..290add55aa 100644 --- a/client/container.go +++ b/client/container.go @@ -121,20 +121,6 @@ func (client *APIClient) ContainerStartExec(ctx context.Context, execid string, return client.hijack(ctx, "/exec/"+execid+"/start", url.Values{}, config, header) } -// ContainerGet returns the detailed information of container. -func (client *APIClient) ContainerGet(ctx context.Context, name string) (*types.ContainerJSON, error) { - resp, err := client.get(ctx, "/containers/"+name+"/json", nil, nil) - if err != nil { - return nil, err - } - - container := types.ContainerJSON{} - err = decodeBody(&container, resp.Body) - ensureCloseReader(resp) - - return &container, err -} - // ContainerRestart restarts a running container. func (client *APIClient) ContainerRestart(ctx context.Context, name string, timeout string) error { q := url.Values{} diff --git a/client/container_get.go b/client/container_get.go new file mode 100644 index 0000000000..3350a7b723 --- /dev/null +++ b/client/container_get.go @@ -0,0 +1,21 @@ +package client + +import ( + "context" + + "github.com/alibaba/pouch/apis/types" +) + +// ContainerGet returns the detailed information of container. +func (client *APIClient) ContainerGet(ctx context.Context, name string) (*types.ContainerJSON, error) { + resp, err := client.get(ctx, "/containers/"+name+"/json", nil, nil) + if err != nil { + return nil, err + } + + container := types.ContainerJSON{} + err = decodeBody(&container, resp.Body) + ensureCloseReader(resp) + + return &container, err +} diff --git a/client/container_get_test.go b/client/container_get_test.go new file mode 100644 index 0000000000..7cec43e4bc --- /dev/null +++ b/client/container_get_test.go @@ -0,0 +1,52 @@ +package client + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + "strings" + "testing" + + "github.com/alibaba/pouch/apis/types" +) + +func TestContainerGetError(t *testing.T) { + client := &APIClient{ + HTTPCli: newMockClient(errorMockResponse(http.StatusInternalServerError, "Server error")), + } + _, err := client.ContainerGet(context.Background(), "nothing") + if err == nil || !strings.Contains(err.Error(), "Server error") { + t.Fatalf("expected a Server Error, got %v", err) + } +} + +func TestContainerGet(t *testing.T) { + expectedURL := "/containers/container_id/json" + + httpClient := newMockClient(func(req *http.Request) (*http.Response, error) { + if !strings.HasPrefix(req.URL.Path, expectedURL) { + return nil, fmt.Errorf("Expected URL '%s', got '%s'", expectedURL, req.URL) + } + containerJSON := types.ContainerJSON{ + Driver: "Driver", + Image: "Image", + } + b, err := json.Marshal(containerJSON) + if err != nil { + return nil, err + } + return &http.Response{ + StatusCode: http.StatusOK, + Body: ioutil.NopCloser(bytes.NewReader([]byte(b))), + }, nil + }) + client := &APIClient{ + HTTPCli: httpClient, + } + if _, err := client.ContainerGet(context.Background(), "container_id"); err != nil { + t.Fatal(err) + } +}