Repository: kubernetes/kompose
Pull request #1366 (2021-03-06 21:51:53)

Add readiness healthcheck generation by label
Related: https://github.com/kubernetes/kompose/issues/1262

Allow the readiness healthcheck to be configured independent from the docker-compose healthcheck (liveness).

The following `docker-compose.yml`
```yaml
version: "3.4"

services:

  some-service:
    image: busybox:latest
    labels:
      kompose.service.healthcheck.readiness.test: CMD curl -f "http://localhost/health one-parameter"
      kompose.service.healthcheck.readiness.interval: 10s
      kompose.service.healthcheck.readiness.timeout: 10s
      kompose.service.healthcheck.readiness.retries: 3
      kompose.service.healthcheck.readiness.start_period: 120s
    healthcheck:
      test: "curl -f http://localhost/health"
      interval: 5s
      timeout: 5s
      retries: 5
      start_period: 120s
    ports:
      - "80:80"
      - "443:443"
```

generates the `deployment.yml`:
```yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  annotations:
    kompose.cmd: kompose.exe convert -f docker-compose.yml
    kompose.service.healthcheck.readiness.interval: 10s
    kompose.service.healthcheck.readiness.retries: "3"
    kompose.service.healthcheck.readiness.start_period: 120s
    kompose.service.healthcheck.readiness.test: CMD curl -f "http://localhost/health one-parameter"
    kompose.service.healthcheck.readiness.timeout: 10s
    kompose.version: 1.22.0 (HEAD)
  creationTimestamp: null
  labels:
    io.kompose.service: some-service
  name: some-service
spec:
  replicas: 1
  selector:
    matchLabels:
      io.kompose.service: some-service
  strategy: {}
  template:
    metadata:
      annotations:
        kompose.cmd: kompose.exe convert -f docker-compose.yml
        kompose.service.healthcheck.readiness.interval: 10s
        kompose.service.healthcheck.readiness.retries: "3"
        kompose.service.healthcheck.readiness.start_period: 120s
        kompose.service.healthcheck.readiness.test: CMD curl -f "http://localhost/health one-parameter"
        kompose.service.healthcheck.readiness.timeout: 10s
        kompose.version: 1.22.0 (HEAD)
      creationTimestamp: null
      labels:
        io.kompose.service: some-service
    spec:
      containers:
        - image: busybox:latest
          livenessProbe:
            exec:
              command:
                - curl
                - -f
                - http://localhost/health
            failureThreshold: 5
            initialDelaySeconds: 120
            periodSeconds: 5
            timeoutSeconds: 5
          name: some-service
          ports:
            - containerPort: 80
            - containerPort: 443
          readinessProbe:
            exec:
              command:
                - curl
                - -f
                - http://localhost/health one-parameter
            failureThreshold: 3
            initialDelaySeconds: 120
            periodSeconds: 10
            timeoutSeconds: 10
          resources: {}
      restartPolicy: Always
status: {}
```

This way, if wanted, both readiness and liveness can be configured.

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes/kompose/pull/1366#" title="Author self-approved">jvitor83</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **janetkuo** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @janetkuo` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes%2Fkompose).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes/kompose/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["janetkuo"]} -->
jvitor83: I think this PR is ready.
Just wondering if is the right choice, like:
1. If we should use the same HealhCheck of compose in both Liveness and Readiness (by having the same bahaviour of swarm/compose that use only one config to both behaviors) - **Not this PR**
    > Drawback: both commands be executed and don't having the possibility of diverge
2. If we should allow a independent config for each behavior (readiness and liveness) and deviate a bit from compose/swarm (that use only one config to both behaviors) - **This PR**
    > Drawback: People should know that in order to achieve the same behavior it need to inform this readiness label in addition to the healtchcheck

I think the option 2 is good enouth (this PR). If the mainteiners think the same, i believe this can be merged.
hangyan: (Sorry for the very late response, it won't happen again). I'm not total against or support this kind of feature. Since i think this project is mainly a single direction map, from compose -> kubernetes. So the main focus is on what compose provides and how they should map to kubernetes. Liveness and Readiness is a mainly a kubernetes feature, so we don't spend time many on this .But i think this PR is good enough to be merged, this can helps a lot of people. There is no perfect soultion on this kind of mismatch. I will review this code now.
hangyan: /lgtm

Patch:
diff --git a/pkg/kobject/kobject.go b/pkg/kobject/kobject.go
index 381bc6e5..4a851d49 100644
--- a/pkg/kobject/kobject.go
+++ b/pkg/kobject/kobject.go
@@ -137,7 +137,7 @@ type ServiceConfig struct {
 	GroupAdd           []int64                     `compose:"group_add"`
 	Volumes            []Volumes                   `compose:""`
 	Secrets            []dockerCliTypes.ServiceSecretConfig
-	HealthChecks       HealthCheck       `compose:""`
+	HealthChecks       HealthChecks      `compose:""`
 	Placement          map[string]string `compose:""`
 	//This is for long LONG SYNTAX link(https://docs.docker.com/compose/compose-file/#long-syntax)
 	Configs []dockerCliTypes.ServiceConfigObjConfig `compose:""`
@@ -147,6 +147,12 @@ type ServiceConfig struct {
 	WithKomposeAnnotation bool `compose:""`
 }
 
+// HealthChecks used to distinguish between liveness and readiness
+type HealthChecks struct {
+	Liveness  HealthCheck
+	Readiness HealthCheck
+}
+
 // HealthCheck the healthcheck configuration for a service
 // "StartPeriod" is not yet added to compose, see:
 // https://github.com/docker/cli/issues/116
diff --git a/pkg/loader/compose/utils.go b/pkg/loader/compose/utils.go
index 12da0866..5653a4d0 100644
--- a/pkg/loader/compose/utils.go
+++ b/pkg/loader/compose/utils.go
@@ -44,6 +44,18 @@ const (
 	LabelImagePullSecret = "kompose.image-pull-secret"
 	// LabelImagePullPolicy defines Kubernetes PodSpec imagePullPolicy.
 	LabelImagePullPolicy = "kompose.image-pull-policy"
+	// HealthCheckReadinessDisable defines readiness health check disable
+	HealthCheckReadinessDisable = "kompose.service.healthcheck.readiness.disable"
+	// HealthCheckReadinessTest defines readiness health check test
+	HealthCheckReadinessTest = "kompose.service.healthcheck.readiness.test"
+	// HealthCheckReadinessInterval defines readiness health check interval
+	HealthCheckReadinessInterval = "kompose.service.healthcheck.readiness.interval"
+	// HealthCheckReadinessTimeout defines readiness health check timeout
+	HealthCheckReadinessTimeout = "kompose.service.healthcheck.readiness.timeout"
+	// HealthCheckReadinessRetries defines readiness health check retries
+	HealthCheckReadinessRetries = "kompose.service.healthcheck.readiness.retries"
+	// HealthCheckReadinessStartPeriod defines readiness health check start period
+	HealthCheckReadinessStartPeriod = "kompose.service.healthcheck.readiness.start_period"
 
 	// ServiceTypeHeadless ...
 	ServiceTypeHeadless = "Headless"
diff --git a/pkg/loader/compose/v3.go b/pkg/loader/compose/v3.go
index cc47a3f6..7d6b8a43 100755
--- a/pkg/loader/compose/v3.go
+++ b/pkg/loader/compose/v3.go
@@ -34,6 +34,7 @@ import (
 
 	"fmt"
 
+	shlex "github.com/google/shlex"
 	"github.com/kubernetes/kompose/pkg/kobject"
 	"github.com/pkg/errors"
 	log "github.com/sirupsen/logrus"
@@ -228,6 +229,66 @@ func loadV3Ports(ports []types.ServicePortConfig, expose []string) []kobject.Por
 	return komposePorts
 }
 
+/* Convert the HealthCheckConfig as designed by Docker to
+a Kubernetes-compatible format.
+*/
+func parseHealthCheckReadiness(labels types.Labels) (kobject.HealthCheck, error) {
+
+	// initialize with CMD as default to not break at return (will be ignored if no test is informed)
+	test := []string{"CMD"}
+	var timeout, interval, retries, startPeriod int32
+	var disable bool
+
+	for key, value := range labels {
+		switch key {
+		case HealthCheckReadinessDisable:
+			disable = cast.ToBool(value)
+		case HealthCheckReadinessTest:
+			if len(value) > 0 {
+				test, _ = shlex.Split(value)
+			}
+		case HealthCheckReadinessInterval:
+			parse, err := time.ParseDuration(value)
+			if err != nil {
+				return kobject.HealthCheck{}, errors.Wrap(err, "unable to parse health check interval variable")
+			}
+			interval = int32(parse.Seconds())
+		case HealthCheckReadinessTimeout:
+			parse, err := time.ParseDuration(value)
+			if err != nil {
+				return kobject.HealthCheck{}, errors.Wrap(err, "unable to parse health check timeout variable")
+			}
+			timeout = int32(parse.Seconds())
+		case HealthCheckReadinessRetries:
+			retries = cast.ToInt32(value)
+		case HealthCheckReadinessStartPeriod:
+			parse, err := time.ParseDuration(value)
+			if err != nil {
+				return kobject.HealthCheck{}, errors.Wrap(err, "unable to parse health check startPeriod variable")
+			}
+			startPeriod = int32(parse.Seconds())
+		}
+	}
+
+	if test[0] == "NONE" {
+		disable = true
+		test = test[1:]
+	}
+	if test[0] == "CMD" || test[0] == "CMD-SHELL" {
+		test = test[1:]
+	}
+
+	// Due to docker/cli adding "CMD-SHELL" to the struct, we remove the first element of composeHealthCheck.Test
+	return kobject.HealthCheck{
+		Test:        test,
+		Timeout:     timeout,
+		Interval:    interval,
+		Retries:     retries,
+		StartPeriod: startPeriod,
+		Disable:     disable,
+	}, nil
+}
+
 /* Convert the HealthCheckConfig as designed by Docker to
 a Kubernetes-compatible format.
 */
@@ -327,15 +388,24 @@ func dockerComposeToKomposeMapping(composeObject *types.Config) (kobject.Kompose
 		// labels
 		serviceConfig.DeployLabels = composeServiceConfig.Deploy.Labels
 
-		// HealthCheck
+		// HealthCheck Liveness
 		if composeServiceConfig.HealthCheck != nil && !composeServiceConfig.HealthCheck.Disable {
 			var err error
-			serviceConfig.HealthChecks, err = parseHealthCheck(*composeServiceConfig.HealthCheck)
+			serviceConfig.HealthChecks.Liveness, err = parseHealthCheck(*composeServiceConfig.HealthCheck)
 			if err != nil {
 				return kobject.KomposeObject{}, errors.Wrap(err, "Unable to parse health check")
 			}
 		}
 
+		// HealthCheck Readiness
+		var readiness, errReadiness = parseHealthCheckReadiness(*&composeServiceConfig.Labels)
+		if readiness.Test != nil && len(readiness.Test) > 0 && len(readiness.Test[0]) > 0 && !readiness.Disable {
+			serviceConfig.HealthChecks.Readiness = readiness
+			if errReadiness != nil {
+				return kobject.KomposeObject{}, errors.Wrap(errReadiness, "Unable to parse health check")
+			}
+		}
+
 		// restart-policy: deploy.restart_policy.condition will rewrite restart option
 		// see: https://docs.docker.com/compose/compose-file/#restart_policy
 		serviceConfig.Restart = composeServiceConfig.Restart
diff --git a/pkg/transformer/kubernetes/k8sutils.go b/pkg/transformer/kubernetes/k8sutils.go
index c7db498d..795e18c4 100644
--- a/pkg/transformer/kubernetes/k8sutils.go
+++ b/pkg/transformer/kubernetes/k8sutils.go
@@ -510,30 +510,53 @@ func (k *Kubernetes) UpdateKubernetesObjects(name string, service kobject.Servic
 		template.Spec.NodeSelector = service.Placement
 		// Configure the HealthCheck
 		// We check to see if it's blank
-		if !reflect.DeepEqual(service.HealthChecks, kobject.HealthCheck{}) {
+		if !reflect.DeepEqual(service.HealthChecks.Liveness, kobject.HealthCheck{}) {
 			probe := api.Probe{}
 
-			if len(service.HealthChecks.Test) > 0 {
+			if len(service.HealthChecks.Liveness.Test) > 0 {
 				probe.Handler = api.Handler{
 					Exec: &api.ExecAction{
-						Command: service.HealthChecks.Test,
+						Command: service.HealthChecks.Liveness.Test,
 					},
 				}
 			} else {
 				return errors.New("Health check must contain a command")
 			}
 
-			probe.TimeoutSeconds = service.HealthChecks.Timeout
-			probe.PeriodSeconds = service.HealthChecks.Interval
-			probe.FailureThreshold = service.HealthChecks.Retries
+			probe.TimeoutSeconds = service.HealthChecks.Liveness.Timeout
+			probe.PeriodSeconds = service.HealthChecks.Liveness.Interval
+			probe.FailureThreshold = service.HealthChecks.Liveness.Retries
 
 			// See issue: https://github.com/docker/cli/issues/116
 			// StartPeriod has been added to docker/cli however, it is not yet added
 			// to compose. Once the feature has been implemented, this will automatically work
-			probe.InitialDelaySeconds = service.HealthChecks.StartPeriod
+			probe.InitialDelaySeconds = service.HealthChecks.Liveness.StartPeriod
 
 			template.Spec.Containers[0].LivenessProbe = &probe
 		}
+		if !reflect.DeepEqual(service.HealthChecks.Readiness, kobject.HealthCheck{}) {
+			probeHealthCheckReadiness := api.Probe{}
+			if len(service.HealthChecks.Readiness.Test) > 0 {
+				probeHealthCheckReadiness.Handler = api.Handler{
+					Exec: &api.ExecAction{
+						Command: service.HealthChecks.Readiness.Test,
+					},
+				}
+			} else {
+				return errors.New("Health check must contain a command")
+			}
+
+			probeHealthCheckReadiness.TimeoutSeconds = service.HealthChecks.Readiness.Timeout
+			probeHealthCheckReadiness.PeriodSeconds = service.HealthChecks.Readiness.Interval
+			probeHealthCheckReadiness.FailureThreshold = service.HealthChecks.Readiness.Retries
+
+			// See issue: https://github.com/docker/cli/issues/116
+			// StartPeriod has been added to docker/cli however, it is not yet added
+			// to compose. Once the feature has been implemented, this will automatically work
+			probeHealthCheckReadiness.InitialDelaySeconds = service.HealthChecks.Readiness.StartPeriod
+
+			template.Spec.Containers[0].ReadinessProbe = &probeHealthCheckReadiness
+		}
 
 		if service.StopGracePeriod != "" {
 			template.Spec.TerminationGracePeriodSeconds, err = DurationStrToSecondsInt(service.StopGracePeriod)


Test patch:
diff --git a/pkg/testutils/kubernetes.go b/pkg/testutils/kubernetes.go
index 89c0c183..465b84d7 100644
--- a/pkg/testutils/kubernetes.go
+++ b/pkg/testutils/kubernetes.go
@@ -2,6 +2,8 @@ package testutils
 
 import (
 	"errors"
+
+	appsv1 "k8s.io/api/apps/v1"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 )
@@ -24,3 +26,25 @@ func CheckForHeadless(objects []runtime.Object) error {
 	}
 	return nil
 }
+
+// CheckForHealthCheckLivenessAndReadiness check if has liveness and readiness in healthcheck configured.
+func CheckForHealthCheckLivenessAndReadiness(objects []runtime.Object) error {
+	serviceCreated := false
+	for _, obj := range objects {
+		if deployment, ok := obj.(*appsv1.Deployment); ok {
+			serviceCreated = true
+
+			// Check if it is a headless services
+			if deployment.Spec.Template.Spec.Containers[0].ReadinessProbe == nil {
+				return errors.New("there is not a ReadinessProbe")
+			}
+			if deployment.Spec.Template.Spec.Containers[0].LivenessProbe == nil {
+				return errors.New("there is not a LivenessGate")
+			}
+		}
+	}
+	if !serviceCreated {
+		return errors.New("no Service created")
+	}
+	return nil
+}
diff --git a/pkg/transformer/kubernetes/k8sutils_test.go b/pkg/transformer/kubernetes/k8sutils_test.go
index 0e301257..ca9c6aee 100644
--- a/pkg/transformer/kubernetes/k8sutils_test.go
+++ b/pkg/transformer/kubernetes/k8sutils_test.go
@@ -359,6 +359,45 @@ func TestIsDir(t *testing.T) {
 	}
 }
 
+// TestServiceWithoutPort this tests if Headless Service is created for services without Port.
+func TestServiceWithHealthCheck(t *testing.T) {
+	service := kobject.ServiceConfig{
+		ContainerName: "name",
+		Image:         "image",
+		ServiceType:   "Headless",
+		HealthChecks: kobject.HealthChecks{
+			Readiness: kobject.HealthCheck{
+				Test:        []string{"arg1", "arg2"},
+				Timeout:     10,
+				Interval:    5,
+				Retries:     3,
+				StartPeriod: 60,
+			},
+			Liveness: kobject.HealthCheck{
+				Test:        []string{"arg1", "arg2"},
+				Timeout:     11,
+				Interval:    6,
+				Retries:     4,
+				StartPeriod: 61,
+			},
+		},
+	}
+
+	komposeObject := kobject.KomposeObject{
+		ServiceConfigs: map[string]kobject.ServiceConfig{"app": service},
+	}
+	k := Kubernetes{}
+
+	objects, err := k.Transform(komposeObject, kobject.ConvertOptions{CreateD: true, Replicas: 1})
+	if err != nil {
+		t.Error(errors.Wrap(err, "k.Transform failed"))
+	}
+	if err := testutils.CheckForHealthCheckLivenessAndReadiness(objects); err != nil {
+		t.Error(err)
+	}
+
+}
+
 // TestServiceWithoutPort this tests if Headless Service is created for services without Port.
 func TestServiceWithoutPort(t *testing.T) {
 	service := kobject.ServiceConfig{