diff --git a/pkg/api/types.go b/pkg/api/types.go index 3c7b2df2c8634..798b799ba21ef 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -136,11 +136,11 @@ type EnvVar struct { // HTTPGetProbe describes a liveness probe based on HTTP Get requests. type HTTPGetProbe struct { - // Path to access on the http server + // Optional: Path to access on the HTTP server. Path string `yaml:"path,omitempty" json:"path,omitempty"` - // Name or number of the port to access on the container - Port string `yaml:"port,omitempty" json:"port,omitempty"` - // Host name to connect to. Optional, default: "localhost" + // Required: Name or number of the port to access on the container. + Port util.IntOrString `yaml:"port,omitempty" json:"port,omitempty"` + // Optional: Host name to connect to, defaults to the pod IP. Host string `yaml:"host,omitempty" json:"host,omitempty"` } diff --git a/pkg/api/v1beta1/types.go b/pkg/api/v1beta1/types.go index 16346bccad208..9bcc814907a4a 100644 --- a/pkg/api/v1beta1/types.go +++ b/pkg/api/v1beta1/types.go @@ -139,11 +139,11 @@ type EnvVar struct { // HTTPGetProbe describes a liveness probe based on HTTP Get requests. type HTTPGetProbe struct { - // Path to access on the http server + // Optional: Path to access on the HTTP server. Path string `yaml:"path,omitempty" json:"path,omitempty"` - // Name or number of the port to access on the container - Port string `yaml:"port,omitempty" json:"port,omitempty"` - // Host name to connect to. Optional, default: "localhost" + // Required: Name or number of the port to access on the container. + Port util.IntOrString `yaml:"port,omitempty" json:"port,omitempty"` + // Optional: Host name to connect to, defaults to the pod IP. Host string `yaml:"host,omitempty" json:"host,omitempty"` } diff --git a/pkg/health/health.go b/pkg/health/health.go index 2610482d2ac24..9136409a40487 100644 --- a/pkg/health/health.go +++ b/pkg/health/health.go @@ -36,11 +36,11 @@ type HTTPGetInterface interface { Get(url string) (*http.Response, error) } -// Check checks if a GET request to the url succeeds. +// DoHTTPCheck checks if a GET request to the url succeeds. // If the HTTP response code is successful (i.e. 400 > code >= 200), it returns Healthy. // If the HTTP response code is unsuccessful, it returns Unhealthy. // It returns Unknown and err if the HTTP communication itself fails. -func Check(url string, client HTTPGetInterface) (Status, error) { +func DoHTTPCheck(url string, client HTTPGetInterface) (Status, error) { res, err := client.Get(url) if err != nil { return Unknown, err diff --git a/pkg/health/health_check.go b/pkg/health/health_check.go index abb06fa7be2a6..9b795b10f0ae3 100644 --- a/pkg/health/health_check.go +++ b/pkg/health/health_check.go @@ -23,6 +23,7 @@ import ( "strconv" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" "github.com/golang/glog" ) @@ -65,38 +66,62 @@ type HTTPHealthChecker struct { client HTTPGetInterface } -func (h *HTTPHealthChecker) findPort(container api.Container, portName string) int64 { +// A helper function to look up a port in a container by name. +// Returns the HostPort if found, -1 if not found. +func findPortByName(container api.Container, portName string) int { for _, port := range container.Ports { if port.Name == portName { - // TODO This means you can only health check exposed ports - return int64(port.HostPort) + return port.HostPort } } return -1 } -// HealthCheck checks if the container is healthy by trying sending HTTP Get requests to the container. -func (h *HTTPHealthChecker) HealthCheck(currentState api.PodState, container api.Container) (Status, error) { +// Get the components of the target URL. For testability. +func getURLParts(currentState api.PodState, container api.Container) (string, int, string, error) { params := container.LivenessProbe.HTTPGet if params == nil { - return Unknown, fmt.Errorf("Error, no HTTP parameters specified: %v", container) + return "", -1, "", fmt.Errorf("no HTTP parameters specified: %v", container) } - port := h.findPort(container, params.Port) - if port == -1 { - var err error - port, err = strconv.ParseInt(params.Port, 10, 0) - if err != nil { - return Unknown, err + port := -1 + switch params.Port.Kind { + case util.IntstrInt: + port = params.Port.IntVal + case util.IntstrString: + port = findPortByName(container, params.Port.StrVal) + if port == -1 { + // Last ditch effort - maybe it was an int stored as string? + var err error + if port, err = strconv.Atoi(params.Port.StrVal); err != nil { + return "", -1, "", err + } } } + if port == -1 { + return "", -1, "", fmt.Errorf("unknown port: %v", params.Port) + } var host string if len(params.Host) > 0 { host = params.Host } else { host = currentState.PodIP } - url := fmt.Sprintf("http://%s:%d%s", host, port, params.Path) - return Check(url, h.client) + + return host, port, params.Path, nil +} + +// Formats a URL from args. For testability. +func formatURL(host string, port int, path string) string { + return fmt.Sprintf("http://%s:%d%s", host, port, path) +} + +// HealthCheck checks if the container is healthy by trying sending HTTP Get requests to the container. +func (h *HTTPHealthChecker) HealthCheck(currentState api.PodState, container api.Container) (Status, error) { + host, port, path, err := getURLParts(currentState, container) + if err != nil { + return Unknown, err + } + return DoHTTPCheck(formatURL(host, port, path), h.client) } type TCPHealthChecker struct{} diff --git a/pkg/health/health_check_test.go b/pkg/health/health_check_test.go index bc9841680cd6a..8d0eaaa374746 100644 --- a/pkg/health/health_check_test.go +++ b/pkg/health/health_check_test.go @@ -25,6 +25,7 @@ import ( "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) const statusServerEarlyShutdown = -1 @@ -59,7 +60,7 @@ func TestHealthChecker(t *testing.T) { container := api.Container{ LivenessProbe: &api.LivenessProbe{ HTTPGet: &api.HTTPGetProbe{ - Port: port, + Port: util.MakeIntOrStringFromString(port), Path: "/foo/bar", Host: host, }, @@ -90,32 +91,91 @@ func TestFindPort(t *testing.T) { }, }, } - checker := HTTPHealthChecker{} - want := int64(8080) - got := checker.findPort(container, "foo") + want := 8080 + got := findPortByName(container, "foo") if got != want { t.Errorf("Expected %v, got %v", want, got) } } +func TestGetURLParts(t *testing.T) { + testCases := []struct { + probe *api.HTTPGetProbe + ok bool + host string + port int + path string + }{ + {&api.HTTPGetProbe{Host: "", Port: util.MakeIntOrStringFromInt(-1), Path: ""}, false, "", -1, ""}, + {&api.HTTPGetProbe{Host: "", Port: util.MakeIntOrStringFromString(""), Path: ""}, false, "", -1, ""}, + {&api.HTTPGetProbe{Host: "", Port: util.MakeIntOrStringFromString("-1"), Path: ""}, false, "", -1, ""}, + {&api.HTTPGetProbe{Host: "", Port: util.MakeIntOrStringFromString("not-found"), Path: ""}, false, "", -1, ""}, + {&api.HTTPGetProbe{Host: "", Port: util.MakeIntOrStringFromString("found"), Path: ""}, true, "127.0.0.1", 93, ""}, + {&api.HTTPGetProbe{Host: "", Port: util.MakeIntOrStringFromInt(76), Path: ""}, true, "127.0.0.1", 76, ""}, + {&api.HTTPGetProbe{Host: "", Port: util.MakeIntOrStringFromString("118"), Path: ""}, true, "127.0.0.1", 118, ""}, + {&api.HTTPGetProbe{Host: "hostname", Port: util.MakeIntOrStringFromInt(76), Path: "path"}, true, "hostname", 76, "path"}, + } + + for _, test := range testCases { + state := api.PodState{PodIP: "127.0.0.1"} + container := api.Container{ + Ports: []api.Port{{Name: "found", HostPort: 93}}, + LivenessProbe: &api.LivenessProbe{ + HTTPGet: test.probe, + Type: "http", + }, + } + host, port, path, err := getURLParts(state, container) + if !test.ok && err == nil { + t.Errorf("Expected error for %+v, got %s:%d/%s", test, host, port, path) + } + if test.ok && err != nil { + t.Errorf("Unexpected error: %v", err) + } + if test.ok { + if host != test.host || port != test.port || path != test.path { + t.Errorf("Expected %s:%d/%s, got %s:%d/%s", + test.host, test.port, test.path, host, port, path) + } + } + } +} + +func TestFormatURL(t *testing.T) { + testCases := []struct { + host string + port int + path string + result string + }{ + {"localhost", 93, "", "http://localhost:93"}, + {"localhost", 93, "/path", "http://localhost:93/path"}, + } + for _, test := range testCases { + url := formatURL(test.host, test.port, test.path) + if url != test.result { + t.Errorf("Expected %s, got %s", test.result, url) + } + } +} + func TestHTTPHealthChecker(t *testing.T) { - httpHealthCheckerTests := []struct { + testCases := []struct { probe *api.HTTPGetProbe status int health Status }{ - {&api.HTTPGetProbe{Host: "httptest"}, http.StatusOK, Healthy}, + // The probe will be filled in below. This is primarily testing that an HTTP GET happens. {&api.HTTPGetProbe{}, http.StatusOK, Healthy}, + {&api.HTTPGetProbe{}, -1, Unhealthy}, {nil, -1, Unknown}, - {&api.HTTPGetProbe{Port: "-1"}, -1, Unknown}, } hc := &HTTPHealthChecker{ client: &http.Client{}, } - for _, httpHealthCheckerTest := range httpHealthCheckerTests { - tt := httpHealthCheckerTest + for _, test := range testCases { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(tt.status) + w.WriteHeader(test.status) })) u, err := url.Parse(ts.URL) if err != nil { @@ -127,28 +187,24 @@ func TestHTTPHealthChecker(t *testing.T) { } container := api.Container{ LivenessProbe: &api.LivenessProbe{ - HTTPGet: tt.probe, + HTTPGet: test.probe, Type: "http", }, } params := container.LivenessProbe.HTTPGet if params != nil { - if params.Port == "" { - params.Port = port - } - if params.Host == "httptest" { - params.Host = host - } + params.Port = util.MakeIntOrStringFromString(port) + params.Host = host } health, err := hc.HealthCheck(api.PodState{PodIP: host}, container) - if tt.health == Unknown && err == nil { + if test.health == Unknown && err == nil { t.Errorf("Expected error") } - if tt.health != Unknown && err != nil { + if test.health != Unknown && err != nil { t.Errorf("Unexpected error: %v", err) } - if health != tt.health { - t.Errorf("Expected %v, got %v", tt.health, health) + if health != test.health { + t.Errorf("Expected %v, got %v", test.health, health) } } } @@ -228,7 +284,7 @@ func TestMuxHealthChecker(t *testing.T) { }, } container.LivenessProbe.Type = tt.probeType - container.LivenessProbe.HTTPGet.Port = port + container.LivenessProbe.HTTPGet.Port = util.MakeIntOrStringFromString(port) container.LivenessProbe.HTTPGet.Host = host health, err := mc.HealthCheck(api.PodState{}, container) if err != nil { diff --git a/pkg/registry/healthy_minion_registry.go b/pkg/registry/healthy_minion_registry.go index 39c99331a4939..658052f13e89d 100644 --- a/pkg/registry/healthy_minion_registry.go +++ b/pkg/registry/healthy_minion_registry.go @@ -49,7 +49,7 @@ func (h *HealthyMinionRegistry) List() (currentMinions []string, err error) { return result, err } for _, minion := range list { - status, err := health.Check(h.makeMinionURL(minion), h.client) + status, err := health.DoHTTPCheck(h.makeMinionURL(minion), h.client) if err != nil { glog.Errorf("%s failed health check with error: %s", minion, err) continue @@ -77,7 +77,7 @@ func (h *HealthyMinionRegistry) Contains(minion string) (bool, error) { if !contains { return false, nil } - status, err := health.Check(h.makeMinionURL(minion), h.client) + status, err := health.DoHTTPCheck(h.makeMinionURL(minion), h.client) if err != nil { return false, err } diff --git a/pkg/util/util.go b/pkg/util/util.go index 1058a50ed46fd..27ab57eb920c0 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -83,6 +83,16 @@ const ( IntstrString // The IntOrString holds a string. ) +// MakeIntOrStringFromInt creates an IntOrString object with an int value. +func MakeIntOrStringFromInt(val int) IntOrString { + return IntOrString{Kind: IntstrInt, IntVal: val} +} + +// MakeIntOrStringFromInt creates an IntOrString object with a string value. +func MakeIntOrStringFromString(val string) IntOrString { + return IntOrString{Kind: IntstrString, StrVal: val} +} + // SetYAML implements the yaml.Setter interface. func (intstr *IntOrString) SetYAML(tag string, value interface{}) bool { switch v := value.(type) { diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index 8e4171b8edc1d..5bd72518245bc 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -71,6 +71,20 @@ func TestHandleCrash(t *testing.T) { } } +func TestMakeIntOrStringFromInt(t *testing.T) { + i := MakeIntOrStringFromInt(93) + if i.Kind != IntstrInt || i.IntVal != 93 { + t.Errorf("Expected IntVal=93, got %+v", i) + } +} + +func TestMakeIntOrStringFromString(t *testing.T) { + i := MakeIntOrStringFromString("76") + if i.Kind != IntstrString || i.StrVal != "76" { + t.Errorf("Expected StrVal=\"76\", got %+v", i) + } +} + type IntOrStringHolder struct { IOrS IntOrString `json:"val" yaml:"val"` }