diff --git a/README.md b/README.md index 45707ba0d0f..725335e4a18 100644 --- a/README.md +++ b/README.md @@ -204,4 +204,3 @@ sudo docker build -t kubernetes/raml2html . sudo docker run --name="docgen" kubernetes/raml2html sudo docker cp docgen:/data/kubernetes.html . ``` - diff --git a/pkg/health/health.go b/pkg/health/health.go index 7ea0faf1b25..2610482d2ac 100644 --- a/pkg/health/health.go +++ b/pkg/health/health.go @@ -42,12 +42,10 @@ type HTTPGetInterface interface { // It returns Unknown and err if the HTTP communication itself fails. func Check(url string, client HTTPGetInterface) (Status, error) { res, err := client.Get(url) - if res.Body != nil { - defer res.Body.Close() - } if err != nil { return Unknown, err } + defer res.Body.Close() if res.StatusCode >= http.StatusOK && res.StatusCode < http.StatusBadRequest { return Healthy, nil } diff --git a/pkg/health/health_check_test.go b/pkg/health/health_check_test.go index 4f4be7293c7..ba36249578c 100644 --- a/pkg/health/health_check_test.go +++ b/pkg/health/health_check_test.go @@ -17,54 +17,62 @@ limitations under the License. package health import ( + "net" "net/http" + "net/http/httptest" + "net/url" "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" ) -// fakeHTTPClient is a fake implementation of HTTPGetInterface. -type fakeHTTPClient struct { - req string - res http.Response - err error -} +const statusServerEarlyShutdown = -1 -func (f *fakeHTTPClient) Get(url string) (*http.Response, error) { - f.req = url - return &f.res, f.err -} - -func TestHttpHealth(t *testing.T) { - fakeClient := fakeHTTPClient{ - res: http.Response{ - StatusCode: http.StatusOK, - }, +func TestHealthChecker(t *testing.T) { + var healthCheckerTests = []struct { + status int + health Status + }{ + {http.StatusOK, Healthy}, + {statusServerEarlyShutdown, Unknown}, + {http.StatusBadRequest, Unhealthy}, + {http.StatusBadGateway, Unhealthy}, + {http.StatusInternalServerError, Unhealthy}, } - - check := HTTPHealthChecker{ - client: &fakeClient, - } - - container := api.Container{ - LivenessProbe: &api.LivenessProbe{ - HTTPGet: &api.HTTPGetProbe{ - Port: "8080", - Path: "/foo/bar", + for _, healthCheckerTest := range healthCheckerTests { + tt := healthCheckerTest + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(tt.status) + })) + u, err := url.Parse(ts.URL) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + host, port, err := net.SplitHostPort(u.Host) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if tt.status == statusServerEarlyShutdown { + ts.Close() + } + container := api.Container{ + LivenessProbe: &api.LivenessProbe{ + HTTPGet: &api.HTTPGetProbe{ + Port: port, + Path: "/foo/bar", + Host: host, + }, + Type: "http", }, - Type: "http", - }, - } - - ok, err := check.HealthCheck(container) - if ok != Healthy { - t.Error("Unexpected unhealthy") - } - if err != nil { - t.Errorf("Unexpected error: %#v", err) - } - if fakeClient.req != "http://localhost:8080/foo/bar" { - t.Errorf("Unexpected url: %s", fakeClient.req) + } + hc := NewHealthChecker() + health, err := hc.HealthCheck(container) + if err != nil && tt.health != Unknown { + t.Errorf("Unexpected error: %v", err) + } + if health != tt.health { + t.Errorf("Expected %v, got %v", tt.health, health) + } } } @@ -81,12 +89,10 @@ func TestFindPort(t *testing.T) { }, }, } - check := HTTPHealthChecker{} - validatePort(t, check.findPort(container, "foo"), 8080) -} - -func validatePort(t *testing.T, port int64, expectedPort int64) { - if port != expectedPort { - t.Errorf("Unexpected port: %d, expected: %d", port, expectedPort) + checker := HTTPHealthChecker{} + want := int64(8080) + got := checker.findPort(container, "foo") + if got != want { + t.Errorf("Expected %v, got %v", want, got) } } diff --git a/pkg/registry/healthy_minion_registry_test.go b/pkg/registry/healthy_minion_registry_test.go index 7ba29a6b6bc..73d17b4e0aa 100644 --- a/pkg/registry/healthy_minion_registry_test.go +++ b/pkg/registry/healthy_minion_registry_test.go @@ -17,6 +17,8 @@ limitations under the License. package registry import ( + "bytes" + "io/ioutil" "net/http" "reflect" "testing" @@ -24,10 +26,15 @@ import ( type alwaysYes struct{} -func (alwaysYes) Get(url string) (*http.Response, error) { +func fakeHTTPResponse(status int) *http.Response { return &http.Response{ - StatusCode: http.StatusOK, - }, nil + StatusCode: status, + Body: ioutil.NopCloser(&bytes.Buffer{}), + } +} + +func (alwaysYes) Get(url string) (*http.Response, error) { + return fakeHTTPResponse(http.StatusOK), nil } func TestBasicDelegation(t *testing.T) { @@ -66,9 +73,9 @@ type notMinion struct { func (n *notMinion) Get(url string) (*http.Response, error) { if url != "http://"+n.minion+":10250/healthz" { - return &http.Response{StatusCode: http.StatusOK}, nil + return fakeHTTPResponse(http.StatusOK), nil } else { - return &http.Response{StatusCode: http.StatusInternalServerError}, nil + return fakeHTTPResponse(http.StatusInternalServerError), nil } }