From f06e68a3ab3c3aeb50e4d4f2ab432880aa5105a0 Mon Sep 17 00:00:00 2001 From: weekface Date: Sun, 21 Jan 2018 10:25:03 +0800 Subject: [PATCH] Update unavailable aggregated APIs to 503s instead of 404s --- .../pkg/apiserver/apiservice_controller.go | 6 -- .../pkg/apiserver/handler_proxy.go | 8 +++ .../pkg/apiserver/handler_proxy_test.go | 57 +++++++++++++++++++ test/e2e/apimachinery/aggregator.go | 4 +- 4 files changed, 68 insertions(+), 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 index 25bfca9af6a33..8eec79589cec3 100644 --- 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 index d1e90acff772a..f9cd5930fbc9e 100644 --- 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 index 11e4c8fa8bc19..5927e27afc42c 100644 --- 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 index 7d52b75954833..d9954e4f70c3a 100644 --- 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 }