Kill LivenessProbe.Type

cmd/kubelet/kubelet.go

@@ -163,9 +163,11 @@ func main() {
 		float32(*registryPullQPS),
 		*registryBurst)
 
-	health.AddHealthChecker("exec", health.NewExecHealthChecker(k))
+	// TODO: These should probably become more plugin-ish: register a factory func
-	health.AddHealthChecker("http", health.NewHTTPHealthChecker(&http.Client{}))
+	// in each checker's init(), iterate those here.
-	health.AddHealthChecker("tcp", &health.TCPHealthChecker{})
+	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)

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'

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'

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'

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'

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
+}

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"},
 			},

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(checker HealthChecker) {
-func AddHealthChecker(key string, checker HealthChecker) {
 	checkerLock.Lock()
 	defer checkerLock.Unlock()
-	if _, found := checkers[key]; found {
+	allCheckers = append(allCheckers, checker)
-		glog.Fatalf("HealthChecker already defined for key %s.", key)
-	}
-	checkers[key] = 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 health checker that can check container it returns Unknown, nil.
-// If there is no matching health checker 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]
+	checker := m.findCheckerFor(container.LivenessProbe)
-	if !ok || checker == nil {
+	if checker == nil {
-		glog.Warningf("Failed to find health checker for %s %s", container.Name, container.LivenessProbe.Type)
+		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 {

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
+		health Status
-		probeType string
 	}{
-		{Healthy, "http"},
+		// TODO: This test should run through a few different checker types.
-		{Unknown, "ftp"},
+		{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)

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
+}

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

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
+}

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

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
+					// Always returns healthy == false
-						Type: "false",
 					},
 				},
 			},