ansible/helm : aling flags and deprecated the old ones (#4654)

**Description of the change:** - Deprecated the flags which were removed in the Golang project in order to keep it better aligned with K8S - Add the new flags - Ensure that that metrics port is passed to the proxy. **Motivation for the change:** - Reduce the complexities for we address kubernetes-sigs/kubebuilder#2015 - Fix docs that were not updated. - Keep Ansible/Helm/Go aligned (Align Helm/Ansible plugins with the changes made for Golang ( go/v3 )) - Closes: Ansible/Helm providing the same flags as Go #4627
operator-framework · Mar 18, 2021 · 348e7de · 348e7de
1 parent 7f6a53c
commit 348e7de
Show file tree

Hide file tree

Showing 31 changed files with 272 additions and 99 deletions.
diff --git a/changelog/fragments/ansible-helm-flags.yaml b/changelog/fragments/ansible-helm-flags.yaml
@@ -0,0 +1,109 @@
+# entries is a list of entries to include in
+# release notes and/or the migration guide
+entries:
+  - description: >
+      (ansible/v1, helm/v1) The flags `--enable-leader-election` and `--metrics-addr` were deprecated in favor of `--leader-elect` and `--metrics-bind-address`, respectively, to follow upstream conventions.
+
+    # kind is one of:
+    # - addition
+    # - change
+    # - deprecation
+    # - removal
+    # - bugfix
+    kind: "deprecation"
+
+    # Is this a breaking change?
+    breaking: false
+
+    # NOTE: ONLY USE `pull_request_override` WHEN ADDING THIS
+    # FILE FOR A PREVIOUSLY MERGED PULL_REQUEST!
+    #
+    # The generator auto-detects the PR number from the commit
+    # message in which this file was originally added.
+    #
+    # What is the pull request number (without the "#")?
+    # pull_request_override: 0
+
+
+    # Migration can be defined to automatically add a section to
+    # the migration guide. This is required for breaking changes.
+    migration:
+      header: (helm/v1) Replace deprecated leader election and metrics address flags
+      body: >
+        Replace deprecated flags `--enable-leader-election` and `--metrics-addr` with `--leader-elect` and `--metrics-bind-address`, respectively.
+  - description: >
+      (helm/v1) Explicitly set `--health-probe-bind-address` in the manager's auth proxy patch.
+
+    # kind is one of:
+    # - addition
+    # - change
+    # - deprecation
+    # - removal
+    # - bugfix
+    kind: "change"
+
+    # Is this a breaking change?
+    breaking: false
+
+    # NOTE: ONLY USE `pull_request_override` WHEN ADDING THIS
+    # FILE FOR A PREVIOUSLY MERGED PULL_REQUEST!
+    #
+    # The generator auto-detects the PR number from the commit
+    # message in which this file was originally added.
+    #
+    # What is the pull request number (without the "#")?
+    # pull_request_override: 0
+
+
+    # Migration can be defined to automatically add a section to
+    # the migration guide. This is required for breaking changes.
+    migration:
+      header: (helm/v1) Explicitly set `--health-probe-bind-address` in the manager's auth proxy patch.
+      body: >
+        Add the arg `--health-probe-bind-address=:8081` to the `config/default/manager_auth_proxy_patch.yaml`:
+        ```yaml
+        - "--health-probe-bind-address=:8081"
+        - "--metrics-bind-address=127.0.0.1:8080"
+        - "--leader-elect"
+        ```
+  - description: >
+      (ansible/v1) Explicitly set `--health-probe-bind-address` in the manager's auth proxy patch.
+
+    # kind is one of:
+    # - addition
+    # - change
+    # - deprecation
+    # - removal
+    # - bugfix
+    kind: "change"
+
+    # Is this a breaking change?
+    breaking: false
+
+    # NOTE: ONLY USE `pull_request_override` WHEN ADDING THIS
+    # FILE FOR A PREVIOUSLY MERGED PULL_REQUEST!
+    #
+    # The generator auto-detects the PR number from the commit
+    # message in which this file was originally added.
+    #
+    # What is the pull request number (without the "#")?
+    # pull_request_override: 0
+
+
+    # Migration can be defined to automatically add a section to
+    # the migration guide. This is required for breaking changes.
+    migration:
+      header: (ansible/v1) Explicitly set `--health-probe-bind-address` in the manager's auth proxy patch.
+      body: >
+        Add the arg `--health-probe-bind-address=:8081` to the `config/default/manager_auth_proxy_patch.yaml`:
+        ```yaml
+        - "--health-probe-bind-address=:6789"
+        - "--metrics-bind-address=127.0.0.1:6789"
+        - "--leader-elect"
+        ```
+
+
+
+
+
+
diff --git a/hack/generate/samples/internal/ansible/advanced_molecule.go b/hack/generate/samples/internal/ansible/advanced_molecule.go
@@ -178,7 +178,7 @@ func (ma *AdvancedMolecule) updateConfig() {
         - "--ansible-args='--vault-password-file /opt/ansible/pwd.yml'"`
 	err = kbtestutils.InsertCode(
 		filepath.Join(ma.ctx.Dir, "config", "default", "manager_auth_proxy_patch.yaml"),
-		"- \"--enable-leader-election\"",
+		"- \"--leader-elect\"",
 		managerAuthArgs)
 	pkg.CheckError("adding vaulting args to the proxy auth", err)
 

diff --git a/internal/ansible/flags/flag.go b/internal/ansible/flags/flag.go
@@ -26,12 +26,12 @@ type Flags struct {
 	ReconcilePeriod         time.Duration
 	WatchesFile             string
 	InjectOwnerRef          bool
-	EnableLeaderElection    bool
+	LeaderElection          bool
 	MaxConcurrentReconciles int
 	AnsibleVerbosity        int
 	AnsibleRolesPath        string
 	AnsibleCollectionsPath  string
-	MetricsAddress          string
+	MetricsBindAddress      string
 	ProbeAddr               string
 	LeaderElectionID        string
 	LeaderElectionNamespace string
@@ -79,11 +79,18 @@ func (f *Flags) AddTo(flagSet *pflag.FlagSet) {
 		"",
 		"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.MetricsAddress,
+	// todo:remove it for 2.0.0
+	flagSet.StringVar(&f.MetricsBindAddress,
 		"metrics-addr",
 		":8080",
 		"The address the metric endpoint binds to",
 	)
+	_ = flagSet.MarkDeprecated("metrics-addr", "use --metrics-bind-address instead")
+	flagSet.StringVar(&f.MetricsBindAddress,
+		"metrics-bind-address",
+		":8080",
+		"The address the metric endpoint binds to",
+	)
 	// todo: for Go/Helm the port used is: 8081
 	// update it to keep the project aligned to the other
 	// types for 2.0
@@ -92,12 +99,20 @@ func (f *Flags) AddTo(flagSet *pflag.FlagSet) {
 		":6789",
 		"The address the probe endpoint binds to.",
 	)
-	flagSet.BoolVar(&f.EnableLeaderElection,
+	// todo:remove it for 2.0.0
+	flagSet.BoolVar(&f.LeaderElection,
 		"enable-leader-election",
 		false,
 		"Enable leader election for controller manager. Enabling this will"+
 			" ensure there is only one active controller manager.",
 	)
+	_ = flagSet.MarkDeprecated("enable-leader-election", "use --leader-elect instead")
+	flagSet.BoolVar(&f.LeaderElection,
+		"leader-elect",
+		false,
+		"Enable leader election for controller manager. Enabling this will"+
+			" ensure there is only one active controller manager.",
+	)
 	flagSet.StringVar(&f.LeaderElectionID,
 		"leader-election-id",
 		"",

diff --git a/internal/cmd/ansible-operator/run/cmd.go b/internal/cmd/ansible-operator/run/cmd.go
@@ -15,6 +15,7 @@
 package run
 
 import (
+	"errors"
 	"flag"
 	"fmt"
 	"os"
@@ -99,12 +100,23 @@ func run(cmd *cobra.Command, f *flags.Flags) {
 		}
 	}
 
+	//todo: remove the following checks for 2.0.0 they are required just because of the flags deprecation
+	if cmd.Flags().Changed("leader-elect") && cmd.Flags().Changed("enable-leader-election") {
+		log.Error(errors.New("only one of --leader-elect and --enable-leader-election may be set"), "invalid flags usage")
+		os.Exit(1)
+	}
+
+	if cmd.Flags().Changed("metrics-addr") && cmd.Flags().Changed("metrics-bind-address") {
+		log.Error(errors.New("only one of --metrics-addr and --metrics-bind-address may be set"), "invalid flags usage")
+		os.Exit(1)
+	}
+
 	// Set default manager options
 	// TODO: probably should expose the host & port as an environment variables
 	options := manager.Options{
-		MetricsBindAddress:         f.MetricsAddress,
+		MetricsBindAddress:         f.MetricsBindAddress,
 		HealthProbeBindAddress:     f.ProbeAddr,
-		LeaderElection:             f.EnableLeaderElection,
+		LeaderElection:             f.LeaderElection,
 		LeaderElectionID:           f.LeaderElectionID,
 		LeaderElectionResourceLock: resourcelock.ConfigMapsResourceLock,
 		LeaderElectionNamespace:    f.LeaderElectionNamespace,

diff --git a/internal/cmd/helm-operator/run/cmd.go b/internal/cmd/helm-operator/run/cmd.go
@@ -15,6 +15,7 @@
 package run
 
 import (
+	"errors"
 	"flag"
 	"fmt"
 	"os"
@@ -84,8 +85,9 @@ func run(cmd *cobra.Command, f *flags.Flags) {
 		os.Exit(1)
 	}
 
-	// Deprecated: OPERATOR_NAME environment variable is an artifact of the legacy operator-sdk project scaffolding.
-	//   Flag `--leader-election-id` should be used instead.
+	// Deprecated: OPERATOR_NAME environment variable is an artifact of the
+	// legacy operator-sdk project scaffolding. Flag `--leader-election-id`
+	// should be used instead.
 	if operatorName, found := os.LookupEnv("OPERATOR_NAME"); found {
 		log.Info("Environment variable OPERATOR_NAME has been deprecated, use --leader-election-id instead.")
 		if cmd.Flags().Lookup("leader-election-id").Changed {
@@ -95,11 +97,22 @@ func run(cmd *cobra.Command, f *flags.Flags) {
 		}
 	}
 
+	//todo: remove the following checks for 2.0.0 they are required just because of the flags deprecation
+	if cmd.Flags().Changed("leader-elect") && cmd.Flags().Changed("enable-leader-election") {
+		log.Error(errors.New("only one of --leader-elect and --enable-leader-election may be set"), "invalid flags usage")
+		os.Exit(1)
+	}
+
+	if cmd.Flags().Changed("metrics-addr") && cmd.Flags().Changed("metrics-bind-address") {
+		log.Error(errors.New("only one of --metrics-addr and --metrics-bind-address may be set"), "invalid flags usage")
+		os.Exit(1)
+	}
+
 	// Set default manager options
 	options := manager.Options{
-		MetricsBindAddress:         f.MetricsAddress,
+		MetricsBindAddress:         f.MetricsBindAddress,
 		HealthProbeBindAddress:     f.ProbeAddr,
-		LeaderElection:             f.EnableLeaderElection,
+		LeaderElection:             f.LeaderElection,
 		LeaderElectionID:           f.LeaderElectionID,
 		LeaderElectionResourceLock: resourcelock.ConfigMapsResourceLock,
 		LeaderElectionNamespace:    f.LeaderElectionNamespace,

diff --git a/internal/helm/flags/flag.go b/internal/helm/flags/flag.go
@@ -25,8 +25,8 @@ import (
 type Flags struct {
 	ReconcilePeriod         time.Duration
 	WatchesFile             string
-	MetricsAddress          string
-	EnableLeaderElection    bool
+	MetricsBindAddress      string
+	LeaderElection          bool
 	LeaderElectionID        string
 	LeaderElectionNamespace string
 	MaxConcurrentReconciles int
@@ -45,20 +45,39 @@ func (f *Flags) AddTo(flagSet *pflag.FlagSet) {
 		"./watches.yaml",
 		"Path to the watches file to use",
 	)
-	flagSet.StringVar(&f.MetricsAddress,
+	// todo:remove it for 2.0.0
+	flagSet.StringVar(&f.MetricsBindAddress,
 		"metrics-addr",
 		":8080",
 		"The address the metric endpoint binds to",
 	)
+	_ = flagSet.MarkDeprecated("metrics-addr", "use --metrics-bind-address instead")
+	flagSet.StringVar(&f.MetricsBindAddress,
+		"metrics-bind-address",
+		":8080",
+		"The address the metric endpoint binds to",
+	)
+	// todo: 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",
 		":8081",
 		"The address the probe endpoint binds to.",
 	)
-	flagSet.BoolVar(&f.EnableLeaderElection,
+	// todo:remove it for 2.0.0
+	flagSet.BoolVar(&f.LeaderElection,
 		"enable-leader-election",
 		false,
-		"Enable leader election for controller manager. Enabling this will ensure there is only one active controller manager.",
+		"Enable leader election for controller manager. Enabling this will"+
+			" ensure there is only one active controller manager.",
+	)
+	_ = flagSet.MarkDeprecated("enable-leader-election", "use --leader-elect instead.")
+	flagSet.BoolVar(&f.LeaderElection,
+		"leader-elect",
+		false,
+		"Enable leader election for controller manager. Enabling this will"+
+			" ensure there is only one active controller manager.",
 	)
 	flagSet.StringVar(&f.LeaderElectionID,
 		"leader-election-id",
@@ -68,7 +87,9 @@ func (f *Flags) AddTo(flagSet *pflag.FlagSet) {
 	flagSet.StringVar(&f.LeaderElectionNamespace,
 		"leader-election-namespace",
 		"",
-		"Namespace in which to create the leader election configmap for holding the leader lock (required if running locally with leader election enabled).",
+		"Namespace in which to create the leader election configmap for"+
+			" holding the leader lock (required if running locally with leader"+
+			" election enabled).",
 	)
 	flagSet.IntVar(&f.MaxConcurrentReconciles,
 		"max-concurrent-reconciles",

diff --git a/internal/plugins/ansible/v1/scaffolds/init.go b/internal/plugins/ansible/v1/scaffolds/init.go
@@ -100,11 +100,11 @@ func (s *initScaffolder) Scaffold() error {
 		&prometheus.Kustomization{},
 		&prometheus.ServiceMonitor{},
 
-		&manager.Manager{Image: imageName},
+		&manager.Config{Image: imageName},
 		&manager.Kustomization{},
 
-		&kdefault.Kustomize{},
-		&kdefault.AuthProxyPatch{},
+		&kdefault.Kustomization{},
+		&kdefault.ManagerAuthProxyPatch{},
 
 		&roles.Placeholder{},
 		&playbooks.Placeholder{},

diff --git a/internal/plugins/ansible/v1/scaffolds/internal/templates/config/kdefault/kustomization.go b/internal/plugins/ansible/v1/scaffolds/internal/templates/config/kdefault/kustomization.go
@@ -23,16 +23,16 @@ import (
 	"sigs.k8s.io/kubebuilder/v3/pkg/model/file"
 )
 
-var _ file.Template = &Kustomize{}
+var _ file.Template = &Kustomization{}
 
-// Kustomize scaffolds the Kustomization file for the default overlay
-type Kustomize struct {
+// Kustomization scaffolds a file that defines the kustomization scheme for the default overlay folder
+type Kustomization struct {
 	file.TemplateMixin
 	file.ProjectNameMixin
 }
 
-// SetTemplateDefaults implements input.Template
-func (f *Kustomize) SetTemplateDefaults() error {
+// SetTemplateDefaults implements file.Template
+func (f *Kustomization) SetTemplateDefaults() error {
 	if f.Path == "" {
 		f.Path = filepath.Join("config", "default", "kustomization.yaml")
 	}
@@ -66,8 +66,8 @@ bases:
 #- ../prometheus
 
 patchesStrategicMerge:
-  # Protect the /metrics endpoint by putting it behind auth.
-  # If you want your controller-manager to expose the /metrics
-  # endpoint w/o any authn/z, please comment the following line.
+# Protect the /metrics endpoint by putting it behind auth.
+# 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
 `