Merge pull request #15958 from piotr-sk/fix/kube_play_liveness_probe

fix: liveness check with http probe
containers · Sep 27, 2022 · a225cb5 · a225cb5
2 parents cbc7185 + 95cb14a
commit a225cb5
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 8 deletions.
diff --git a/pkg/specgen/generate/kube/kube.go b/pkg/specgen/generate/kube/kube.go
@@ -500,27 +500,33 @@ func setupLivenessProbe(s *specgen.SpecGenerator, containerYAML v1.Container, re
 		probe := containerYAML.LivenessProbe
 		probeHandler := probe.Handler
 
-		// append `exit 1` to `cmd` so healthcheck can be marked as `unhealthy`.
-		// append `kill 1` to `cmd` if appropriate restart policy is configured.
-		if restartPolicy == "always" || restartPolicy == "onfailure" {
-			// container will be restarted so we can kill init.
-			failureCmd = "kill 1"
-		}
-
 		// configure healthcheck on the basis of Handler Actions.
 		switch {
 		case probeHandler.Exec != nil:
 			execString := strings.Join(probeHandler.Exec.Command, " ")
 			commandString = fmt.Sprintf("%s || %s", execString, failureCmd)
 		case probeHandler.HTTPGet != nil:
-			commandString = fmt.Sprintf("curl %s://%s:%d/%s  || %s", probeHandler.HTTPGet.Scheme, probeHandler.HTTPGet.Host, probeHandler.HTTPGet.Port.IntValue(), probeHandler.HTTPGet.Path, failureCmd)
+			// set defaults as in https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/#http-probes
+			var uriScheme v1.URIScheme = "http"
+			if probeHandler.HTTPGet.Scheme != "" {
+				uriScheme = probeHandler.HTTPGet.Scheme
+			}
+			host := "localhost" // Kubernetes default is host IP, but with Podman there is only one node
+			if probeHandler.HTTPGet.Host != "" {
+				host = probeHandler.HTTPGet.Host
+			}
+			commandString = fmt.Sprintf("curl -f %s://%s:%d%s || %s", uriScheme, host, probeHandler.HTTPGet.Port.IntValue(), probeHandler.HTTPGet.Path, failureCmd)
 		case probeHandler.TCPSocket != nil:
 			commandString = fmt.Sprintf("nc -z -v %s %d || %s", probeHandler.TCPSocket.Host, probeHandler.TCPSocket.Port.IntValue(), failureCmd)
 		}
 		s.HealthConfig, err = makeHealthCheck(commandString, probe.PeriodSeconds, probe.FailureThreshold, probe.TimeoutSeconds, probe.InitialDelaySeconds)
 		if err != nil {
 			return err
 		}
+		// if restart policy is in place, ensure the health check enforces it
+		if restartPolicy == "always" || restartPolicy == "onfailure" {
+			s.HealthCheckOnFailureAction = define.HealthCheckOnFailureActionRestart
+		}
 		return nil
 	}
 	return nil

diff --git a/pkg/specgen/generate/kube/play_test.go b/pkg/specgen/generate/kube/play_test.go
@@ -11,6 +11,8 @@ import (
 	v1 "github.com/containers/podman/v4/pkg/k8s.io/api/core/v1"
 	"github.com/containers/podman/v4/pkg/k8s.io/apimachinery/pkg/api/resource"
 	v12 "github.com/containers/podman/v4/pkg/k8s.io/apimachinery/pkg/apis/meta/v1"
+	"github.com/containers/podman/v4/pkg/k8s.io/apimachinery/pkg/util/intstr"
+	"github.com/containers/podman/v4/pkg/specgen"
 	"github.com/docker/docker/pkg/system"
 	"github.com/stretchr/testify/assert"
 )
@@ -858,3 +860,60 @@ var (
 		},
 	}
 )
+
+func TestHttpLivenessProbe(t *testing.T) {
+	tests := []struct {
+		name          string
+		specGenerator specgen.SpecGenerator
+		container     v1.Container
+		restartPolicy string
+		succeed       bool
+		expectedURL   string
+	}{
+		{
+			"HttpLivenessProbeUrlSetCorrectly",
+			specgen.SpecGenerator{},
+			v1.Container{
+				LivenessProbe: &v1.Probe{
+					Handler: v1.Handler{
+						HTTPGet: &v1.HTTPGetAction{
+							Scheme: "http",
+							Host:   "127.0.0.1",
+							Port:   intstr.FromInt(8080),
+							Path:   "/health",
+						},
+					},
+				},
+			},
+			"always",
+			true,
+			"http://127.0.0.1:8080/health",
+		},
+		{
+			"HttpLivenessProbeUrlUsesDefaults",
+			specgen.SpecGenerator{},
+			v1.Container{
+				LivenessProbe: &v1.Probe{
+					Handler: v1.Handler{
+						HTTPGet: &v1.HTTPGetAction{
+							Port: intstr.FromInt(80),
+							Path: "/",
+						},
+					},
+				},
+			},
+			"always",
+			true,
+			"http://localhost:80/",
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			err := setupLivenessProbe(&test.specGenerator, test.container, test.restartPolicy)
+			assert.Equal(t, err == nil, test.succeed)
+			assert.Contains(t, test.specGenerator.ContainerHealthCheckConfig.HealthConfig.Test, test.expectedURL)
+		})
+	}
+}