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 f9cd5930fbc..7d3d603b5d7 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 @@ -120,7 +120,8 @@ func (r *proxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { location.Scheme = "https" rloc, err := r.serviceResolver.ResolveEndpoint(handlingInfo.serviceNamespace, handlingInfo.serviceName) if err != nil { - http.Error(w, fmt.Sprintf("missing route (%s)", err.Error()), http.StatusInternalServerError) + glog.Errorf("error resolving %s/%s: %v", handlingInfo.serviceNamespace, handlingInfo.serviceName, err) + http.Error(w, "service unavailable", http.StatusServiceUnavailable) return } location.Host = rloc.Host 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 5927e27afc4..b994cd18f88 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 @@ -18,6 +18,7 @@ package apiserver import ( "crypto/tls" + "fmt" "io/ioutil" "net/http" "net/http/httptest" @@ -84,13 +85,11 @@ func (*fakeRequestContextMapper) Update(req *http.Request, context genericapireq type mockedRouter struct { destinationHost string + err error } func (r *mockedRouter) ResolveEndpoint(namespace, name string) (*url.URL, error) { - return &url.URL{ - Scheme: "https", - Host: r.destinationHost, - }, nil + return &url.URL{Scheme: "https", Host: r.destinationHost}, r.err } func TestProxyHandler(t *testing.T) { @@ -109,6 +108,8 @@ func TestProxyHandler(t *testing.T) { path string apiService *apiregistration.APIService + serviceResolver ServiceResolver + expectedStatusCode int expectedBody string expectedCalled bool @@ -220,6 +221,29 @@ func TestProxyHandler(t *testing.T) { }, expectedStatusCode: http.StatusServiceUnavailable, }, + "service unresolveable": { + user: &user.DefaultInfo{ + Name: "username", + Groups: []string{"one", "two"}, + }, + path: "/request/path", + serviceResolver: &mockedRouter{err: fmt.Errorf("unresolveable")}, + apiService: &apiregistration.APIService{ + ObjectMeta: metav1.ObjectMeta{Name: "v1.foo"}, + Spec: apiregistration.APIServiceSpec{ + Service: &apiregistration.ServiceReference{Name: "bad-service", Namespace: "test-ns"}, + Group: "foo", + Version: "v1", + CABundle: testCACrt, + }, + Status: apiregistration.APIServiceStatus{ + Conditions: []apiregistration.APIServiceCondition{ + {Type: apiregistration.Available, Status: apiregistration.ConditionTrue}, + }, + }, + }, + expectedStatusCode: http.StatusServiceUnavailable, + }, "fail on bad serving cert": { user: &user.DefaultInfo{ Name: "username", @@ -247,9 +271,13 @@ func TestProxyHandler(t *testing.T) { target.Reset() func() { + serviceResolver := tc.serviceResolver + if serviceResolver == nil { + serviceResolver = &mockedRouter{destinationHost: targetServer.Listener.Addr().String()} + } handler := &proxyHandler{ localDelegate: http.NewServeMux(), - serviceResolver: &mockedRouter{destinationHost: targetServer.Listener.Addr().String()}, + serviceResolver: serviceResolver, proxyTransport: &http.Transport{}, } handler.contextMapper = &fakeRequestContextMapper{user: tc.user} diff --git a/test/e2e/apimachinery/aggregator.go b/test/e2e/apimachinery/aggregator.go index d9954e4f70c..8a04e70633d 100644 --- a/test/e2e/apimachinery/aggregator.go +++ b/test/e2e/apimachinery/aggregator.go @@ -291,6 +291,13 @@ func TestSampleAPIServer(f *framework.Framework, image string) { }) framework.ExpectNoError(err, "creating role binding %s:sample-apiserver to access configMap", namespace) + // Wait for the extension apiserver to be up and healthy + // kubectl get deployments -n && status == Running + // NOTE: aggregated apis should generally be set up in there own namespace (). As the test framework + // is setting up a new namespace, we are just using that. + err = framework.WaitForDeploymentComplete(client, deployment) + framework.ExpectNoError(err, "deploying extension apiserver in namespace %s", namespace) + // kubectl create -f apiservice.yaml _, err = aggrclient.ApiregistrationV1beta1().APIServices().Create(&apiregistrationv1beta1.APIService{ ObjectMeta: metav1.ObjectMeta{Name: "v1alpha1.wardle.k8s.io"}, @@ -308,12 +315,6 @@ func TestSampleAPIServer(f *framework.Framework, image string) { }) framework.ExpectNoError(err, "creating apiservice %s with namespace %s", "v1alpha1.wardle.k8s.io", namespace) - // Wait for the extension apiserver to be up and healthy - // kubectl get deployments -n && status == Running - // NOTE: aggregated apis should generally be set up in there own namespace (). As the test framework - // is setting up a new namespace, we are just using that. - err = framework.WaitForDeploymentComplete(client, deployment) - 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") diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 5142760840a..65d411369f4 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1200,6 +1200,10 @@ func hasRemainingContent(c clientset.Interface, clientPool dynamic.ClientPool, n if apierrs.IsMethodNotSupported(err) || apierrs.IsNotFound(err) || apierrs.IsForbidden(err) { continue } + // skip unavailable servers + if apierrs.IsServiceUnavailable(err) { + continue + } return false, err } unstructuredList, ok := obj.(*unstructured.UnstructuredList)