remove todo

pkg/apiserver/validator.go

@@ -17,15 +17,16 @@ limitations under the License.
 package apiserver
 
 import (
-	"crypto/tls"
-	"fmt"
-	"io/ioutil"
-	"net"
 	"net/http"
-	"strconv"
 
 	"k8s.io/kubernetes/pkg/probe"
+	httpprober "k8s.io/kubernetes/pkg/probe/http"
 	"k8s.io/kubernetes/pkg/util"
+	"time"
+)
+
+const (
+	probeTimeOut = time.Minute
 )
 
 // TODO: this basic interface is duplicated in N places.  consolidate?
@@ -51,41 +52,25 @@ type ServerStatus struct {
 	Err        string       `json:"err,omitempty"`
 }
 
-// TODO: can this use pkg/probe/http
+func (server *Server) DoServerCheck(prober httpprober.HTTPProber) (probe.Result, string, error) {
-func (server *Server) DoServerCheck(rt http.RoundTripper) (probe.Result, string, error) {
+	scheme := "http"
-	var client *http.Client
-	scheme := "http://"
 	if server.EnableHTTPS {
-		// TODO(roberthbailey): The servers that use HTTPS are currently the
+		scheme = "https"
-		// kubelets, and we should be using a standard kubelet client library
-		// to talk to them rather than a separate http client.
-		transport := util.SetTransportDefaults(&http.Transport{
-			TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
-		})
-
-		client = &http.Client{Transport: transport}
-		scheme = "https://"
-	} else {
-		client = &http.Client{Transport: rt}
 	}
+	url := util.FormatURL(scheme, server.Addr, server.Port, server.Path)
+
+	result, data, err := prober.Probe(url, probeTimeOut)
 
-	resp, err := client.Get(scheme + net.JoinHostPort(server.Addr, strconv.Itoa(server.Port)) + server.Path)
 	if err != nil {
 		return probe.Unknown, "", err
 	}
-	defer resp.Body.Close()
+	if result == probe.Failure {
-	data, err := ioutil.ReadAll(resp.Body)
+		return probe.Failure, string(data), err
-	if err != nil {
-		return probe.Unknown, string(data), err
-	}
-	if resp.StatusCode != http.StatusOK {
-		return probe.Failure, string(data),
-			fmt.Errorf("unhealthy http status code: %d (%s)", resp.StatusCode, resp.Status)
 	}
 	if server.Validate != nil {
-		if err := server.Validate(data); err != nil {
+		if err := server.Validate([]byte(data)); err != nil {
 			return probe.Failure, string(data), err
 		}
 	}
-	return probe.Success, string(data), nil
+	return result, string(data), nil
 }

pkg/apiserver/validator_test.go

@@ -17,31 +17,29 @@ limitations under the License.
 package apiserver
 
 import (
-	"bytes"
 	"errors"
 	"fmt"
-	"io/ioutil"
-	"net/http"
 	"testing"
 
 	"k8s.io/kubernetes/pkg/probe"
+	"net/url"
+	"time"
 )
 
-type fakeRoundTripper struct {
+type fakeHttpProber struct {
-	err  error
+	result probe.Result
-	resp *http.Response
+	body   string
-	url  string
+	err    error
 }
 
-func (f *fakeRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
+func (f *fakeHttpProber) Probe(*url.URL, time.Duration) (probe.Result, string, error) {
-	f.url = req.URL.String()
+	return f.result, f.body, f.err
-	return f.resp, f.err
 }
 
 func alwaysError([]byte) error { return errors.New("test error") }
 
 func matchError(data []byte) error {
-	if string(data) == "bar" {
+	if string(data) != "bar" {
 		return errors.New("match error")
 	}
 	return nil
@@ -49,47 +47,44 @@ func matchError(data []byte) error {
 
 func TestValidate(t *testing.T) {
 	tests := []struct {
-		err            error
+		probeResult probe.Result
-		data           string
+		probeData   string
-		expectedStatus probe.Result
+		probeErr    error
-		code           int
+
-		expectErr      bool
+		expectResult probe.Result
-		validator      ValidatorFn
+		expectData   string
+		expectErr    bool
+
+		validator ValidatorFn
 	}{
-		{fmt.Errorf("test error"), "", probe.Unknown, 500 /*ignored*/, true, nil},
+		{probe.Unknown, "", fmt.Errorf("probe error"), probe.Unknown, "", true, nil},
-		{nil, "foo", probe.Success, 200, false, nil},
+		{probe.Failure, "", nil, probe.Failure, "", false, nil},
-		{nil, "foo", probe.Failure, 500, true, nil},
+		{probe.Success, "foo", nil, probe.Failure, "foo", true, matchError},
-		{nil, "foo", probe.Failure, 200, true, alwaysError},
+		{probe.Success, "foo", nil, probe.Success, "foo", false, nil},
-		{nil, "foo", probe.Success, 200, false, matchError},
 	}
 
 	s := Server{Addr: "foo.com", Port: 8080, Path: "/healthz"}
 
 	for _, test := range tests {
-		fakeRT := &fakeRoundTripper{
+		fakeProber := &fakeHttpProber{
-			err: test.err,
+			result: test.probeResult,
-			resp: &http.Response{
+			body:   test.probeData,
-				Body:       ioutil.NopCloser(bytes.NewBufferString(test.data)),
+			err:    test.probeErr,
-				StatusCode: test.code,
-			},
 		}
+
 		s.Validate = test.validator
-		status, data, err := s.DoServerCheck(fakeRT)
+		result, data, err := s.DoServerCheck(fakeProber)
-		expect := fmt.Sprintf("http://%s:%d/healthz", s.Addr, s.Port)
-		if fakeRT.url != expect {
-			t.Errorf("expected %s, got %s", expect, fakeRT.url)
-		}
 		if test.expectErr && err == nil {
-			t.Errorf("unexpected non-error")
+			t.Error("unexpected non-error")
 		}
 		if !test.expectErr && err != nil {
 			t.Errorf("unexpected error: %v", err)
 		}
-		if data != test.data {
+		if data != test.expectData {
-			t.Errorf("expected empty string, got %s", status)
+			t.Errorf("expected %s, got %s", test.expectData, data)
 		}
-		if status != test.expectedStatus {
+		if result != test.expectResult {
-			t.Errorf("expected %s, got %s", test.expectedStatus, status)
+			t.Errorf("expected %s, got %s", test.expectResult, result)
 		}
 	}
 }

pkg/registry/componentstatus/rest.go

@@ -18,25 +18,25 @@ package componentstatus
 
 import (
 	"fmt"
-	"net/http"
 
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/apiserver"
 	"k8s.io/kubernetes/pkg/probe"
+	httpprober "k8s.io/kubernetes/pkg/probe/http"
 	"k8s.io/kubernetes/pkg/runtime"
 )
 
 type REST struct {
 	GetServersToValidate func() map[string]apiserver.Server
-	rt                   http.RoundTripper
+	prober               httpprober.HTTPProber
 }
 
 // NewStorage returns a new REST.
 func NewStorage(serverRetriever func() map[string]apiserver.Server) *REST {
 	return &REST{
 		GetServersToValidate: serverRetriever,
-		rt:                   http.DefaultTransport,
+		prober:               httpprober.New(),
 	}
 }
 
@@ -84,8 +84,7 @@ func ToConditionStatus(s probe.Result) api.ConditionStatus {
 }
 
 func (rs *REST) getComponentStatus(name string, server apiserver.Server) *api.ComponentStatus {
-	transport := rs.rt
+	status, msg, err := server.DoServerCheck(rs.prober)
-	status, msg, err := server.DoServerCheck(transport)
 	errorMsg := ""
 	if err != nil {
 		errorMsg = err.Error()

pkg/registry/componentstatus/rest_test.go

@@ -17,34 +17,33 @@ limitations under the License.
 package componentstatus
 
 import (
-	"bytes"
 	"fmt"
-	"io/ioutil"
-	"net/http"
 	"reflect"
 	"strings"
 	"testing"
 
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/apiserver"
+	"k8s.io/kubernetes/pkg/probe"
 	"k8s.io/kubernetes/pkg/util"
+	"net/url"
+	"time"
 )
 
-type fakeRoundTripper struct {
+type fakeHttpProber struct {
-	err  error
+	result probe.Result
-	resp *http.Response
+	body   string
-	url  string
+	err    error
 }
 
-func (f *fakeRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
+func (f *fakeHttpProber) Probe(*url.URL, time.Duration) (probe.Result, string, error) {
-	f.url = req.URL.String()
+	return f.result, f.body, f.err
-	return f.resp, f.err
 }
 
 type testResponse struct {
-	code int
+	result probe.Result
-	data string
+	data   string
-	err  error
+	err    error
 }
 
 func NewTestREST(resp testResponse) *REST {
@@ -54,12 +53,10 @@ func NewTestREST(resp testResponse) *REST {
 				"test1": {Addr: "testserver1", Port: 8000, Path: "/healthz"},
 			}
 		},
-		rt: &fakeRoundTripper{
+		prober: &fakeHttpProber{
-			err: resp.err,
+			result: resp.result,
-			resp: &http.Response{
+			body:   resp.data,
-				Body:       ioutil.NopCloser(bytes.NewBufferString(resp.data)),
+			err:    resp.err,
-				StatusCode: resp.code,
-			},
 		},
 	}
 }
@@ -75,7 +72,7 @@ func createTestStatus(name string, status api.ConditionStatus, msg string, err s
 }
 
 func TestList_NoError(t *testing.T) {
-	r := NewTestREST(testResponse{code: 200, data: "ok"})
+	r := NewTestREST(testResponse{result: probe.Success, data: "ok"})
 	got, err := r.List(api.NewContext(), nil)
 	if err != nil {
 		t.Fatalf("Unexpected error: %v", err)
@@ -89,14 +86,14 @@ func TestList_NoError(t *testing.T) {
 }
 
 func TestList_FailedCheck(t *testing.T) {
-	r := NewTestREST(testResponse{code: 500, data: ""})
+	r := NewTestREST(testResponse{result: probe.Failure, data: ""})
 	got, err := r.List(api.NewContext(), nil)
 	if err != nil {
 		t.Fatalf("Unexpected error: %v", err)
 	}
 	expect := &api.ComponentStatusList{
 		Items: []api.ComponentStatus{
-			*(createTestStatus("test1", api.ConditionFalse, "", "unhealthy http status code: 500 ()"))},
+			*(createTestStatus("test1", api.ConditionFalse, "", ""))},
 	}
 	if e, a := expect, got; !reflect.DeepEqual(e, a) {
 		t.Errorf("Got unexpected object. Diff: %s", util.ObjectDiff(e, a))
@@ -104,14 +101,14 @@ func TestList_FailedCheck(t *testing.T) {
 }
 
 func TestList_UnknownError(t *testing.T) {
-	r := NewTestREST(testResponse{code: 500, data: "", err: fmt.Errorf("fizzbuzz error")})
+	r := NewTestREST(testResponse{result: probe.Unknown, data: "", err: fmt.Errorf("fizzbuzz error")})
 	got, err := r.List(api.NewContext(), nil)
 	if err != nil {
 		t.Fatalf("Unexpected error: %v", err)
 	}
 	expect := &api.ComponentStatusList{
 		Items: []api.ComponentStatus{
-			*(createTestStatus("test1", api.ConditionUnknown, "", "Get http://testserver1:8000/healthz: fizzbuzz error"))},
+			*(createTestStatus("test1", api.ConditionUnknown, "", "fizzbuzz error"))},
 	}
 	if e, a := expect, got; !reflect.DeepEqual(e, a) {
 		t.Errorf("Got unexpected object. Diff: %s", util.ObjectDiff(e, a))
@@ -119,7 +116,7 @@ func TestList_UnknownError(t *testing.T) {
 }
 
 func TestGet_NoError(t *testing.T) {
-	r := NewTestREST(testResponse{code: 200, data: "ok"})
+	r := NewTestREST(testResponse{result: probe.Success, data: "ok"})
 	got, err := r.Get(api.NewContext(), "test1")
 	if err != nil {
 		t.Fatalf("Unexpected error: %v", err)
@@ -131,7 +128,7 @@ func TestGet_NoError(t *testing.T) {
 }
 
 func TestGet_BadName(t *testing.T) {
-	r := NewTestREST(testResponse{code: 200, data: "ok"})
+	r := NewTestREST(testResponse{result: probe.Success, data: "ok"})
 	_, err := r.Get(api.NewContext(), "invalidname")
 	if err == nil {
 		t.Fatalf("Expected error, but did not get one")

pkg/util/http.go

@@ -23,6 +23,7 @@ import (
 	"net"
 	"net/http"
 	"net/url"
+	"strconv"
 	"strings"
 )
 
@@ -102,3 +103,11 @@ func TLSClientConfig(transport http.RoundTripper) (*tls.Config, error) {
 		return nil, fmt.Errorf("unknown transport type: %v", transport)
 	}
 }
+
+func FormatURL(scheme string, host string, port int, path string) *url.URL {
+	return &url.URL{
+		Scheme: scheme,
+		Host:   net.JoinHostPort(host, strconv.Itoa(port)),
+		Path:   path,
+	}
+}