Merge pull request #745 from smarterclayton/add_structured_reason

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

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

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

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 {

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

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

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

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

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

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