Repository: apache/camel-k
Pull request #975 (2019-10-01 07:41:45)

fix: Kaniko cache default spec is not updated when calling the status endpoint
Follows-up on #922. The default setting is updated using the status endpoint that does not take update to the `spec` section into account. That leaves the default setting for Kaniko cache unapplied and causes nil pointer dereference panic later on. 


Patch:
diff --git a/pkg/builder/kaniko/publisher.go b/pkg/builder/kaniko/publisher.go
index 488fe6d56..fd3766ca5 100644
--- a/pkg/builder/kaniko/publisher.go
+++ b/pkg/builder/kaniko/publisher.go
@@ -20,6 +20,7 @@ package kaniko
 import (
 	"fmt"
 	"io/ioutil"
+	"os"
 	"path"
 	"strconv"
 	"time"
@@ -44,6 +45,12 @@ func publisher(ctx *builder.Context) error {
 	image := ctx.Build.Platform.Build.Registry.Address + "/" + organization + "/camel-k-" + ctx.Build.Meta.Name + ":" + ctx.Build.Meta.ResourceVersion
 	baseDir, _ := path.Split(ctx.Archive)
 	contextDir := path.Join(baseDir, "context")
+
+	err := os.Mkdir(contextDir, 0777)
+	if err != nil {
+		return err
+	}
+
 	if err := tar.Extract(ctx.Archive, contextDir); err != nil {
 		return err
 	}
@@ -54,7 +61,7 @@ func publisher(ctx *builder.Context) error {
 		ADD . /deployments
 	`)
 
-	err := ioutil.WriteFile(path.Join(contextDir, "Dockerfile"), dockerFileContent, 0777)
+	err = ioutil.WriteFile(path.Join(contextDir, "Dockerfile"), dockerFileContent, 0777)
 	if err != nil {
 		return err
 	}
diff --git a/pkg/cmd/run.go b/pkg/cmd/run.go
index 51b71254b..97e4ff6be 100644
--- a/pkg/cmd/run.go
+++ b/pkg/cmd/run.go
@@ -209,7 +209,7 @@ func (o *runCmdOptions) run(_ *cobra.Command, args []string) error {
 				return err
 			}
 
-			if *integrationPhase == v1alpha1.IntegrationPhaseRunning || *integrationPhase == v1alpha1.IntegrationPhaseError {
+			if integrationPhase == nil || *integrationPhase == v1alpha1.IntegrationPhaseRunning || *integrationPhase == v1alpha1.IntegrationPhaseError {
 				break
 			}
 
diff --git a/pkg/controller/integrationplatform/initialize.go b/pkg/controller/integrationplatform/initialize.go
index a33102706..d90a06cf3 100644
--- a/pkg/controller/integrationplatform/initialize.go
+++ b/pkg/controller/integrationplatform/initialize.go
@@ -55,7 +55,6 @@ func (action *initializeAction) CanHandle(platform *v1alpha1.IntegrationPlatform
 }
 
 func (action *initializeAction) Handle(ctx context.Context, platform *v1alpha1.IntegrationPlatform) (*v1alpha1.IntegrationPlatform, error) {
-
 	duplicate, err := action.isDuplicate(ctx, platform)
 	if err != nil {
 		return nil, err
@@ -109,15 +108,15 @@ func (action *initializeAction) Handle(ctx context.Context, platform *v1alpha1.I
 		}
 	}
 
-	if platform.Spec.Build.PublishStrategy == v1alpha1.IntegrationPlatformBuildPublishStrategyKaniko && platform.Spec.Build.Registry.Address == "" {
-		action.L.Info("No registry specified for publishing images")
-	}
-
 	err = action.setDefaults(ctx, platform)
 	if err != nil {
 		return nil, err
 	}
 
+	if platform.Spec.Build.PublishStrategy == v1alpha1.IntegrationPlatformBuildPublishStrategyKaniko && platform.Spec.Build.Registry.Address == "" {
+		action.L.Info("No registry specified for publishing images")
+	}
+
 	if platform.Spec.Build.Maven.Timeout.Duration != 0 {
 		action.L.Infof("Maven Timeout set to %s", platform.Spec.Build.Maven.Timeout.Duration)
 	}
@@ -136,13 +135,6 @@ func (action *initializeAction) Handle(ctx context.Context, platform *v1alpha1.I
 			return nil, err
 		}
 
-		defaultKanikoBuildCache := true
-		// Check if the KanikoBuildCache has been initialized
-		if platform.Spec.Build.KanikoBuildCache == nil {
-			//if not initialized then default it to true
-			platform.Spec.Build.KanikoBuildCache = &defaultKanikoBuildCache
-		}
-
 		// Check if the operator is running in the same namespace before starting the cache warmer
 		if platform.Namespace == platformutil.GetOperatorNamespace() && *platform.Spec.Build.KanikoBuildCache {
 			// Create the Kaniko warmer pod that caches the base image into the Camel K builder volume
@@ -255,6 +247,13 @@ func (action *initializeAction) setDefaults(ctx context.Context, platform *v1alp
 		}
 	}
 
+	if platform.Spec.Build.PublishStrategy == v1alpha1.IntegrationPlatformBuildPublishStrategyKaniko && platform.Spec.Build.KanikoBuildCache == nil {
+		// Default to using Kaniko cache warmer
+		defaultKanikoBuildCache := true
+		platform.Spec.Build.KanikoBuildCache = &defaultKanikoBuildCache
+		action.L.Infof("Kaniko cache set to %t", *platform.Spec.Build.KanikoBuildCache)
+	}
+
 	action.L.Infof("CamelVersion set to %s", platform.Spec.Build.CamelVersion)
 	action.L.Infof("RuntimeVersion set to %s", platform.Spec.Build.RuntimeVersion)
 	action.L.Infof("BaseImage set to %s", platform.Spec.Build.BaseImage)
diff --git a/pkg/trait/jolokia.go b/pkg/trait/jolokia.go
index bfe617452..37721ba6e 100644
--- a/pkg/trait/jolokia.go
+++ b/pkg/trait/jolokia.go
@@ -79,15 +79,32 @@ func (t *jolokiaTrait) Configure(e *Environment) (bool, error) {
 }
 
 func (t *jolokiaTrait) Apply(e *Environment) (err error) {
+	containerName := defaultContainerName
+	dt := e.Catalog.GetTrait(containerTraitID)
+	if dt != nil {
+		containerName = dt.(*containerTrait).Name
+	}
+
+	container := e.Resources.GetContainerByName(containerName)
+	if container == nil {
+		e.Integration.Status.SetCondition(
+			v1alpha1.IntegrationConditionJolokiaAvailable,
+			corev1.ConditionFalse,
+			v1alpha1.IntegrationConditionContainerNotAvailableReason,
+			"",
+		)
+		return nil
+	}
+
 	if t.Enabled == nil || !*t.Enabled {
 		// Deactivate the Jolokia Java agent
 		// Note: the AB_JOLOKIA_OFF environment variable acts as an option flag
-		envvar.SetVal(&e.EnvVars, "AB_JOLOKIA_OFF", "true")
+		envvar.SetVal(&container.Env, "AB_JOLOKIA_OFF", "true")
 		return nil
 	}
 
 	// Need to set it explicitly as it default to true
-	envvar.SetVal(&e.EnvVars, "AB_JOLOKIA_AUTH_OPENSHIFT", "false")
+	envvar.SetVal(&container.Env, "AB_JOLOKIA_AUTH_OPENSHIFT", "false")
 
 	// Configure the Jolokia Java agent
 	// Populate first with the extra options
@@ -113,25 +130,7 @@ func (t *jolokiaTrait) Apply(e *Environment) (err error) {
 	for k, v := range options {
 		optionValues = append(optionValues, k+"="+v)
 	}
-	envvar.SetVal(&e.EnvVars, "AB_JOLOKIA_OPTS", strings.Join(optionValues, ","))
-
-	containerName := defaultContainerName
-	dt := e.Catalog.GetTrait(containerTraitID)
-	if dt != nil {
-		containerName = dt.(*containerTrait).Name
-	}
-
-	container := e.Resources.GetContainerByName(containerName)
-	if container == nil {
-		e.Integration.Status.SetCondition(
-			v1alpha1.IntegrationConditionJolokiaAvailable,
-			corev1.ConditionFalse,
-			v1alpha1.IntegrationConditionContainerNotAvailableReason,
-			"",
-		)
-
-		return nil
-	}
+	envvar.SetVal(&container.Env, "AB_JOLOKIA_OPTS", strings.Join(optionValues, ","))
 
 	containerPort := corev1.ContainerPort{
 		Name:          "jolokia",
diff --git a/pkg/trait/prometheus.go b/pkg/trait/prometheus.go
index f52ad54c1..b5dd77d9f 100644
--- a/pkg/trait/prometheus.go
+++ b/pkg/trait/prometheus.go
@@ -56,94 +56,101 @@ func (t *prometheusTrait) Configure(e *Environment) (bool, error) {
 }
 
 func (t *prometheusTrait) Apply(e *Environment) (err error) {
+	containerName := defaultContainerName
+	dt := e.Catalog.GetTrait(containerTraitID)
+	if dt != nil {
+		containerName = dt.(*containerTrait).Name
+	}
+
+	container := e.Resources.GetContainerByName(containerName)
+	if container == nil {
+		e.Integration.Status.SetCondition(
+			v1alpha1.IntegrationConditionPrometheusAvailable,
+			corev1.ConditionFalse,
+			v1alpha1.IntegrationConditionContainerNotAvailableReason,
+			"",
+		)
+		return nil
+	}
+
 	if t.Enabled == nil || !*t.Enabled {
 		// Deactivate the Prometheus Java agent
 		// Note: the AB_PROMETHEUS_OFF environment variable acts as an option flag
-		envvar.SetVal(&e.EnvVars, "AB_PROMETHEUS_OFF", "true")
+		envvar.SetVal(&container.Env, "AB_PROMETHEUS_OFF", "true")
 		return nil
 	}
 
-	// Configure the Prometheus Java agent
-	envvar.SetVal(&e.EnvVars, "AB_PROMETHEUS_PORT", strconv.Itoa(t.Port))
-
-	service, servicePort := t.configureServicePort(e)
-	container, containerPort := t.configureContainerPort(e)
-
 	condition := v1alpha1.IntegrationCondition{
 		Type:   v1alpha1.IntegrationConditionPrometheusAvailable,
 		Status: corev1.ConditionTrue,
 		Reason: v1alpha1.IntegrationConditionPrometheusAvailableReason,
 	}
 
-	if servicePort != nil {
-		service.Spec.Ports = append(service.Spec.Ports, *servicePort)
-		condition.Message = fmt.Sprintf("%s(%s/%d) -> ", service.Name, servicePort.Name, servicePort.Port)
-	} else {
-		condition.Status = corev1.ConditionFalse
-		condition.Reason = v1alpha1.IntegrationConditionServiceNotAvailableReason
-	}
-
-	if containerPort != nil {
-		container.Ports = append(container.Ports, *containerPort)
-		condition.Message += fmt.Sprintf("%s(%s/%d)", container.Name, containerPort.Name, containerPort.ContainerPort)
-	} else {
-		condition.Status = corev1.ConditionFalse
-		condition.Reason = v1alpha1.IntegrationConditionContainerNotAvailableReason
-	}
+	// Configure the Prometheus Java agent
+	envvar.SetVal(&container.Env, "AB_PROMETHEUS_PORT", strconv.Itoa(t.Port))
 
-	e.Integration.Status.SetConditions(condition)
+	// Add the container port
+	containerPort := t.getContainerPort()
+	container.Ports = append(container.Ports, *containerPort)
+	condition.Message = fmt.Sprintf("%s(%s/%d)", container.Name, containerPort.Name, containerPort.ContainerPort)
 
-	if condition.Status == corev1.ConditionFalse {
-		return nil
+	// Retrieve the service or create a new one if the service trait is enabled
+	serviceEnabled := false
+	service := e.Resources.GetServiceForIntegration(e.Integration)
+	if service == nil {
+		trait := e.Catalog.GetTrait(serviceTraitID)
+		if serviceTrait, ok := trait.(*serviceTrait); ok {
+			serviceEnabled = serviceTrait.isEnabled()
+		}
+		if serviceEnabled {
+			// add a new service if not already created
+			service = getServiceFor(e)
+			e.Resources.Add(service)
+		}
 	}
 
-	if t.ServiceMonitor {
+	// Add the service port and service monitor resource
+	// A better strategy may be needed when the Knative profile is active
+	if serviceEnabled {
+		servicePort := t.getServicePort()
+		service.Spec.Ports = append(service.Spec.Ports, *servicePort)
+		condition.Message = fmt.Sprintf("%s(%s/%d) -> ", service.Name, servicePort.Name, servicePort.Port) + condition.Message
+
 		// Add the ServiceMonitor resource
-		smt, err := t.getServiceMonitorFor(e)
-		if err != nil {
-			return err
+		if t.ServiceMonitor {
+			smt, err := t.getServiceMonitorFor(e)
+			if err != nil {
+				return err
+			}
+			e.Resources.Add(smt)
 		}
-		e.Resources.Add(smt)
+	} else {
+		condition.Status = corev1.ConditionFalse
+		condition.Reason = v1alpha1.IntegrationConditionServiceNotAvailableReason
 	}
 
+	e.Integration.Status.SetConditions(condition)
+
 	return nil
 }
 
-func (t *prometheusTrait) configureContainerPort(e *Environment) (*corev1.Container, *corev1.ContainerPort) {
-	containerName := defaultContainerName
-	dt := e.Catalog.GetTrait(containerTraitID)
-	if dt != nil {
-		containerName = dt.(*containerTrait).Name
-	}
-
-	container := e.Resources.GetContainerByName(containerName)
-	if container == nil {
-		return nil, nil
-	}
-
+func (t *prometheusTrait) getContainerPort() *corev1.ContainerPort {
 	containerPort := corev1.ContainerPort{
 		Name:          prometheusPortName,
 		ContainerPort: int32(t.Port),
 		Protocol:      corev1.ProtocolTCP,
 	}
-
-	return container, &containerPort
+	return &containerPort
 }
 
-func (t *prometheusTrait) configureServicePort(e *Environment) (*corev1.Service, *corev1.ServicePort) {
-	service := e.Resources.GetServiceForIntegration(e.Integration)
-	if service == nil {
-		return nil, nil
-	}
-
+func (t *prometheusTrait) getServicePort() *corev1.ServicePort {
 	servicePort := corev1.ServicePort{
 		Name:       prometheusPortName,
 		Port:       int32(t.Port),
 		Protocol:   corev1.ProtocolTCP,
 		TargetPort: intstr.FromString(prometheusPortName),
 	}
-
-	return service, &servicePort
+	return &servicePort
 }
 
 func (t *prometheusTrait) getServiceMonitorFor(e *Environment) (*monitoringv1.ServiceMonitor, error) {
@@ -171,7 +178,7 @@ func (t *prometheusTrait) getServiceMonitorFor(e *Environment) (*monitoringv1.Se
 			},
 			Endpoints: []monitoringv1.Endpoint{
 				{
-					Port: "prometheus",
+					Port: prometheusPortName,
 				},
 			},
 		},
diff --git a/pkg/trait/service.go b/pkg/trait/service.go
index ae204c035..3efb446a1 100644
--- a/pkg/trait/service.go
+++ b/pkg/trait/service.go
@@ -30,16 +30,23 @@ type serviceTrait struct {
 	Auto      *bool `property:"auto"`
 }
 
-const httpPortName = "http"
+const (
+	serviceTraitID = "service"
+	httpPortName   = "http"
+)
 
 func newServiceTrait() *serviceTrait {
 	return &serviceTrait{
-		BaseTrait: newBaseTrait("service"),
+		BaseTrait: newBaseTrait(serviceTraitID),
 	}
 }
 
+func (t *serviceTrait) isEnabled() bool {
+	return t.Enabled == nil || *t.Enabled
+}
+
 func (t *serviceTrait) Configure(e *Environment) (bool, error) {
-	if t.Enabled != nil && !*t.Enabled {
+	if !t.isEnabled() {
 		e.Integration.Status.SetCondition(
 			v1alpha1.IntegrationConditionServiceAvailable,
 			corev1.ConditionFalse,
@@ -83,32 +90,34 @@ func (t *serviceTrait) Configure(e *Environment) (bool, error) {
 	return true, nil
 }
 
-func (t *serviceTrait) Apply(e *Environment) (err error) {
+func (t *serviceTrait) Apply(e *Environment) error {
 	svc := e.Resources.GetServiceForIntegration(e.Integration)
+	// add a new service if not already created
 	if svc == nil {
-		svc := corev1.Service{
-			TypeMeta: metav1.TypeMeta{
-				Kind:       "Service",
-				APIVersion: "v1",
-			},
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      e.Integration.Name,
-				Namespace: e.Integration.Namespace,
-				Labels: map[string]string{
-					"camel.apache.org/integration": e.Integration.Name,
-				},
+		svc = getServiceFor(e)
+		e.Resources.Add(svc)
+	}
+	return nil
+}
+
+func getServiceFor(e *Environment) *corev1.Service {
+	return &corev1.Service{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "Service",
+			APIVersion: "v1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      e.Integration.Name,
+			Namespace: e.Integration.Namespace,
+			Labels: map[string]string{
+				"camel.apache.org/integration": e.Integration.Name,
 			},
-			Spec: corev1.ServiceSpec{
-				Ports: []corev1.ServicePort{},
-				Selector: map[string]string{
-					"camel.apache.org/integration": e.Integration.Name,
-				},
+		},
+		Spec: corev1.ServiceSpec{
+			Ports: []corev1.ServicePort{},
+			Selector: map[string]string{
+				"camel.apache.org/integration": e.Integration.Name,
 			},
-		}
-
-		// add a new service if not already created
-		e.Resources.Add(&svc)
+		},
 	}
-
-	return nil
 }
diff --git a/pkg/trait/trait_catalog.go b/pkg/trait/trait_catalog.go
index 5dfacabec..b68231447 100644
--- a/pkg/trait/trait_catalog.go
+++ b/pkg/trait/trait_catalog.go
@@ -184,6 +184,8 @@ func (c *Catalog) traitsFor(environment *Environment) []Trait {
 			c.tAffinity,
 			c.tKnativeService,
 			c.tContainer,
+			c.tJolokia,
+			c.tPrometheus,
 			c.tClasspath,
 			c.tProbes,
 			c.tIstio,


Test patch:
diff --git a/pkg/trait/jolokia_test.go b/pkg/trait/jolokia_test.go
index 30381f6bf..5474fde09 100644
--- a/pkg/trait/jolokia_test.go
+++ b/pkg/trait/jolokia_test.go
@@ -113,12 +113,13 @@ func TestApplyJolokiaTraitNominalShouldSucceed(t *testing.T) {
 
 	err := trait.Apply(environment)
 
+	container := environment.Resources.GetContainerByName(defaultContainerName)
+
 	assert.Nil(t, err)
-	test.EnvVarHasValue(t, environment.EnvVars, "AB_JOLOKIA_AUTH_OPENSHIFT", "false")
-	test.EnvVarHasValue(t, environment.EnvVars, "AB_JOLOKIA_OPTS", "port=8778")
+	test.EnvVarHasValue(t, container.Env, "AB_JOLOKIA_AUTH_OPENSHIFT", "false")
+	test.EnvVarHasValue(t, container.Env, "AB_JOLOKIA_OPTS", "port=8778")
 	assert.Len(t, environment.Integration.Status.Conditions, 1)
 
-	container := environment.Resources.GetContainerByName("integration")
 	assert.NotNil(t, container)
 	assert.Len(t, container.Ports, 1)
 	containerPort := container.Ports[0]
@@ -158,8 +159,10 @@ func TestApplyJolokiaTraitWithOptionShouldOverrideDefault(t *testing.T) {
 
 	err := trait.Apply(environment)
 
+	container := environment.Resources.GetContainerByName(defaultContainerName)
+
 	assert.Nil(t, err)
-	ev := envvar.Get(environment.EnvVars, "AB_JOLOKIA_OPTS")
+	ev := envvar.Get(container.Env, "AB_JOLOKIA_OPTS")
 	assert.NotNil(t, ev)
 	assert.Contains(t, ev.Value, "port=8778", "host=explicit-host", "discoveryEnabled=true", "protocol=http", "caCert=.cacert")
 	assert.Contains(t, ev.Value, "extendedClientCheck=false", "clientPrincipal=cn:any", "useSslClientAuthentication=false")
@@ -181,8 +184,10 @@ func TestApplyDisabledJolokiaTraitShouldNotSucceed(t *testing.T) {
 
 	err := trait.Apply(environment)
 
+	container := environment.Resources.GetContainerByName(defaultContainerName)
+
 	assert.Nil(t, err)
-	test.EnvVarHasValue(t, environment.EnvVars, "AB_JOLOKIA_OFF", "true")
+	test.EnvVarHasValue(t, container.Env, "AB_JOLOKIA_OFF", "true")
 }
 
 func TestSetDefaultJolokiaOptionShoudlNotOverrideOptionsMap(t *testing.T) {
@@ -283,7 +288,6 @@ func TestAddWrongTypeOptionToJolokiaOptionsDoesNothing(t *testing.T) {
 }
 
 func createNominalJolokiaTest() (*jolokiaTrait, *Environment) {
-
 	trait := newJolokiaTrait()
 	enabled := true
 	trait.Enabled = &enabled