Cleaning up the operations code in client

duda · Jan 29, 2015 · dc92d3c · dc92d3c
1 parent fcb1cd3
commit dc92d3c
Show file tree

Hide file tree

Showing 13 changed files with 38 additions and 230 deletions.
diff --git a/cmd/integration/integration.go b/cmd/integration/integration.go
@@ -135,7 +135,6 @@ func startComponents(manifestURL string) (apiServerURL string) {
 	}
 
 	cl := client.NewOrDie(&client.Config{Host: apiServer.URL, Version: testapi.Version()})
-	cl.PollPeriod = time.Millisecond * 100
 
 	helper, err := master.NewEtcdHelper(etcdClient, "")
 	if err != nil {

diff --git a/cmd/kubernetes/kubernetes.go b/cmd/kubernetes/kubernetes.go
@@ -22,7 +22,6 @@ package main
 
 import (
 	"fmt"
-	"time"
 
 	kubeletapp "github.com/GoogleCloudPlatform/kubernetes/cmd/kubelet/app"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -61,7 +60,6 @@ func startComponents(etcdClient tools.EtcdClient, cl *client.Client, addr string
 func newApiClient(addr string, port int) *client.Client {
 	apiServerURL := fmt.Sprintf("http://%s:%d", addr, port)
 	cl := client.NewOrDie(&client.Config{Host: apiServerURL, Version: testapi.Version()})
-	cl.PollPeriod = time.Second * 1
 	return cl
 }
 

diff --git a/pkg/api/types.go b/pkg/api/types.go
@@ -821,12 +821,12 @@ type Status struct {
 	TypeMeta `json:",inline"`
 	ListMeta `json:"metadata,omitempty"`
 
-	// One of: "Success", "Failure", "Working" (for operations not yet completed)
+	// One of: "Success" or "Failure"
 	Status string `json:"status,omitempty"`
 	// A human-readable description of the status of this operation.
 	Message string `json:"message,omitempty"`
 	// A machine-readable description of why this operation is in the
-	// "Failure" or "Working" status. If this value is empty there
+	// "Failure" status. If this value is empty there
 	// is no information available. A Reason clarifies an HTTP status
 	// code but does not override it.
 	Reason StatusReason `json:"reason,omitempty"`
@@ -862,7 +862,6 @@ type StatusDetails struct {
 const (
 	StatusSuccess = "Success"
 	StatusFailure = "Failure"
-	StatusWorking = "Working"
 )
 
 // StatusReason is an enumeration of possible failure causes.  Each StatusReason
@@ -973,7 +972,7 @@ type StatusCause struct {
 
 // CauseType is a machine readable value providing more detail about what
 // occured in a status response. An operation may have multiple causes for a
-// status (whether Failure, Success, or Working).
+// status (whether Failure or Success).
 type CauseType string
 
 const (

diff --git a/pkg/api/v1beta1/types.go b/pkg/api/v1beta1/types.go
@@ -636,15 +636,15 @@ type Binding struct {
 // import both.
 type Status struct {
 	TypeMeta `json:",inline"`
-	// One of: "Success", "Failure", "Working" (for operations not yet completed)
-	Status string `json:"status,omitempty" description:"status of the operation; either Working (not yet completed), Success, or Failure"`
+	// One of: "Success" or "Failure".
+	Status string `json:"status,omitempty" description:"status of the operation; either Success, or Failure"`
 	// A human-readable description of the status of this operation.
 	Message string `json:"message,omitempty" description:"human-readable description of the status of this operation"`
 	// A machine-readable description of why this operation is in the
-	// "Failure" or "Working" status. If this value is empty there
+	// "Failure" status. If this value is empty there
 	// is no information available. A Reason clarifies an HTTP status
 	// code but does not override it.
-	Reason StatusReason `json:"reason,omitempty" description:"machine-readable description of why this operation is in the 'Failure' or 'Working' status; if this value is empty there is no information available; a reason clarifies an HTTP status code but does not override it"`
+	Reason StatusReason `json:"reason,omitempty" description:"machine-readable description of why this operation is in the 'Failure' status; if this value is empty there is no information available; a reason clarifies an HTTP status code but does not override it"`
 	// Extended data associated with the reason.  Each reason may define its
 	// own extended details. This field is optional and the data returned
 	// is not guaranteed to conform to any schema except that defined by
@@ -676,7 +676,6 @@ type StatusDetails struct {
 const (
 	StatusSuccess = "Success"
 	StatusFailure = "Failure"
-	StatusWorking = "Working"
 )
 
 // StatusReason is an enumeration of possible failure causes.  Each StatusReason
@@ -739,7 +738,7 @@ type StatusCause struct {
 
 // CauseType is a machine readable value providing more detail about what
 // occured in a status response. An operation may have multiple causes for a
-// status (whether Failure, Success, or Working).
+// status (whether Failure or Success).
 type CauseType string
 
 const (

diff --git a/pkg/api/v1beta2/types.go b/pkg/api/v1beta2/types.go
@@ -597,15 +597,15 @@ type Binding struct {
 // import both.
 type Status struct {
 	TypeMeta `json:",inline"`
-	// One of: "Success", "Failure", "Working" (for operations not yet completed)
-	Status string `json:"status,omitempty" description:"status of the operation; either Working (not yet completed), Success, or Failure"`
+	// One of: "Success" or "Failure"
+	Status string `json:"status,omitempty" description:"status of the operation; either Success or Failure"`
 	// A human-readable description of the status of this operation.
 	Message string `json:"message,omitempty" description:"human-readable description of the status of this operation"`
 	// A machine-readable description of why this operation is in the
-	// "Failure" or "Working" status. If this value is empty there
+	// "Failure" status. If this value is empty there
 	// is no information available. A Reason clarifies an HTTP status
 	// code but does not override it.
-	Reason StatusReason `json:"reason,omitempty" description:"machine-readable description of why this operation is in the 'Failure' or 'Working' status; if this value is empty there is no information available; a reason clarifies an HTTP status code but does not override it"`
+	Reason StatusReason `json:"reason,omitempty" description:"machine-readable description of why this operation is in the 'Failure' status; if this value is empty there is no information available; a reason clarifies an HTTP status code but does not override it"`
 	// Extended data associated with the reason.  Each reason may define its
 	// own extended details. This field is optional and the data returned
 	// is not guaranteed to conform to any schema except that defined by
@@ -637,7 +637,6 @@ type StatusDetails struct {
 const (
 	StatusSuccess = "Success"
 	StatusFailure = "Failure"
-	StatusWorking = "Working"
 )
 
 // StatusReason is an enumeration of possible failure causes.  Each StatusReason
@@ -713,7 +712,7 @@ type StatusCause struct {
 
 // CauseType is a machine readable value providing more detail about what
 // occured in a status response. An operation may have multiple causes for a
-// status (whether Failure, Success, or Working).
+// status (whether Failure or Success).
 type CauseType string
 
 const (

diff --git a/pkg/api/v1beta3/types.go b/pkg/api/v1beta3/types.go
@@ -832,12 +832,12 @@ type Status struct {
 	TypeMeta `json:",inline"`
 	ListMeta `json:"metadata,omitempty"`
 
-	// One of: "Success", "Failure", "Working" (for operations not yet completed)
+	// One of: "Success" or "Failure"
 	Status string `json:"status,omitempty"`
 	// A human-readable description of the status of this operation.
 	Message string `json:"message,omitempty"`
 	// A machine-readable description of why this operation is in the
-	// "Failure" or "Working" status. If this value is empty there
+	// "Failure" status. If this value is empty there
 	// is no information available. A Reason clarifies an HTTP status
 	// code but does not override it.
 	Reason StatusReason `json:"reason,omitempty"`
@@ -872,7 +872,6 @@ type StatusDetails struct {
 const (
 	StatusSuccess = "Success"
 	StatusFailure = "Failure"
-	StatusWorking = "Working"
 )
 
 // StatusReason is an enumeration of possible failure causes.  Each StatusReason
@@ -948,7 +947,7 @@ type StatusCause struct {
 
 // CauseType is a machine readable value providing more detail about what
 // occured in a status response. An operation may have multiple causes for a
-// status (whether Failure, Success, or Working).
+// status (whether Failure or Success).
 type CauseType string
 
 const (

diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go
@@ -74,15 +74,14 @@ func interfacesFor(version string) (*meta.VersionInterfaces, error) {
 func init() {
 	// Certain API objects are returned regardless of the contents of storage:
 	// api.Status is returned in errors
-	// api.Operation/api.OperationList are returned by /operations
 
 	// "internal" version
 	api.Scheme.AddKnownTypes("", &Simple{}, &SimpleList{},
-		&api.Status{}, &api.Operation{}, &api.OperationList{})
+		&api.Status{})
 	// "version" version
 	// TODO: Use versioned api objects?
 	api.Scheme.AddKnownTypes(testVersion, &Simple{}, &SimpleList{},
-		&api.Status{}, &api.Operation{}, &api.OperationList{})
+		&api.Status{})
 
 	defMapper := meta.NewDefaultRESTMapper(
 		versions,

diff --git a/pkg/client/request.go b/pkg/client/request.go
@@ -42,12 +42,6 @@ import (
 // are therefore not allowed to set manually.
 var specialParams = util.NewStringSet("timeout")
 
-// PollFunc is called when a server operation returns 202 accepted. The name of the
-// operation is extracted from the response and passed to this function. Return a
-// request to retrieve the result of the operation, or false for the second argument
-// if polling should end.
-type PollFunc func(name string) (*Request, bool)
-
 // HTTPClient is an interface for testing a request object.
 type HTTPClient interface {
 	Do(req *http.Request) (*http.Response, error)
@@ -86,10 +80,6 @@ type Request struct {
 	baseURL *url.URL
 	codec   runtime.Codec
 
-	// optional, will be invoked if the server returns a 202 to decide
-	// whether to poll.
-	poller PollFunc
-
 	// If true, add "?namespace=<namespace>" as a query parameter, if false put ns/<namespace> in path
 	// Query parameter is considered legacy behavior
 	namespaceInQuery bool
@@ -305,22 +295,6 @@ func (r *Request) Body(obj interface{}) *Request {
 	return r
 }
 
-// NoPoll indicates a server "working" response should be returned as an error
-func (r *Request) NoPoll() *Request {
-	return r.Poller(nil)
-}
-
-// Poller indicates this request should use the specified poll function to determine whether
-// a server "working" response should be retried. The poller is responsible for waiting or
-// outputting messages to the client.
-func (r *Request) Poller(poller PollFunc) *Request {
-	if r.err != nil {
-		return r
-	}
-	r.poller = poller
-	return r
-}
-
 func (r *Request) finalURL() string {
 	p := r.path
 	if r.namespaceSet && !r.namespaceInQuery && len(r.namespace) > 0 {
@@ -430,7 +404,7 @@ func (r *Request) Stream() (io.ReadCloser, error) {
 }
 
 // Do formats and executes the request. Returns a Result object for easy response
-// processing. Handles polling the server in the event a continuation was sent.
+// processing.
 //
 // Error type:
 //  * If the request can't be constructed, or an error happened earlier while building its
@@ -464,10 +438,6 @@ func (r *Request) Do() Result {
 		}
 
 		respBody, created, err := r.transformResponse(resp, req)
-		if poll, ok := r.shouldPoll(err); ok {
-			r = poll
-			continue
-		}
 
 		// Check to see if we got a 429 Too Many Requests response code.
 		if resp.StatusCode == errors.StatusTooManyRequests {
@@ -487,27 +457,6 @@ func (r *Request) Do() Result {
 	}
 }
 
-// shouldPoll checks the server error for an incomplete operation
-// and if found returns a request that would check the response.
-// If no polling is necessary or possible, it will return false.
-func (r *Request) shouldPoll(err error) (*Request, bool) {
-	if err == nil || r.poller == nil {
-		return nil, false
-	}
-	apistatus, ok := err.(APIStatus)
-	if !ok {
-		return nil, false
-	}
-	status := apistatus.Status()
-	if status.Status != api.StatusWorking {
-		return nil, false
-	}
-	if status.Details == nil || len(status.Details.ID) == 0 {
-		return nil, false
-	}
-	return r.poller(status.Details.ID)
-}
-
 // transformResponse converts an API response into a structured API object.
 func (r *Request) transformResponse(resp *http.Response, req *http.Request) ([]byte, bool, error) {
 	defer resp.Body.Close()

diff --git a/pkg/client/request_test.go b/pkg/client/request_test.go
@@ -44,10 +44,6 @@ import (
 	watchjson "github.com/GoogleCloudPlatform/kubernetes/pkg/watch/json"
 )
 
-func skipPolling(name string) (*Request, bool) {
-	return nil, false
-}
-
 func TestRequestWithErrorWontChange(t *testing.T) {
 	original := Request{err: errors.New("test")}
 	r := original
@@ -61,9 +57,7 @@ func TestRequestWithErrorWontChange(t *testing.T) {
 		Namespace("new").
 		Resource("foos").
 		Name("bars").
-		NoPoll().
 		Body("foo").
-		Poller(skipPolling).
 		Timeout(time.Millisecond)
 	if changed != &r {
 		t.Errorf("returned request should point to the same object")
@@ -768,90 +762,6 @@ func TestBody(t *testing.T) {
 	}
 }
 
-func TestSetPoller(t *testing.T) {
-	c := NewOrDie(&Config{})
-	r := c.Get()
-	if c.PollPeriod == 0 {
-		t.Errorf("polling should be on by default")
-	}
-	if r.poller == nil {
-		t.Errorf("polling should be on by default")
-	}
-	r.NoPoll()
-	if r.poller != nil {
-		t.Errorf("'NoPoll' doesn't work")
-	}
-}
-
-func TestPolling(t *testing.T) {
-	objects := []runtime.Object{
-		&api.Status{Status: api.StatusWorking, Details: &api.StatusDetails{ID: "1234"}},
-		&api.Status{Status: api.StatusWorking, Details: &api.StatusDetails{ID: "1234"}},
-		&api.Status{Status: api.StatusWorking, Details: &api.StatusDetails{ID: "1234"}},
-		&api.Status{Status: api.StatusWorking, Details: &api.StatusDetails{ID: "1234"}},
-		&api.Status{Status: api.StatusSuccess},
-	}
-
-	callNumber := 0
-	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if callNumber == 0 {
-			if r.URL.Path != "/api/v1beta1/" {
-				t.Fatalf("unexpected request URL path %s", r.URL.Path)
-			}
-		} else {
-			if r.URL.Path != "/api/v1beta1/operations/1234" {
-				t.Fatalf("unexpected request URL path %s", r.URL.Path)
-			}
-		}
-		t.Logf("About to write %d", callNumber)
-		data, err := v1beta1.Codec.Encode(objects[callNumber])
-		if err != nil {
-			t.Errorf("Unexpected encode error")
-		}
-		callNumber++
-		w.Write(data)
-	}))
-
-	c := NewOrDie(&Config{Host: testServer.URL, Version: "v1beta1", Username: "user", Password: "pass"})
-	c.PollPeriod = 1 * time.Millisecond
-	trials := []func(){
-		func() {
-			// Check that we do indeed poll when asked to.
-			obj, err := c.Get().Do().Get()
-			if err != nil {
-				t.Errorf("Unexpected error: %v %#v", err, err)
-				return
-			}
-			if s, ok := obj.(*api.Status); !ok || s.Status != api.StatusSuccess {
-				t.Errorf("Unexpected return object: %#v", obj)
-				return
-			}
-			if callNumber != len(objects) {
-				t.Errorf("Unexpected number of calls: %v", callNumber)
-			}
-		},
-		func() {
-			// Check that we don't poll when asked not to.
-			obj, err := c.Get().NoPoll().Do().Get()
-			if err == nil {
-				t.Errorf("Unexpected non error: %v", obj)
-				return
-			}
-			if se, ok := err.(APIStatus); !ok || se.Status().Status != api.StatusWorking {
-				t.Errorf("Unexpected kind of error: %#v", err)
-				return
-			}
-			if callNumber != 1 {
-				t.Errorf("Unexpected number of calls: %v", callNumber)
-			}
-		},
-	}
-	for _, f := range trials {
-		callNumber = 0
-		f()
-	}
-}
-
 func authFromReq(r *http.Request) (*Config, bool) {
 	auth, ok := r.Header["Authorization"]
 	if !ok {