From 1a23f5a79f34cb70fa65de9b3902428a16a366ca Mon Sep 17 00:00:00 2001
From: Daniel Smith <dbsmith@google.com>
Date: Tue, 30 Aug 2016 16:56:56 -0700
Subject: [PATCH] Log useful information on 500's

* include error message in error (!!)
* add test verifying error message is correct for service ip allocation
---
 pkg/api/errors/errors.go               |  2 +-
 pkg/apiserver/apiserver.go             |  8 +++-
 pkg/httplog/log.go                     |  6 +++
 test/integration/master/master_test.go | 52 ++++++++++++++++++++++++++
 4 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/pkg/api/errors/errors.go b/pkg/api/errors/errors.go
index 432bbed855..44bfe82f86 100644
--- a/pkg/api/errors/errors.go
+++ b/pkg/api/errors/errors.go
@@ -325,7 +325,7 @@ func NewGenericServerResponse(code int, verb string, qualifiedResource unversion
 	default:
 		if code >= 500 {
 			reason = unversioned.StatusReasonInternalError
-			message = "an error on the server has prevented the request from succeeding"
+			message = fmt.Sprintf("an error on the server (%q) has prevented the request from succeeding", serverMessage)
 		}
 	}
 	switch {
diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go
index 54839a058d..0e1530eeb8 100644
--- a/pkg/apiserver/apiserver.go
+++ b/pkg/apiserver/apiserver.go
@@ -266,7 +266,13 @@ func InstallServiceErrorHandler(s runtime.NegotiatedSerializer, container *restf
 }
 
 func serviceErrorHandler(s runtime.NegotiatedSerializer, requestResolver *RequestInfoResolver, apiVersions []string, serviceErr restful.ServiceError, request *restful.Request, response *restful.Response) {
-	errorNegotiated(apierrors.NewGenericServerResponse(serviceErr.Code, "", api.Resource(""), "", "", 0, false), s, unversioned.GroupVersion{}, response.ResponseWriter, request.Request)
+	errorNegotiated(
+		apierrors.NewGenericServerResponse(serviceErr.Code, "", api.Resource(""), "", serviceErr.Message, 0, false),
+		s,
+		unversioned.GroupVersion{},
+		response.ResponseWriter,
+		request.Request,
+	)
 }
 
 // Adds a service to return the supported api versions at the legacy /api.
diff --git a/pkg/httplog/log.go b/pkg/httplog/log.go
index 2c42bd781b..4a4894cee0 100644
--- a/pkg/httplog/log.go
+++ b/pkg/httplog/log.go
@@ -60,6 +60,8 @@ type respLogger struct {
 	addedInfo      string
 	startTime      time.Time
 
+	captureErrorOutput bool
+
 	req *http.Request
 	w   http.ResponseWriter
 
@@ -175,6 +177,9 @@ func (rl *respLogger) Write(b []byte) (int, error) {
 	if !rl.statusRecorded {
 		rl.recordStatus(http.StatusOK) // Default if WriteHeader hasn't been called
 	}
+	if rl.captureErrorOutput {
+		rl.Addf("logging error output: %q\n", string(b))
+	}
 	return rl.w.Write(b)
 }
 
@@ -213,6 +218,7 @@ func (rl *respLogger) recordStatus(status int) {
 		stack := make([]byte, 50*1024)
 		stack = stack[:runtime.Stack(stack, false)]
 		rl.statusStack = "\n" + string(stack)
+		rl.captureErrorOutput = true
 	} else {
 		rl.statusStack = ""
 	}
diff --git a/test/integration/master/master_test.go b/test/integration/master/master_test.go
index 5334d9814a..f1e80fe238 100644
--- a/test/integration/master/master_test.go
+++ b/test/integration/master/master_test.go
@@ -23,6 +23,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
+	"net"
 	"net/http"
 	"strings"
 	"testing"
@@ -414,3 +415,54 @@ func TestMasterService(t *testing.T) {
 		t.Errorf("unexpected error: %v", err)
 	}
 }
+
+func TestServiceAlloc(t *testing.T) {
+	cfg := framework.NewIntegrationTestMasterConfig()
+	_, cidr, err := net.ParseCIDR("192.168.0.0/30")
+	if err != nil {
+		t.Fatalf("bad cidr: %v", err)
+	}
+	cfg.ServiceClusterIPRange = cidr
+	_, s := framework.RunAMaster(cfg)
+	defer s.Close()
+
+	client := client.NewOrDie(&restclient.Config{Host: s.URL, ContentConfig: restclient.ContentConfig{GroupVersion: testapi.Default.GroupVersion()}})
+
+	svc := func(i int) *api.Service {
+		return &api.Service{
+			ObjectMeta: api.ObjectMeta{
+				Name: fmt.Sprintf("svc-%v", i),
+			},
+			Spec: api.ServiceSpec{
+				Type: api.ServiceTypeClusterIP,
+				Ports: []api.ServicePort{
+					{Port: 80},
+				},
+			},
+		}
+	}
+
+	// Make a service.
+	if _, err := client.Services(api.NamespaceDefault).Create(svc(1)); err != nil {
+		t.Fatalf("got unexpected error: %v", err)
+	}
+
+	// Make a second service. It will fail because we're out of cluster IPs
+	if _, err := client.Services(api.NamespaceDefault).Create(svc(2)); err != nil {
+		if !strings.Contains(err.Error(), "range is full") {
+			t.Errorf("unexpected error text: %v", err)
+		}
+	} else {
+		t.Fatalf("unexpected sucess")
+	}
+
+	// Delete the first service.
+	if err := client.Services(api.NamespaceDefault).Delete(svc(1).ObjectMeta.Name); err != nil {
+		t.Fatalf("got unexpected error: %v", err)
+	}
+
+	// This time creating the second service should work.
+	if _, err := client.Services(api.NamespaceDefault).Create(svc(2)); err != nil {
+		t.Fatalf("got unexpected error: %v", err)
+	}
+}