From 61a494d303839d67875680a43dcf71955864e0d5 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Sun, 22 Jun 2014 14:18:01 -0700 Subject: [PATCH 1/5] First step of combination. --- cmd/cloudcfg/cloudcfg.go | 18 ++--- cmd/controller-manager/controller-manager.go | 7 +- cmd/integration/integration.go | 9 +-- cmd/localkube/localkube.go | 7 +- pkg/client/client.go | 71 ++++++++++------- pkg/client/client_test.go | 76 ++++++++++++------- pkg/{cloudcfg => client}/request.go | 29 ++----- pkg/{cloudcfg => client}/request_test.go | 55 ++++++++++++-- pkg/cloudcfg/cloudcfg.go | 7 +- pkg/cloudcfg/cloudcfg_test.go | 50 +----------- pkg/controller/replication_controller_test.go | 56 +++++--------- 11 files changed, 189 insertions(+), 196 deletions(-) rename pkg/{cloudcfg => client}/request.go (86%) rename pkg/{cloudcfg => client}/request_test.go (65%) diff --git a/cmd/cloudcfg/cloudcfg.go b/cmd/cloudcfg/cloudcfg.go index c27d7b26a9..dabea2fccb 100644 --- a/cmd/cloudcfg/cloudcfg.go +++ b/cmd/cloudcfg/cloudcfg.go @@ -147,7 +147,7 @@ func executeAPIRequest(method string, auth *kube_client.AuthInfo) bool { return false } - s := cloudcfg.New(*httpServer, auth) + s := kube_client.New(*httpServer, auth) r := s.Verb(verb). Path("api/v1beta1"). Path(parseStorage()). @@ -187,18 +187,16 @@ func executeControllerRequest(method string, auth *kube_client.AuthInfo) bool { return flag.Arg(1) } + c := kube_client.New(*httpServer, auth) + var err error switch method { case "stop": - err = cloudcfg.StopController(parseController(), kube_client.Client{Host: *httpServer, Auth: auth}) + err = cloudcfg.StopController(parseController(), c) case "rm": - err = cloudcfg.DeleteController(parseController(), kube_client.Client{Host: *httpServer, Auth: auth}) + err = cloudcfg.DeleteController(parseController(), c) case "rollingupdate": - client := &kube_client.Client{ - Host: *httpServer, - Auth: auth, - } - err = cloudcfg.Update(parseController(), client, *updatePeriod) + err = cloudcfg.Update(parseController(), c, *updatePeriod) case "run": if len(flag.Args()) != 4 { log.Fatal("usage: cloudcfg [OPTIONS] run ") @@ -209,7 +207,7 @@ func executeControllerRequest(method string, auth *kube_client.AuthInfo) bool { if err != nil { log.Fatalf("Error parsing replicas: %#v", err) } - err = cloudcfg.RunController(image, name, replicas, kube_client.Client{Host: *httpServer, Auth: auth}, *portSpec, *servicePort) + err = cloudcfg.RunController(image, name, replicas, c, *portSpec, *servicePort) case "resize": args := flag.Args() if len(args) < 3 { @@ -220,7 +218,7 @@ func executeControllerRequest(method string, auth *kube_client.AuthInfo) bool { if err != nil { log.Fatalf("Error parsing replicas: %#v", err) } - err = cloudcfg.ResizeController(name, replicas, kube_client.Client{Host: *httpServer, Auth: auth}) + err = cloudcfg.ResizeController(name, replicas, c) default: return false } diff --git a/cmd/controller-manager/controller-manager.go b/cmd/controller-manager/controller-manager.go index 7b24f1a429..418a6aaba8 100644 --- a/cmd/controller-manager/controller-manager.go +++ b/cmd/controller-manager/controller-manager.go @@ -47,10 +47,9 @@ func main() { // Set up logger for etcd client etcd.SetLogger(log.New(os.Stderr, "etcd ", log.LstdFlags)) - controllerManager := controller.MakeReplicationManager(etcd.NewClient([]string{*etcd_servers}), - client.Client{ - Host: "http://" + *master, - }) + controllerManager := controller.MakeReplicationManager( + etcd.NewClient([]string{*etcd_servers}), + client.New("http://"+*master, nil)) controllerManager.Run(10 * time.Second) select {} diff --git a/cmd/integration/integration.go b/cmd/integration/integration.go index 9a5c49196c..845b6363b3 100644 --- a/cmd/integration/integration.go +++ b/cmd/integration/integration.go @@ -49,10 +49,7 @@ func main() { }, "/api/v1beta1") server := httptest.NewServer(apiserver) - controllerManager := controller.MakeReplicationManager(etcd.NewClient(servers), - client.Client{ - Host: server.URL, - }) + controllerManager := controller.MakeReplicationManager(etcd.NewClient(servers), client.New(server.URL, nil)) controllerManager.Run(10 * time.Second) @@ -61,9 +58,7 @@ func main() { // Wait for the synchronization threads to come up. time.Sleep(time.Second * 10) - kubeClient := client.Client{ - Host: server.URL, - } + kubeClient := client.New(server.URL, nil) data, err := ioutil.ReadFile("api/examples/controller.json") if err != nil { log.Fatalf("Unexpected error: %#v", err) diff --git a/cmd/localkube/localkube.go b/cmd/localkube/localkube.go index 804e609ae3..282ecb726a 100644 --- a/cmd/localkube/localkube.go +++ b/cmd/localkube/localkube.go @@ -86,10 +86,9 @@ func api_server() { // Starts up a controller manager. Never returns. func controller_manager() { - controllerManager := controller.MakeReplicationManager(etcd.NewClient([]string{*etcd_server}), - client.Client{ - Host: fmt.Sprintf("http://%s:%d", *master_address, *master_port), - }) + controllerManager := controller.MakeReplicationManager( + etcd.NewClient([]string{*etcd_server}), + client.New(fmt.Sprintf("http://%s:%d", *master_address, *master_port), nil)) controllerManager.Run(20 * time.Second) select {} diff --git a/pkg/client/client.go b/pkg/client/client.go index c51a877ab9..2fc8cb694e 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -59,47 +59,62 @@ type AuthInfo struct { // Client is the actual implementation of a Kubernetes client. // Host is the http://... base for the URL type Client struct { - Host string - Auth *AuthInfo + host string + auth *AuthInfo httpClient *http.Client } -// Underlying base implementation of performing a request. -// method is the HTTP method (e.g. "GET") -// path is the path on the host to hit -// requestBody is the body of the request. Can be nil. -// target the interface to marshal the JSON response into. Can be nil. -func (client Client) rawRequest(method, path string, requestBody io.Reader, target interface{}) ([]byte, error) { - request, err := http.NewRequest(method, client.makeURL(path), requestBody) +// Create a new client object. +func New(host string, auth *AuthInfo) *Client { + return &Client{ + auth: auth, + host: host, + httpClient: &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, + }, + }, + }, + } +} + +// Execute a request, adds authentication (if auth != nil), and HTTPS cert ignoring. +func (c *Client) doRequest(request *http.Request) ([]byte, error) { + if c.auth != nil { + request.SetBasicAuth(c.auth.User, c.auth.Password) + } + response, err := c.httpClient.Do(request) if err != nil { return []byte{}, err } - if client.Auth != nil { - request.SetBasicAuth(client.Auth.User, client.Auth.Password) - } - tr := &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, - } - var httpClient *http.Client - if client.httpClient != nil { - httpClient = client.httpClient - } else { - httpClient = &http.Client{Transport: tr} - } - response, err := httpClient.Do(request) - if err != nil { - return nil, err - } defer response.Body.Close() body, err := ioutil.ReadAll(response.Body) if err != nil { return body, err } if response.StatusCode < http.StatusOK || response.StatusCode > http.StatusPartialContent { - return nil, fmt.Errorf("request [%s %s] failed (%d) %s: %s", method, client.makeURL(path), response.StatusCode, response.Status, string(body)) + return nil, fmt.Errorf("request [%#v] failed (%d) %s: %s", request, response.StatusCode, response.Status, string(body)) + } + return body, err +} + +// Underlying base implementation of performing a request. +// method is the HTTP method (e.g. "GET") +// path is the path on the host to hit +// requestBody is the body of the request. Can be nil. +// target the interface to marshal the JSON response into. Can be nil. +func (c *Client) rawRequest(method, path string, requestBody io.Reader, target interface{}) ([]byte, error) { + request, err := http.NewRequest(method, c.makeURL(path), requestBody) + if err != nil { + return []byte{}, err + } + body, err := c.doRequest(request) + if err != nil { + return body, err } if target != nil { - err = json.Unmarshal(body, target) + err = api.DecodeInto(body, target) } if err != nil { log.Printf("Failed to parse: %s\n", string(body)) @@ -109,7 +124,7 @@ func (client Client) rawRequest(method, path string, requestBody io.Reader, targ } func (client Client) makeURL(path string) string { - return client.Host + "/api/v1beta1/" + path + return client.host + "/api/v1beta1/" + path } // EncodeSelector transforms a selector expressed as a key/value map, into a diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index bbdf20edf3..07061e980e 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -38,7 +38,7 @@ func makeUrl(suffix string) string { func TestListEmptyPods(t *testing.T) { c := &TestClient{ - Request: Request{Method: "GET", Path: "/pods"}, + Request: testRequest{Method: "GET", Path: "/pods"}, Response: Response{StatusCode: 200, Body: api.PodList{}}, } podList, err := c.Setup().ListPods(nil) @@ -47,7 +47,7 @@ func TestListEmptyPods(t *testing.T) { func TestListPods(t *testing.T) { c := &TestClient{ - Request: Request{Method: "GET", Path: "/pods"}, + Request: testRequest{Method: "GET", Path: "/pods"}, Response: Response{StatusCode: 200, Body: api.PodList{ Items: []api.Pod{ @@ -76,7 +76,7 @@ func validateLabels(a, b string) bool { func TestListPodsLabels(t *testing.T) { c := &TestClient{ - Request: Request{Method: "GET", Path: "/pods", Query: url.Values{"labels": []string{"foo=bar,name=baz"}}}, + Request: testRequest{Method: "GET", Path: "/pods", Query: url.Values{"labels": []string{"foo=bar,name=baz"}}}, Response: Response{ StatusCode: 200, Body: api.PodList{ @@ -103,7 +103,7 @@ func TestListPodsLabels(t *testing.T) { func TestGetPod(t *testing.T) { c := &TestClient{ - Request: Request{Method: "GET", Path: "/pods/foo"}, + Request: testRequest{Method: "GET", Path: "/pods/foo"}, Response: Response{ StatusCode: 200, Body: api.Pod{ @@ -123,7 +123,7 @@ func TestGetPod(t *testing.T) { func TestDeletePod(t *testing.T) { c := &TestClient{ - Request: Request{Method: "DELETE", Path: "/pods/foo"}, + Request: testRequest{Method: "DELETE", Path: "/pods/foo"}, Response: Response{StatusCode: 200}, } err := c.Setup().DeletePod("foo") @@ -141,7 +141,7 @@ func TestCreatePod(t *testing.T) { }, } c := &TestClient{ - Request: Request{Method: "POST", Path: "/pods", Body: requestPod}, + Request: testRequest{Method: "POST", Path: "/pods", Body: requestPod}, Response: Response{ StatusCode: 200, Body: requestPod, @@ -163,7 +163,7 @@ func TestUpdatePod(t *testing.T) { }, } c := &TestClient{ - Request: Request{Method: "PUT", Path: "/pods/foo"}, + Request: testRequest{Method: "PUT", Path: "/pods/foo"}, Response: Response{StatusCode: 200, Body: requestPod}, } receivedPod, err := c.Setup().UpdatePod(requestPod) @@ -172,7 +172,7 @@ func TestUpdatePod(t *testing.T) { func TestGetController(t *testing.T) { c := &TestClient{ - Request: Request{Method: "GET", Path: "/replicationControllers/foo"}, + Request: testRequest{Method: "GET", Path: "/replicationControllers/foo"}, Response: Response{ StatusCode: 200, Body: api.ReplicationController{ @@ -200,7 +200,7 @@ func TestUpdateController(t *testing.T) { }, } c := &TestClient{ - Request: Request{Method: "PUT", Path: "/replicationControllers/foo"}, + Request: testRequest{Method: "PUT", Path: "/replicationControllers/foo"}, Response: Response{ StatusCode: 200, Body: api.ReplicationController{ @@ -223,7 +223,7 @@ func TestUpdateController(t *testing.T) { func TestDeleteController(t *testing.T) { c := &TestClient{ - Request: Request{Method: "DELETE", Path: "/replicationControllers/foo"}, + Request: testRequest{Method: "DELETE", Path: "/replicationControllers/foo"}, Response: Response{StatusCode: 200}, } err := c.Setup().DeleteReplicationController("foo") @@ -237,7 +237,7 @@ func TestCreateController(t *testing.T) { }, } c := &TestClient{ - Request: Request{Method: "POST", Path: "/replicationControllers", Body: requestController}, + Request: testRequest{Method: "POST", Path: "/replicationControllers", Body: requestController}, Response: Response{ StatusCode: 200, Body: api.ReplicationController{ @@ -267,7 +267,7 @@ func body(obj interface{}, raw *string) *string { return raw } -type Request struct { +type testRequest struct { Method string Path string Header string @@ -284,7 +284,7 @@ type Response struct { type TestClient struct { *Client - Request Request + Request testRequest Response Response Error bool server *httptest.Server @@ -306,9 +306,9 @@ func (c *TestClient) Setup() *TestClient { } c.server = httptest.NewTLSServer(c.handler) if c.Client == nil { - c.Client = &Client{} + c.Client = New("", nil) } - c.Client.Host = c.server.URL + c.Client.host = c.server.URL c.QueryValidator = map[string]func(string, string) bool{} return c } @@ -355,7 +355,7 @@ func (c *TestClient) Validate(t *testing.T, received interface{}, err error) { func TestGetService(t *testing.T) { c := &TestClient{ - Request: Request{Method: "GET", Path: "/services/1"}, + Request: testRequest{Method: "GET", Path: "/services/1"}, Response: Response{StatusCode: 200, Body: &api.Service{JSONBase: api.JSONBase{ID: "service-1"}}}, } response, err := c.Setup().GetService("1") @@ -364,7 +364,7 @@ func TestGetService(t *testing.T) { func TestCreateService(t *testing.T) { c := (&TestClient{ - Request: Request{Method: "POST", Path: "/services", Body: &api.Service{JSONBase: api.JSONBase{ID: "service-1"}}}, + Request: testRequest{Method: "POST", Path: "/services", Body: &api.Service{JSONBase: api.JSONBase{ID: "service-1"}}}, Response: Response{StatusCode: 200, Body: &api.Service{JSONBase: api.JSONBase{ID: "service-1"}}}, }).Setup() response, err := c.Setup().CreateService(api.Service{JSONBase: api.JSONBase{ID: "service-1"}}) @@ -373,7 +373,7 @@ func TestCreateService(t *testing.T) { func TestUpdateService(t *testing.T) { c := &TestClient{ - Request: Request{Method: "PUT", Path: "/services/service-1", Body: &api.Service{JSONBase: api.JSONBase{ID: "service-1"}}}, + Request: testRequest{Method: "PUT", Path: "/services/service-1", Body: &api.Service{JSONBase: api.JSONBase{ID: "service-1"}}}, Response: Response{StatusCode: 200, Body: &api.Service{JSONBase: api.JSONBase{ID: "service-1"}}}, } response, err := c.Setup().UpdateService(api.Service{JSONBase: api.JSONBase{ID: "service-1"}}) @@ -382,7 +382,7 @@ func TestUpdateService(t *testing.T) { func TestDeleteService(t *testing.T) { c := &TestClient{ - Request: Request{Method: "DELETE", Path: "/services/1"}, + Request: testRequest{Method: "DELETE", Path: "/services/1"}, Response: Response{StatusCode: 200}, } err := c.Setup().DeleteService("1") @@ -391,16 +391,40 @@ func TestDeleteService(t *testing.T) { func TestMakeRequest(t *testing.T) { testClients := []TestClient{ - {Request: Request{Method: "GET", Path: "/good"}, Response: Response{StatusCode: 200}}, - {Request: Request{Method: "GET", Path: "/bad%ZZ"}, Error: true}, - {Client: &Client{Auth: &AuthInfo{"foo", "bar"}}, Request: Request{Method: "GET", Path: "/auth", Header: "Authorization"}, Response: Response{StatusCode: 200}}, - {Client: &Client{httpClient: http.DefaultClient}, Request: Request{Method: "GET", Path: "/nocertificate"}, Error: true}, - {Request: Request{Method: "GET", Path: "/error"}, Response: Response{StatusCode: 500}, Error: true}, - {Request: Request{Method: "POST", Path: "/faildecode"}, Response: Response{StatusCode: 200, Body: "aaaaa"}, Target: &struct{}{}, Error: true}, - {Request: Request{Method: "GET", Path: "/failread"}, Response: Response{StatusCode: 200, Body: "aaaaa"}, Target: &struct{}{}, Error: true}, + {Request: testRequest{Method: "GET", Path: "/good"}, Response: Response{StatusCode: 200}}, + {Request: testRequest{Method: "GET", Path: "/bad%ZZ"}, Error: true}, + {Client: New("", &AuthInfo{"foo", "bar"}), Request: testRequest{Method: "GET", Path: "/auth", Header: "Authorization"}, Response: Response{StatusCode: 200}}, + {Client: &Client{httpClient: http.DefaultClient}, Request: testRequest{Method: "GET", Path: "/nocertificate"}, Error: true}, + {Request: testRequest{Method: "GET", Path: "/error"}, Response: Response{StatusCode: 500}, Error: true}, + {Request: testRequest{Method: "POST", Path: "/faildecode"}, Response: Response{StatusCode: 200, Body: "aaaaa"}, Target: &struct{}{}, Error: true}, + {Request: testRequest{Method: "GET", Path: "/failread"}, Response: Response{StatusCode: 200, Body: "aaaaa"}, Target: &struct{}{}, Error: true}, } for _, c := range testClients { response, err := c.Setup().rawRequest(c.Request.Method, c.Request.Path[1:], nil, c.Target) c.Validate(t, response, err) } } + +func TestDoRequest(t *testing.T) { + expectedBody := `{ "items": []}` + fakeHandler := util.FakeHandler{ + StatusCode: 200, + ResponseBody: expectedBody, + T: t, + } + testServer := httptest.NewTLSServer(&fakeHandler) + request, _ := http.NewRequest("GET", testServer.URL+"/foo/bar", nil) + auth := AuthInfo{User: "user", Password: "pass"} + c := New(testServer.URL, &auth) + body, err := c.doRequest(request) + if request.Header["Authorization"] == nil { + t.Errorf("Request is missing authorization header: %#v", *request) + } + if err != nil { + t.Error("Unexpected error") + } + if string(body) != expectedBody { + t.Errorf("Expected body: '%s', saw: '%s'", expectedBody, body) + } + fakeHandler.ValidateRequest(t, "/foo/bar", "GET", nil) +} diff --git a/pkg/cloudcfg/request.go b/pkg/client/request.go similarity index 86% rename from pkg/cloudcfg/request.go rename to pkg/client/request.go index cea3d98a0a..3c711fe376 100644 --- a/pkg/cloudcfg/request.go +++ b/pkg/client/request.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package cloudcfg +package client import ( "bytes" @@ -26,46 +26,33 @@ import ( "time" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/GoogleCloudPlatform/kubernetes/pkg/client" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" ) // Server contains info locating a kubernetes api server. // Example usage: // auth, err := LoadAuth(filename) -// s := New(url, auth) -// resp, err := s.Verb("GET"). +// c := New(url, auth) +// resp, err := c.Verb("GET"). // Path("api/v1beta1"). // Path("pods"). // Selector("area=staging"). // Timeout(10*time.Second). // Do() // list, ok := resp.(api.PodList) -type Server struct { - auth *client.AuthInfo - rawUrl string -} - -// Create a new server object. -func New(serverUrl string, auth *client.AuthInfo) *Server { - return &Server{ - auth: auth, - rawUrl: serverUrl, - } -} // Begin a request with a verb (GET, POST, PUT, DELETE) -func (s *Server) Verb(verb string) *Request { +func (c *Client) Verb(verb string) *Request { return &Request{ verb: verb, - s: s, + c: c, path: "/", } } // Request allows for building up a request to a server in a chained fashion. type Request struct { - s *Server + c *Client err error verb string path string @@ -118,7 +105,7 @@ func (r *Request) Do() (interface{}, error) { if r.err != nil { return nil, r.err } - finalUrl := r.s.rawUrl + r.path + finalUrl := r.c.host + r.path query := url.Values{} if r.selector != nil { query.Add("labels", r.selector.String()) @@ -150,7 +137,7 @@ func (r *Request) Do() (interface{}, error) { if err != nil { return nil, err } - str, err := doRequest(req, r.s.auth) + str, err := r.c.doRequest(req) if err != nil { return nil, err } diff --git a/pkg/cloudcfg/request_test.go b/pkg/client/request_test.go similarity index 65% rename from pkg/cloudcfg/request_test.go rename to pkg/client/request_test.go index d05fbe6316..41070591cd 100644 --- a/pkg/cloudcfg/request_test.go +++ b/pkg/client/request_test.go @@ -14,16 +14,16 @@ See the License for the specific language governing permissions and limitations under the License. */ -package cloudcfg +package client import ( + "io/ioutil" "net/http/httptest" "reflect" "testing" "time" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/GoogleCloudPlatform/kubernetes/pkg/client" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) @@ -37,7 +37,7 @@ func TestDoRequestNewWay(t *testing.T) { T: t, } testServer := httptest.NewTLSServer(&fakeHandler) - auth := client.AuthInfo{User: "user", Password: "pass"} + auth := AuthInfo{User: "user", Password: "pass"} s := New(testServer.URL, &auth) obj, err := s.Verb("POST"). Path("foo/bar"). @@ -65,7 +65,7 @@ func TestDoRequestNewWay(t *testing.T) { } func TestDoRequestNewWayObj(t *testing.T) { - reqObj := &api.Pod{} + reqObj := &api.Pod{JSONBase: api.JSONBase{ID: "foo"}} reqBodyExpected, _ := api.Encode(reqObj) expectedObj := &api.Service{Port: 12345} expectedBody, _ := api.Encode(expectedObj) @@ -75,7 +75,7 @@ func TestDoRequestNewWayObj(t *testing.T) { T: t, } testServer := httptest.NewTLSServer(&fakeHandler) - auth := client.AuthInfo{User: "user", Password: "pass"} + auth := AuthInfo{User: "user", Password: "pass"} s := New(testServer.URL, &auth) obj, err := s.Verb("POST"). Path("foo/bar"). @@ -102,3 +102,48 @@ func TestDoRequestNewWayObj(t *testing.T) { t.Errorf("Request is missing authorization header: %#v", *fakeHandler.RequestReceived) } } + +func TestDoRequestNewWayFile(t *testing.T) { + reqObj := &api.Pod{JSONBase: api.JSONBase{ID: "foo"}} + reqBodyExpected, err := api.Encode(reqObj) + expectNoError(t, err) + file, err := ioutil.TempFile("", "foo") + expectNoError(t, err) + _, err = file.Write(reqBodyExpected) + expectNoError(t, err) + + expectedObj := &api.Service{Port: 12345} + expectedBody, _ := api.Encode(expectedObj) + fakeHandler := util.FakeHandler{ + StatusCode: 200, + ResponseBody: string(expectedBody), + T: t, + } + testServer := httptest.NewTLSServer(&fakeHandler) + auth := AuthInfo{User: "user", Password: "pass"} + s := New(testServer.URL, &auth) + obj, err := s.Verb("POST"). + Path("foo/bar"). + Path("baz"). + Selector("name=foo"). + Timeout(time.Second). + Body(file.Name()). + Do() + if err != nil { + t.Errorf("Unexpected error: %v %#v", err, err) + return + } + if obj == nil { + t.Error("nil obj") + } else if !reflect.DeepEqual(obj, expectedObj) { + t.Errorf("Expected: %#v, got %#v", expectedObj, obj) + } + tmpStr := string(reqBodyExpected) + fakeHandler.ValidateRequest(t, "/foo/bar/baz", "POST", &tmpStr) + if fakeHandler.RequestReceived.URL.RawQuery != "labels=name%3Dfoo&timeout=1s" { + t.Errorf("Unexpected query: %v", fakeHandler.RequestReceived.URL.RawQuery) + } + if fakeHandler.RequestReceived.Header["Authorization"] == nil { + t.Errorf("Request is missing authorization header: %#v", *fakeHandler.RequestReceived) + } +} diff --git a/pkg/cloudcfg/cloudcfg.go b/pkg/cloudcfg/cloudcfg.go index 0d17707d7a..0bd6a195a8 100644 --- a/pkg/cloudcfg/cloudcfg.go +++ b/pkg/cloudcfg/cloudcfg.go @@ -17,13 +17,10 @@ limitations under the License. package cloudcfg import ( - "bytes" - "crypto/tls" "encoding/json" "fmt" "io/ioutil" "log" - "net/http" "os" "strconv" "strings" @@ -90,7 +87,7 @@ func Update(name string, client client.ClientInterface, updatePeriod time.Durati return nil } -// RequestWithBody is a helper method that creates an HTTP request with the specified url, method +/*// RequestWithBody is a helper method that creates an HTTP request with the specified url, method // and a body read from 'configFile' func requestWithBody(configFile, url, method string) (*http.Request, error) { if len(configFile) == 0 { @@ -127,7 +124,7 @@ func doRequest(request *http.Request, auth *client.AuthInfo) ([]byte, error) { defer response.Body.Close() body, err := ioutil.ReadAll(response.Body) return body, err -} +}*/ // StopController stops a controller named 'name' by setting replicas to zero func StopController(name string, client client.ClientInterface) error { diff --git a/pkg/cloudcfg/cloudcfg_test.go b/pkg/cloudcfg/cloudcfg_test.go index 1379530913..eb586c878b 100644 --- a/pkg/cloudcfg/cloudcfg_test.go +++ b/pkg/cloudcfg/cloudcfg_test.go @@ -19,14 +19,11 @@ package cloudcfg import ( "encoding/json" "io/ioutil" - "net/http" - "net/http/httptest" "os" "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/client" - "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) // TODO: This doesn't reduce typing enough to make it worth the less readable errors. Remove. @@ -147,29 +144,6 @@ func TestUpdateNoPods(t *testing.T) { validateAction(Action{action: "list-pods"}, client.actions[1], t) } -func TestDoRequest(t *testing.T) { - expectedBody := `{ "items": []}` - fakeHandler := util.FakeHandler{ - StatusCode: 200, - ResponseBody: expectedBody, - T: t, - } - testServer := httptest.NewTLSServer(&fakeHandler) - request, _ := http.NewRequest("GET", testServer.URL+"/foo/bar", nil) - auth := client.AuthInfo{User: "user", Password: "pass"} - body, err := doRequest(request, &auth) - if request.Header["Authorization"] == nil { - t.Errorf("Request is missing authorization header: %#v", *request) - } - if err != nil { - t.Error("Unexpected error") - } - if string(body) != expectedBody { - t.Errorf("Expected body: '%s', saw: '%s'", expectedBody, body) - } - fakeHandler.ValidateRequest(t, "/foo/bar", "GET", nil) -} - func TestRunController(t *testing.T) { fakeClient := FakeKubeClient{} name := "name" @@ -283,6 +257,7 @@ func TestCloudCfgDeleteControllerWithReplicas(t *testing.T) { } } +/* func TestRequestWithBodyNoSuchFile(t *testing.T) { request, err := requestWithBody("non/existent/file.json", "http://www.google.com", "GET") if request != nil { @@ -291,7 +266,7 @@ func TestRequestWithBodyNoSuchFile(t *testing.T) { if err == nil { t.Error("Unexpected non-error") } -} +}*/ func TestLoadAuthInfo(t *testing.T) { testAuthInfo := &client.AuthInfo{ @@ -326,27 +301,6 @@ func TestLoadAuthInfo(t *testing.T) { } } -func TestRequestWithBody(t *testing.T) { - file, err := ioutil.TempFile("", "foo") - expectNoError(t, err) - data, err := json.Marshal(api.Pod{JSONBase: api.JSONBase{ID: "foo"}}) - expectNoError(t, err) - _, err = file.Write(data) - expectNoError(t, err) - request, err := requestWithBody(file.Name(), "http://www.google.com", "GET") - if request == nil { - t.Error("Unexpected nil result") - } - if err != nil { - t.Errorf("Unexpected error: %#v") - } - dataOut, err := ioutil.ReadAll(request.Body) - expectNoError(t, err) - if string(data) != string(dataOut) { - t.Errorf("Mismatched data. Expected %s, got %s", data, dataOut) - } -} - func validatePort(t *testing.T, p api.Port, external int, internal int) { if p.HostPort != external || p.ContainerPort != internal { t.Errorf("Unexpected port: %#v != (%d, %d)", p, external, internal) diff --git a/pkg/controller/replication_controller_test.go b/pkg/controller/replication_controller_test.go index b9be3e1ad1..6a41ddf42e 100644 --- a/pkg/controller/replication_controller_test.go +++ b/pkg/controller/replication_controller_test.go @@ -111,13 +111,11 @@ func TestSyncReplicationControllerDoesNothing(t *testing.T) { ResponseBody: string(body), } testServer := httptest.NewTLSServer(&fakeHandler) - client := client.Client{ - Host: testServer.URL, - } + client := client.New(testServer.URL, nil) fakePodControl := FakePodControl{} - manager := MakeReplicationManager(nil, &client) + manager := MakeReplicationManager(nil, client) manager.podControl = &fakePodControl controllerSpec := makeReplicationController(2) @@ -133,13 +131,11 @@ func TestSyncReplicationControllerDeletes(t *testing.T) { ResponseBody: string(body), } testServer := httptest.NewTLSServer(&fakeHandler) - client := client.Client{ - Host: testServer.URL, - } + client := client.New(testServer.URL, nil) fakePodControl := FakePodControl{} - manager := MakeReplicationManager(nil, &client) + manager := MakeReplicationManager(nil, client) manager.podControl = &fakePodControl controllerSpec := makeReplicationController(1) @@ -155,13 +151,11 @@ func TestSyncReplicationControllerCreates(t *testing.T) { ResponseBody: string(body), } testServer := httptest.NewTLSServer(&fakeHandler) - client := client.Client{ - Host: testServer.URL, - } + client := client.New(testServer.URL, nil) fakePodControl := FakePodControl{} - manager := MakeReplicationManager(nil, &client) + manager := MakeReplicationManager(nil, client) manager.podControl = &fakePodControl controllerSpec := makeReplicationController(2) @@ -177,9 +171,7 @@ func TestCreateReplica(t *testing.T) { ResponseBody: string(body), } testServer := httptest.NewTLSServer(&fakeHandler) - client := client.Client{ - Host: testServer.URL, - } + client := client.New(testServer.URL, nil) podControl := RealPodControl{ kubeClient: client, @@ -222,13 +214,11 @@ func TestHandleWatchResponseNotSet(t *testing.T) { ResponseBody: string(body), } testServer := httptest.NewTLSServer(&fakeHandler) - client := client.Client{ - Host: testServer.URL, - } + client := client.New(testServer.URL, nil) fakePodControl := FakePodControl{} - manager := MakeReplicationManager(nil, &client) + manager := MakeReplicationManager(nil, client) manager.podControl = &fakePodControl _, err := manager.handleWatchResponse(&etcd.Response{ Action: "update", @@ -243,13 +233,11 @@ func TestHandleWatchResponseNoNode(t *testing.T) { ResponseBody: string(body), } testServer := httptest.NewTLSServer(&fakeHandler) - client := client.Client{ - Host: testServer.URL, - } + client := client.New(testServer.URL, nil) fakePodControl := FakePodControl{} - manager := MakeReplicationManager(nil, &client) + manager := MakeReplicationManager(nil, client) manager.podControl = &fakePodControl _, err := manager.handleWatchResponse(&etcd.Response{ Action: "set", @@ -266,13 +254,11 @@ func TestHandleWatchResponseBadData(t *testing.T) { ResponseBody: string(body), } testServer := httptest.NewTLSServer(&fakeHandler) - client := client.Client{ - Host: testServer.URL, - } + client := client.New(testServer.URL, nil) fakePodControl := FakePodControl{} - manager := MakeReplicationManager(nil, &client) + manager := MakeReplicationManager(nil, client) manager.podControl = &fakePodControl _, err := manager.handleWatchResponse(&etcd.Response{ Action: "set", @@ -292,13 +278,11 @@ func TestHandleWatchResponse(t *testing.T) { ResponseBody: string(body), } testServer := httptest.NewTLSServer(&fakeHandler) - client := client.Client{ - Host: testServer.URL, - } + client := client.New(testServer.URL, nil) fakePodControl := FakePodControl{} - manager := MakeReplicationManager(nil, &client) + manager := MakeReplicationManager(nil, client) manager.podControl = &fakePodControl controller := makeReplicationController(2) @@ -326,13 +310,11 @@ func TestHandleWatchResponseDelete(t *testing.T) { ResponseBody: string(body), } testServer := httptest.NewTLSServer(&fakeHandler) - client := client.Client{ - Host: testServer.URL, - } + client := client.New(testServer.URL, nil) fakePodControl := FakePodControl{} - manager := MakeReplicationManager(nil, &client) + manager := MakeReplicationManager(nil, client) manager.podControl = &fakePodControl controller := makeReplicationController(2) @@ -417,9 +399,7 @@ func TestSyncronize(t *testing.T) { T: t, } testServer := httptest.NewTLSServer(&fakeHandler) - client := client.Client{ - Host: testServer.URL, - } + client := client.New(testServer.URL, nil) manager := MakeReplicationManager(fakeEtcd, client) fakePodControl := FakePodControl{} manager.podControl = &fakePodControl From 5ce54bb77ba6bf250966a64bea7c56931ed9e130 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Sun, 22 Jun 2014 17:02:48 -0700 Subject: [PATCH 2/5] Use new method. --- cmd/cloudcfg/cloudcfg.go | 4 +- pkg/client/client.go | 145 ++++++----------------- pkg/client/client_test.go | 5 +- pkg/client/request.go | 127 ++++++++++++++------ pkg/client/request_test.go | 35 ++++-- pkg/cloudcfg/cloudcfg.go | 5 +- pkg/cloudcfg/cloudcfg_test.go | 3 +- pkg/controller/replication_controller.go | 4 +- 8 files changed, 169 insertions(+), 159 deletions(-) diff --git a/cmd/cloudcfg/cloudcfg.go b/cmd/cloudcfg/cloudcfg.go index dabea2fccb..8675ebec7f 100644 --- a/cmd/cloudcfg/cloudcfg.go +++ b/cmd/cloudcfg/cloudcfg.go @@ -151,11 +151,11 @@ func executeAPIRequest(method string, auth *kube_client.AuthInfo) bool { r := s.Verb(verb). Path("api/v1beta1"). Path(parseStorage()). - Selector(*selector) + ParseSelector(*selector) if method == "create" || method == "update" { r.Body(readConfig(parseStorage())) } - obj, err := r.Do() + obj, err := r.Do().Get() if err != nil { log.Fatalf("Got request error: %v\n", err) return false diff --git a/pkg/client/client.go b/pkg/client/client.go index 2fc8cb694e..444fa65bb7 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -17,23 +17,20 @@ limitations under the License. package client import ( - "bytes" "crypto/tls" - "encoding/json" "fmt" "io" "io/ioutil" "log" "net/http" - "net/url" - "strings" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" ) // ClientInterface holds the methods for clients of Kubenetes, an interface to allow mock testing type ClientInterface interface { - ListPods(selector map[string]string) (api.PodList, error) + ListPods(selector labels.Selector) (api.PodList, error) GetPod(name string) (api.Pod, error) DeletePod(name string) error CreatePod(api.Pod) (api.Pod, error) @@ -123,143 +120,79 @@ func (c *Client) rawRequest(method, path string, requestBody io.Reader, target i return body, err } -func (client Client) makeURL(path string) string { +func (client *Client) makeURL(path string) string { return client.host + "/api/v1beta1/" + path } -// EncodeSelector transforms a selector expressed as a key/value map, into a -// comma separated, key=value encoding. -func EncodeSelector(selector map[string]string) string { - parts := make([]string, 0, len(selector)) - for key, value := range selector { - parts = append(parts, key+"="+value) - } - return url.QueryEscape(strings.Join(parts, ",")) -} - -// DecodeSelector transforms a selector from a comma separated, key=value format into -// a key/value map. -func DecodeSelector(selector string) map[string]string { - result := map[string]string{} - if len(selector) == 0 { - return result - } - parts := strings.Split(selector, ",") - for _, part := range parts { - pieces := strings.Split(part, "=") - if len(pieces) == 2 { - result[pieces[0]] = pieces[1] - } else { - log.Printf("Invalid selector: %s", selector) - } - } - return result -} - // ListPods takes a selector, and returns the list of pods that match that selector -func (client Client) ListPods(selector map[string]string) (api.PodList, error) { - path := "pods" - if selector != nil && len(selector) > 0 { - path += "?labels=" + EncodeSelector(selector) - } - var result api.PodList - _, err := client.rawRequest("GET", path, nil, &result) - return result, err +func (client *Client) ListPods(selector labels.Selector) (result api.PodList, err error) { + err = client.Get().Path("pods").Selector(selector).Do().Into(&result) + return } // GetPod takes the name of the pod, and returns the corresponding Pod object, and an error if it occurs -func (client Client) GetPod(name string) (api.Pod, error) { - var result api.Pod - _, err := client.rawRequest("GET", "pods/"+name, nil, &result) - return result, err +func (client *Client) GetPod(name string) (result api.Pod, err error) { + err = client.Get().Path("pods").Path(name).Do().Into(&result) + return } // DeletePod takes the name of the pod, and returns an error if one occurs -func (client Client) DeletePod(name string) error { - _, err := client.rawRequest("DELETE", "pods/"+name, nil, nil) - return err +func (client *Client) DeletePod(name string) error { + return client.Delete().Path("pods").Path(name).Do().Error() } // CreatePod takes the representation of a pod. Returns the server's representation of the pod, and an error, if it occurs -func (client Client) CreatePod(pod api.Pod) (api.Pod, error) { - var result api.Pod - body, err := json.Marshal(pod) - if err == nil { - _, err = client.rawRequest("POST", "pods", bytes.NewBuffer(body), &result) - } - return result, err +func (client *Client) CreatePod(pod api.Pod) (result api.Pod, err error) { + err = client.Post().Path("pods").Body(pod).Do().Into(&result) + return } // UpdatePod takes the representation of a pod to update. Returns the server's representation of the pod, and an error, if it occurs -func (client Client) UpdatePod(pod api.Pod) (api.Pod, error) { - var result api.Pod - body, err := json.Marshal(pod) - if err == nil { - _, err = client.rawRequest("PUT", "pods/"+pod.ID, bytes.NewBuffer(body), &result) - } - return result, err +func (client *Client) UpdatePod(pod api.Pod) (result api.Pod, err error) { + err = client.Put().Path("pods").Path(pod.ID).Body(pod).Do().Into(&result) + return } // GetReplicationController returns information about a particular replication controller -func (client Client) GetReplicationController(name string) (api.ReplicationController, error) { - var result api.ReplicationController - _, err := client.rawRequest("GET", "replicationControllers/"+name, nil, &result) - return result, err +func (client *Client) GetReplicationController(name string) (result api.ReplicationController, err error) { + err = client.Get().Path("replicationControllers").Path(name).Do().Into(&result) + return } // CreateReplicationController creates a new replication controller -func (client Client) CreateReplicationController(controller api.ReplicationController) (api.ReplicationController, error) { - var result api.ReplicationController - body, err := json.Marshal(controller) - if err == nil { - _, err = client.rawRequest("POST", "replicationControllers", bytes.NewBuffer(body), &result) - } - return result, err +func (client *Client) CreateReplicationController(controller api.ReplicationController) (result api.ReplicationController, err error) { + err = client.Post().Path("replicationControllers").Body(controller).Do().Into(&result) + return } // UpdateReplicationController updates an existing replication controller -func (client Client) UpdateReplicationController(controller api.ReplicationController) (api.ReplicationController, error) { - var result api.ReplicationController - body, err := json.Marshal(controller) - if err == nil { - _, err = client.rawRequest("PUT", "replicationControllers/"+controller.ID, bytes.NewBuffer(body), &result) - } - return result, err +func (client *Client) UpdateReplicationController(controller api.ReplicationController) (result api.ReplicationController, err error) { + err = client.Put().Path("replicationControllers").Path(controller.ID).Body(controller).Do().Into(&result) + return } -func (client Client) DeleteReplicationController(name string) error { - _, err := client.rawRequest("DELETE", "replicationControllers/"+name, nil, nil) - return err +func (client *Client) DeleteReplicationController(name string) error { + return client.Delete().Path("replicationControllers").Path(name).Do().Error() } // GetReplicationController returns information about a particular replication controller -func (client Client) GetService(name string) (api.Service, error) { - var result api.Service - _, err := client.rawRequest("GET", "services/"+name, nil, &result) - return result, err +func (client *Client) GetService(name string) (result api.Service, err error) { + err = client.Get().Path("services").Path(name).Do().Into(&result) + return } // CreateReplicationController creates a new replication controller -func (client Client) CreateService(svc api.Service) (api.Service, error) { - var result api.Service - body, err := json.Marshal(svc) - if err == nil { - _, err = client.rawRequest("POST", "services", bytes.NewBuffer(body), &result) - } - return result, err +func (client *Client) CreateService(svc api.Service) (result api.Service, err error) { + err = client.Post().Path("services").Body(svc).Do().Into(&result) + return } // UpdateReplicationController updates an existing replication controller -func (client Client) UpdateService(svc api.Service) (api.Service, error) { - var result api.Service - body, err := json.Marshal(svc) - if err == nil { - _, err = client.rawRequest("PUT", "services/"+svc.ID, bytes.NewBuffer(body), &result) - } - return result, err +func (client *Client) UpdateService(svc api.Service) (result api.Service, err error) { + err = client.Put().Path("services").Path(svc.ID).Body(svc).Do().Into(&result) + return } -func (client Client) DeleteService(name string) error { - _, err := client.rawRequest("DELETE", "services/"+name, nil, nil) - return err +func (client *Client) DeleteService(name string) error { + return client.Delete().Path("services").Path(name).Do().Error() } diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 07061e980e..41563790a3 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -17,7 +17,6 @@ limitations under the License. package client import ( - "encoding/json" "net/http" "net/http/httptest" "net/url" @@ -96,7 +95,7 @@ func TestListPodsLabels(t *testing.T) { } c.Setup() c.QueryValidator["labels"] = validateLabels - selector := map[string]string{"foo": "bar", "name": "baz"} + selector := labels.Set{"foo": "bar", "name": "baz"}.AsSelector() receivedPodList, err := c.ListPods(selector) c.Validate(t, receivedPodList, err) } @@ -260,7 +259,7 @@ func TestCreateController(t *testing.T) { func body(obj interface{}, raw *string) *string { if obj != nil { - bs, _ := json.Marshal(obj) + bs, _ := api.Encode(obj) body := string(bs) return &body } diff --git a/pkg/client/request.go b/pkg/client/request.go index 3c711fe376..c75e13f7a6 100644 --- a/pkg/client/request.go +++ b/pkg/client/request.go @@ -34,7 +34,6 @@ import ( // auth, err := LoadAuth(filename) // c := New(url, auth) // resp, err := c.Verb("GET"). -// Path("api/v1beta1"). // Path("pods"). // Selector("area=staging"). // Timeout(10*time.Second). @@ -46,17 +45,39 @@ func (c *Client) Verb(verb string) *Request { return &Request{ verb: verb, c: c, - path: "/", + path: "/api/v1beta1", } } +// Begin a POST request. +func (c *Client) Post() *Request { + return c.Verb("POST") +} + +// Begin a PUT request. +func (c *Client) Put() *Request { + return c.Verb("PUT") +} + +// Begin a GET request. +func (c *Client) Get() *Request { + return c.Verb("GET") +} + +// Begin a DELETE request. +func (c *Client) Delete() *Request { + return c.Verb("DELETE") +} + // Request allows for building up a request to a server in a chained fashion. +// Any errors are stored until the end of your call, so you only have to +// check once. type Request struct { c *Client err error verb string path string - body interface{} + body io.Reader selector labels.Selector timeout time.Duration } @@ -70,8 +91,8 @@ func (r *Request) Path(item string) *Request { return r } -// Use the given item as a resource label selector. Optional. -func (r *Request) Selector(item string) *Request { +// Parse the given string as a resource label selector. Optional. +func (r *Request) ParseSelector(item string) *Request { if r.err != nil { return r } @@ -79,6 +100,15 @@ func (r *Request) Selector(item string) *Request { return r } +// Use the given selector. +func (r *Request) Selector(s labels.Selector) *Request { + if r.err != nil { + return r + } + r.selector = s + return r +} + // Use the given duration as a timeout. Optional. func (r *Request) Timeout(d time.Duration) *Request { if r.err != nil { @@ -96,14 +126,31 @@ func (r *Request) Body(obj interface{}) *Request { if r.err != nil { return r } - r.body = obj + switch t := obj.(type) { + case string: + data, err := ioutil.ReadFile(t) + if err != nil { + r.err = err + return r + } + r.body = bytes.NewBuffer(data) + case []byte: + r.body = bytes.NewBuffer(t) + default: + data, err := api.Encode(obj) + if err != nil { + r.err = err + return r + } + r.body = bytes.NewBuffer(data) + } return r } -// Format and xecute the request. Returns the API object received, or an error. -func (r *Request) Do() (interface{}, error) { +// Format and execute the request. +func (r *Request) Do() Result { if r.err != nil { - return nil, r.err + return Result{err: r.err} } finalUrl := r.c.host + r.path query := url.Values{} @@ -114,32 +161,42 @@ func (r *Request) Do() (interface{}, error) { query.Add("timeout", r.timeout.String()) } finalUrl += "?" + query.Encode() - var body io.Reader - if r.body != nil { - switch t := r.body.(type) { - case string: - data, err := ioutil.ReadFile(t) - if err != nil { - return nil, err - } - body = bytes.NewBuffer(data) - case []byte: - body = bytes.NewBuffer(t) - default: - data, err := api.Encode(r.body) - if err != nil { - return nil, err - } - body = bytes.NewBuffer(data) - } - } - req, err := http.NewRequest(r.verb, finalUrl, body) + req, err := http.NewRequest(r.verb, finalUrl, r.body) if err != nil { - return nil, err + return Result{err: err} } - str, err := r.c.doRequest(req) - if err != nil { - return nil, err - } - return api.Decode([]byte(str)) + respBody, err := r.c.doRequest(req) + return Result{respBody, err} +} + +// Result contains the result of calling Request.Do(). +type Result struct { + body []byte + err error +} + +// Raw returns the raw result. +func (r Result) Raw() ([]byte, error) { + return r.body, r.err +} + +// Get returns the result as an object. +func (r Result) Get() (interface{}, error) { + if r.err != nil { + return nil, r.err + } + return api.Decode(r.body) +} + +// Into stores the result into obj, if possible.. +func (r Result) Into(obj interface{}) error { + if r.err != nil { + return r.err + } + return api.DecodeInto(r.body, obj) +} + +// Returns the error executing the request, nil if no error occurred. +func (r Result) Error() error { + return r.err } diff --git a/pkg/client/request_test.go b/pkg/client/request_test.go index 41070591cd..a9bc8c1c39 100644 --- a/pkg/client/request_test.go +++ b/pkg/client/request_test.go @@ -24,6 +24,7 @@ import ( "time" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) @@ -42,10 +43,10 @@ func TestDoRequestNewWay(t *testing.T) { obj, err := s.Verb("POST"). Path("foo/bar"). Path("baz"). - Selector("name=foo"). + ParseSelector("name=foo"). Timeout(time.Second). Body([]byte(reqBody)). - Do() + Do().Get() if err != nil { t.Errorf("Unexpected error: %v %#v", err, err) return @@ -55,7 +56,7 @@ func TestDoRequestNewWay(t *testing.T) { } else if !reflect.DeepEqual(obj, expectedObj) { t.Errorf("Expected: %#v, got %#v", expectedObj, obj) } - fakeHandler.ValidateRequest(t, "/foo/bar/baz", "POST", &reqBody) + fakeHandler.ValidateRequest(t, "/api/v1beta1/foo/bar/baz", "POST", &reqBody) if fakeHandler.RequestReceived.URL.RawQuery != "labels=name%3Dfoo&timeout=1s" { t.Errorf("Unexpected query: %v", fakeHandler.RequestReceived.URL.RawQuery) } @@ -80,10 +81,10 @@ func TestDoRequestNewWayObj(t *testing.T) { obj, err := s.Verb("POST"). Path("foo/bar"). Path("baz"). - Selector("name=foo"). + Selector(labels.Set{"name": "foo"}.AsSelector()). Timeout(time.Second). Body(reqObj). - Do() + Do().Get() if err != nil { t.Errorf("Unexpected error: %v %#v", err, err) return @@ -94,7 +95,7 @@ func TestDoRequestNewWayObj(t *testing.T) { t.Errorf("Expected: %#v, got %#v", expectedObj, obj) } tmpStr := string(reqBodyExpected) - fakeHandler.ValidateRequest(t, "/foo/bar/baz", "POST", &tmpStr) + fakeHandler.ValidateRequest(t, "/api/v1beta1/foo/bar/baz", "POST", &tmpStr) if fakeHandler.RequestReceived.URL.RawQuery != "labels=name%3Dfoo&timeout=1s" { t.Errorf("Unexpected query: %v", fakeHandler.RequestReceived.URL.RawQuery) } @@ -125,10 +126,10 @@ func TestDoRequestNewWayFile(t *testing.T) { obj, err := s.Verb("POST"). Path("foo/bar"). Path("baz"). - Selector("name=foo"). + ParseSelector("name=foo"). Timeout(time.Second). Body(file.Name()). - Do() + Do().Get() if err != nil { t.Errorf("Unexpected error: %v %#v", err, err) return @@ -139,7 +140,7 @@ func TestDoRequestNewWayFile(t *testing.T) { t.Errorf("Expected: %#v, got %#v", expectedObj, obj) } tmpStr := string(reqBodyExpected) - fakeHandler.ValidateRequest(t, "/foo/bar/baz", "POST", &tmpStr) + fakeHandler.ValidateRequest(t, "/api/v1beta1/foo/bar/baz", "POST", &tmpStr) if fakeHandler.RequestReceived.URL.RawQuery != "labels=name%3Dfoo&timeout=1s" { t.Errorf("Unexpected query: %v", fakeHandler.RequestReceived.URL.RawQuery) } @@ -147,3 +148,19 @@ func TestDoRequestNewWayFile(t *testing.T) { t.Errorf("Request is missing authorization header: %#v", *fakeHandler.RequestReceived) } } + +func TestVerbs(t *testing.T) { + c := New("", nil) + if r := c.Post(); r.verb != "POST" { + t.Errorf("Post verb is wrong") + } + if r := c.Put(); r.verb != "PUT" { + t.Errorf("Put verb is wrong") + } + if r := c.Get(); r.verb != "GET" { + t.Errorf("Get verb is wrong") + } + if r := c.Delete(); r.verb != "DELETE" { + t.Errorf("Delete verb is wrong") + } +} diff --git a/pkg/cloudcfg/cloudcfg.go b/pkg/cloudcfg/cloudcfg.go index 0bd6a195a8..41e93b4f3c 100644 --- a/pkg/cloudcfg/cloudcfg.go +++ b/pkg/cloudcfg/cloudcfg.go @@ -28,6 +28,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/client" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "gopkg.in/v1/yaml" ) @@ -71,9 +72,9 @@ func Update(name string, client client.ClientInterface, updatePeriod time.Durati if err != nil { return err } - labels := controller.DesiredState.ReplicaSelector + s := labels.Set(controller.DesiredState.ReplicaSelector).AsSelector() - podList, err := client.ListPods(labels) + podList, err := client.ListPods(s) if err != nil { return err } diff --git a/pkg/cloudcfg/cloudcfg_test.go b/pkg/cloudcfg/cloudcfg_test.go index eb586c878b..eb3cbcc50e 100644 --- a/pkg/cloudcfg/cloudcfg_test.go +++ b/pkg/cloudcfg/cloudcfg_test.go @@ -24,6 +24,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/client" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" ) // TODO: This doesn't reduce typing enough to make it worth the less readable errors. Remove. @@ -44,7 +45,7 @@ type FakeKubeClient struct { ctrl api.ReplicationController } -func (client *FakeKubeClient) ListPods(selector map[string]string) (api.PodList, error) { +func (client *FakeKubeClient) ListPods(selector labels.Selector) (api.PodList, error) { client.actions = append(client.actions, Action{action: "list-pods"}) return client.pods, nil } diff --git a/pkg/controller/replication_controller.go b/pkg/controller/replication_controller.go index a3876f2d48..5658dc6d42 100644 --- a/pkg/controller/replication_controller.go +++ b/pkg/controller/replication_controller.go @@ -26,6 +26,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/client" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" "github.com/coreos/go-etcd/etcd" ) @@ -177,7 +178,8 @@ func (rm *ReplicationManager) filterActivePods(pods []api.Pod) []api.Pod { } func (rm *ReplicationManager) syncReplicationController(controllerSpec api.ReplicationController) error { - podList, err := rm.kubeClient.ListPods(controllerSpec.DesiredState.ReplicaSelector) + s := labels.Set(controllerSpec.DesiredState.ReplicaSelector).AsSelector() + podList, err := rm.kubeClient.ListPods(s) if err != nil { return err } From 72809f8e67e09d6d4ffcb79d294b917703700172 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Sun, 22 Jun 2014 18:14:32 -0700 Subject: [PATCH 3/5] catch 202 early --- pkg/client/client.go | 17 +++++++++++++++++ pkg/client/client_test.go | 34 ++++++++++++++++++++++++++++++++++ pkg/client/request.go | 8 ++++++++ 3 files changed, 59 insertions(+) diff --git a/pkg/client/client.go b/pkg/client/client.go index 444fa65bb7..a33dd85ff7 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -47,6 +47,16 @@ type ClientInterface interface { DeleteService(string) error } +// StatusErr might get returned from an api call if your request is still being processed +// and hence the expected return data is not available yet. +type StatusErr struct { + Status api.Status +} + +func (s *StatusErr) Error() string { + return fmt.Sprintf("Status: %v (%#v)", s.Status.Status, s) +} + // AuthInfo is used to store authorization information type AuthInfo struct { User string @@ -93,6 +103,13 @@ func (c *Client) doRequest(request *http.Request) ([]byte, error) { if response.StatusCode < http.StatusOK || response.StatusCode > http.StatusPartialContent { return nil, fmt.Errorf("request [%#v] failed (%d) %s: %s", request, response.StatusCode, response.Status, string(body)) } + if response.StatusCode == http.StatusAccepted { + var status api.Status + if err := api.DecodeInto(body, &status); err == nil { + return nil, &StatusErr{status} + } + // Sometimes the server returns 202 even though it completely handled the request. + } return body, err } diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 41563790a3..51f7b4f353 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -427,3 +427,37 @@ func TestDoRequest(t *testing.T) { } fakeHandler.ValidateRequest(t, "/foo/bar", "GET", nil) } + +func TestDoRequestAccepted(t *testing.T) { + status := api.Status{Status: api.StatusWorking} + expectedBody, _ := api.Encode(status) + fakeHandler := util.FakeHandler{ + StatusCode: 202, + ResponseBody: string(expectedBody), + T: t, + } + testServer := httptest.NewTLSServer(&fakeHandler) + request, _ := http.NewRequest("GET", testServer.URL+"/foo/bar", nil) + auth := AuthInfo{User: "user", Password: "pass"} + c := New(testServer.URL, &auth) + body, err := c.doRequest(request) + if request.Header["Authorization"] == nil { + t.Errorf("Request is missing authorization header: %#v", *request) + } + if err == nil { + t.Error("Unexpected non-error") + return + } + se, ok := err.(*StatusErr) + if !ok { + t.Errorf("Unexpected kind of error: %#v", err) + return + } + if !reflect.DeepEqual(se.Status, status) { + t.Errorf("Unexpected status: %#v", se.Status) + } + if body != nil { + t.Errorf("Expected nil body, but saw: '%s'", body) + } + fakeHandler.ValidateRequest(t, "/foo/bar", "GET", nil) +} diff --git a/pkg/client/request.go b/pkg/client/request.go index c75e13f7a6..a8ad1da0bb 100644 --- a/pkg/client/request.go +++ b/pkg/client/request.go @@ -166,6 +166,14 @@ func (r *Request) Do() Result { return Result{err: err} } respBody, err := r.c.doRequest(req) + if err != nil { + if statusErr, ok := err.(*StatusErr); ok { + // TODO: using the information in statusErr, + // loop querying the server to wait and retrieve + // the actual result. + _ = statusErr + } + } return Result{respBody, err} } From dccfe8046aab576989ec088fe8642381343c8680 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Sun, 22 Jun 2014 18:26:13 -0700 Subject: [PATCH 4/5] Remove dead code --- pkg/cloudcfg/cloudcfg.go | 39 -------------------------------- pkg/cloudcfg/cloudcfg_test.go | 11 --------- pkg/cloudcfg/resource_printer.go | 2 -- 3 files changed, 52 deletions(-) diff --git a/pkg/cloudcfg/cloudcfg.go b/pkg/cloudcfg/cloudcfg.go index 41e93b4f3c..cafaaddaa4 100644 --- a/pkg/cloudcfg/cloudcfg.go +++ b/pkg/cloudcfg/cloudcfg.go @@ -88,45 +88,6 @@ func Update(name string, client client.ClientInterface, updatePeriod time.Durati return nil } -/*// RequestWithBody is a helper method that creates an HTTP request with the specified url, method -// and a body read from 'configFile' -func requestWithBody(configFile, url, method string) (*http.Request, error) { - if len(configFile) == 0 { - return nil, fmt.Errorf("empty config file.") - } - data, err := ioutil.ReadFile(configFile) - if err != nil { - return nil, err - } - return requestWithBodyData(data, url, method) -} - -// RequestWithBodyData is a helper method that creates an HTTP request with the specified url, method -// and body data -func requestWithBodyData(data []byte, url, method string) (*http.Request, error) { - request, err := http.NewRequest(method, url, bytes.NewBuffer(data)) - request.ContentLength = int64(len(data)) - return request, err -} - -// Execute a request, adds authentication (if auth != nil), and HTTPS cert ignoring. -func doRequest(request *http.Request, auth *client.AuthInfo) ([]byte, error) { - if auth != nil { - request.SetBasicAuth(auth.User, auth.Password) - } - tr := &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, - } - client := &http.Client{Transport: tr} - response, err := client.Do(request) - if err != nil { - return []byte{}, err - } - defer response.Body.Close() - body, err := ioutil.ReadAll(response.Body) - return body, err -}*/ - // StopController stops a controller named 'name' by setting replicas to zero func StopController(name string, client client.ClientInterface) error { controller, err := client.GetReplicationController(name) diff --git a/pkg/cloudcfg/cloudcfg_test.go b/pkg/cloudcfg/cloudcfg_test.go index eb3cbcc50e..b016fccca3 100644 --- a/pkg/cloudcfg/cloudcfg_test.go +++ b/pkg/cloudcfg/cloudcfg_test.go @@ -258,17 +258,6 @@ func TestCloudCfgDeleteControllerWithReplicas(t *testing.T) { } } -/* -func TestRequestWithBodyNoSuchFile(t *testing.T) { - request, err := requestWithBody("non/existent/file.json", "http://www.google.com", "GET") - if request != nil { - t.Error("Unexpected non-nil result") - } - if err == nil { - t.Error("Unexpected non-error") - } -}*/ - func TestLoadAuthInfo(t *testing.T) { testAuthInfo := &client.AuthInfo{ User: "TestUser", diff --git a/pkg/cloudcfg/resource_printer.go b/pkg/cloudcfg/resource_printer.go index 02c13791a9..fe914efe94 100644 --- a/pkg/cloudcfg/resource_printer.go +++ b/pkg/cloudcfg/resource_printer.go @@ -162,8 +162,6 @@ func (h *HumanReadablePrinter) printStatus(status *api.Status, w io.Writer) erro return err } -// TODO replace this with something that returns a concrete printer object, rather than -// having the secondary switch below. func (h *HumanReadablePrinter) Print(data []byte, output io.Writer) error { var mapObj map[string]interface{} if err := json.Unmarshal([]byte(data), &mapObj); err != nil { From 175e998258c68bf48d36ec4297faaa8d29c3a4e3 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 23 Jun 2014 21:55:11 -0700 Subject: [PATCH 5/5] Fix nil/[]byte{} consistency, and other review comments. --- pkg/client/client.go | 4 +-- pkg/client/client_test.go | 62 ++++++++++++--------------------------- 2 files changed, 21 insertions(+), 45 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index a33dd85ff7..3a67d898cd 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -93,7 +93,7 @@ func (c *Client) doRequest(request *http.Request) ([]byte, error) { } response, err := c.httpClient.Do(request) if err != nil { - return []byte{}, err + return nil, err } defer response.Body.Close() body, err := ioutil.ReadAll(response.Body) @@ -121,7 +121,7 @@ func (c *Client) doRequest(request *http.Request) ([]byte, error) { func (c *Client) rawRequest(method, path string, requestBody io.Reader, target interface{}) ([]byte, error) { request, err := http.NewRequest(method, c.makeURL(path), requestBody) if err != nil { - return []byte{}, err + return nil, err } body, err := c.doRequest(request) if err != nil { diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 51f7b4f353..080c83d5d5 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -36,7 +36,7 @@ func makeUrl(suffix string) string { } func TestListEmptyPods(t *testing.T) { - c := &TestClient{ + c := &testClient{ Request: testRequest{Method: "GET", Path: "/pods"}, Response: Response{StatusCode: 200, Body: api.PodList{}}, } @@ -45,7 +45,7 @@ func TestListEmptyPods(t *testing.T) { } func TestListPods(t *testing.T) { - c := &TestClient{ + c := &testClient{ Request: testRequest{Method: "GET", Path: "/pods"}, Response: Response{StatusCode: 200, Body: api.PodList{ @@ -74,7 +74,7 @@ func validateLabels(a, b string) bool { } func TestListPodsLabels(t *testing.T) { - c := &TestClient{ + c := &testClient{ Request: testRequest{Method: "GET", Path: "/pods", Query: url.Values{"labels": []string{"foo=bar,name=baz"}}}, Response: Response{ StatusCode: 200, @@ -101,7 +101,7 @@ func TestListPodsLabels(t *testing.T) { } func TestGetPod(t *testing.T) { - c := &TestClient{ + c := &testClient{ Request: testRequest{Method: "GET", Path: "/pods/foo"}, Response: Response{ StatusCode: 200, @@ -121,7 +121,7 @@ func TestGetPod(t *testing.T) { } func TestDeletePod(t *testing.T) { - c := &TestClient{ + c := &testClient{ Request: testRequest{Method: "DELETE", Path: "/pods/foo"}, Response: Response{StatusCode: 200}, } @@ -139,7 +139,7 @@ func TestCreatePod(t *testing.T) { "name": "baz", }, } - c := &TestClient{ + c := &testClient{ Request: testRequest{Method: "POST", Path: "/pods", Body: requestPod}, Response: Response{ StatusCode: 200, @@ -161,7 +161,7 @@ func TestUpdatePod(t *testing.T) { "name": "baz", }, } - c := &TestClient{ + c := &testClient{ Request: testRequest{Method: "PUT", Path: "/pods/foo"}, Response: Response{StatusCode: 200, Body: requestPod}, } @@ -170,7 +170,7 @@ func TestUpdatePod(t *testing.T) { } func TestGetController(t *testing.T) { - c := &TestClient{ + c := &testClient{ Request: testRequest{Method: "GET", Path: "/replicationControllers/foo"}, Response: Response{ StatusCode: 200, @@ -198,7 +198,7 @@ func TestUpdateController(t *testing.T) { ID: "foo", }, } - c := &TestClient{ + c := &testClient{ Request: testRequest{Method: "PUT", Path: "/replicationControllers/foo"}, Response: Response{ StatusCode: 200, @@ -221,7 +221,7 @@ func TestUpdateController(t *testing.T) { } func TestDeleteController(t *testing.T) { - c := &TestClient{ + c := &testClient{ Request: testRequest{Method: "DELETE", Path: "/replicationControllers/foo"}, Response: Response{StatusCode: 200}, } @@ -235,7 +235,7 @@ func TestCreateController(t *testing.T) { ID: "foo", }, } - c := &TestClient{ + c := &testClient{ Request: testRequest{Method: "POST", Path: "/replicationControllers", Body: requestController}, Response: Response{ StatusCode: 200, @@ -281,7 +281,7 @@ type Response struct { RawBody *string } -type TestClient struct { +type testClient struct { *Client Request testRequest Response Response @@ -296,7 +296,7 @@ type TestClient struct { QueryValidator map[string]func(string, string) bool } -func (c *TestClient) Setup() *TestClient { +func (c *testClient) Setup() *testClient { c.handler = &util.FakeHandler{ StatusCode: c.Response.StatusCode, } @@ -312,7 +312,7 @@ func (c *TestClient) Setup() *TestClient { return c } -func (c *TestClient) Validate(t *testing.T, received interface{}, err error) { +func (c *testClient) Validate(t *testing.T, received interface{}, err error) { defer c.server.Close() if c.Error { @@ -353,7 +353,7 @@ func (c *TestClient) Validate(t *testing.T, received interface{}, err error) { } func TestGetService(t *testing.T) { - c := &TestClient{ + c := &testClient{ Request: testRequest{Method: "GET", Path: "/services/1"}, Response: Response{StatusCode: 200, Body: &api.Service{JSONBase: api.JSONBase{ID: "service-1"}}}, } @@ -362,7 +362,7 @@ func TestGetService(t *testing.T) { } func TestCreateService(t *testing.T) { - c := (&TestClient{ + c := (&testClient{ Request: testRequest{Method: "POST", Path: "/services", Body: &api.Service{JSONBase: api.JSONBase{ID: "service-1"}}}, Response: Response{StatusCode: 200, Body: &api.Service{JSONBase: api.JSONBase{ID: "service-1"}}}, }).Setup() @@ -371,7 +371,7 @@ func TestCreateService(t *testing.T) { } func TestUpdateService(t *testing.T) { - c := &TestClient{ + c := &testClient{ Request: testRequest{Method: "PUT", Path: "/services/service-1", Body: &api.Service{JSONBase: api.JSONBase{ID: "service-1"}}}, Response: Response{StatusCode: 200, Body: &api.Service{JSONBase: api.JSONBase{ID: "service-1"}}}, } @@ -380,7 +380,7 @@ func TestUpdateService(t *testing.T) { } func TestDeleteService(t *testing.T) { - c := &TestClient{ + c := &testClient{ Request: testRequest{Method: "DELETE", Path: "/services/1"}, Response: Response{StatusCode: 200}, } @@ -389,7 +389,7 @@ func TestDeleteService(t *testing.T) { } func TestMakeRequest(t *testing.T) { - testClients := []TestClient{ + testClients := []testClient{ {Request: testRequest{Method: "GET", Path: "/good"}, Response: Response{StatusCode: 200}}, {Request: testRequest{Method: "GET", Path: "/bad%ZZ"}, Error: true}, {Client: New("", &AuthInfo{"foo", "bar"}), Request: testRequest{Method: "GET", Path: "/auth", Header: "Authorization"}, Response: Response{StatusCode: 200}}, @@ -404,30 +404,6 @@ func TestMakeRequest(t *testing.T) { } } -func TestDoRequest(t *testing.T) { - expectedBody := `{ "items": []}` - fakeHandler := util.FakeHandler{ - StatusCode: 200, - ResponseBody: expectedBody, - T: t, - } - testServer := httptest.NewTLSServer(&fakeHandler) - request, _ := http.NewRequest("GET", testServer.URL+"/foo/bar", nil) - auth := AuthInfo{User: "user", Password: "pass"} - c := New(testServer.URL, &auth) - body, err := c.doRequest(request) - if request.Header["Authorization"] == nil { - t.Errorf("Request is missing authorization header: %#v", *request) - } - if err != nil { - t.Error("Unexpected error") - } - if string(body) != expectedBody { - t.Errorf("Expected body: '%s', saw: '%s'", expectedBody, body) - } - fakeHandler.ValidateRequest(t, "/foo/bar", "GET", nil) -} - func TestDoRequestAccepted(t *testing.T) { status := api.Status{Status: api.StatusWorking} expectedBody, _ := api.Encode(status)