diff --git a/cmd/kubelet/kubelet.go b/cmd/kubelet/kubelet.go index b3e7a3069a..d8a919be68 100644 --- a/cmd/kubelet/kubelet.go +++ b/cmd/kubelet/kubelet.go @@ -163,9 +163,11 @@ func main() { float32(*registryPullQPS), *registryBurst) - health.AddHealthChecker("exec", health.NewExecHealthChecker(k)) - health.AddHealthChecker("http", health.NewHTTPHealthChecker(&http.Client{})) - health.AddHealthChecker("tcp", &health.TCPHealthChecker{}) + // TODO: These should probably become more plugin-ish: register a factory func + // in each checker's init(), iterate those here. + health.AddHealthChecker(health.NewExecHealthChecker(k)) + health.AddHealthChecker(health.NewHTTPHealthChecker(&http.Client{})) + health.AddHealthChecker(&health.TCPHealthChecker{}) // start the kubelet go util.Forever(func() { k.Run(cfg.Updates()) }, 0) diff --git a/pkg/api/types.go b/pkg/api/types.go index 6916cc24a8..ff1a32f2f4 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -171,8 +171,6 @@ type ExecAction struct { // LivenessProbe describes a liveness probe to be examined to the container. // TODO: pass structured data to the actions, and document that data here. type LivenessProbe struct { - // Type of liveness probe. Current legal values "http", "tcp" - Type string `yaml:"type,omitempty" json:"type,omitempty"` // HTTPGetProbe parameters, required if Type == 'http' HTTPGet *HTTPGetAction `yaml:"httpGet,omitempty" json:"httpGet,omitempty"` // TCPSocketProbe parameter, required if Type == 'tcp' diff --git a/pkg/api/v1beta1/types.go b/pkg/api/v1beta1/types.go index 9225b592e9..028333c959 100644 --- a/pkg/api/v1beta1/types.go +++ b/pkg/api/v1beta1/types.go @@ -181,8 +181,6 @@ type ExecAction struct { // LivenessProbe describes a liveness probe to be examined to the container. // TODO: pass structured data to the actions, and document that data here. type LivenessProbe struct { - // Type of liveness probe. Current legal values "http", "tcp" - Type string `yaml:"type,omitempty" json:"type,omitempty"` // HTTPGetProbe parameters, required if Type == 'http' HTTPGet *HTTPGetAction `yaml:"httpGet,omitempty" json:"httpGet,omitempty"` // TCPSocketProbe parameter, required if Type == 'tcp' diff --git a/pkg/api/v1beta2/types.go b/pkg/api/v1beta2/types.go index 44cdfb4be2..9031ee8ffa 100644 --- a/pkg/api/v1beta2/types.go +++ b/pkg/api/v1beta2/types.go @@ -180,8 +180,6 @@ type ExecAction struct { // LivenessProbe describes a liveness probe to be examined to the container. // TODO: pass structured data to the actions, and document that data here. type LivenessProbe struct { - // Type of liveness probe. Current legal values "http", "tcp" - Type string `yaml:"type,omitempty" json:"type,omitempty"` // HTTPGetProbe parameters, required if Type == 'http' HTTPGet *HTTPGetAction `yaml:"httpGet,omitempty" json:"httpGet,omitempty"` // TCPSocketProbe parameter, required if Type == 'tcp' diff --git a/pkg/api/v1beta3/types.go b/pkg/api/v1beta3/types.go index 382bb1932e..9f76944285 100644 --- a/pkg/api/v1beta3/types.go +++ b/pkg/api/v1beta3/types.go @@ -171,8 +171,6 @@ type ExecAction struct { // LivenessProbe describes a liveness probe to be examined to the container. // TODO: pass structured data to the actions, and document that data here. type LivenessProbe struct { - // Type of liveness probe. Current legal values "http", "tcp" - Type string `yaml:"type,omitempty" json:"type,omitempty"` // HTTPGetProbe parameters, required if Type == 'http' HTTPGet *HTTPGetAction `yaml:"httpGet,omitempty" json:"httpGet,omitempty"` // TCPSocketProbe parameter, required if Type == 'tcp' diff --git a/pkg/health/exec.go b/pkg/health/exec.go index efcfec9150..ba95f2f3ce 100644 --- a/pkg/health/exec.go +++ b/pkg/health/exec.go @@ -57,3 +57,7 @@ func (e *ExecHealthChecker) HealthCheck(podFullName string, currentState api.Pod } return Healthy, nil } + +func (e *ExecHealthChecker) CanCheck(probe *api.LivenessProbe) bool { + return probe.Exec != nil +} diff --git a/pkg/health/exec_test.go b/pkg/health/exec_test.go index 44727779a9..28e1a9ccaf 100644 --- a/pkg/health/exec_test.go +++ b/pkg/health/exec_test.go @@ -49,22 +49,19 @@ func TestExec(t *testing.T) { checker := ExecHealthChecker{&fake} tests := []healthCheckTest{ // Missing parameters - {Unknown, &api.LivenessProbe{Type: "exec"}, true, nil, nil}, + {Unknown, &api.LivenessProbe{}, true, nil, nil}, // Ok {Healthy, &api.LivenessProbe{ - Type: "exec", Exec: &api.ExecAction{Command: []string{"ls", "-l"}}, }, false, []byte("OK"), nil}, // Run returns error {Unknown, &api.LivenessProbe{ - Type: "exec", Exec: &api.ExecAction{ Command: []string{"ls", "-l"}, }, }, true, []byte("OK, NOT"), fmt.Errorf("test error")}, // Command error {Unhealthy, &api.LivenessProbe{ - Type: "exec", Exec: &api.ExecAction{ Command: []string{"ls", "-l"}, }, diff --git a/pkg/health/health.go b/pkg/health/health.go index 80c0b60437..c978b00b7b 100644 --- a/pkg/health/health.go +++ b/pkg/health/health.go @@ -36,54 +36,61 @@ const ( // HealthChecker defines an abstract interface for checking container health. type HealthChecker interface { HealthCheck(podFullName string, currentState api.PodState, container api.Container) (Status, error) + CanCheck(probe *api.LivenessProbe) bool } -// protects checkers +// protects allCheckers var checkerLock = sync.Mutex{} -var checkers = map[string]HealthChecker{} +var allCheckers = []HealthChecker{} // AddHealthChecker adds a health checker to the list of known HealthChecker objects. // Any subsequent call to NewHealthChecker will know about this HealthChecker. -// Panics if 'key' is already present. -func AddHealthChecker(key string, checker HealthChecker) { +func AddHealthChecker(checker HealthChecker) { checkerLock.Lock() defer checkerLock.Unlock() - if _, found := checkers[key]; found { - glog.Fatalf("HealthChecker already defined for key %s.", key) - } - checkers[key] = checker + allCheckers = append(allCheckers, checker) } // NewHealthChecker creates a new HealthChecker which supports multiple types of liveness probes. func NewHealthChecker() HealthChecker { checkerLock.Lock() defer checkerLock.Unlock() - input := map[string]HealthChecker{} - for key, value := range checkers { - input[key] = value - } return &muxHealthChecker{ - checkers: input, + checkers: append([]HealthChecker{}, allCheckers...), } } // muxHealthChecker bundles multiple implementations of HealthChecker of different types. type muxHealthChecker struct { - checkers map[string]HealthChecker + // Given a LivenessProbe, cycle through each known checker and see if it supports + // the specific kind of probe (by returning non-nil). + checkers []HealthChecker +} + +func (m *muxHealthChecker) findCheckerFor(probe *api.LivenessProbe) HealthChecker { + for i := range m.checkers { + if m.checkers[i].CanCheck(probe) { + return m.checkers[i] + } + } + return nil } // HealthCheck delegates the health-checking of the container to one of the bundled implementations. -// It chooses an implementation according to container.LivenessProbe.Type. -// If there is no matching health checker it returns Unknown, nil. +// If there is no health checker that can check container it returns Unknown, nil. func (m *muxHealthChecker) HealthCheck(podFullName string, currentState api.PodState, container api.Container) (Status, error) { - checker, ok := m.checkers[container.LivenessProbe.Type] - if !ok || checker == nil { - glog.Warningf("Failed to find health checker for %s %s", container.Name, container.LivenessProbe.Type) + checker := m.findCheckerFor(container.LivenessProbe) + if checker == nil { + glog.Warningf("Failed to find health checker for %s %+v", container.Name, container.LivenessProbe) return Unknown, nil } return checker.HealthCheck(podFullName, currentState, container) } +func (m *muxHealthChecker) CanCheck(probe *api.LivenessProbe) bool { + return m.findCheckerFor(probe) != nil +} + // findPortByName is 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 { diff --git a/pkg/health/health_test.go b/pkg/health/health_test.go index 246f869f11..6c08a90bde 100644 --- a/pkg/health/health_test.go +++ b/pkg/health/health_test.go @@ -30,7 +30,7 @@ import ( const statusServerEarlyShutdown = -1 func TestHealthChecker(t *testing.T) { - AddHealthChecker("http", &HTTPHealthChecker{client: &http.Client{}}) + AddHealthChecker(&HTTPHealthChecker{client: &http.Client{}}) var healthCheckerTests = []struct { status int health Status @@ -64,7 +64,6 @@ func TestHealthChecker(t *testing.T) { Path: "/foo/bar", Host: host, }, - Type: "http", }, } hc := NewHealthChecker() @@ -100,19 +99,16 @@ func TestFindPortByName(t *testing.T) { func TestMuxHealthChecker(t *testing.T) { muxHealthCheckerTests := []struct { - health Status - probeType string + health Status }{ - {Healthy, "http"}, - {Unknown, "ftp"}, + // TODO: This test should run through a few different checker types. + {Healthy}, } mc := &muxHealthChecker{ - checkers: make(map[string]HealthChecker), + checkers: []HealthChecker{ + &HTTPHealthChecker{client: &http.Client{}}, + }, } - hc := &HTTPHealthChecker{ - client: &http.Client{}, - } - mc.checkers["http"] = hc for _, muxHealthCheckerTest := range muxHealthCheckerTests { tt := muxHealthCheckerTest ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -131,7 +127,6 @@ func TestMuxHealthChecker(t *testing.T) { HTTPGet: &api.HTTPGetAction{}, }, } - container.LivenessProbe.Type = tt.probeType container.LivenessProbe.HTTPGet.Port = util.NewIntOrStringFromString(port) container.LivenessProbe.HTTPGet.Host = host health, err := mc.HealthCheck("test", api.PodState{}, container) diff --git a/pkg/health/http.go b/pkg/health/http.go index 8ee739dc59..18ced9faaf 100644 --- a/pkg/health/http.go +++ b/pkg/health/http.go @@ -105,3 +105,7 @@ func (h *HTTPHealthChecker) HealthCheck(podFullName string, currentState api.Pod } return DoHTTPCheck(formatURL(host, port, path), h.client) } + +func (h *HTTPHealthChecker) CanCheck(probe *api.LivenessProbe) bool { + return probe.HTTPGet != nil +} diff --git a/pkg/health/http_test.go b/pkg/health/http_test.go index d2067231bd..5fb57217ce 100644 --- a/pkg/health/http_test.go +++ b/pkg/health/http_test.go @@ -51,7 +51,6 @@ func TestGetURLParts(t *testing.T) { Ports: []api.Port{{Name: "found", HostPort: 93}}, LivenessProbe: &api.LivenessProbe{ HTTPGet: test.probe, - Type: "http", }, } host, port, path, err := getURLParts(state, container) @@ -117,7 +116,6 @@ func TestHTTPHealthChecker(t *testing.T) { container := api.Container{ LivenessProbe: &api.LivenessProbe{ HTTPGet: test.probe, - Type: "http", }, } params := container.LivenessProbe.HTTPGet diff --git a/pkg/health/tcp.go b/pkg/health/tcp.go index 1bcfb15f30..7253c7597d 100644 --- a/pkg/health/tcp.go +++ b/pkg/health/tcp.go @@ -81,3 +81,7 @@ func (t *TCPHealthChecker) HealthCheck(podFullName string, currentState api.PodS } return DoTCPCheck(net.JoinHostPort(host, strconv.Itoa(port))) } + +func (t *TCPHealthChecker) CanCheck(probe *api.LivenessProbe) bool { + return probe.TCPSocket != nil +} diff --git a/pkg/health/tcp_test.go b/pkg/health/tcp_test.go index bd17328a11..2689ae615d 100644 --- a/pkg/health/tcp_test.go +++ b/pkg/health/tcp_test.go @@ -49,7 +49,6 @@ func TestGetTCPAddrParts(t *testing.T) { Ports: []api.Port{{Name: "found", HostPort: 93}}, LivenessProbe: &api.LivenessProbe{ TCPSocket: test.probe, - Type: "tcp", }, } host, port, err := getTCPAddrParts(state, container) @@ -95,7 +94,6 @@ func TestTcpHealthChecker(t *testing.T) { container := api.Container{ LivenessProbe: &api.LivenessProbe{ TCPSocket: test.probe, - Type: "tcp", }, } params := container.LivenessProbe.TCPSocket diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 5509bf7b62..e9ebfbf030 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -457,6 +457,10 @@ func (f *FalseHealthChecker) HealthCheck(podFullName string, state api.PodState, return health.Unhealthy, nil } +func (f *FalseHealthChecker) CanCheck(probe *api.LivenessProbe) bool { + return true +} + func TestSyncPodBadHash(t *testing.T) { kubelet, _, fakeDocker := newTestKubelet(t) kubelet.healthChecker = &FalseHealthChecker{} @@ -522,8 +526,7 @@ func TestSyncPodUnhealthy(t *testing.T) { Containers: []api.Container{ {Name: "bar", LivenessProbe: &api.LivenessProbe{ - // Always returns healthy == false - Type: "false", + // Always returns healthy == false }, }, },