Merge pull request #11076 from nikhiljindal/apiError

Updating apiserver to not return 500 when a service without pods is proxied
kubernetes · Jul 24, 2015 · 1a908a2 · 1a908a2
2 parents 0a715a7 + 1517b66
commit 1a908a2
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 3 deletions.
diff --git a/examples/guestbook/frontend-service.yaml b/examples/guestbook/frontend-service.yaml
@@ -10,6 +10,6 @@ spec:
   # type: LoadBalancer
   ports:
     # the port that this service should serve on
-  - port: 80
+    - port: 80
   selector:
     name: frontend
diff --git a/pkg/api/errors/errors.go b/pkg/api/errors/errors.go
@@ -193,6 +193,16 @@ func NewBadRequest(reason string) error {
 	}}
 }
 
+// NewServiceUnavailable creates an error that indicates that the requested service is unavailable.
+func NewServiceUnavailable(reason string) error {
+	return &StatusError{api.Status{
+		Status:  api.StatusFailure,
+		Code:    http.StatusServiceUnavailable,
+		Reason:  api.StatusReasonServiceUnavailable,
+		Message: reason,
+	}}
+}
+
 // NewMethodNotSupported returns an error indicating the requested action is not supported on this kind.
 func NewMethodNotSupported(kind, action string) error {
 	return &StatusError{api.Status{

diff --git a/pkg/api/types.go b/pkg/api/types.go
@@ -1708,6 +1708,12 @@ const (
 	//   "causes" - The original error
 	// Status code 500
 	StatusReasonInternalError = "InternalError"
+
+	// StatusReasonServiceUnavailable means that the request itself was valid,
+	// but the requested service is unavailable at this time.
+	// Retrying the request after some time might succeed.
+	// Status code 503
+	StatusReasonServiceUnavailable StatusReason = "ServiceUnavailable"
 )
 
 // StatusCause provides more information about an api.Status failure, including

diff --git a/pkg/registry/service/rest.go b/pkg/registry/service/rest.go
@@ -300,7 +300,7 @@ func (rs *REST) ResourceLocation(ctx api.Context, id string) (*url.URL, http.Rou
 		return nil, nil, err
 	}
 	if len(eps.Subsets) == 0 {
-		return nil, nil, fmt.Errorf("no endpoints available for %q", svcName)
+		return nil, nil, errors.NewServiceUnavailable(fmt.Sprintf("no endpoints available for service %q", svcName))
 	}
 	// Pick a random Subset to start searching from.
 	ssSeed := rand.Intn(len(eps.Subsets))
@@ -320,7 +320,7 @@ func (rs *REST) ResourceLocation(ctx api.Context, id string) (*url.URL, http.Rou
 			}
 		}
 	}
-	return nil, nil, fmt.Errorf("no endpoints available for %q", id)
+	return nil, nil, errors.NewServiceUnavailable(fmt.Sprintf("no endpoints available for service %q", id))
 }
 
 // This is O(N), but we expect haystack to be small;

diff --git a/test/integration/auth_test.go b/test/integration/auth_test.go
@@ -199,6 +199,16 @@ var aBinding string = `
 }
 `
 
+var emptyEndpoints string = `
+{
+  "kind": "Endpoints",
+  "apiVersion": "v1",
+  "metadata": {
+    "name": "a"%s
+  }
+}
+`
+
 var aEndpoints string = `
 {
   "kind": "Endpoints",
@@ -243,6 +253,7 @@ var code405 = map[int]bool{405: true}
 var code409 = map[int]bool{409: true}
 var code422 = map[int]bool{422: true}
 var code500 = map[int]bool{500: true}
+var code503 = map[int]bool{503: true}
 
 // To ensure that a POST completes before a dependent GET, set a timeout.
 func addTimeoutFlag(URLString string) string {
@@ -299,8 +310,14 @@ func getTestRequests() []struct {
 		{"GET", path("services", "", ""), "", code200},
 		{"GET", path("services", api.NamespaceDefault, ""), "", code200},
 		{"POST", timeoutPath("services", api.NamespaceDefault, ""), aService, code201},
+		// Create an endpoint for the service (this is done automatically by endpoint controller
+		// whenever a service is created, but this test does not run that controller)
+		{"POST", timeoutPath("endpoints", api.NamespaceDefault, ""), emptyEndpoints, code201},
+		// Should return service unavailable when endpoint.subset is empty.
+		{"GET", pathWithPrefix("proxy", "services", api.NamespaceDefault, "a") + "/", "", code503},
 		{"PUT", timeoutPath("services", api.NamespaceDefault, "a"), aService, code200},
 		{"GET", path("services", api.NamespaceDefault, "a"), "", code200},
+		{"DELETE", timeoutPath("endpoints", api.NamespaceDefault, "a"), "", code200},
 		{"DELETE", timeoutPath("services", api.NamespaceDefault, "a"), "", code200},
 
 		// Normal methods on replicationControllers