Repository: apache/camel-k
Pull request #1345 (2020-03-13 18:22:05)

Fix probes port detection
<!-- Description -->




<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE". 

You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes. 
-->

**Release Note**
```release-note
NONE
```


Patch:
diff --git a/pkg/trait/container.go b/pkg/trait/container.go
index 416e844ab..6028936ea 100644
--- a/pkg/trait/container.go
+++ b/pkg/trait/container.go
@@ -41,7 +41,6 @@ const (
 	defaultContainerName = "integration"
 	defaultContainerPort = 8080
 	defaultServicePort   = 80
-	defaultProbePort     = 8081
 	defaultProbePath     = "/health"
 	containerTraitID     = "container"
 )
@@ -81,10 +80,10 @@ type containerTrait struct {
 
 	// ProbesEnabled enable/disable probes on the container (default `false`)
 	ProbesEnabled bool `property:"probes-enabled"`
-	// ProbePort configures the port on which the probes are exposed if the container is not exposed
-	// through an `http` port (default `8081`). Note that the value has no effect for Knative service
+	// ProbePort configures the port on which the probes are exposed, by default it inhierit the
+	// value from the `http` port configured on the container. Note that the value has no effect for Knative service
 	// as the port should be the same as the port declared by the container.
-	ProbePort int `property:"probe-port"`
+	ProbePort *int `property:"probe-port"`
 	// Path to access on the probe ( default `/health`). Note that this property is not supported
 	// on quarkus runtime and setting it will result in the integration failing to start.
 	ProbePath string `property:"probe-path"`
@@ -123,7 +122,7 @@ func newContainerTrait() Trait {
 		ServicePortName: httpPortName,
 		Name:            defaultContainerName,
 		ProbesEnabled:   false,
-		ProbePort:       defaultProbePort,
+		ProbePort:       nil,
 		ProbePath:       defaultProbePath,
 	}
 }
@@ -181,6 +180,7 @@ func (t *containerTrait) configureDependencies(e *Environment) {
 	}
 }
 
+// nolint:gocyclo
 func (t *containerTrait) configureContainer(e *Environment) error {
 	container := corev1.Container{
 		Name:  t.Name,
@@ -203,32 +203,40 @@ func (t *containerTrait) configureContainer(e *Environment) error {
 	if t.Expose != nil && *t.Expose {
 		t.configureService(e, &container)
 	}
-	if props := e.ComputeApplicationProperties(); props != nil {
-		e.Resources.Add(props)
-	}
 
 	//
 	// Deployment
 	//
 	if err := e.Resources.VisitDeploymentE(func(deployment *appsv1.Deployment) error {
+		if t.ProbesEnabled {
+			var port int
+
+			switch {
+			case t.ProbePort != nil:
+				port = *t.ProbePort
+			case t.Expose != nil && *t.Expose && t.PortName == httpPortName:
+				port = t.Port
+			default:
+				return fmt.Errorf("unable to determine the port probes should be bound to")
+			}
+
+			if err := t.configureProbes(e, &container, port, t.ProbePath); err != nil {
+				return err
+			}
+		}
+
 		for _, envVar := range e.EnvVars {
 			envvar.SetVar(&container.Env, envVar)
 		}
+		if props := e.ComputeApplicationProperties(); props != nil {
+			e.Resources.Add(props)
+		}
 
 		e.ConfigureVolumesAndMounts(
 			&deployment.Spec.Template.Spec.Volumes,
 			&container.VolumeMounts,
 		)
 
-		port := t.Port
-		if t.PortName != httpPortName {
-			port = t.ProbePort
-		}
-
-		if err := t.configureProbes(e, &container, port, t.ProbePath); err != nil {
-			return err
-		}
-
 		deployment.Spec.Template.Spec.Containers = append(deployment.Spec.Template.Spec.Containers, container)
 
 		return nil
@@ -240,6 +248,13 @@ func (t *containerTrait) configureContainer(e *Environment) error {
 	// Knative Service
 	//
 	if err := e.Resources.VisitKnativeServiceE(func(service *serving.Service) error {
+		if t.ProbesEnabled {
+			// don't set the port on Knative service as it is not allowed.
+			if err := t.configureProbes(e, &container, 0, t.ProbePath); err != nil {
+				return err
+			}
+		}
+
 		for _, env := range e.EnvVars {
 			switch {
 			case env.ValueFrom == nil:
@@ -254,17 +269,15 @@ func (t *containerTrait) configureContainer(e *Environment) error {
 				envvar.SetVar(&container.Env, env)
 			}
 		}
+		if props := e.ComputeApplicationProperties(); props != nil {
+			e.Resources.Add(props)
+		}
 
 		e.ConfigureVolumesAndMounts(
 			&service.Spec.ConfigurationSpec.Template.Spec.Volumes,
 			&container.VolumeMounts,
 		)
 
-		// don't set the port on Knative service as it is not allowed.
-		if err := t.configureProbes(e, &container, 0, t.ProbePath); err != nil {
-			return err
-		}
-
 		service.Spec.ConfigurationSpec.Template.Spec.Containers = append(service.Spec.ConfigurationSpec.Template.Spec.Containers, container)
 
 		return nil
@@ -276,24 +289,35 @@ func (t *containerTrait) configureContainer(e *Environment) error {
 	// CronJob
 	//
 	if err := e.Resources.VisitCronJobE(func(cron *v1beta1.CronJob) error {
+		if t.ProbesEnabled {
+			var port int
+
+			switch {
+			case t.ProbePort != nil:
+				port = *t.ProbePort
+			case t.Expose != nil && *t.Expose && t.PortName == httpPortName:
+				port = t.Port
+			default:
+				return fmt.Errorf("unable to determine the port probes should be bound to")
+			}
+
+			if err := t.configureProbes(e, &container, port, t.ProbePath); err != nil {
+				return err
+			}
+		}
+
 		for _, envVar := range e.EnvVars {
 			envvar.SetVar(&container.Env, envVar)
 		}
+		if props := e.ComputeApplicationProperties(); props != nil {
+			e.Resources.Add(props)
+		}
 
 		e.ConfigureVolumesAndMounts(
 			&cron.Spec.JobTemplate.Spec.Template.Spec.Volumes,
 			&container.VolumeMounts,
 		)
 
-		port := t.Port
-		if t.PortName != httpPortName {
-			port = t.ProbePort
-		}
-
-		if err := t.configureProbes(e, &container, port, t.ProbePath); err != nil {
-			return err
-		}
-
 		cron.Spec.JobTemplate.Spec.Template.Spec.Containers = append(cron.Spec.JobTemplate.Spec.Template.Spec.Containers, container)
 
 		return nil
@@ -391,10 +415,6 @@ func (t *containerTrait) configureResources(_ *Environment, container *corev1.Co
 	}
 }
 func (t *containerTrait) configureProbes(e *Environment, container *corev1.Container, port int, path string) error {
-	if !t.ProbesEnabled {
-		return nil
-	}
-
 	if e.ApplicationProperties == nil {
 		e.ApplicationProperties = make(map[string]string)
 	}


Test patch:
diff --git a/pkg/trait/container_probes_test.go b/pkg/trait/container_probes_test.go
index 930710b8d..5d2cde667 100644
--- a/pkg/trait/container_probes_test.go
+++ b/pkg/trait/container_probes_test.go
@@ -84,7 +84,6 @@ func TestProbesDepsQuarkus(t *testing.T) {
 	env.Integration.Status.Phase = v1.IntegrationPhaseInitialization
 
 	ctr := newTestContainerTrait()
-	ctr.ProbePort = 9191
 
 	ok, err := ctr.Configure(&env)
 	assert.Nil(t, err)
@@ -102,8 +101,10 @@ func TestProbesOnDeployment(t *testing.T) {
 	env.Integration.Status.Phase = v1.IntegrationPhaseDeploying
 	env.Resources.Add(&target)
 
+	expose := true
+
 	ctr := newTestContainerTrait()
-	ctr.ProbePort = 9191
+	ctr.Expose = &expose
 	ctr.LivenessTimeout = 1234
 
 	err := ctr.Apply(&env)
@@ -125,19 +126,21 @@ func TestProbesOnDeploymentWithNoHttpPort(t *testing.T) {
 	env.Integration.Status.Phase = v1.IntegrationPhaseDeploying
 	env.Resources.Add(&target)
 
+	probePort := 9191
+
 	ctr := newTestContainerTrait()
 	ctr.PortName = "custom"
-	ctr.ProbePort = 9191
+	ctr.ProbePort = &probePort
 	ctr.LivenessTimeout = 1234
 
 	err := ctr.Apply(&env)
 	assert.Nil(t, err)
 
 	assert.Equal(t, "", target.Spec.Template.Spec.Containers[0].LivenessProbe.HTTPGet.Host)
-	assert.Equal(t, int32(9191), target.Spec.Template.Spec.Containers[0].LivenessProbe.HTTPGet.Port.IntVal)
+	assert.Equal(t, int32(probePort), target.Spec.Template.Spec.Containers[0].LivenessProbe.HTTPGet.Port.IntVal)
 	assert.Equal(t, defaultProbePath, target.Spec.Template.Spec.Containers[0].LivenessProbe.HTTPGet.Path)
 	assert.Equal(t, "", target.Spec.Template.Spec.Containers[0].ReadinessProbe.HTTPGet.Host)
-	assert.Equal(t, int32(9191), target.Spec.Template.Spec.Containers[0].ReadinessProbe.HTTPGet.Port.IntVal)
+	assert.Equal(t, int32(probePort), target.Spec.Template.Spec.Containers[0].ReadinessProbe.HTTPGet.Port.IntVal)
 	assert.Equal(t, defaultProbePath, target.Spec.Template.Spec.Containers[0].ReadinessProbe.HTTPGet.Path)
 	assert.Equal(t, int32(1234), target.Spec.Template.Spec.Containers[0].LivenessProbe.TimeoutSeconds)
 }
@@ -149,8 +152,10 @@ func TestProbesOnKnativeService(t *testing.T) {
 	env.Integration.Status.Phase = v1.IntegrationPhaseDeploying
 	env.Resources.Add(&target)
 
+	expose := true
+
 	ctr := newTestContainerTrait()
-	ctr.ProbePort = 9191
+	ctr.Expose = &expose
 	ctr.LivenessTimeout = 1234
 
 	err := ctr.Apply(&env)
@@ -172,9 +177,11 @@ func TestProbesOnKnativeServiceWithNoHttpPort(t *testing.T) {
 	env.Integration.Status.Phase = v1.IntegrationPhaseDeploying
 	env.Resources.Add(&target)
 
+	probePort := 9191
+
 	ctr := newTestContainerTrait()
 	ctr.PortName = "custom"
-	ctr.ProbePort = 9191
+	ctr.ProbePort = &probePort
 	ctr.LivenessTimeout = 1234
 
 	err := ctr.Apply(&env)