This is an automated email from the ASF dual-hosted git repository.

pcongiusti pushed a commit to branch release-2.3.x
in repository https://gitbox.apache.org/repos/asf/camel-k.git


The following commit(s) were added to refs/heads/release-2.3.x by this push:
     new 446172ffd fix(trait): controller strategy default service port name
446172ffd is described below

commit 446172ffdde30c64b8c96ec0d6d8ee49ea724dc5
Author: Pasquale Congiusti <pasquale.congiu...@gmail.com>
AuthorDate: Wed Apr 24 10:29:20 2024 +0200

    fix(trait): controller strategy default service port name
    
    Using a func that determine the default port name based on the controller 
strategy adopted
    
    Close #5409
---
 pkg/trait/container.go             | 14 +++++-------
 pkg/trait/container_probes_test.go |  4 ++--
 pkg/trait/container_test.go        |  2 +-
 pkg/trait/health.go                | 15 +++++--------
 pkg/trait/prometheus.go            | 40 ++-------------------------------
 pkg/trait/route.go                 |  5 ++++-
 pkg/trait/trait_types.go           | 46 +++++++++++++++++++++++++++++++++++++-
 7 files changed, 66 insertions(+), 60 deletions(-)

diff --git a/pkg/trait/container.go b/pkg/trait/container.go
index 2beca4dfb..526c651de 100644
--- a/pkg/trait/container.go
+++ b/pkg/trait/container.go
@@ -43,11 +43,10 @@ import (
 )
 
 const (
-       defaultContainerName     = "integration"
-       defaultContainerPort     = 8080
-       defaultContainerPortName = "http"
-       defaultServicePort       = 80
-       containerTraitID         = "container"
+       defaultContainerName = "integration"
+       defaultContainerPort = 8080
+       defaultServicePort   = 80
+       containerTraitID     = "container"
 )
 
 type containerTrait struct {
@@ -261,15 +260,14 @@ func (t *containerTrait) configureContainer(e 
*Environment) error {
 func (t *containerTrait) configureService(e *Environment, container 
*corev1.Container, isKnative bool) {
        name := t.PortName
        if name == "" {
-               name = defaultContainerPortName
+               name = e.determineDefaultContainerPortName()
        }
        containerPort := corev1.ContainerPort{
+               Name:          name,
                ContainerPort: int32(t.Port),
                Protocol:      corev1.ProtocolTCP,
        }
        if !isKnative {
-               // Knative does not want name=http
-               containerPort.Name = name
                // The service is managed by Knative, so, we only take care of 
this when it's managed by us
                service := e.Resources.GetServiceForIntegration(e.Integration)
                if service != nil {
diff --git a/pkg/trait/container_probes_test.go 
b/pkg/trait/container_probes_test.go
index 87a070f94..d56857525 100644
--- a/pkg/trait/container_probes_test.go
+++ b/pkg/trait/container_probes_test.go
@@ -207,10 +207,10 @@ func TestProbesOnKnativeService(t *testing.T) {
        container := env.GetIntegrationContainer()
 
        assert.Equal(t, "", container.LivenessProbe.HTTPGet.Host)
-       assert.Equal(t, int32(0), container.LivenessProbe.HTTPGet.Port.IntVal)
+       assert.Equal(t, int32(defaultContainerPort), 
container.LivenessProbe.HTTPGet.Port.IntVal)
        assert.Equal(t, defaultLivenessProbePath, 
container.LivenessProbe.HTTPGet.Path)
        assert.Equal(t, "", container.ReadinessProbe.HTTPGet.Host)
-       assert.Equal(t, int32(0), container.ReadinessProbe.HTTPGet.Port.IntVal)
+       assert.Equal(t, int32(defaultContainerPort), 
container.ReadinessProbe.HTTPGet.Port.IntVal)
        assert.Equal(t, defaultReadinessProbePath, 
container.ReadinessProbe.HTTPGet.Path)
        assert.Equal(t, int32(1234), container.LivenessProbe.TimeoutSeconds)
 }
diff --git a/pkg/trait/container_test.go b/pkg/trait/container_test.go
index 176dd9794..dca6e1dd4 100644
--- a/pkg/trait/container_test.go
+++ b/pkg/trait/container_test.go
@@ -653,5 +653,5 @@ func TestKnativeServiceContainerPorts(t *testing.T) {
        container := environment.GetIntegrationContainer()
        assert.Len(t, container.Ports, 1)
        assert.Equal(t, int32(8081), container.Ports[0].ContainerPort)
-       assert.Equal(t, "", container.Ports[0].Name)
+       assert.Equal(t, defaultKnativeContainerPortName, 
container.Ports[0].Name)
 }
diff --git a/pkg/trait/health.go b/pkg/trait/health.go
index a214b7144..c1a3e8b9c 100644
--- a/pkg/trait/health.go
+++ b/pkg/trait/health.go
@@ -87,17 +87,14 @@ func (t *healthTrait) Apply(e *Environment) error {
        if container == nil {
                return fmt.Errorf("unable to find integration container: %s", 
e.Integration.Name)
        }
+
        var port *intstr.IntOrString
-       // Use the default named HTTP container port if it exists.
-       // For Knative, the Serving webhook is responsible for setting the 
user-land port,
-       // and associating the probes with the corresponding port.
-       if containerPort := e.getIntegrationContainerPort(); containerPort != 
nil && containerPort.Name == defaultContainerPortName {
-               p := intstr.FromString(defaultContainerPortName)
-               port = &p
-       } else if e.GetTrait(knativeServiceTraitID) == nil {
-               p := intstr.FromInt(defaultContainerPort)
-               port = &p
+       containerPort := e.getIntegrationContainerPort()
+       if containerPort == nil {
+               containerPort = e.createContainerPort()
        }
+       p := intstr.FromInt(int(containerPort.ContainerPort))
+       port = &p
 
        if e.CamelCatalog.Runtime.Capabilities["health"].Metadata != nil {
                t.setCatalogConfiguration(container, port, 
e.CamelCatalog.Runtime.Capabilities["health"].Metadata)
diff --git a/pkg/trait/prometheus.go b/pkg/trait/prometheus.go
index bbc93c9d1..8612dc859 100644
--- a/pkg/trait/prometheus.go
+++ b/pkg/trait/prometheus.go
@@ -64,7 +64,7 @@ func (t *prometheusTrait) Apply(e *Environment) error {
                        v1.IntegrationConditionPrometheusAvailable,
                        corev1.ConditionFalse,
                        v1.IntegrationConditionContainerNotAvailableReason,
-                       "",
+                       "integration container not available",
                )
                return nil
        }
@@ -75,14 +75,9 @@ func (t *prometheusTrait) Apply(e *Environment) error {
                Reason: v1.IntegrationConditionPrometheusAvailableReason,
        }
 
-       controller, err := e.DetermineControllerStrategy()
-       if err != nil {
-               return err
-       }
-
        containerPort := e.getIntegrationContainerPort()
        if containerPort == nil {
-               containerPort = t.getContainerPort(e, controller)
+               containerPort = e.createContainerPort()
                container.Ports = append(container.Ports, *containerPort)
        }
 
@@ -91,14 +86,6 @@ func (t *prometheusTrait) Apply(e *Environment) error {
        // Add the PodMonitor resource
        if pointer.BoolDeref(t.PodMonitor, false) {
                portName := containerPort.Name
-               // Knative defaults to naming the userland container port 
"user-port".
-               // Let's rely on that default, granted it is not officially 
part of the Knative
-               // runtime contract.
-               // See 
https://github.com/knative/specs/blob/main/specs/serving/runtime-contract.md
-               if portName == "" && controller == 
ControllerStrategyKnativeService {
-                       portName = "user-port"
-               }
-
                podMonitor, err := t.getPodMonitorFor(e, portName)
                if err != nil {
                        return err
@@ -114,29 +101,6 @@ func (t *prometheusTrait) Apply(e *Environment) error {
        return nil
 }
 
-func (t *prometheusTrait) getContainerPort(e *Environment, controller 
ControllerStrategy) *corev1.ContainerPort {
-       var name string
-       var port int
-
-       if t := e.Catalog.GetTrait(containerTraitID); t != nil {
-               if ct, ok := t.(*containerTrait); ok {
-                       name = ct.PortName
-                       port = ct.Port
-               }
-       }
-
-       // Let's rely on Knative default HTTP negotiation
-       if name == "" && controller != ControllerStrategyKnativeService {
-               name = defaultContainerPortName
-       }
-
-       return &corev1.ContainerPort{
-               Name:          name,
-               ContainerPort: int32(port),
-               Protocol:      corev1.ProtocolTCP,
-       }
-}
-
 func (t *prometheusTrait) getPodMonitorFor(e *Environment, portName string) 
(*monitoringv1.PodMonitor, error) {
        labels, err := keyValuePairArrayAsStringMap(t.PodMonitorLabels)
        if err != nil {
diff --git a/pkg/trait/route.go b/pkg/trait/route.go
index 6816352e7..80794c050 100644
--- a/pkg/trait/route.go
+++ b/pkg/trait/route.go
@@ -80,12 +80,15 @@ func (t *routeTrait) Configure(e *Environment) (bool, 
*TraitCondition, error) {
 }
 
 func (t *routeTrait) Apply(e *Environment) error {
-       servicePortName := defaultContainerPortName
+       servicePortName := ""
        if dt := e.Catalog.GetTrait(containerTraitID); dt != nil {
                if ct, ok := dt.(*containerTrait); ok {
                        servicePortName = ct.ServicePortName
                }
        }
+       if servicePortName == "" {
+               servicePortName = e.determineDefaultContainerPortName()
+       }
 
        tlsConfig, err := t.getTLSConfig(e)
        if err != nil {
diff --git a/pkg/trait/trait_types.go b/pkg/trait/trait_types.go
index 46690b28b..2527ebe6f 100644
--- a/pkg/trait/trait_types.go
+++ b/pkg/trait/trait_types.go
@@ -50,6 +50,11 @@ const (
        sourceLoaderAnnotation      = "camel.apache.org/source.loader"
        sourceNameAnnotation        = "camel.apache.org/source.name"
        sourceCompressionAnnotation = "camel.apache.org/source.compression"
+
+       defaultContainerPortName = "http"
+       // Knative does not want name=http, it only supports http1 (HTTP/1) and 
h2c (HTTP/2)
+       // 
https://github.com/knative/specs/blob/main/specs/serving/runtime-contract.md#protocols-and-ports
+       defaultKnativeContainerPortName = "h2c"
 )
 
 var capabilityDynamicProperty = regexp.MustCompile(`(\$\{([^}]*)\})`)
@@ -337,6 +342,19 @@ func (e *Environment) DetermineControllerStrategy() 
(ControllerStrategy, error)
        return defaultStrategy, nil
 }
 
+// determineDefaultContainerPortName determines the default port name, 
according the controller strategy used.
+func (e *Environment) determineDefaultContainerPortName() string {
+       controller, err := e.DetermineControllerStrategy()
+       if err != nil {
+               log.WithValues("Function", 
"trait.determineDefaultContainerPortName").Errorf(err, "could not determine 
controller strategy, using default deployment container name")
+               return defaultContainerPortName
+       }
+       if controller == ControllerStrategyKnativeService {
+               return defaultKnativeContainerPortName
+       }
+       return defaultContainerPortName
+}
+
 func (e *Environment) getControllerStrategyChoosers() 
[]ControllerStrategySelector {
        var res []ControllerStrategySelector
        for _, t := range e.ConfiguredTraits {
@@ -712,14 +730,17 @@ func (e *Environment) getIntegrationContainerPort() 
*corev1.ContainerPort {
                return nil
        }
 
+       // User specified port name
        portName := ""
        if t := e.Catalog.GetTrait(containerTraitID); t != nil {
                if ct, ok := t.(*containerTrait); ok {
                        portName = ct.PortName
                }
        }
+
+       // default port name (may change according the controller strategy, ie 
Knative)
        if portName == "" {
-               portName = defaultContainerPortName
+               portName = e.determineDefaultContainerPortName()
        }
 
        for i, port := range container.Ports {
@@ -731,6 +752,29 @@ func (e *Environment) getIntegrationContainerPort() 
*corev1.ContainerPort {
        return nil
 }
 
+// createContainerPort creates a new container port with values taken from 
Container trait or default.
+func (e *Environment) createContainerPort() *corev1.ContainerPort {
+       var name string
+       var port int
+
+       if t := e.Catalog.GetTrait(containerTraitID); t != nil {
+               if ct, ok := t.(*containerTrait); ok {
+                       name = ct.PortName
+                       port = ct.Port
+               }
+       }
+
+       if name == "" {
+               name = e.determineDefaultContainerPortName()
+       }
+
+       return &corev1.ContainerPort{
+               Name:          name,
+               ContainerPort: int32(port),
+               Protocol:      corev1.ProtocolTCP,
+       }
+}
+
 // CapabilityPropertyKey returns the key or expand any variable provided in 
it. vars variable contain the
 // possible dynamic values to use.
 func CapabilityPropertyKey(camelPropertyKey string, vars map[string]string) 
string {

Reply via email to