Skip to content

Commit

Permalink
fix(trait): controller strategy default service port name
Browse files Browse the repository at this point in the history
Using a func that determine the default port name based on the controller strategy adopted

Close apache#5409
  • Loading branch information
squakez committed Apr 26, 2024
1 parent 5e6c159 commit 698b470
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 60 deletions.
14 changes: 6 additions & 8 deletions pkg/trait/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions pkg/trait/container_probes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion pkg/trait/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
15 changes: 6 additions & 9 deletions pkg/trait/health.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
40 changes: 2 additions & 38 deletions pkg/trait/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (t *prometheusTrait) Apply(e *Environment) error {
v1.IntegrationConditionPrometheusAvailable,
corev1.ConditionFalse,
v1.IntegrationConditionContainerNotAvailableReason,
"",
"integration container not available",
)
return nil
}
Expand All @@ -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)
}

Expand All @@ -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
Expand All @@ -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 {
Expand Down
5 changes: 4 additions & 1 deletion pkg/trait/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
46 changes: 45 additions & 1 deletion pkg/trait/trait_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(`(\$\{([^}]*)\})`)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down

0 comments on commit 698b470

Please sign in to comment.