Skip to content

K
k3s
Commit 0345211e authored by Brian Grant's avatar Brian Grant
Browse files
Options

Merge pull request #3921 from nikhiljindal/OperationClean

Some more operations code cleanup
parents 35d59e62 dc92d3c7
Hide whitespace changes
Inline Side-by-side
Showing with 38 additions and 230 deletions
cmd/integration/integration.go
View file @ 0345211e
......@@ -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 {
......
cmd/kubernetes/kubernetes.go
View file @ 0345211e
......@@ -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
}
......
pkg/api/types.go
View file @ 0345211e
......@@ -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 (
......
pkg/api/v1beta1/types.go
View file @ 0345211e
......@@ -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 (
......
pkg/api/v1beta2/types.go
View file @ 0345211e
......@@ -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 (
......
pkg/api/v1beta3/types.go
View file @ 0345211e
......@@ -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 (
......
pkg/apiserver/apiserver_test.go
View file @ 0345211e
......@@ -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,
......
pkg/client/request.go
View file @ 0345211e
......@@ -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()
......
pkg/client/request_test.go
View file @ 0345211e
......@@ -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 {
......
pkg/client/restclient.go
View file @ 0345211e
......@@ -22,8 +22,6 @@ import (
"time"
"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
"github.com/golang/glog"
)
// RESTClient imposes common Kubernetes API conventions on a set of resource paths.
......@@ -51,12 +49,7 @@ type RESTClient struct {
// used.
Client HTTPClient
// Set the poll behavior of this client. If not set the DefaultPoll method will
// be called.
Poller PollFunc
PollPeriod time.Duration
Timeout time.Duration
Timeout time.Duration
}
// NewRESTClient creates a new RESTClient. This client performs generic REST functions
......@@ -78,9 +71,6 @@ func NewRESTClient(baseURL *url.URL, apiVersion string, c runtime.Codec, legacyB
Codec: c,
LegacyBehavior: legacyBehavior,
// Poll frequently
PollPeriod: time.Second * 2,
}
}
......@@ -102,11 +92,7 @@ func (c *RESTClient) Verb(verb string) *Request {
// if c.Client != nil {
// timeout = c.Client.Timeout
// }
poller := c.Poller
if poller == nil {
poller = c.DefaultPoll
}
return NewRequest(c.Client, verb, c.baseURL, c.Codec, c.LegacyBehavior, c.LegacyBehavior).Poller(poller).Timeout(c.Timeout)
return NewRequest(c.Client, verb, c.baseURL, c.Codec, c.LegacyBehavior, c.LegacyBehavior).Timeout(c.Timeout)
}
// Post begins a POST request. Short for c.Verb("POST").
......@@ -129,22 +115,6 @@ func (c *RESTClient) Delete() *Request {
return c.Verb("DELETE")
}
// PollFor makes a request to do a single poll of the completion of the given operation.
func (c *RESTClient) Operation(name string) *Request {
return c.Get().Resource("operations").Name(name).NoPoll()
}
// DefaultPoll performs a polling action based on the PollPeriod set on the Client.
func (c *RESTClient) DefaultPoll(name string) (*Request, bool) {
if c.PollPeriod == 0 {
return nil, false
}
glog.Infof("Waiting for completion of operation %s", name)
time.Sleep(c.PollPeriod)
// Make a poll request
return c.Operation(name).Poller(c.DefaultPoll), true
}
// APIVersion returns the APIVersion this RESTClient is expected to use.
func (c *RESTClient) APIVersion() string {
return c.apiVersion
......
pkg/client/restclient_test.go
View file @ 0345211e
......@@ -157,10 +157,10 @@ func TestValidatesHostParameter(t *testing.T) {
}
func TestDoRequestBearer(t *testing.T) {
status := &api.Status{Status: api.StatusWorking}
status := &api.Status{Status: api.StatusFailure}
expectedBody, _ := latest.Codec.Encode(status)
fakeHandler := util.FakeHandler{
StatusCode: 202,
StatusCode: 400,
ResponseBody: string(expectedBody),
T: t,
}
......@@ -187,11 +187,11 @@ func TestDoRequestBearer(t *testing.T) {
}
}
func TestDoRequestAccepted(t *testing.T) {
status := &api.Status{Status: api.StatusWorking}
func TestDoRequestWithoutPassword(t *testing.T) {
status := &api.Status{Status: api.StatusFailure}
expectedBody, _ := latest.Codec.Encode(status)
fakeHandler := util.FakeHandler{
StatusCode: 202,
StatusCode: 400,
ResponseBody: string(expectedBody),
T: t,
}
......@@ -228,11 +228,11 @@ func TestDoRequestAccepted(t *testing.T) {
fakeHandler.ValidateRequest(t, "/"+testapi.Version()+"/test", "GET", nil)
}
func TestDoRequestAcceptedSuccess(t *testing.T) {
func TestDoRequestSuccess(t *testing.T) {
status := &api.Status{Status: api.StatusSuccess}
expectedBody, _ := latest.Codec.Encode(status)
fakeHandler := util.FakeHandler{
StatusCode: 202,
StatusCode: 200,
ResponseBody: string(expectedBody),
T: t,
}
......@@ -339,10 +339,3 @@ func TestDoRequestCreated(t *testing.T) {
}
fakeHandler.ValidateRequest(t, "/"+testapi.Version()+"/test", "GET", nil)
}
func TestDefaultPoll(t *testing.T) {
c := &RESTClient{PollPeriod: 0}
if req, ok := c.DefaultPoll("test"); req != nil || ok {
t.Errorf("expected nil request and not poll")
}
}
pkg/registry/controller/rest.go
View file @ 0345211e
......@@ -18,7 +18,6 @@ package controller
import (
"fmt"
"time"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
......@@ -38,17 +37,15 @@ type PodLister interface {
// REST implements apiserver.RESTStorage for the replication controller service.
type REST struct {
registry Registry
podLister PodLister
pollPeriod time.Duration
registry Registry
podLister PodLister
}
// NewREST returns a new apiserver.RESTStorage for the given registry and PodLister.
func NewREST(registry Registry, podLister PodLister) *REST {
return &REST{
registry: registry,
podLister: podLister,
pollPeriod: time.Second * 10,
registry: registry,
podLister: podLister,
}
}
......
pkg/registry/controller/rest_test.go
View file @ 0345211e
......@@ -243,9 +243,8 @@ func TestCreateController(t *testing.T) {
},
}
storage := REST{
registry: &mockRegistry,
podLister: &mockPodRegistry,
pollPeriod: time.Millisecond * 1,
registry: &mockRegistry,
podLister: &mockPodRegistry,
}
controller := &api.ReplicationController{
ObjectMeta: api.ObjectMeta{Name: "test"},
......@@ -278,9 +277,8 @@ func TestCreateController(t *testing.T) {
func TestControllerStorageValidatesCreate(t *testing.T) {
mockRegistry := registrytest.ControllerRegistry{}
storage := REST{
registry: &mockRegistry,
podLister: nil,
pollPeriod: time.Millisecond * 1,
registry: &mockRegistry,
podLister: nil,
}
failureCases := map[string]api.ReplicationController{
"empty ID": {
......@@ -309,9 +307,8 @@ func TestControllerStorageValidatesCreate(t *testing.T) {
func TestControllerStorageValidatesUpdate(t *testing.T) {
mockRegistry := registrytest.ControllerRegistry{}
storage := REST{
registry: &mockRegistry,
podLister: nil,
pollPeriod: time.Millisecond * 1,
registry: &mockRegistry,
podLister: nil,
}
failureCases := map[string]api.ReplicationController{
"empty ID": {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment