feat(ansible): make ansible verbosity configurable (#2087)

* feat(ansible): make ansible verbosity configurable

Make it possible to configure the verbosity of `ansible-runner` command
invocations via command line flags. The default value should be 2 to
avoid unexpected changes in behavior. Also, like `maxWorkers` the
`ansibleVerbosity` can be overridden per GVK with an environment
variable of the form `ANSIBLE_VERBOSITY_$KIND_$GROUP`.

This PR:

- Adds new command line flag `ansible-verbosity` for Ansible based
  operators
- Add internal `cmdFuncType` to runner pkg for use when constructing
  command functions

* feat(ansible): address documentation comments

- word choice in advanced_options
- refer to correct environment key in getAnsibleVerbosity()

* Ansible verbosity change goes with unreleased

Author: djzager

CHANGELOG.md

@@ -3,6 +3,7 @@
 ### Added
 
 - Added `Operator Version: X.Y.Z` information in the operator logs.([#1953](https://github.com/operator-framework/operator-sdk/pull/1953))
+- Make Ansible verbosity configurable via the `ansible-verbosity` flag. ([#2087](https://github.com/operator-framework/operator-sdk/pull/2087))
 
 ### Changed
 

doc/ansible/dev/advanced_options.md

@@ -11,7 +11,7 @@ Some features can be overridden per resource via an annotation on that CR. The o
 
 | Feature | Yaml Key | Description| Annotation for override | default | Documentation |
 |---------|----------|------------|-------------------------|---------|---------------|
-| Reconcile Period | `reconcilePeriod`  | time between reconcile runs for a particular CR  | ansbile.operator-sdk/reconcile-period  | 1m | |
+| Reconcile Period | `reconcilePeriod`  | time between reconcile runs for a particular CR  | ansible.operator-sdk/reconcile-period  | 1m | |
 | Manage Status | `manageStatus` | Allows the ansible operator to manage the conditions section of each resource's status section. | | true | |
 | Watching Dependent Resources | `watchDependentResources` | Allows the ansible operator to dynamically watch resources that are created by ansible | | true | [dependent_watches.md](dependent_watches.md) |
 | Watching Cluster-Scoped Resources | `watchClusterScopedResources` | Allows the ansible operator to watch cluster-scoped resources that are created by ansible | | false | |
@@ -111,3 +111,44 @@ From this data, we can see that the environment variable will be
     - name: WORKER_MEMCACHED_CACHE_EXAMPLE_COM
       value: "6"
 ```
+
+## Ansible Verbosity
+
+Setting the verbosity at which `ansible-runner` is run controls how verbose the
+output of `ansible-playbook` will be. The normal rules for verbosity apply
+here, where higher values mean more output. Acceptable values range from 0
+(only the most severe messages are output) to 7 (all debugging messages are
+output).
+
+There are two ways to configure the verbosity argument to the `ansible-runner`
+command:
+
+1. Operator **authors and admins** can set the Ansible verbosity by including
+   extra args to the operator container in the operator deployment.
+1. Operator **admins** can set Ansible verbosity by setting an environment
+   variable in the format `ANSIBLE_VERBOSITY_<kind>_<group>`. This variable must
+   be all uppercase and all periods (e.g. in the group name) are replaced with
+   underscore.
+
+### Example
+
+For demonstration purposes, let us assume that we have a database operator that
+supports two Kinds -- `MongoDB` and `PostgreSQL` -- in the `db.example.com`
+Group. We have only recently implemented the support for the `MongoDB` Kind so
+we want reconciles for this Kind to be more verbose. Our operator container's
+spec in our `deploy/operator.yaml` might look something like:
+
+```yaml
+- name: operator
+  image: "quay.io/example/database-operator:v1.0.0"
+  imagePullPolicy: "Always"
+  args:
+    # This value applies to all GVKs specified in watches.yaml
+    # that are not overriden by environment variables.
+    - "--ansible-verbosity"
+    - "1"
+  env:
+    # Override the verbosity for the MongoDB kind
+    - name: ANSIBLE_VERBOSITY_MONGODB_DB_EXAMPLE_COM
+      value: "4"
+```

pkg/ansible/flags/flag.go

@@ -25,8 +25,9 @@ import (
 // AnsibleOperatorFlags - Options to be used by an ansible operator
 type AnsibleOperatorFlags struct {
 	watch.WatchFlags
-	InjectOwnerRef bool
-	MaxWorkers     int
+	InjectOwnerRef   bool
+	MaxWorkers       int
+	AnsibleVerbosity int
 }
 
 // AddTo - Add the ansible operator flags to the the flagset
@@ -47,5 +48,12 @@ func AddTo(flagSet *pflag.FlagSet, helpTextPrefix ...string) *AnsibleOperatorFla
 			"Maximum number of workers to use. Overridden by environment variable."),
 			" "),
 	)
+	flagSet.IntVar(&aof.AnsibleVerbosity,
+		"ansible-verbosity",
+		2,
+		strings.Join(append(helpTextPrefix,
+			"Ansible verbosity. Overridden by environment variable."),
+			" "),
+	)
 	return aof
 }

pkg/ansible/run.go

@@ -89,7 +89,7 @@ func Run(flags *aoflags.AnsibleOperatorFlags) error {
 
 	var gvks []schema.GroupVersionKind
 	cMap := controllermap.NewControllerMap()
-	watches, err := watches.Load(flags.WatchesFile, flags.MaxWorkers)
+	watches, err := watches.Load(flags.WatchesFile, flags.MaxWorkers, flags.AnsibleVerbosity)
 	if err != nil {
 		log.Error(err, "Failed to load watches.")
 		return err

pkg/ansible/runner/runner.go

@@ -52,62 +52,65 @@ type Runner interface {
 	GetFinalizer() (string, bool)
 }
 
+func ansibleVerbosityString(verbosity int) string {
+	if verbosity > 0 {
+		return fmt.Sprintf("-%v", strings.Repeat("v", verbosity))
+	}
+	return ""
+}
+
+type cmdFuncType func(ident, inputDirPath string, maxArtifacts int) *exec.Cmd
+
+func playbookCmdFunc(verbosity, path string) cmdFuncType {
+	return func(ident, inputDirPath string, maxArtifacts int) *exec.Cmd {
+		return exec.Command("ansible-runner", verbosity, "--rotate-artifacts", fmt.Sprintf("%v", maxArtifacts), "-p", path, "-i", ident, "run", inputDirPath)
+	}
+}
+
+func roleCmdFunc(verbosity, path string) cmdFuncType {
+	rolePath, roleName := filepath.Split(path)
+	return func(ident, inputDirPath string, maxArtifacts int) *exec.Cmd {
+		return exec.Command("ansible-runner", verbosity, "--rotate-artifacts", fmt.Sprintf("%v", maxArtifacts), "--role", roleName, "--roles-path", rolePath, "--hosts", "localhost", "-i", ident, "run", inputDirPath)
+	}
+}
+
 // New - creates a Runner from a Watch struct
 func New(watch watches.Watch) (Runner, error) {
-	// handle role or playbook
 	var path string
-	var cmdFunc func(ident, inputDirPath string, maxArtifacts int) *exec.Cmd
+	var cmdFunc, finalizerCmdFunc cmdFuncType
 
 	err := watch.Validate()
 	if err != nil {
 		log.Error(err, "Failed to validate watch")
 		return nil, err
 	}
+	verbosityString := ansibleVerbosityString(watch.AnsibleVerbosity)
 
 	switch {
 	case watch.Playbook != "":
 		path = watch.Playbook
-		cmdFunc = func(ident, inputDirPath string, maxArtifacts int) *exec.Cmd {
-			return exec.Command("ansible-runner", "-vv", "--rotate-artifacts", fmt.Sprintf("%v", maxArtifacts), "-p", path, "-i", ident, "run", inputDirPath)
-		}
+		cmdFunc = playbookCmdFunc(verbosityString, path)
 	case watch.Role != "":
 		path = watch.Role
-		cmdFunc = func(ident, inputDirPath string, maxArtifacts int) *exec.Cmd {
-			rolePath, roleName := filepath.Split(path)
-			return exec.Command("ansible-runner", "-vv", "--rotate-artifacts", fmt.Sprintf("%v", maxArtifacts), "--role", roleName, "--roles-path", rolePath, "--hosts", "localhost", "-i", ident, "run", inputDirPath)
-		}
-	default:
-		return nil, fmt.Errorf("must specify Role or Path")
+		cmdFunc = roleCmdFunc(verbosityString, path)
 	}
 
 	// handle finalizer
-	var finalizer *watches.Finalizer
-	var finalizerCmdFunc func(ident, inputDirPath string, maxArtifacts int) *exec.Cmd
 	switch {
 	case watch.Finalizer == nil:
-		finalizer = nil
 		finalizerCmdFunc = nil
 	case watch.Finalizer.Playbook != "":
-		finalizer = watch.Finalizer
-		finalizerCmdFunc = func(ident, inputDirPath string, maxArtifacts int) *exec.Cmd {
-			return exec.Command("ansible-runner", "-vv", "--rotate-artifacts", fmt.Sprintf("%v", maxArtifacts), "-p", finalizer.Playbook, "-i", ident, "run", inputDirPath)
-		}
+		finalizerCmdFunc = playbookCmdFunc(verbosityString, watch.Finalizer.Playbook)
 	case watch.Finalizer.Role != "":
-		finalizer = watch.Finalizer
-		finalizerCmdFunc = func(ident, inputDirPath string, maxArtifacts int) *exec.Cmd {
-			path := strings.TrimRight(finalizer.Role, "/")
-			rolePath, roleName := filepath.Split(path)
-			return exec.Command("ansible-runner", "-vv", "--rotate-artifacts", fmt.Sprintf("%v", maxArtifacts), "--role", roleName, "--roles-path", rolePath, "--hosts", "localhost", "-i", ident, "run", inputDirPath)
-		}
+		finalizerCmdFunc = roleCmdFunc(verbosityString, watch.Finalizer.Role)
 	case len(watch.Finalizer.Vars) != 0:
-		finalizer = watch.Finalizer
 		finalizerCmdFunc = cmdFunc
 	}
 
 	return &runner{
 		Path:               path,
 		cmdFunc:            cmdFunc,
-		Finalizer:          finalizer,
+		Finalizer:          watch.Finalizer,
 		finalizerCmdFunc:   finalizerCmdFunc,
 		GVK:                watch.GroupVersionKind,
 		maxRunnerArtifacts: watch.MaxRunnerArtifacts,

pkg/ansible/runner/runner_test.go

@@ -15,7 +15,6 @@
 package runner
 
 import (
-	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -27,32 +26,20 @@ import (
 	"github.com/operator-framework/operator-sdk/pkg/ansible/watches"
 )
 
-func playbookCmd(path string, ident string, inputDirPath string, maxArtifacts int) *exec.Cmd {
-	return exec.Command("ansible-runner", "-vv", "--rotate-artifacts", fmt.Sprintf("%v", maxArtifacts), "-p", path, "-i", ident, "run", inputDirPath)
-}
-
-func roleCmd(path string, ident string, inputDirPath string, maxArtifacts int) *exec.Cmd {
-	rolePath, roleName := filepath.Split(path)
-	return exec.Command("ansible-runner", "-vv", "--rotate-artifacts", fmt.Sprintf("%v", maxArtifacts), "--role", roleName, "--roles-path", rolePath, "--hosts", "localhost", "-i", ident, "run", inputDirPath)
-}
-
-type cmdFuncType func(ident, inputDirPath string, maxArtifacts int) *exec.Cmd
-
-func checkCmdFunc(t *testing.T, cmdFunc cmdFuncType, role, playbook string, watchesMaxArtifacts, runnerMaxArtifacts int) {
+func checkCmdFunc(t *testing.T, cmdFunc cmdFuncType, playbook, role string, verbosity int) {
 	ident := "test"
 	inputDirPath := "/test/path"
-	var path string
+	maxArtifacts := 1
 	var expectedCmd, gotCmd *exec.Cmd
+	verbosityString := ansibleVerbosityString(verbosity)
 	switch {
 	case playbook != "":
-		path = playbook
-		expectedCmd = playbookCmd(path, ident, inputDirPath, watchesMaxArtifacts)
+		expectedCmd = playbookCmdFunc(verbosityString, playbook)(ident, inputDirPath, maxArtifacts)
 	case role != "":
-		path = role
-		expectedCmd = roleCmd(path, ident, inputDirPath, watchesMaxArtifacts)
+		expectedCmd = roleCmdFunc(verbosityString, role)(ident, inputDirPath, maxArtifacts)
 	}
 
-	gotCmd = cmdFunc(ident, inputDirPath, runnerMaxArtifacts)
+	gotCmd = cmdFunc(ident, inputDirPath, maxArtifacts)
 
 	if expectedCmd.Path != gotCmd.Path {
 		t.Fatalf("Unexpected cmd path %v expected cmd path %v", gotCmd.Path, expectedCmd.Path)
@@ -171,7 +158,7 @@ func TestNew(t *testing.T) {
 			}
 
 			// Check the cmdFunc
-			checkCmdFunc(t, testRunnerStruct.cmdFunc, testWatch.Role, testWatch.Playbook, testWatch.MaxRunnerArtifacts, testRunnerStruct.maxRunnerArtifacts)
+			checkCmdFunc(t, testRunnerStruct.cmdFunc, testWatch.Playbook, testWatch.Role, testWatch.AnsibleVerbosity)
 
 			// Check finalizer
 			if testRunnerStruct.Finalizer != testWatch.Finalizer {
@@ -184,12 +171,32 @@ func TestNew(t *testing.T) {
 				}
 
 				if len(testWatch.Finalizer.Vars) == 0 {
-					checkCmdFunc(t, testRunnerStruct.finalizerCmdFunc, testWatch.Finalizer.Role, testWatch.Finalizer.Playbook, testWatch.MaxRunnerArtifacts, testRunnerStruct.maxRunnerArtifacts)
+					checkCmdFunc(t, testRunnerStruct.cmdFunc, testWatch.Finalizer.Playbook, testWatch.Finalizer.Role, testWatch.AnsibleVerbosity)
 				} else {
 					// when finalizer vars is set the finalizerCmdFunc should be the same as the cmdFunc
-					checkCmdFunc(t, testRunnerStruct.finalizerCmdFunc, testWatch.Role, testWatch.Playbook, testWatch.MaxRunnerArtifacts, testRunnerStruct.maxRunnerArtifacts)
+					checkCmdFunc(t, testRunnerStruct.finalizerCmdFunc, testWatch.Playbook, testWatch.Role, testWatch.AnsibleVerbosity)
 				}
 			}
 		})
 	}
 }
+
+func TestAnsibleVerbosityString(t *testing.T) {
+	testCases := []struct {
+		verbosity      int
+		expectedString string
+	}{
+		{verbosity: -1, expectedString: ""},
+		{verbosity: 0, expectedString: ""},
+		{verbosity: 1, expectedString: "-v"},
+		{verbosity: 2, expectedString: "-vv"},
+		{verbosity: 7, expectedString: "-vvvvvvv"},
+	}
+
+	for _, tc := range testCases {
+		gotString := ansibleVerbosityString(tc.verbosity)
+		if tc.expectedString != gotString {
+			t.Fatalf("Unexpected string %v expected %v", gotString, tc.expectedString)
+		}
+	}
+}

pkg/ansible/watches/testdata/valid.yaml.tmpl

@@ -68,3 +68,16 @@
   group: app.example.com
   kind: MaxWorkersEnv
   role: {{ .ValidRole }}
+- version: v1alpha1
+  group: app.example.com
+  kind: AnsibleVerbosityDefault
+  role: {{ .ValidRole }}
+- version: v1alpha1
+  group: app.example.com
+  kind: AnsibleVerbosityIgnored
+  role: {{ .ValidRole }}
+  ansibleVerbosity: 5
+- version: v1alpha1
+  group: app.example.com
+  kind: AnsibleVerbosityEnv
+  role: {{ .ValidRole }}

pkg/ansible/watches/watches.go

@@ -48,7 +48,8 @@ type Watch struct {
 	Finalizer                   *Finalizer              `yaml:"finalizer"`
 
 	// Not configurable via watches.yaml
-	MaxWorkers int `yaml:"maxWorkers"`
+	MaxWorkers       int `yaml:"maxWorkers"`
+	AnsibleVerbosity int `yaml:"ansibleVerbosity"`
 }
 
 // Finalizer - Expose finalizer to be used by a user.
@@ -68,7 +69,8 @@ var (
 	watchClusterScopedResourcesDefault = false
 
 	// these are overridden by cmdline flags
-	maxWorkersDefault = 1
+	maxWorkersDefault       = 1
+	ansibleVerbosityDefault = 2
 )
 
 // UnmarshalYAML - implements the yaml.Unmarshaler interface for Watch.
@@ -130,6 +132,7 @@ func (w *Watch) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	w.WatchDependentResources = tmp.WatchDependentResources
 	w.WatchClusterScopedResources = tmp.WatchClusterScopedResources
 	w.Finalizer = tmp.Finalizer
+	w.AnsibleVerbosity = getAnsibleVerbosity(gvk, ansibleVerbosityDefault)
 
 	return nil
 }
@@ -176,12 +179,14 @@ func New(gvk schema.GroupVersionKind, role, playbook string, finalizer *Finalize
 		WatchDependentResources:     watchDependentResourcesDefault,
 		WatchClusterScopedResources: watchClusterScopedResourcesDefault,
 		Finalizer:                   finalizer,
+		AnsibleVerbosity:            ansibleVerbosityDefault,
 	}
 }
 
 // Load - loads a slice of Watches from the watches file from the CLI
-func Load(path string, maxWorkers int) ([]Watch, error) {
+func Load(path string, maxWorkers, ansibleVerbosity int) ([]Watch, error) {
 	maxWorkersDefault = maxWorkers
+	ansibleVerbosityDefault = ansibleVerbosity
 	b, err := ioutil.ReadFile(path)
 	if err != nil {
 		log.Error(err, "Failed to get config file")
@@ -251,7 +256,7 @@ func verifyAnsiblePath(playbook string, role string) error {
 }
 
 // if the WORKER_* environment variable is set, use that value.
-// Otherwise, use the value from the CLI. This is definitely
+// Otherwise, use defValue. This is definitely
 // counter-intuitive but it allows the operator admin adjust the
 // number of workers based on their cluster resources. While the
 // author may use the CLI option to specify a suggested
@@ -276,3 +281,30 @@ func getMaxWorkers(gvk schema.GroupVersionKind, defValue int) int {
 	}
 	return maxWorkers
 }
+
+// if the ANSIBLE_VERBOSITY_* environment variable is set, use that value.
+// Otherwise, use defValue.
+func getAnsibleVerbosity(gvk schema.GroupVersionKind, defValue int) int {
+	envVar := strings.ToUpper(strings.Replace(
+		fmt.Sprintf("ANSIBLE_VERBOSITY_%s_%s", gvk.Kind, gvk.Group),
+		".",
+		"_",
+		-1,
+	))
+	ansibleVerbosity, err := strconv.Atoi(os.Getenv(envVar))
+	if err != nil {
+		log.Info("Failed to parse %v from environment. Using default %v", envVar, defValue)
+		return defValue
+	}
+
+	// Use default value when value doesn't make sense
+	if ansibleVerbosity < 0 {
+		log.Info("Value %v not valid. Using default %v", ansibleVerbosity, defValue)
+		return defValue
+	}
+	if ansibleVerbosity > 7 {
+		log.Info("Value %v not valid. Using default %v", ansibleVerbosity, defValue)
+		return defValue
+	}
+	return ansibleVerbosity
+}