Skip to content

Commit

Permalink
fix(trait): bool trait props should be pointer, otherwise omitted in CR
Browse files Browse the repository at this point in the history
  • Loading branch information
tadayosi committed Dec 10, 2020
1 parent de30c13 commit 3a2564d
Show file tree
Hide file tree
Showing 10 changed files with 95 additions and 40 deletions.
37 changes: 37 additions & 0 deletions pkg/cmd/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"testing"

v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
"github.com/apache/camel-k/pkg/trait"
"github.com/apache/camel-k/pkg/util/test"
"github.com/spf13/cobra"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -172,3 +173,39 @@ func TestRunPropertyFileFlag(t *testing.T) {
assert.Equal(t, `d=c\=e`, spec.Configuration[2].Value)
assert.Equal(t, `f=g\:h`, spec.Configuration[3].Value)
}

func TestConfigureTraits(t *testing.T) {
options, rootCmd := kamelTestPreAddCommandInit()
runCmdOptions := addTestRunCmd(options, rootCmd)
kamelTestPostAddCommandInit(t, rootCmd)
_, err := test.ExecuteCommand(rootCmd, "run",
"--trait", "affinity.pod-affinity=false",
"--trait", "container.probes-enabled=false",
"--trait", "environment.container-meta=false",
"--trait", "jvm.print-command=false",
"--trait", "prometheus.service-monitor=false",
"example.js")
if err != nil {
t.Error(err)
}
client, err := runCmdOptions.GetCmdClient()
if err != nil {
t.Error(err)
}
catalog := trait.NewCatalog(runCmdOptions.Context, client)

traits, err := configureTraits(runCmdOptions.Traits, catalog)

assert.Nil(t, err)
assert.Len(t, traits, 5)
assertTraitConfiguration(t, traits, "affinity", `{"podAffinity":false}`)
assertTraitConfiguration(t, traits, "container", `{"probesEnabled":false}`)
assertTraitConfiguration(t, traits, "environment", `{"containerMeta":false}`)
assertTraitConfiguration(t, traits, "jvm", `{"printCommand":false}`)
assertTraitConfiguration(t, traits, "prometheus", `{"serviceMonitor":false}`)
}

func assertTraitConfiguration(t *testing.T, traits map[string]v1.TraitSpec, trait string, expected string) {
assert.Contains(t, traits, trait)
assert.Equal(t, expected, string(traits[trait].Configuration.RawMessage))
}
20 changes: 11 additions & 9 deletions pkg/trait/affinity.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ import (
type affinityTrait struct {
BaseTrait `property:",squash"`
// Always co-locates multiple replicas of the integration in the same node (default *false*).
PodAffinity bool `property:"pod-affinity" json:"podAffinity,omitempty"`
PodAffinity *bool `property:"pod-affinity" json:"podAffinity,omitempty"`
// Never co-locates multiple replicas of the integration in the same node (default *false*).
PodAntiAffinity bool `property:"pod-anti-affinity" json:"podAntiAffinity,omitempty"`
PodAntiAffinity *bool `property:"pod-anti-affinity" json:"podAntiAffinity,omitempty"`
// Defines a set of nodes the integration pod(s) are eligible to be scheduled on, based on labels on the node.
NodeAffinityLabels []string `property:"node-affinity-labels" json:"nodeAffinityLabels,omitempty"`
// Defines a set of pods (namely those matching the label selector, relative to the given namespace) that the
Expand All @@ -54,16 +54,18 @@ type affinityTrait struct {

func newAffinityTrait() Trait {
return &affinityTrait{
BaseTrait: NewBaseTrait("affinity", 1300),
BaseTrait: NewBaseTrait("affinity", 1300),
PodAffinity: &[]bool{false}[0],
PodAntiAffinity: &[]bool{false}[0],
}
}

func (t *affinityTrait) Configure(e *Environment) (bool, error) {
if t.Enabled == nil || !*t.Enabled {
if isNilOrFalse(t.Enabled) {
return false, nil
}

if t.PodAffinity && t.PodAntiAffinity {
if isTrue(t.PodAffinity) && isTrue(t.PodAntiAffinity) {
return false, fmt.Errorf("both pod affinity and pod anti-affinity can't be set simultaneously")
}

Expand Down Expand Up @@ -136,7 +138,7 @@ func (t *affinityTrait) addNodeAffinity(_ *Environment, deployment *appsv1.Deplo
}

func (t *affinityTrait) addPodAffinity(e *Environment, deployment *appsv1.Deployment) error {
if !t.PodAffinity && len(t.PodAffinityLabels) == 0 {
if isNilOrFalse(t.PodAffinity) && len(t.PodAffinityLabels) == 0 {
return nil
}

Expand All @@ -161,7 +163,7 @@ func (t *affinityTrait) addPodAffinity(e *Environment, deployment *appsv1.Deploy
}
}

if t.PodAffinity {
if isTrue(t.PodAffinity) {
labelSelectorRequirements = append(labelSelectorRequirements, metav1.LabelSelectorRequirement{
Key: v1.IntegrationLabel,
Operator: metav1.LabelSelectorOpIn,
Expand Down Expand Up @@ -192,7 +194,7 @@ func (t *affinityTrait) addPodAffinity(e *Environment, deployment *appsv1.Deploy
}

func (t *affinityTrait) addPodAntiAffinity(e *Environment, deployment *appsv1.Deployment) error {
if !t.PodAntiAffinity && len(t.PodAntiAffinityLabels) == 0 {
if isNilOrFalse(t.PodAntiAffinity) && len(t.PodAntiAffinityLabels) == 0 {
return nil
}

Expand All @@ -217,7 +219,7 @@ func (t *affinityTrait) addPodAntiAffinity(e *Environment, deployment *appsv1.De
}
}

if t.PodAntiAffinity {
if isTrue(t.PodAntiAffinity) {
labelSelectorRequirements = append(labelSelectorRequirements, metav1.LabelSelectorRequirement{
Key: v1.IntegrationLabel,
Operator: metav1.LabelSelectorOpIn,
Expand Down
8 changes: 4 additions & 4 deletions pkg/trait/affinity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ func TestConfigureAffinityTraitDoesSucceed(t *testing.T) {

func TestConfigureAffinityTraitWithConflictingAffinitiesFails(t *testing.T) {
affinityTrait, environment, _ := createNominalAffinityTest()
affinityTrait.PodAffinity = true
affinityTrait.PodAntiAffinity = true
affinityTrait.PodAffinity = &[]bool{true}[0]
affinityTrait.PodAntiAffinity = &[]bool{true}[0]
configured, err := affinityTrait.Configure(environment)

assert.False(t, configured)
Expand Down Expand Up @@ -83,7 +83,7 @@ func TestApplyNodeAffinityLabelsDoesSucceed(t *testing.T) {

func TestApplyPodAntiAffinityLabelsDoesSucceed(t *testing.T) {
affinityTrait, environment, deployment := createNominalAffinityTest()
affinityTrait.PodAntiAffinity = true
affinityTrait.PodAntiAffinity = &[]bool{true}[0]
affinityTrait.PodAntiAffinityLabels = []string{"criteria != value"}

err := affinityTrait.Apply(environment)
Expand All @@ -105,7 +105,7 @@ func TestApplyPodAntiAffinityLabelsDoesSucceed(t *testing.T) {

func TestApplyPodAffinityLabelsDoesSucceed(t *testing.T) {
affinityTrait, environment, deployment := createNominalAffinityTest()
affinityTrait.PodAffinity = true
affinityTrait.PodAffinity = &[]bool{true}[0]
affinityTrait.PodAffinityLabels = []string{"!criteria"}

err := affinityTrait.Apply(environment)
Expand Down
14 changes: 7 additions & 7 deletions pkg/trait/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ type containerTrait struct {
Name string `property:"name" json:"name,omitempty"`

// ProbesEnabled enable/disable probes on the container (default `false`)
ProbesEnabled bool `property:"probes-enabled" json:"probesEnabled,omitempty"`
ProbesEnabled *bool `property:"probes-enabled" json:"probesEnabled,omitempty"`
// 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" json:"probePath,omitempty"`
Expand Down Expand Up @@ -113,7 +113,7 @@ func newContainerTrait() Trait {
ServicePort: defaultServicePort,
ServicePortName: httpPortName,
Name: defaultContainerName,
ProbesEnabled: false,
ProbesEnabled: &[]bool{false}[0],
ProbePath: defaultProbePath,
}
}
Expand All @@ -127,7 +127,7 @@ func (t *containerTrait) Configure(e *Environment) (bool, error) {
return false, nil
}

if t.Auto == nil || *t.Auto {
if isNilOrTrue(t.Auto) {
if t.Expose == nil {
e := e.Resources.GetServiceForIntegration(e.Integration) != nil
t.Expose = &e
Expand Down Expand Up @@ -155,7 +155,7 @@ func (t *containerTrait) IsPlatformTrait() bool {
}

func (t *containerTrait) configureDependencies(e *Environment) {
if !t.ProbesEnabled {
if isNilOrFalse(t.ProbesEnabled) {
return
}

Expand Down Expand Up @@ -207,7 +207,7 @@ func (t *containerTrait) configureContainer(e *Environment) error {
// Deployment
//
if err := e.Resources.VisitDeploymentE(func(deployment *appsv1.Deployment) error {
if t.ProbesEnabled && t.PortName == httpPortName {
if isTrue(t.ProbesEnabled) && t.PortName == httpPortName {
if err := t.configureProbes(e, &container, t.Port, t.ProbePath); err != nil {
return err
}
Expand Down Expand Up @@ -236,7 +236,7 @@ func (t *containerTrait) configureContainer(e *Environment) error {
// Knative Service
//
if err := e.Resources.VisitKnativeServiceE(func(service *serving.Service) error {
if t.ProbesEnabled && t.PortName == httpPortName {
if isTrue(t.ProbesEnabled) && t.PortName == httpPortName {
// 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
Expand Down Expand Up @@ -277,7 +277,7 @@ func (t *containerTrait) configureContainer(e *Environment) error {
// CronJob
//
if err := e.Resources.VisitCronJobE(func(cron *v1beta1.CronJob) error {
if t.ProbesEnabled && t.PortName == httpPortName {
if isTrue(t.ProbesEnabled) && t.PortName == httpPortName {
if err := t.configureProbes(e, &container, t.Port, t.ProbePath); err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/trait/container_probes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func newTestProbesEnv(t *testing.T, provider v1.RuntimeProvider) Environment {

func newTestContainerTrait() *containerTrait {
tr := newContainerTrait().(*containerTrait)
tr.ProbesEnabled = true
tr.ProbesEnabled = &[]bool{true}[0]

return tr
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/trait/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
// +camel-k:trait=environment
type environmentTrait struct {
BaseTrait `property:",squash"`
ContainerMeta bool `property:"container-meta" json:"containerMeta,omitempty"`
ContainerMeta *bool `property:"container-meta" json:"containerMeta,omitempty"`
}

const (
Expand All @@ -53,7 +53,7 @@ func newEnvironmentTrait() Trait {
return &environmentTrait{
BaseTrait: NewBaseTrait("environment", 800),
// Enable injection of NAMESPACE and POD_NAME environment variables.
ContainerMeta: true,
ContainerMeta: &[]bool{true}[0],
}
}

Expand All @@ -74,7 +74,7 @@ func (t *environmentTrait) Apply(e *Environment) error {
envvar.SetVal(&e.EnvVars, envVarMountPathConfigMaps, ConfigMapsMountPath)
envvar.SetVal(&e.EnvVars, envVarMountPathSecrets, SecretsMountPath)

if t.ContainerMeta {
if isNilOrTrue(t.ContainerMeta) {
envvar.SetValFrom(&e.EnvVars, envVarNamespace, "metadata.namespace")
envvar.SetValFrom(&e.EnvVars, envVarPodName, "metadata.name")
}
Expand Down
14 changes: 7 additions & 7 deletions pkg/trait/jvm.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ import (
type jvmTrait struct {
BaseTrait `property:",squash"`
// Activates remote debugging, so that a debugger can be attached to the JVM, e.g., using port-forwarding
Debug bool `property:"debug" json:"debug,omitempty"`
Debug *bool `property:"debug" json:"debug,omitempty"`
// Suspends the target JVM immediately before the main class is loaded
DebugSuspend bool `property:"debug-suspend" json:"debugSuspend,omitempty"`
DebugSuspend *bool `property:"debug-suspend" json:"debugSuspend,omitempty"`
// Prints the command used the start the JVM in the container logs (default `true`)
PrintCommand bool `property:"print-command" json:"printCommand,omitempty"`
PrintCommand *bool `property:"print-command" json:"printCommand,omitempty"`
// Transport address at which to listen for the newly launched JVM (default `*:5005`)
DebugAddress string `property:"debug-address" json:"debugAddress,omitempty"`
// A list of JVM options
Expand All @@ -54,7 +54,7 @@ func newJvmTrait() Trait {
return &jvmTrait{
BaseTrait: NewBaseTrait("jvm", 2000),
DebugAddress: "*:5005",
PrintCommand: true,
PrintCommand: &[]bool{true}[0],
}
}

Expand Down Expand Up @@ -115,9 +115,9 @@ func (t *jvmTrait) Apply(e *Environment) error {
args := container.Args

// Remote debugging
if t.Debug {
if isTrue(t.Debug) {
suspend := "n"
if t.DebugSuspend {
if isTrue(t.DebugSuspend) {
suspend = "y"
}
args = append(args,
Expand Down Expand Up @@ -167,7 +167,7 @@ func (t *jvmTrait) Apply(e *Environment) error {
args = append(args, "-cp", strings.Join(items, ":"))
args = append(args, e.CamelCatalog.Runtime.ApplicationClass)

if t.PrintCommand {
if isNilOrTrue(t.PrintCommand) {
args = append([]string{"exec", "java"}, args...)
container.Command = []string{"/bin/sh", "-c"}
cmd := strings.Join(args, " ")
Expand Down
6 changes: 3 additions & 3 deletions pkg/trait/jvm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ func TestApplyJvmTraitWithKNativeResource(t *testing.T) {

func TestApplyJvmTraitWithDebugEnabled(t *testing.T) {
trait, environment := createNominalJvmTest()
trait.Debug = true
trait.DebugSuspend = true
trait.Debug = &[]bool{true}[0]
trait.DebugSuspend = &[]bool{true}[0]

d := appsv1.Deployment{
Spec: appsv1.DeploymentSpec{
Expand Down Expand Up @@ -204,7 +204,7 @@ func createJvmTestWithKitType(kitType string) (*jvmTrait, *Environment) {
trait := newJvmTrait().(*jvmTrait)
enabled := true
trait.Enabled = &enabled
trait.PrintCommand = false
trait.PrintCommand = &[]bool{false}[0]
trait.Ctx = context.TODO()
trait.Client = client

Expand Down
12 changes: 6 additions & 6 deletions pkg/trait/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ import (
"github.com/apache/camel-k/pkg/util"
)

// The Prometheus trait configures a Prometheus-compatible endpoint. This trait also exposes the integration with
//`Service` and `ServiceMonitor` resources, so that the endpoint can be scraped automatically, when using the
// The Prometheus trait configures a Prometheus-compatible endpoint. This trait also exposes the integration with
//`Service` and `ServiceMonitor` resources, so that the endpoint can be scraped automatically, when using the
// Prometheus Operator.
//
// The metrics exposed vary depending on the configured runtime. With the default Quarkus runtime, metrics are
// exposed using MicroProfile Metrics. While with the Java main runtime, metrics are exposed using the Prometheus
// exposed using MicroProfile Metrics. While with the Java main runtime, metrics are exposed using the Prometheus
// JMX exporter.
//
// WARNING: The creation of the `ServiceMonitor` resource requires the https://github.com/coreos/prometheus-operator[Prometheus Operator]
Expand All @@ -54,7 +54,7 @@ type prometheusTrait struct {
// The Prometheus endpoint port (default `9779`, or `8080` with Quarkus).
Port *int `property:"port" json:"port,omitempty"`
// Whether a `ServiceMonitor` resource is created (default `true`).
ServiceMonitor bool `property:"service-monitor" json:"serviceMonitor,omitempty"`
ServiceMonitor *bool `property:"service-monitor" json:"serviceMonitor,omitempty"`
// The `ServiceMonitor` resource labels, applicable when `service-monitor` is `true`.
ServiceMonitorLabels []string `property:"service-monitor-labels" json:"serviceMonitorLabels,omitempty"`
// To use a custom ConfigMap containing the Prometheus JMX exporter configuration (under the `content` ConfigMap key).
Expand All @@ -72,7 +72,7 @@ const (
func newPrometheusTrait() Trait {
return &prometheusTrait{
BaseTrait: NewBaseTrait("prometheus", 1900),
ServiceMonitor: true,
ServiceMonitor: &[]bool{true}[0],
}
}

Expand Down Expand Up @@ -182,7 +182,7 @@ func (t *prometheusTrait) Apply(e *Environment) (err error) {
condition.Message = fmt.Sprintf("%s(%s/%d) -> ", service.Name, servicePort.Name, servicePort.Port) + condition.Message

// Add the ServiceMonitor resource
if t.ServiceMonitor {
if isNilOrTrue(t.ServiceMonitor) {
smt, err := t.getServiceMonitorFor(e)
if err != nil {
return err
Expand Down
16 changes: 16 additions & 0 deletions pkg/trait/trait_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,22 @@ type ControllerStrategySelector interface {
ControllerStrategySelectorOrder() int
}

func isTrue(b *bool) bool {
return b != nil && *b
}

func isNilOrTrue(b *bool) bool {
return b == nil || *b
}

func isFalse(b *bool) bool {
return b != nil && !*b
}

func isNilOrFalse(b *bool) bool {
return b == nil || !*b
}

/* Environment */

// A Environment provides the context where the trait is executed
Expand Down

0 comments on commit 3a2564d

Please sign in to comment.