From 698b47052d44058e9b5cccb2111823b2b3ff5e01 Mon Sep 17 00:00:00 2001 From: Pasquale Congiusti Date: Wed, 24 Apr 2024 10:29:20 +0200 Subject: [PATCH] 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 2beca4dfb8..526c651de2 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 87a070f941..d568575252 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 176dd97946..dca6e1dd47 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 a214b71447..7f764fb7b8 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.FromInt32(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 bbc93c9d1a..8612dc8597 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 6816352e77..80794c050c 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 46690b28b2..2527ebe6f6 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 {