[v1.6.x] {ansible,helm}-operator run: add component config flag --config (#4780)

Co-authored-by: Eric Stroczynski <[email protected]>

Author: openshift-cherrypick-robot

changelog/fragments/add-componentconfig-flag.yaml

@@ -0,0 +1,28 @@
+entries:
+  - description: >
+      For Ansible- and Helm-based operators, add the `--config` flag, which was mistakenly
+      not added to either ansible-/helm-operator binary when file support was originally added.
+    kind: bugfix
+    migration:
+      header: Add the manager config patch to config/default/kustomization.yaml
+      body: >
+        The scaffolded `--config` flag was not added to either ansible-/helm-operator binary
+        when [config file](https://master.book.kubebuilder.io/component-config-tutorial/tutorial.html)
+        support was originally added, so does not currently work.
+        The `--config` flag supports configuration of both binaries by file;
+        this method of configuration only applies to the underlying
+        [controller manager](https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/manager#Manager),
+        not the operator as a whole. To optionally configure the operator's Deployment
+        with a config file, make the following update to config/default/kustomization.yaml:
+
+        ```diff
+        # If you want your controller-manager to expose the /metrics
+        # endpoint w/o any authn/z, please comment the following line.
+        - manager_auth_proxy_patch.yaml
+        +
+        +# Mount the controller config file for loading manager configurations
+        +# through a ComponentConfig type
+        +- manager_config_patch.yaml
+        ```
+
+        This feature is opt-in: flags can be used as-is or to override config file values.

internal/ansible/flags/flag.go

@@ -19,6 +19,8 @@ import (
 	"time"
 
 	"github.com/spf13/pflag"
+	"k8s.io/client-go/tools/leaderelection/resourcelock"
+	"sigs.k8s.io/controller-runtime/pkg/manager"
 )
 
 // Flags - Options to be used by an ansible operator
@@ -37,18 +39,26 @@ type Flags struct {
 	LeaderElectionNamespace string
 	GracefulShutdownTimeout time.Duration
 	AnsibleArgs             string
+
+	// Path to a controller-runtime componentconfig file.
+	// If this is empty, use default values.
+	ManagerConfigPath string
+
+	// If not nil, used to deduce which flags were set in the CLI.
+	flagSet *pflag.FlagSet
 }
 
-const AnsibleRolesPathEnvVar = "ANSIBLE_ROLES_PATH"
-const AnsibleCollectionsPathEnvVar = "ANSIBLE_COLLECTIONS_PATH"
+const (
+	AnsibleRolesPathEnvVar       = "ANSIBLE_ROLES_PATH"
+	AnsibleCollectionsPathEnvVar = "ANSIBLE_COLLECTIONS_PATH"
+)
 
 // AddTo - Add the ansible operator flags to the the flagset
 func (f *Flags) AddTo(flagSet *pflag.FlagSet) {
-	flagSet.DurationVar(&f.ReconcilePeriod,
-		"reconcile-period",
-		time.Minute,
-		"Default reconcile period for controllers",
-	)
+	// Store flagset internally to be used for lookups later.
+	f.flagSet = flagSet
+
+	// Ansible flags.
 	flagSet.StringVar(&f.WatchesFile,
 		"watches-file",
 		"./watches.yaml",
@@ -59,11 +69,6 @@ func (f *Flags) AddTo(flagSet *pflag.FlagSet) {
 		true,
 		"The ansible operator will inject owner references unless this flag is false",
 	)
-	flagSet.IntVar(&f.MaxConcurrentReconciles,
-		"max-concurrent-reconciles",
-		runtime.NumCPU(),
-		"Maximum number of concurrent reconciles for controllers. Overridden by environment variable.",
-	)
 	flagSet.IntVar(&f.AnsibleVerbosity,
 		"ansible-verbosity",
 		2,
@@ -77,9 +82,36 @@ func (f *Flags) AddTo(flagSet *pflag.FlagSet) {
 	flagSet.StringVar(&f.AnsibleCollectionsPath,
 		"ansible-collections-path",
 		"",
-		"Path to installed Ansible Collections. If set, collections should be located in {{value}}/ansible_collections/. If unset, collections are assumed to be in ~/.ansible/collections or /usr/share/ansible/collections.",
+		"Path to installed Ansible Collections. If set, collections should be located in {{value}}/ansible_collections/. "+
+			"If unset, collections are assumed to be in ~/.ansible/collections or /usr/share/ansible/collections.",
+	)
+	flagSet.StringVar(&f.AnsibleArgs,
+		"ansible-args",
+		"",
+		"Ansible args. Allows user to specify arbitrary arguments for ansible-based operators.",
+	)
+
+	// Controller flags.
+	flagSet.DurationVar(&f.ReconcilePeriod,
+		"reconcile-period",
+		time.Minute,
+		"Default reconcile period for controllers",
+	)
+	flagSet.IntVar(&f.MaxConcurrentReconciles,
+		"max-concurrent-reconciles",
+		runtime.NumCPU(),
+		"Maximum number of concurrent reconciles for controllers. Overridden by environment variable.",
+	)
+
+	// Controller manager flags.
+	flagSet.StringVar(&f.ManagerConfigPath,
+		"config",
+		"",
+		"The controller will load its initial configuration from this file. "+
+			"Omit this flag to use the default configuration values. "+
+			"Command-line flags override configuration from this file.",
 	)
-	// todo:remove it for 2.0.0
+	// TODO(2.0.0): remove
 	flagSet.StringVar(&f.MetricsBindAddress,
 		"metrics-addr",
 		":8080",
@@ -91,15 +123,14 @@ func (f *Flags) AddTo(flagSet *pflag.FlagSet) {
 		":8080",
 		"The address the metric endpoint binds to",
 	)
-	// todo: for Go/Helm the port used is: 8081
+	// TODO(2.0.0): for Go/Helm the port used is: 8081
 	// update it to keep the project aligned to the other
-	// types for 2.0
 	flagSet.StringVar(&f.ProbeAddr,
 		"health-probe-bind-address",
 		":6789",
 		"The address the probe endpoint binds to.",
 	)
-	// todo:remove it for 2.0.0
+	// TODO(2.0.0): remove
 	flagSet.BoolVar(&f.LeaderElection,
 		"enable-leader-election",
 		false,
@@ -131,9 +162,43 @@ func (f *Flags) AddTo(flagSet *pflag.FlagSet) {
 		"The amount of time that will be spent waiting"+
 			" for runners to gracefully exit.",
 	)
-	flagSet.StringVar(&f.AnsibleArgs,
-		"ansible-args",
-		"",
-		"Ansible args. Allows user to specify arbitrary arguments for ansible-based operators.",
-	)
+}
+
+// ToManagerOptions uses the flag set in f to configure options.
+// Values of options take precedence over flag defaults,
+// as values are assume to have been explicitly set.
+func (f *Flags) ToManagerOptions(options manager.Options) manager.Options {
+	// Alias FlagSet.Changed so options are still updated when fields are empty.
+	changed := func(flagName string) bool {
+		return f.flagSet.Changed(flagName)
+	}
+	if f.flagSet == nil {
+		changed = func(flagName string) bool { return false }
+	}
+
+	// TODO(2.0.0): remove metrics-addr
+	if changed("metrics-bind-address") || changed("metrics-addr") || options.MetricsBindAddress == "" {
+		options.MetricsBindAddress = f.MetricsBindAddress
+	}
+	if changed("health-probe-bind-address") || options.HealthProbeBindAddress == "" {
+		options.HealthProbeBindAddress = f.ProbeAddr
+	}
+	// TODO(2.0.0): remove enable-leader-election
+	if changed("leader-elect") || changed("enable-leader-election") || !options.LeaderElection {
+		options.LeaderElection = f.LeaderElection
+	}
+	if changed("leader-election-id") || options.LeaderElectionID == "" {
+		options.LeaderElectionID = f.LeaderElectionID
+	}
+	if changed("leader-election-namespace") || options.LeaderElectionNamespace == "" {
+		options.LeaderElectionNamespace = f.LeaderElectionNamespace
+	}
+	if options.LeaderElectionResourceLock == "" {
+		options.LeaderElectionResourceLock = resourcelock.ConfigMapsResourceLock
+	}
+	if changed("graceful-shutdown-timeout") || options.GracefulShutdownTimeout == nil {
+		options.GracefulShutdownTimeout = &f.GracefulShutdownTimeout
+	}
+
+	return options
 }

internal/ansible/flags/flag_test.go

@@ -0,0 +1,72 @@
+// Copyright 2021 The Operator-SDK Authors
+//
+// Licensed 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 flags_test
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"github.com/spf13/pflag"
+	"sigs.k8s.io/controller-runtime/pkg/manager"
+
+	"github.com/operator-framework/operator-sdk/internal/ansible/flags"
+)
+
+var _ = Describe("Flags", func() {
+	Describe("ToManagerOptions", func() {
+		var (
+			f       *flags.Flags
+			flagSet *pflag.FlagSet
+			options manager.Options
+		)
+		BeforeEach(func() {
+			f = &flags.Flags{}
+			flagSet = pflag.NewFlagSet("test", pflag.ExitOnError)
+			f.AddTo(flagSet)
+		})
+
+		When("the flag is set", func() {
+			It("uses the flag value when corresponding option value is empty", func() {
+				expOptionValue := ":5678"
+				options.MetricsBindAddress = ""
+				parseArgs(flagSet, "--metrics-bind-address", expOptionValue)
+				Expect(f.ToManagerOptions(options).MetricsBindAddress).To(Equal(expOptionValue))
+			})
+			It("uses the flag value when corresponding option value is not empty", func() {
+				expOptionValue := ":5678"
+				options.MetricsBindAddress = ":1234"
+				parseArgs(flagSet, "--metrics-bind-address", expOptionValue)
+				Expect(f.ToManagerOptions(options).MetricsBindAddress).To(Equal(expOptionValue))
+			})
+		})
+		When("the flag is not set", func() {
+			It("uses the default flag value when corresponding option value is empty", func() {
+				expOptionValue := ":8080"
+				options.MetricsBindAddress = ""
+				parseArgs(flagSet)
+				Expect(f.ToManagerOptions(options).MetricsBindAddress).To(Equal(expOptionValue))
+			})
+			It("uses the option value when corresponding option value is not empty", func() {
+				expOptionValue := ":1234"
+				options.MetricsBindAddress = expOptionValue
+				parseArgs(flagSet)
+				Expect(f.ToManagerOptions(options).MetricsBindAddress).To(Equal(expOptionValue))
+			})
+		})
+	})
+})
+
+func parseArgs(fs *pflag.FlagSet, extraArgs ...string) {
+	Expect(fs.Parse(extraArgs)).To(Succeed())
+}

internal/ansible/flags/suite_test.go

@@ -0,0 +1,27 @@
+// Copyright 2021 The Operator-SDK Authors
+//
+// Licensed 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 flags_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func TestFlags(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Flags Suite")
+}