diff --git a/pkg/api/types.go b/pkg/api/types.go index 861f2727240..3c7b2df2c86 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -336,21 +336,43 @@ type MinionList struct { } // Status is a return value for calls that don't return other objects. -// Arguably, this could go in apiserver, but I'm including it here so clients needn't +// TODO: this could go in apiserver, but I'm including it here so clients needn't // import both. type Status struct { JSONBase `json:",inline" yaml:",inline"` // One of: "success", "failure", "working" (for operations not yet completed) - // TODO: if "working", include an operation identifier so final status can be - // checked. Status string `json:"status,omitempty" yaml:"status,omitempty"` - // Details about the status. May be an error description or an - // operation number for later polling. - Details string `json:"details,omitempty" yaml:"details,omitempty"` + // A human readable description of the status of this operation. + Message string `json:"message,omitempty" yaml:"message,omitempty"` + // A 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 ReasonType `json:"reason,omitempty" yaml:"reason,omitempty"` + // 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 + // the reason type. + Details *StatusDetails `json:"details,omitempty" yaml:"details,omitempty"` // Suggested HTTP return code for this status, 0 if not set. Code int `json:"code,omitempty" yaml:"code,omitempty"` } +// StatusDetails is a set of additional properties that MAY be set by the +// server to provide additional information about a response. The Reason +// field of a Status object defines what attributes will be set. Clients +// must ignore fields that do not match the defined type of each attribute, +// and should assume that any attribute may be empty, invalid, or under +// defined. +type StatusDetails struct { + // The ID attribute of the resource associated with the status ReasonType + // (when there is a single ID which can be described). + ID string `json:"id,omitempty" yaml:"id,omitempty"` + // The kind attribute of the resource associated with the status ReasonType. + // On some operations may differ from the requested resource Kind. + Kind string `json:"kind,omitempty" yaml:"kind,omitempty"` +} + // Values of Status.Status const ( StatusSuccess = "success" @@ -358,6 +380,55 @@ const ( StatusWorking = "working" ) +// ReasonType is an enumeration of possible failure causes. Each ReasonType +// must map to a single HTTP status code, but multiple reasons may map +// to the same HTTP status code. +// TODO: move to apiserver +type ReasonType string + +const ( + // ReasonTypeUnknown means the server has declined to indicate a specific reason. + // The details field may contain other information about this error. + // Status code 500. + ReasonTypeUnknown ReasonType = "" + + // ReasonTypeWorking means the server is processing this request and will complete + // at a future time. + // Details (optional): + // "kind" string - the name of the resource being referenced ("operation" today) + // "id" string - the identifier of the Operation resource where updates + // will be returned + // Headers (optional): + // "Location" - HTTP header populated with a URL that can retrieved the final + // status of this operation. + // Status code 202 + ReasonTypeWorking ReasonType = "working" + + // ResourceTypeNotFound means one or more resources required for this operation + // could not be found. + // Details (optional): + // "kind" string - the kind attribute of the missing resource + // on some operations may differ from the requested + // resource. + // "id" string - the identifier of the missing resource + // Status code 404 + ReasonTypeNotFound ReasonType = "not_found" + + // ReasonTypeAlreadyExists means the resource you are creating already exists. + // Details (optional): + // "kind" string - the kind attribute of the conflicting resource + // "id" string - the identifier of the conflicting resource + // Status code 409 + ReasonTypeAlreadyExists ReasonType = "already_exists" + + // ResourceTypeConflict means the requested update operation cannot be completed + // due to a conflict in the operation. The client may need to alter the request. + // Each resource may define custom details that indicate the nature of the + // conflict. + // Status code 409 + ReasonTypeConflict ReasonType = "conflict" +) + // ServerOp is an operation delivered to API clients. type ServerOp struct { JSONBase `yaml:",inline" json:",inline"` diff --git a/pkg/api/v1beta1/types.go b/pkg/api/v1beta1/types.go index 925eefc73f0..16346bccad2 100644 --- a/pkg/api/v1beta1/types.go +++ b/pkg/api/v1beta1/types.go @@ -339,21 +339,43 @@ type MinionList struct { } // Status is a return value for calls that don't return other objects. -// Arguably, this could go in apiserver, but I'm including it here so clients needn't +// TODO: this could go in apiserver, but I'm including it here so clients needn't // import both. type Status struct { JSONBase `json:",inline" yaml:",inline"` // One of: "success", "failure", "working" (for operations not yet completed) - // TODO: if "working", include an operation identifier so final status can be - // checked. Status string `json:"status,omitempty" yaml:"status,omitempty"` - // Details about the status. May be an error description or an - // operation number for later polling. - Details string `json:"details,omitempty" yaml:"details,omitempty"` + // A human readable description of the status of this operation. + Message string `json:"message,omitempty" yaml:"message,omitempty"` + // A 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 ReasonType `json:"reason,omitempty" yaml:"reason,omitempty"` + // 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 + // the reason type. + Details *StatusDetails `json:"details,omitempty" yaml:"details,omitempty"` // Suggested HTTP return code for this status, 0 if not set. Code int `json:"code,omitempty" yaml:"code,omitempty"` } +// StatusDetails is a set of additional properties that MAY be set by the +// server to provide additional information about a response. The Reason +// field of a Status object defines what attributes will be set. Clients +// must ignore fields that do not match the defined type of each attribute, +// and should assume that any attribute may be empty, invalid, or under +// defined. +type StatusDetails struct { + // The ID attribute of the resource associated with the status ReasonType + // (when there is a single ID which can be described). + ID string `json:"id,omitempty" yaml:"id,omitempty"` + // The kind attribute of the resource associated with the status ReasonType. + // On some operations may differ from the requested resource Kind. + Kind string `json:"kind,omitempty" yaml:"kind,omitempty"` +} + // Values of Status.Status const ( StatusSuccess = "success" @@ -361,6 +383,55 @@ const ( StatusWorking = "working" ) +// ReasonType is an enumeration of possible failure causes. Each ReasonType +// must map to a single HTTP status code, but multiple reasons may map +// to the same HTTP status code. +// TODO: move to apiserver +type ReasonType string + +const ( + // ReasonTypeUnknown means the server has declined to indicate a specific reason. + // The details field may contain other information about this error. + // Status code 500. + ReasonTypeUnknown ReasonType = "" + + // ReasonTypeWorking means the server is processing this request and will complete + // at a future time. + // Details (optional): + // "kind" string - the name of the resource being referenced ("operation" today) + // "id" string - the identifier of the Operation resource where updates + // will be returned + // Headers (optional): + // "Location" - HTTP header populated with a URL that can retrieved the final + // status of this operation. + // Status code 202 + ReasonTypeWorking ReasonType = "working" + + // ResourceTypeNotFound means one or more resources required for this operation + // could not be found. + // Details (optional): + // "kind" string - the kind attribute of the missing resource + // on some operations may differ from the requested + // resource. + // "id" string - the identifier of the missing resource + // Status code 404 + ReasonTypeNotFound ReasonType = "not_found" + + // ReasonTypeAlreadyExists means the resource you are creating already exists. + // Details (optional): + // "kind" string - the kind attribute of the conflicting resource + // "id" string - the identifier of the conflicting resource + // Status code 409 + ReasonTypeAlreadyExists ReasonType = "already_exists" + + // ResourceTypeConflict means the requested update operation cannot be completed + // due to a conflict in the operation. The client may need to alter the request. + // Each resource may define custom details that indicate the nature of the + // conflict. + // Status code 409 + ReasonTypeConflict ReasonType = "conflict" +) + // ServerOp is an operation delivered to API clients. type ServerOp struct { JSONBase `yaml:",inline" json:",inline"` diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index 824410d28c9..1c29efaf3c0 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -493,7 +493,7 @@ func TestCreate(t *testing.T) { t.Errorf("unexpected error: %v", err) } - if itemOut.Status != api.StatusWorking || itemOut.Details == "" { + if itemOut.Status != api.StatusWorking || itemOut.Details == nil || itemOut.Details.ID == "" { t.Errorf("Unexpected status: %#v (%s)", itemOut, string(body)) } } @@ -621,7 +621,7 @@ func TestAsyncDelayReturnsError(t *testing.T) { server := httptest.NewServer(handler) status := expectApiStatus(t, "DELETE", fmt.Sprintf("%s/prefix/version/foo/bar", server.URL), nil, http.StatusInternalServerError) - if status.Status != api.StatusFailure || status.Details == "" { + if status.Status != api.StatusFailure || status.Message != "error" || status.Details != nil { t.Errorf("Unexpected status %#v", status) } } @@ -642,11 +642,11 @@ func TestAsyncCreateError(t *testing.T) { data, _ := api.Encode(simple) status := expectApiStatus(t, "POST", fmt.Sprintf("%s/prefix/version/foo", server.URL), data, http.StatusAccepted) - if status.Status != api.StatusWorking || status.Details == "" { + if status.Status != api.StatusWorking || status.Details == nil || status.Details.ID == "" { t.Errorf("Unexpected status %#v", status) } - otherStatus := expectApiStatus(t, "GET", fmt.Sprintf("%s/prefix/version/operations/%s", server.URL, status.Details), []byte{}, http.StatusAccepted) + otherStatus := expectApiStatus(t, "GET", fmt.Sprintf("%s/prefix/version/operations/%s", server.URL, status.Details.ID), []byte{}, http.StatusAccepted) if !reflect.DeepEqual(status, otherStatus) { t.Errorf("Expected %#v, Got %#v", status, otherStatus) } @@ -654,10 +654,10 @@ func TestAsyncCreateError(t *testing.T) { ch <- struct{}{} time.Sleep(time.Millisecond) - finalStatus := expectApiStatus(t, "GET", fmt.Sprintf("%s/prefix/version/operations/%s?after=1", server.URL, status.Details), []byte{}, http.StatusOK) + finalStatus := expectApiStatus(t, "GET", fmt.Sprintf("%s/prefix/version/operations/%s?after=1", server.URL, status.Details.ID), []byte{}, http.StatusOK) expectedStatus := &api.Status{ Code: http.StatusInternalServerError, - Details: "error", + Message: "error", Status: api.StatusFailure, } if !reflect.DeepEqual(expectedStatus, finalStatus) { @@ -721,7 +721,7 @@ func TestSyncCreateTimeout(t *testing.T) { simple := Simple{Name: "foo"} data, _ := api.Encode(simple) itemOut := expectApiStatus(t, "POST", server.URL+"/prefix/version/foo?sync=true&timeout=4ms", data, http.StatusAccepted) - if itemOut.Status != api.StatusWorking || itemOut.Details == "" { + if itemOut.Status != api.StatusWorking || itemOut.Details == nil || itemOut.Details.ID == "" { t.Errorf("Unexpected status %#v", itemOut) } } diff --git a/pkg/apiserver/async.go b/pkg/apiserver/async.go index 0868f18867a..6e7d6df2d19 100644 --- a/pkg/apiserver/async.go +++ b/pkg/apiserver/async.go @@ -44,7 +44,7 @@ func MakeAsync(fn WorkFunc) <-chan interface{} { } channel <- &api.Status{ Status: api.StatusFailure, - Details: err.Error(), + Message: err.Error(), Code: status, } } else { diff --git a/pkg/apiserver/operation.go b/pkg/apiserver/operation.go index 99489d9adfd..c8560edebfe 100644 --- a/pkg/apiserver/operation.go +++ b/pkg/apiserver/operation.go @@ -206,7 +206,8 @@ func (op *Operation) StatusOrResult() (description interface{}, finished bool) { if op.finished == nil { return api.Status{ Status: api.StatusWorking, - Details: op.ID, + Reason: api.ReasonTypeWorking, + Details: &api.StatusDetails{ID: op.ID, Kind: "operation"}, }, false } return op.result, true diff --git a/pkg/apiserver/operation_test.go b/pkg/apiserver/operation_test.go index 009542de447..cda5ee9bb40 100644 --- a/pkg/apiserver/operation_test.go +++ b/pkg/apiserver/operation_test.go @@ -177,11 +177,11 @@ func TestOpGet(t *testing.T) { t.Errorf("unexpected error: %v", err) } - if itemOut.Status != api.StatusWorking || itemOut.Details == "" { - t.Errorf("Unexpected status: %#v (%s)", itemOut, string(body)) + if itemOut.Status != api.StatusWorking || itemOut.Details == nil || itemOut.Details.ID == "" { + t.Fatalf("Unexpected status: %#v (%s)", itemOut, string(body)) } - req2, err := http.NewRequest("GET", server.URL+"/prefix/version/operations/"+itemOut.Details, nil) + req2, err := http.NewRequest("GET", server.URL+"/prefix/version/operations/"+itemOut.Details.ID, nil) if err != nil { t.Errorf("unexpected error: %v", err) } diff --git a/pkg/client/request.go b/pkg/client/request.go index 47904f230d9..729aa13281a 100644 --- a/pkg/client/request.go +++ b/pkg/client/request.go @@ -246,13 +246,18 @@ func (r *Request) Do() Result { if err != nil { if statusErr, ok := err.(*StatusErr); ok { if statusErr.Status.Status == api.StatusWorking && r.pollPeriod != 0 { - glog.Infof("Waiting for completion of /operations/%s", statusErr.Status.Details) - time.Sleep(r.pollPeriod) - // Make a poll request - pollOp := r.c.PollFor(statusErr.Status.Details).PollPeriod(r.pollPeriod) - // Could also say "return r.Do()" but this way doesn't grow the callstack. - r = pollOp - continue + if statusErr.Status.Details != nil { + id := statusErr.Status.Details.ID + if len(id) > 0 { + glog.Infof("Waiting for completion of /operations/%s", id) + time.Sleep(r.pollPeriod) + // Make a poll request + pollOp := r.c.PollFor(id).PollPeriod(r.pollPeriod) + // Could also say "return r.Do()" but this way doesn't grow the callstack. + r = pollOp + continue + } + } } } } diff --git a/pkg/client/request_test.go b/pkg/client/request_test.go index 613be01fb11..739599bb4a5 100644 --- a/pkg/client/request_test.go +++ b/pkg/client/request_test.go @@ -258,10 +258,10 @@ func TestSetPollPeriod(t *testing.T) { func TestPolling(t *testing.T) { objects := []interface{}{ - &api.Status{Status: api.StatusWorking, Details: "1234"}, - &api.Status{Status: api.StatusWorking, Details: "1234"}, - &api.Status{Status: api.StatusWorking, Details: "1234"}, - &api.Status{Status: api.StatusWorking, Details: "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.StatusWorking, Details: &api.StatusDetails{ID: "1234"}}, &api.Status{Status: api.StatusSuccess}, } diff --git a/pkg/kubecfg/proxy_server.go b/pkg/kubecfg/proxy_server.go index 11b986ee9e0..ccc5f387516 100644 --- a/pkg/kubecfg/proxy_server.go +++ b/pkg/kubecfg/proxy_server.go @@ -58,7 +58,7 @@ func (s *ProxyServer) doError(w http.ResponseWriter, err error) { w.Header().Add("Content-type", "application/json") data, _ := api.Encode(api.Status{ Status: api.StatusFailure, - Details: fmt.Sprintf("internal error: %#v", err), + Message: fmt.Sprintf("internal error: %#v", err), }) w.Write(data) } diff --git a/pkg/registry/podstorage_test.go b/pkg/registry/podstorage_test.go index 3d9abfa17f7..7cf4ba50ca6 100644 --- a/pkg/registry/podstorage_test.go +++ b/pkg/registry/podstorage_test.go @@ -36,8 +36,8 @@ func expectApiStatusError(t *testing.T, ch <-chan interface{}, msg string) { t.Errorf("Expected an api.Status object, was %#v", out) return } - if msg != status.Details { - t.Errorf("Expected %#v, was %s", msg, status.Details) + if msg != status.Message { + t.Errorf("Expected %#v, was %s", msg, status.Message) } }