Fixed binding of command line flags

Fixes #1128

Signed-off-by: Juraci Paixão Kröhling <[email protected]>

go.mod

@@ -16,6 +16,7 @@ require (
 	github.com/pkg/errors v0.8.1
 	github.com/sirupsen/logrus v1.4.2
 	github.com/spf13/cobra v0.0.5
+	github.com/spf13/pflag v1.0.5
 	github.com/spf13/viper v1.4.0
 	github.com/stretchr/testify v1.4.0
 	github.com/uber/jaeger-client-go v2.20.1+incompatible

pkg/cmd/generate/main.go

@@ -27,16 +27,13 @@ func NewGenerateCommand() *cobra.Command {
 		Long: `Generate YAML manifests from Jaeger CRD.
 
 Defaults to reading Jaeger CRD from standard input and writing the manifest file to standard output, override with --cr <filename> and --output <filename>.`,
-		RunE: generate,
+		PreRunE: start.BindFlags,
+		RunE:    generate,
 	}
 
+	start.AddFlags(cmd)
 	cmd.Flags().String("cr", "/dev/stdin", "Input Jaeger CRD")
-	viper.BindPFlag("cr", cmd.Flags().Lookup("cr"))
-
 	cmd.Flags().String("output", "/dev/stdout", "Where to print the generated YAML documents")
-	viper.BindPFlag("output", cmd.Flags().Lookup("output"))
-
-	start.AddFlags(cmd)
 
 	return cmd
 }

pkg/cmd/start/main.go

@@ -6,6 +6,7 @@ import (
 
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 	"github.com/spf13/viper"
 	"sigs.k8s.io/controller-runtime/pkg/manager/signals"
 
@@ -17,97 +18,62 @@ import (
 // command.
 func AddFlags(cmd *cobra.Command) {
 	cmd.Flags().String("log-level", "info", "The log-level for the operator. Possible values: trace, debug, info, warning, error, fatal, panic")
-	viper.BindPFlag("log-level", cmd.Flags().Lookup("log-level"))
-
 	cmd.Flags().String("jaeger-version", version.DefaultJaeger(), "Deprecated: the Jaeger version is now managed entirely by the operator. This option is currently no-op.")
-
 	cmd.Flags().String("jaeger-agent-image", "jaegertracing/jaeger-agent", "The Docker image for the Jaeger Agent")
-	viper.BindPFlag("jaeger-agent-image", cmd.Flags().Lookup("jaeger-agent-image"))
-
 	cmd.Flags().String("jaeger-query-image", "jaegertracing/jaeger-query", "The Docker image for the Jaeger Query")
-	viper.BindPFlag("jaeger-query-image", cmd.Flags().Lookup("jaeger-query-image"))
-
 	cmd.Flags().String("jaeger-collector-image", "jaegertracing/jaeger-collector", "The Docker image for the Jaeger Collector")
-	viper.BindPFlag("jaeger-collector-image", cmd.Flags().Lookup("jaeger-collector-image"))
-
 	cmd.Flags().String("jaeger-ingester-image", "jaegertracing/jaeger-ingester", "The Docker image for the Jaeger Ingester")
-	viper.BindPFlag("jaeger-ingester-image", cmd.Flags().Lookup("jaeger-ingester-image"))
-
 	cmd.Flags().String("jaeger-all-in-one-image", "jaegertracing/all-in-one", "The Docker image for the Jaeger all-in-one")
-	viper.BindPFlag("jaeger-all-in-one-image", cmd.Flags().Lookup("jaeger-all-in-one-image"))
-
 	cmd.Flags().String("jaeger-cassandra-schema-image", "jaegertracing/jaeger-cassandra-schema", "The Docker image for the Jaeger Cassandra Schema")
-	viper.BindPFlag("jaeger-cassandra-schema-image", cmd.Flags().Lookup("jaeger-cassandra-schema-image"))
-
 	cmd.Flags().String("jaeger-spark-dependencies-image", "jaegertracing/spark-dependencies", "The Docker image for the Spark Dependencies Job")
-	viper.BindPFlag("jaeger-spark-dependencies-image", cmd.Flags().Lookup("jaeger-spark-dependencies-image"))
-
 	cmd.Flags().String("jaeger-es-index-cleaner-image", "jaegertracing/jaeger-es-index-cleaner", "The Docker image for the Jaeger Elasticsearch Index Cleaner")
-	viper.BindPFlag("jaeger-es-index-cleaner-image", cmd.Flags().Lookup("jaeger-es-index-cleaner-image"))
-
 	cmd.Flags().String("jaeger-es-rollover-image", "jaegertracing/jaeger-es-rollover", "The Docker image for the Jaeger Elasticsearch Rollover")
-	viper.BindPFlag("jaeger-es-rollover-image", cmd.Flags().Lookup("jaeger-es-rollover-image"))
-
 	cmd.Flags().String("openshift-oauth-proxy-image", "openshift/oauth-proxy:latest", "The Docker image location definition for the OpenShift OAuth Proxy")
-	viper.BindPFlag("openshift-oauth-proxy-image", cmd.Flags().Lookup("openshift-oauth-proxy-image"))
-
 	cmd.Flags().String("openshift-oauth-proxy-imagestream-ns", "", "The namespace for the OpenShift OAuth Proxy imagestream")
-	viper.BindPFlag("openshift-oauth-proxy-imagestream-ns", cmd.Flags().Lookup("openshift-oauth-proxy-imagestream-ns"))
-
 	cmd.Flags().String("openshift-oauth-proxy-imagestream-name", "", "The name for the OpenShift OAuth Proxy imagestream")
-	viper.BindPFlag("openshift-oauth-proxy-imagestream-name", cmd.Flags().Lookup("openshift-oauth-proxy-imagestream-name"))
-
 	cmd.Flags().String("platform", "auto-detect", "The target platform the operator will run. Possible values: 'kubernetes', 'openshift', 'auto-detect'")
-	viper.BindPFlag("platform", cmd.Flags().Lookup("platform"))
-
 	cmd.Flags().String("es-provision", "auto", "Whether to auto-provision an Elasticsearch cluster for suitable Jaeger instances. Possible values: 'yes', 'no', 'auto'. When set to 'auto' and the API name 'logging.openshift.io' is available, auto-provisioning is enabled.")
-	viper.BindPFlag("es-provision", cmd.Flags().Lookup("es-provision"))
-
 	cmd.Flags().String("kafka-provision", "auto", "Whether to auto-provision a Kafka cluster for suitable Jaeger instances. Possible values: 'yes', 'no', 'auto'. When set to 'auto' and the API name 'kafka.strimzi.io' is available, auto-provisioning is enabled.")
-	viper.BindPFlag("kafka-provision", cmd.Flags().Lookup("kafka-provision"))
+	cmd.Flags().Bool("kafka-provisioning-minimal", false, "(unsupported) Whether to provision Kafka clusters with minimal requirements, suitable for demos and tests.")
 
 	docURL := fmt.Sprintf("https://www.jaegertracing.io/docs/%s", version.DefaultJaegerMajorMinor())
 	cmd.Flags().String("documentation-url", docURL, "The URL for the 'Documentation' menu item")
-	viper.BindPFlag("documentation-url", cmd.Flags().Lookup("documentation-url"))
-
-	cmd.Flags().Bool("kafka-provisioning-minimal", false, "(unsupported) Whether to provision Kafka clusters with minimal requirements, suitable for demos and tests.")
-	viper.BindPFlag("kafka-provisioning-minimal", cmd.Flags().Lookup("kafka-provisioning-minimal"))
 }
 
 // NewStartCommand starts the Jaeger Operator
 func NewStartCommand() *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "start",
-		Short: "Starts a new Jaeger Operator",
-		Long:  "Starts a new Jaeger Operator",
-		Run: func(cmd *cobra.Command, args []string) {
-			start(cmd, args)
-		},
+		Use:     "start",
+		Short:   "Starts a new Jaeger Operator",
+		Long:    "Starts a new Jaeger Operator",
+		PreRunE: bindFlags,
+		RunE:    start,
 	}
 
 	AddFlags(cmd)
-
-	// Operator specific flags here. Any flags affecting manifest generation should be added to AddFlags instead
 	cmd.Flags().String("metrics-host", "0.0.0.0", "The host to bind the metrics port")
-	viper.BindPFlag("metrics-host", cmd.Flags().Lookup("metrics-host"))
-
 	cmd.Flags().Int32("metrics-port", 8383, "The metrics port")
-	viper.BindPFlag("metrics-port", cmd.Flags().Lookup("metrics-port"))
-
 	cmd.Flags().Int32("cr-metrics-port", 8686, "The metrics port for Operator and/or Custom Resource based metrics")
-	viper.BindPFlag("cr-metrics-port", cmd.Flags().Lookup("cr-metrics-port"))
-
 	cmd.Flags().String("jaeger-agent-hostport", "localhost:6831", "The location for the Jaeger Agent")
-	viper.BindPFlag("jaeger-agent-hostport", cmd.Flags().Lookup("jaeger-agent-hostport"))
-
 	cmd.Flags().Bool("tracing-enabled", false, "Whether the Operator should report its own spans to a Jaeger instance")
-	viper.BindPFlag("tracing-enabled", cmd.Flags().Lookup("tracing-enabled"))
 
 	return cmd
 }
 
-func start(cmd *cobra.Command, args []string) {
+func start(cmd *cobra.Command, args []string) error {
 	mgr := bootstrap(context.Background())
+
 	// Start the Cmd
-	log.Fatal(mgr.Start(signals.SetupSignalHandler()))
+	err := mgr.Start(signals.SetupSignalHandler())
+	log.Fatal(err)
+	return err
+}
+
+// BindFlags iterates over all registered flags in the command and binds them to viper
+func BindFlags(cmd *cobra.Command, args []string) error {
+	cmd.Flags().VisitAll(func(flag *pflag.Flag) {
+		viper.BindPFlag(flag.Name, flag)
+	})
+
+	return nil
 }

pkg/service/collector.go

@@ -7,9 +7,10 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
+	"github.com/spf13/viper"
+
 	v1 "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1"
 	"github.com/jaegertracing/jaeger-operator/pkg/util"
-	"github.com/spf13/viper"
 )
 
 // NewCollectorServices returns a new Kubernetes service for Jaeger Collector backed by the pods matching the selector