Update unavailable aggregated APIs to 503s instead of 404s

kubernetes · Jan 21, 2018 · f06e68a · f06e68a
1 parent 3af0b57
commit f06e68a
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 7 deletions.
diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/apiservice_controller.go b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/apiservice_controller.go
@@ -80,12 +80,6 @@ func (c *APIServiceRegistrationController) sync(key string) error {
 		return err
 	}
 
-	// remove registration handling for APIServices which are not available
-	if !apiregistration.IsAPIServiceConditionTrue(apiService, apiregistration.Available) {
-		c.apiHandlerManager.RemoveAPIService(key)
-		return nil
-	}
-
 	return c.apiHandlerManager.AddAPIService(apiService)
 }
 

diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy.go b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy.go
@@ -74,6 +74,8 @@ type proxyHandlingInfo struct {
 	serviceName string
 	// namespace is the namespace the service lives in
 	serviceNamespace string
+	// serviceAvailable indicates this APIService is available or not
+	serviceAvailable bool
 }
 
 func (r *proxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
@@ -92,6 +94,11 @@ func (r *proxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		return
 	}
 
+	if !handlingInfo.serviceAvailable {
+		http.Error(w, "service unavailable", http.StatusServiceUnavailable)
+		return
+	}
+
 	if handlingInfo.transportBuildingError != nil {
 		http.Error(w, handlingInfo.transportBuildingError.Error(), http.StatusInternalServerError)
 		return
@@ -205,6 +212,7 @@ func (r *proxyHandler) updateAPIService(apiService *apiregistrationapi.APIServic
 		},
 		serviceName:      apiService.Spec.Service.Name,
 		serviceNamespace: apiService.Spec.Service.Namespace,
+		serviceAvailable: apiregistrationapi.IsAPIServiceConditionTrue(apiService, apiregistrationapi.Available),
 	}
 	newInfo.proxyRoundTripper, newInfo.transportBuildingError = restclient.TransportFor(newInfo.restConfig)
 	if newInfo.transportBuildingError == nil && r.proxyTransport != nil && r.proxyTransport.Dial != nil {

diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy_test.go b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy_test.go
@@ -125,6 +125,11 @@ func TestProxyHandler(t *testing.T) {
 					Group:   "foo",
 					Version: "v1",
 				},
+				Status: apiregistration.APIServiceStatus{
+					Conditions: []apiregistration.APIServiceCondition{
+						{Type: apiregistration.Available, Status: apiregistration.ConditionTrue},
+					},
+				},
 			},
 			expectedStatusCode: http.StatusInternalServerError,
 			expectedBody:       "missing user",
@@ -143,6 +148,11 @@ func TestProxyHandler(t *testing.T) {
 					Version:               "v1",
 					InsecureSkipTLSVerify: true,
 				},
+				Status: apiregistration.APIServiceStatus{
+					Conditions: []apiregistration.APIServiceCondition{
+						{Type: apiregistration.Available, Status: apiregistration.ConditionTrue},
+					},
+				},
 			},
 			expectedStatusCode: http.StatusOK,
 			expectedCalled:     true,
@@ -170,6 +180,11 @@ func TestProxyHandler(t *testing.T) {
 					Version:  "v1",
 					CABundle: testCACrt,
 				},
+				Status: apiregistration.APIServiceStatus{
+					Conditions: []apiregistration.APIServiceCondition{
+						{Type: apiregistration.Available, Status: apiregistration.ConditionTrue},
+					},
+				},
 			},
 			expectedStatusCode: http.StatusOK,
 			expectedCalled:     true,
@@ -183,6 +198,28 @@ func TestProxyHandler(t *testing.T) {
 				"X-Remote-Group":    {"one", "two"},
 			},
 		},
+		"service unavailable": {
+			user: &user.DefaultInfo{
+				Name:   "username",
+				Groups: []string{"one", "two"},
+			},
+			path: "/request/path",
+			apiService: &apiregistration.APIService{
+				ObjectMeta: metav1.ObjectMeta{Name: "v1.foo"},
+				Spec: apiregistration.APIServiceSpec{
+					Service:  &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns"},
+					Group:    "foo",
+					Version:  "v1",
+					CABundle: testCACrt,
+				},
+				Status: apiregistration.APIServiceStatus{
+					Conditions: []apiregistration.APIServiceCondition{
+						{Type: apiregistration.Available, Status: apiregistration.ConditionFalse},
+					},
+				},
+			},
+			expectedStatusCode: http.StatusServiceUnavailable,
+		},
 		"fail on bad serving cert": {
 			user: &user.DefaultInfo{
 				Name:   "username",
@@ -196,6 +233,11 @@ func TestProxyHandler(t *testing.T) {
 					Group:   "foo",
 					Version: "v1",
 				},
+				Status: apiregistration.APIServiceStatus{
+					Conditions: []apiregistration.APIServiceCondition{
+						{Type: apiregistration.Available, Status: apiregistration.ConditionTrue},
+					},
+				},
 			},
 			expectedStatusCode: http.StatusServiceUnavailable,
 		},
@@ -269,6 +311,11 @@ func TestProxyUpgrade(t *testing.T) {
 					Version:  "v1",
 					Service:  &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns"},
 				},
+				Status: apiregistration.APIServiceStatus{
+					Conditions: []apiregistration.APIServiceCondition{
+						{Type: apiregistration.Available, Status: apiregistration.ConditionTrue},
+					},
+				},
 			},
 			ExpectError:  false,
 			ExpectCalled: true,
@@ -281,6 +328,11 @@ func TestProxyUpgrade(t *testing.T) {
 					Version: "v1",
 					Service: &apiregistration.ServiceReference{Name: "invalid-service", Namespace: "invalid-ns"},
 				},
+				Status: apiregistration.APIServiceStatus{
+					Conditions: []apiregistration.APIServiceCondition{
+						{Type: apiregistration.Available, Status: apiregistration.ConditionTrue},
+					},
+				},
 			},
 			ExpectError:  false,
 			ExpectCalled: true,
@@ -293,6 +345,11 @@ func TestProxyUpgrade(t *testing.T) {
 					Version:  "v1",
 					Service:  &apiregistration.ServiceReference{Name: "invalid-service", Namespace: "invalid-ns"},
 				},
+				Status: apiregistration.APIServiceStatus{
+					Conditions: []apiregistration.APIServiceCondition{
+						{Type: apiregistration.Available, Status: apiregistration.ConditionTrue},
+					},
+				},
 			},
 			ExpectError:  true,
 			ExpectCalled: false,

diff --git a/test/e2e/apimachinery/aggregator.go b/test/e2e/apimachinery/aggregator.go
@@ -314,7 +314,6 @@ func TestSampleAPIServer(f *framework.Framework, image string) {
 	// is setting up a new namespace, we are just using that.
 	err = framework.WaitForDeploymentComplete(client, deployment)
 
-	// We seem to need to do additional waiting until the extension api service is actually up.
 	err = wait.Poll(100*time.Millisecond, 30*time.Second, func() (bool, error) {
 		request := restClient.Get().AbsPath("/apis/wardle.k8s.io/v1alpha1/namespaces/default/flunders")
 		request.SetHeader("Accept", "application/json")
@@ -324,6 +323,9 @@ func TestSampleAPIServer(f *framework.Framework, image string) {
 			if !ok {
 				return false, err
 			}
+			if status.Status().Code == 503 {
+				return false, nil
+			}
 			if status.Status().Code == 404 && strings.HasPrefix(err.Error(), "the server could not find the requested resource") {
 				return false, nil
 			}