Repository: apache/camel-k
Pull request #2202 (2021-04-12 09:59:46)

chore(cli): Fix error handling in local commands
**Release Note**
```release-note
NONE
```



Patch:
diff --git a/pkg/cmd/install.go b/pkg/cmd/install.go
index 0adbac4ce..1692c15e9 100644
--- a/pkg/cmd/install.go
+++ b/pkg/cmd/install.go
@@ -128,6 +128,7 @@ func newCmdInstall(rootCmdOptions *RootCmdOptions) (*cobra.Command, *installCmdO
 
 	// Pod settings
 	cmd.Flags().StringArrayP("toleration", "", nil, "Add a Toleration to the operator Pod")
+	cmd.Flags().StringArrayP("node-selector", "", nil, "Add a NodeSelector to the operator Pod")
 
 	// save
 	cmd.Flags().Bool("save", false, "Save the install parameters into the default kamel configuration file (kamel-config.yaml)")
@@ -175,6 +176,7 @@ type installCmdOptions struct {
 	Properties              []string `mapstructure:"properties"`
 	TraitProfile            string   `mapstructure:"trait-profile"`
 	Tolerations             []string `mapstructure:"tolerations"`
+	NodeSelectors           []string `mapstructure:"node-selectors"`
 	HTTPProxySecret         string   `mapstructure:"http-proxy-secret"`
 
 	registry         v1.IntegrationPlatformRegistrySpec
@@ -221,7 +223,7 @@ func (o *installCmdOptions) install(cobraCmd *cobra.Command, _ []string) error {
 		if installViaOLM {
 			fmt.Fprintln(cobraCmd.OutOrStdout(), "OLM is available in the cluster")
 			var installed bool
-			if installed, err = olm.Install(o.Context, olmClient, o.Namespace, o.Global, o.olmOptions, collection, o.Tolerations); err != nil {
+			if installed, err = olm.Install(o.Context, olmClient, o.Namespace, o.Global, o.olmOptions, collection, o.Tolerations, o.NodeSelectors); err != nil {
 				return err
 			}
 			if !installed {
@@ -272,7 +274,8 @@ func (o *installCmdOptions) install(cobraCmd *cobra.Command, _ []string) error {
 					Enabled: o.Monitoring,
 					Port:    o.MonitoringPort,
 				},
-				Tolerations: o.Tolerations,
+				Tolerations:   o.Tolerations,
+				NodeSelectors: o.NodeSelectors,
 			}
 			err = install.OperatorOrCollect(o.Context, c, cfg, collection, o.Force)
 			if err != nil {
diff --git a/pkg/cmd/local_build.go b/pkg/cmd/local_build.go
index 2e4219417..b9606ddfc 100644
--- a/pkg/cmd/local_build.go
+++ b/pkg/cmd/local_build.go
@@ -20,9 +20,10 @@ package cmd
 import (
 	"fmt"
 
-	"github.com/apache/camel-k/pkg/util"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
+
+	"github.com/apache/camel-k/pkg/util"
 )
 
 func newCmdLocalBuild(rootCmdOptions *RootCmdOptions) (*cobra.Command, *localBuildCmdOptions) {
@@ -105,7 +106,7 @@ func (command *localBuildCmdOptions) validate(args []string) error {
 	// Validate properties file.
 	err = validateFiles(command.PropertyFiles)
 	if err != nil {
-		return nil
+		return err
 	}
 
 	// ContainerRegistry should only be specified when building the base image.
@@ -163,8 +164,7 @@ func (command *localBuildCmdOptions) init(args []string) error {
 }
 
 func (command *localBuildCmdOptions) run(cmd *cobra.Command, args []string) error {
-	dependenciesList := []string{}
-	propertyFilesList := []string{}
+	var dependenciesList, propertyFilesList []string
 	routeFiles := args
 	if !command.BaseImage {
 		// Fetch dependencies.
@@ -173,7 +173,7 @@ func (command *localBuildCmdOptions) run(cmd *cobra.Command, args []string) erro
 			return err
 		}
 
-		propertyFiles := []string{}
+		var propertyFiles []string
 		if !command.DependenciesOnly {
 			// Manage integration properties which may come from files or CLI.
 			propertyFiles, err = updateIntegrationProperties(command.Properties, command.PropertyFiles, false)
diff --git a/pkg/cmd/local_run.go b/pkg/cmd/local_run.go
index 3e51d3573..db8c4b490 100644
--- a/pkg/cmd/local_run.go
+++ b/pkg/cmd/local_run.go
@@ -20,9 +20,10 @@ package cmd
 import (
 	"fmt"
 
-	"github.com/apache/camel-k/pkg/util"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
+
+	"github.com/apache/camel-k/pkg/util"
 )
 
 func newCmdLocalRun(rootCmdOptions *RootCmdOptions) (*cobra.Command, *localRunCmdOptions) {
@@ -146,7 +147,7 @@ func (command *localRunCmdOptions) run(cmd *cobra.Command, args []string) error
 
 	hasIntegrationDir := command.IntegrationDirectory != ""
 
-	dependencies := []string{}
+	var dependencies []string
 	if hasIntegrationDir {
 		localBuildDependencies, err := getLocalBuildDependencies(command.IntegrationDirectory)
 		if err != nil {
@@ -201,13 +202,13 @@ func (command *localRunCmdOptions) run(cmd *cobra.Command, args []string) error
 			return err
 		}
 	} else {
-		propertieDir := util.GetLocalPropertiesDir()
+		propertiesDir := util.GetLocalPropertiesDir()
 		if hasIntegrationDir {
-			propertieDir = getCustomPropertiesDir(command.IntegrationDirectory)
+			propertiesDir = getCustomPropertiesDir(command.IntegrationDirectory)
 		}
 
 		// Run integration locally.
-		err := RunLocalIntegrationRunCommand(command.Context, propertyFiles, dependencies, routes, propertieDir, cmd.OutOrStdout(), cmd.ErrOrStderr())
+		err := RunLocalIntegrationRunCommand(command.Context, propertyFiles, dependencies, routes, propertiesDir, cmd.OutOrStdout(), cmd.ErrOrStderr())
 		if err != nil {
 			return err
 		}
diff --git a/pkg/cmd/util_dependencies.go b/pkg/cmd/util_dependencies.go
index 0dbbcb19b..be0dfb7e3 100644
--- a/pkg/cmd/util_dependencies.go
+++ b/pkg/cmd/util_dependencies.go
@@ -311,7 +311,7 @@ func validateFiles(args []string) error {
 	for _, arg := range args {
 		err := validateFile(arg)
 		if err != nil {
-			return nil
+			return err
 		}
 	}
 
@@ -354,7 +354,7 @@ func validateIntegrationFiles(args []string) error {
 	// Validate integration files.
 	err := validateFiles(args)
 	if err != nil {
-		return nil
+		return err
 	}
 
 	return nil
@@ -388,7 +388,10 @@ func updateIntegrationProperties(properties []string, propertyFiles []string, ha
 	var relocatedPropertyFiles []string
 	for _, propertyFile := range propertyFiles {
 		relocatedPropertyFile := path.Join(util.GetLocalPropertiesDir(), path.Base(propertyFile))
-		util.CopyFile(propertyFile, relocatedPropertyFile)
+		_, err = util.CopyFile(propertyFile, relocatedPropertyFile)
+		if err != nil {
+			return nil, err
+		}
 		relocatedPropertyFiles = append(relocatedPropertyFiles, relocatedPropertyFile)
 	}
 
@@ -424,7 +427,10 @@ func updateIntegrationDependencies(dependencies []string) error {
 		} else {
 			targetPath = path.Join(util.GetLocalDependenciesDir(), path.Base(dependency))
 		}
-		util.CopyFile(dependency, targetPath)
+		_, err = util.CopyFile(dependency, targetPath)
+		if err != nil {
+			return err
+		}
 	}
 
 	return nil
@@ -437,7 +443,10 @@ func updateIntegrationRoutes(routes []string) error {
 	}
 
 	for _, route := range routes {
-		util.CopyFile(route, path.Join(util.GetLocalRoutesDir(), path.Base(route)))
+		_, err = util.CopyFile(route, path.Join(util.GetLocalRoutesDir(), path.Base(route)))
+		if err != nil {
+			return err
+		}
 	}
 
 	return nil
diff --git a/pkg/install/operator.go b/pkg/install/operator.go
index 1fc6cf7c9..50d4e5b35 100644
--- a/pkg/install/operator.go
+++ b/pkg/install/operator.go
@@ -55,6 +55,7 @@ type OperatorConfiguration struct {
 	Health                OperatorHealthConfiguration
 	Monitoring            OperatorMonitoringConfiguration
 	Tolerations           []string
+	NodeSelectors         []string
 }
 
 // OperatorHealthConfiguration --
@@ -99,6 +100,18 @@ func OperatorOrCollect(ctx context.Context, c client.Client, cfg OperatorConfigu
 			}
 		}
 
+		if cfg.NodeSelectors != nil {
+			if d, ok := o.(*appsv1.Deployment); ok {
+				if d.Labels["camel.apache.org/component"] == "operator" {
+					nodeSelector, err := kubernetes.NewNodeSelectors(cfg.NodeSelectors)
+					if err != nil {
+						fmt.Println("Warning: could not parse the configured node selectors!")
+					}
+					d.Spec.Template.Spec.NodeSelector = nodeSelector
+				}
+			}
+		}
+
 		if d, ok := o.(*appsv1.Deployment); ok {
 			if d.Labels["camel.apache.org/component"] == "operator" {
 				// Metrics endpoint port
diff --git a/pkg/util/kubernetes/factory.go b/pkg/util/kubernetes/factory.go
index a85c4723c..956627c1e 100644
--- a/pkg/util/kubernetes/factory.go
+++ b/pkg/util/kubernetes/factory.go
@@ -26,8 +26,9 @@ import (
 )
 
 var validTaintRegexp = regexp.MustCompile(`^([\w\/_\-\.]+)(=)?([\w_\-\.]+)?:(NoSchedule|NoExecute|PreferNoSchedule):?(\d*)?$`)
+var validNodeSelectorRegexp = regexp.MustCompile(`^([\w\/_\-\.]+)=([\w_\-\.]+)$`)
 
-// GetTolerations build an array of Tolerations from an array of string
+// NewTolerations build an array of Tolerations from an array of string
 func NewTolerations(taints []string) ([]corev1.Toleration, error) {
 	tolerations := make([]corev1.Toleration, 0)
 	for _, t := range taints {
@@ -60,3 +61,17 @@ func NewTolerations(taints []string) ([]corev1.Toleration, error) {
 
 	return tolerations, nil
 }
+
+// NewNodeSelectors build a map of NodeSelectors from an array of string
+func NewNodeSelectors(nsArray []string) (map[string]string, error) {
+	nodeSelectors := make(map[string]string)
+	for _, ns := range nsArray {
+		if !validNodeSelectorRegexp.MatchString(ns) {
+			return nil, fmt.Errorf("could not match node selector %v", ns)
+		}
+		// Parse the regexp groups
+		groups := validNodeSelectorRegexp.FindStringSubmatch(ns)
+		nodeSelectors[groups[1]] = groups[2]
+	}
+	return nodeSelectors, nil
+}
diff --git a/pkg/util/olm/operator.go b/pkg/util/olm/operator.go
index 896b24194..bb9ce2af8 100644
--- a/pkg/util/olm/operator.go
+++ b/pkg/util/olm/operator.go
@@ -138,7 +138,7 @@ func HasPermissionToInstall(ctx context.Context, client client.Client, namespace
 }
 
 // Install creates a subscription for the OLM package
-func Install(ctx context.Context, client client.Client, namespace string, global bool, options Options, collection *kubernetes.Collection, tolerations []string) (bool, error) {
+func Install(ctx context.Context, client client.Client, namespace string, global bool, options Options, collection *kubernetes.Collection, tolerations []string, nodeSelectors []string) (bool, error) {
 	options = fillDefaults(options)
 	if installed, err := IsOperatorInstalled(ctx, client, namespace, global, options); err != nil {
 		return false, err
@@ -171,6 +171,10 @@ func Install(ctx context.Context, client client.Client, namespace string, global
 	if err != nil {
 		return false, errors.Wrap(err, fmt.Sprintf("could not set tolerations"))
 	}
+	err = maybeSetNodeSelectors(&sub, nodeSelectors)
+	if err != nil {
+		return false, errors.Wrap(err, fmt.Sprintf("could not set node selectors"))
+	}
 
 	if collection != nil {
 		collection.Add(&sub)
@@ -216,6 +220,17 @@ func maybeSetTolerations(sub *operatorsv1alpha1.Subscription, tolArray []string)
 	return nil
 }
 
+func maybeSetNodeSelectors(sub *operatorsv1alpha1.Subscription, nsArray []string) error {
+	if nsArray != nil {
+		nodeSelectors, err := kubernetes.NewNodeSelectors(nsArray)
+		if err != nil {
+			return err
+		}
+		sub.Spec.Config.NodeSelector = nodeSelectors
+	}
+	return nil
+}
+
 // Uninstall removes CSV and subscription from the namespace
 func Uninstall(ctx context.Context, client client.Client, namespace string, global bool, options Options) error {
 	sub, err := findSubscription(ctx, client, namespace, global, options)


Test patch:
diff --git a/pkg/util/kubernetes/factory_test.go b/pkg/util/kubernetes/factory_test.go
new file mode 100644
index 000000000..1bda9f343
--- /dev/null
+++ b/pkg/util/kubernetes/factory_test.go
@@ -0,0 +1,136 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package kubernetes
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	v1 "k8s.io/api/core/v1"
+)
+
+func TestValidTolerations(t *testing.T) {
+	validTolerations := [][]string{
+		{"key=value:NoSchedule"},
+		{"key=value:NoExecute"},
+		{"key=value:PreferNoSchedule"},
+		{"key=value:NoSchedule:120"},
+		{"key=value:NoExecute:120"},
+		{"key=value:PreferNoSchedule:120"},
+		{"existKey:NoSchedule"},
+		{"existKey:NoExecute"},
+		{"existKey:PreferNoSchedule"},
+		{"existKey:NoSchedule:120"},
+		{"existKey:NoExecute:120"},
+		{"existKey:PreferNoSchedule:120"},
+	}
+	for _, vd := range validTolerations {
+		_, err := NewTolerations(vd)
+		assert.Nil(t, err)
+	}
+}
+
+func TestInvalidTolerations(t *testing.T) {
+	validTolerations := [][]string{
+		{"key-NoSchedule"},
+		{"key=value:Something"},
+		{"key@wrong=value:PreferNoSchedule"},
+		{"key=value%wrong:NoSchedule:120"},
+		{"existKey"},
+		{"existKey:"},
+		{"existKey:Something"},
+		{"existKey:PreferNoSchedule:something"},
+	}
+	for _, vd := range validTolerations {
+		_, err := NewTolerations(vd)
+		assert.NotNil(t, err)
+	}
+}
+
+func TestValueTolerations(t *testing.T) {
+	tolerations := []string{
+		"key=value:NoSchedule",
+		"key=value:NoExecute:120",
+		"existKey:PreferNoSchedule",
+		"existKey:NoSchedule:120",
+	}
+	toleration, err := NewTolerations(tolerations)
+	assert.Nil(t, err)
+	assert.Equal(t, 4, len(toleration))
+
+	assert.Equal(t, "key", toleration[0].Key)
+	assert.Equal(t, v1.TolerationOpEqual, toleration[0].Operator)
+	assert.Equal(t, "value", toleration[0].Value)
+	assert.Equal(t, v1.TaintEffectNoSchedule, toleration[0].Effect)
+
+	assert.Equal(t, "key", toleration[1].Key)
+	assert.Equal(t, v1.TolerationOpEqual, toleration[1].Operator)
+	assert.Equal(t, "value", toleration[1].Value)
+	assert.Equal(t, v1.TaintEffectNoExecute, toleration[1].Effect)
+	assert.Equal(t, int64(120), *toleration[1].TolerationSeconds)
+
+	assert.Equal(t, "existKey", toleration[2].Key)
+	assert.Equal(t, v1.TolerationOpExists, toleration[2].Operator)
+	assert.Equal(t, "", toleration[2].Value)
+	assert.Equal(t, v1.TaintEffectPreferNoSchedule, toleration[2].Effect)
+
+	assert.Equal(t, "existKey", toleration[3].Key)
+	assert.Equal(t, v1.TolerationOpExists, toleration[3].Operator)
+	assert.Equal(t, "", toleration[3].Value)
+	assert.Equal(t, v1.TaintEffectNoSchedule, toleration[3].Effect)
+	assert.Equal(t, int64(120), *toleration[3].TolerationSeconds)
+}
+
+func TestValidNodeSelectors(t *testing.T) {
+	validNodeSelectors := [][]string{
+		{"key1=value"},
+		{"kubernetes.io/hostname=worker0"},
+		{"disktype=ssd"},
+		{"key=path-to-value"},
+		{"keyNum=123"},
+	}
+	for _, vds := range validNodeSelectors {
+		_, err := NewNodeSelectors(vds)
+		assert.Nil(t, err)
+	}
+}
+
+func TestInvalidNodeSelectors(t *testing.T) {
+	validNodeSelectors := [][]string{
+		{"key1"},
+		{"kubernetes.io@hostname=worker0"},
+		{"key=path/to/value"},
+	}
+	for _, vds := range validNodeSelectors {
+		_, err := NewNodeSelectors(vds)
+		assert.NotNil(t, err)
+	}
+}
+
+func TestValueNodeSelectors(t *testing.T) {
+	nodeSelectorsArray := []string{
+		"key=value",
+		"kubernetes.io/hostname=worker0",
+	}
+	nodeSelectors, err := NewNodeSelectors(nodeSelectorsArray)
+	assert.Nil(t, err)
+	assert.Equal(t, 2, len(nodeSelectors))
+
+	assert.Equal(t, "value", nodeSelectors["key"])
+	assert.Equal(t, "worker0", nodeSelectors["kubernetes.io/hostname"])
+}