Evolve the api.Status object with Reason/Details

Contains breaking API change on api.Status#Details (type change)

Turn Details from string -> StatusDetails - a general
bucket for keyed error behavior.  Define an open enumeration
ReasonType exposed as Reason on the status object to provide
machine readable subcategorization beyond HTTP Status Code. Define
a human readable field Message which is common convention (previously
this was joined into Details).

Precedence order: HTTP Status Code, Reason, Details. apiserver would
impose restraints on the ReasonTypes defined by the main apiobject,
and ensure their use is consistent.

There are four long term scenarios this change supports:

1. Allow a client access to a machine readable field that can be
   easily switched on for improving or translating the generic
   server Message.

2. Return a 404 when a composite operation on multiple resources
   fails with enough data so that a client can distinguish which
   item does not exist.  E.g. resource Parent and resource Child,
   POST /parents/1/children to create a new Child, but /parents/1
   is deleted.  POST returns 404, ReasonTypeNotFound, and
   Details.ID = "1", Details.Kind = "parent"

3. Allow a client to receive validation data that is keyed by
   attribute for building user facing UIs around field submission.
   Validation is usually expressed as map[string][]string, but
   that type is less appropriate for many other uses.

4. Allow specific API errors to return more granular failure status
   for specific operations.  An example might be a minion proxy,
   where the operation that failed may be both proxying OR the
   minion itself.  In this case a reason may be defined "proxy_failed"
   corresponding to 502, where the Details field may be extended
   to contain a nested error object.

At this time only ID and Kind are exposed
This commit is contained in:
Clayton Coleman 2014-07-31 14:12:26 -04:00
parent 83aeae7658
commit 4f88b778a6
10 changed files with 186 additions and 38 deletions

View File

@ -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"`

View File

@ -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"`

View File

@ -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)
}
}

View File

@ -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 {

View File

@ -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

View File

@ -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)
}

View File

@ -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
}
}
}
}
}

View File

@ -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},
}

View File

@ -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)
}

View File

@ -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)
}
}