Update unavailable aggregated APIs to 503s instead of 404s

This commit is contained in:
weekface 2018-01-21 10:25:03 +08:00
parent 3af0b57e80
commit f06e68a3ab
4 changed files with 68 additions and 7 deletions

View File

@ -80,12 +80,6 @@ func (c *APIServiceRegistrationController) sync(key string) error {
return err 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) return c.apiHandlerManager.AddAPIService(apiService)
} }

View File

@ -74,6 +74,8 @@ type proxyHandlingInfo struct {
serviceName string serviceName string
// namespace is the namespace the service lives in // namespace is the namespace the service lives in
serviceNamespace string serviceNamespace string
// serviceAvailable indicates this APIService is available or not
serviceAvailable bool
} }
func (r *proxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { 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 return
} }
if !handlingInfo.serviceAvailable {
http.Error(w, "service unavailable", http.StatusServiceUnavailable)
return
}
if handlingInfo.transportBuildingError != nil { if handlingInfo.transportBuildingError != nil {
http.Error(w, handlingInfo.transportBuildingError.Error(), http.StatusInternalServerError) http.Error(w, handlingInfo.transportBuildingError.Error(), http.StatusInternalServerError)
return return
@ -205,6 +212,7 @@ func (r *proxyHandler) updateAPIService(apiService *apiregistrationapi.APIServic
}, },
serviceName: apiService.Spec.Service.Name, serviceName: apiService.Spec.Service.Name,
serviceNamespace: apiService.Spec.Service.Namespace, serviceNamespace: apiService.Spec.Service.Namespace,
serviceAvailable: apiregistrationapi.IsAPIServiceConditionTrue(apiService, apiregistrationapi.Available),
} }
newInfo.proxyRoundTripper, newInfo.transportBuildingError = restclient.TransportFor(newInfo.restConfig) newInfo.proxyRoundTripper, newInfo.transportBuildingError = restclient.TransportFor(newInfo.restConfig)
if newInfo.transportBuildingError == nil && r.proxyTransport != nil && r.proxyTransport.Dial != nil { if newInfo.transportBuildingError == nil && r.proxyTransport != nil && r.proxyTransport.Dial != nil {

View File

@ -125,6 +125,11 @@ func TestProxyHandler(t *testing.T) {
Group: "foo", Group: "foo",
Version: "v1", Version: "v1",
}, },
Status: apiregistration.APIServiceStatus{
Conditions: []apiregistration.APIServiceCondition{
{Type: apiregistration.Available, Status: apiregistration.ConditionTrue},
},
},
}, },
expectedStatusCode: http.StatusInternalServerError, expectedStatusCode: http.StatusInternalServerError,
expectedBody: "missing user", expectedBody: "missing user",
@ -143,6 +148,11 @@ func TestProxyHandler(t *testing.T) {
Version: "v1", Version: "v1",
InsecureSkipTLSVerify: true, InsecureSkipTLSVerify: true,
}, },
Status: apiregistration.APIServiceStatus{
Conditions: []apiregistration.APIServiceCondition{
{Type: apiregistration.Available, Status: apiregistration.ConditionTrue},
},
},
}, },
expectedStatusCode: http.StatusOK, expectedStatusCode: http.StatusOK,
expectedCalled: true, expectedCalled: true,
@ -170,6 +180,11 @@ func TestProxyHandler(t *testing.T) {
Version: "v1", Version: "v1",
CABundle: testCACrt, CABundle: testCACrt,
}, },
Status: apiregistration.APIServiceStatus{
Conditions: []apiregistration.APIServiceCondition{
{Type: apiregistration.Available, Status: apiregistration.ConditionTrue},
},
},
}, },
expectedStatusCode: http.StatusOK, expectedStatusCode: http.StatusOK,
expectedCalled: true, expectedCalled: true,
@ -183,6 +198,28 @@ func TestProxyHandler(t *testing.T) {
"X-Remote-Group": {"one", "two"}, "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": { "fail on bad serving cert": {
user: &user.DefaultInfo{ user: &user.DefaultInfo{
Name: "username", Name: "username",
@ -196,6 +233,11 @@ func TestProxyHandler(t *testing.T) {
Group: "foo", Group: "foo",
Version: "v1", Version: "v1",
}, },
Status: apiregistration.APIServiceStatus{
Conditions: []apiregistration.APIServiceCondition{
{Type: apiregistration.Available, Status: apiregistration.ConditionTrue},
},
},
}, },
expectedStatusCode: http.StatusServiceUnavailable, expectedStatusCode: http.StatusServiceUnavailable,
}, },
@ -269,6 +311,11 @@ func TestProxyUpgrade(t *testing.T) {
Version: "v1", Version: "v1",
Service: &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns"}, Service: &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns"},
}, },
Status: apiregistration.APIServiceStatus{
Conditions: []apiregistration.APIServiceCondition{
{Type: apiregistration.Available, Status: apiregistration.ConditionTrue},
},
},
}, },
ExpectError: false, ExpectError: false,
ExpectCalled: true, ExpectCalled: true,
@ -281,6 +328,11 @@ func TestProxyUpgrade(t *testing.T) {
Version: "v1", Version: "v1",
Service: &apiregistration.ServiceReference{Name: "invalid-service", Namespace: "invalid-ns"}, Service: &apiregistration.ServiceReference{Name: "invalid-service", Namespace: "invalid-ns"},
}, },
Status: apiregistration.APIServiceStatus{
Conditions: []apiregistration.APIServiceCondition{
{Type: apiregistration.Available, Status: apiregistration.ConditionTrue},
},
},
}, },
ExpectError: false, ExpectError: false,
ExpectCalled: true, ExpectCalled: true,
@ -293,6 +345,11 @@ func TestProxyUpgrade(t *testing.T) {
Version: "v1", Version: "v1",
Service: &apiregistration.ServiceReference{Name: "invalid-service", Namespace: "invalid-ns"}, Service: &apiregistration.ServiceReference{Name: "invalid-service", Namespace: "invalid-ns"},
}, },
Status: apiregistration.APIServiceStatus{
Conditions: []apiregistration.APIServiceCondition{
{Type: apiregistration.Available, Status: apiregistration.ConditionTrue},
},
},
}, },
ExpectError: true, ExpectError: true,
ExpectCalled: false, ExpectCalled: false,

View File

@ -314,7 +314,6 @@ func TestSampleAPIServer(f *framework.Framework, image string) {
// is setting up a new namespace, we are just using that. // is setting up a new namespace, we are just using that.
err = framework.WaitForDeploymentComplete(client, deployment) 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) { 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 := restClient.Get().AbsPath("/apis/wardle.k8s.io/v1alpha1/namespaces/default/flunders")
request.SetHeader("Accept", "application/json") request.SetHeader("Accept", "application/json")
@ -324,6 +323,9 @@ func TestSampleAPIServer(f *framework.Framework, image string) {
if !ok { if !ok {
return false, err 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") { if status.Status().Code == 404 && strings.HasPrefix(err.Error(), "the server could not find the requested resource") {
return false, nil return false, nil
} }