From 42574edc549c373c98f2493bb45f347a73f42f94 Mon Sep 17 00:00:00 2001 From: David Zager Date: Wed, 21 Aug 2019 15:15:19 -0400 Subject: [PATCH 1/7] feat(ansible): make ansible verbosity configurable The primary objective of this change is to make the verbosity of individual `ansible-runner` command invocations configurable (not just "-vv"). In order to accomplish this: * A new flag was added to the AnsibleOperatorFlags (ansible-verbosity) with a default value of 2. * The entire flagset is given to the watches pkg Load() to provide one place for handling command line arguments, watches config values, and environment variables. This makes an individual watch more useful when creating a runner and adding a GVK to the controller. Also, this puts new fields on the Watch struct (MaxWorkers and AnsibleVerbosity). * New functions were added to the watches pkg -- New(gvk) instantiates a Watch with sensible defaults. Validate() is used to verify a given watch meets certain criteria (legit path to playbook/role and proper finalizer). This makes it possible for someone who only knows their GVK and role/playbook to easily create a watch that can be used with the runner pkg. * When creating a runner from a watch, we will now validate it using watches.Validate(). Essentially the watches pkg should know what is and isn't a valid watch. * New helper functions in the runner pkg -- ansibleVerbosityString to convert an integer AnsibleVerbosity to a string (ie. 2 becomes "-vv"), playbookCmdFunc, and roleCmdFunc consolidate the logic for creating the cmdFund. --- pkg/ansible/flags/flag.go | 12 +- pkg/ansible/run.go | 47 +- pkg/ansible/run_test.go | 56 -- pkg/ansible/runner/runner.go | 62 ++- pkg/ansible/runner/runner_test.go | 198 +++---- pkg/ansible/runner/testdata/playbook.yml | 0 .../runner/testdata/roles/role/tasks.yaml | 0 pkg/ansible/watches/testdata/valid.yaml.tmpl | 10 + pkg/ansible/watches/watches.go | 154 ++++-- pkg/ansible/watches/watches_test.go | 505 +++++++++++++++++- 10 files changed, 761 insertions(+), 283 deletions(-) create mode 100644 pkg/ansible/runner/testdata/playbook.yml create mode 100644 pkg/ansible/runner/testdata/roles/role/tasks.yaml diff --git a/pkg/ansible/flags/flag.go b/pkg/ansible/flags/flag.go index ab882718ec..6cf8802000 100644 --- a/pkg/ansible/flags/flag.go +++ b/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 } diff --git a/pkg/ansible/run.go b/pkg/ansible/run.go index 1a359381b4..98fd5373bf 100644 --- a/pkg/ansible/run.go +++ b/pkg/ansible/run.go @@ -19,30 +19,28 @@ import ( "fmt" "os" "runtime" - "strconv" - "strings" "github.com/operator-framework/operator-sdk/pkg/ansible/controller" - aoflags "github.com/operator-framework/operator-sdk/pkg/ansible/flags" - proxy "github.com/operator-framework/operator-sdk/pkg/ansible/proxy" "github.com/operator-framework/operator-sdk/pkg/ansible/proxy/controllermap" "github.com/operator-framework/operator-sdk/pkg/ansible/runner" "github.com/operator-framework/operator-sdk/pkg/ansible/watches" "github.com/operator-framework/operator-sdk/pkg/k8sutil" - kubemetrics "github.com/operator-framework/operator-sdk/pkg/kube-metrics" "github.com/operator-framework/operator-sdk/pkg/leader" "github.com/operator-framework/operator-sdk/pkg/metrics" "github.com/operator-framework/operator-sdk/pkg/restmapper" - sdkVersion "github.com/operator-framework/operator-sdk/version" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/intstr" - "sigs.k8s.io/controller-runtime/pkg/client/config" - logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/manager/signals" + + aoflags "github.com/operator-framework/operator-sdk/pkg/ansible/flags" + proxy "github.com/operator-framework/operator-sdk/pkg/ansible/proxy" + kubemetrics "github.com/operator-framework/operator-sdk/pkg/kube-metrics" + sdkVersion "github.com/operator-framework/operator-sdk/version" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + logf "sigs.k8s.io/controller-runtime/pkg/log" ) var ( @@ -91,7 +89,7 @@ func Run(flags *aoflags.AnsibleOperatorFlags) error { var gvks []schema.GroupVersionKind cMap := controllermap.NewControllerMap() - watches, err := watches.Load(flags.WatchesFile) + watches, err := watches.Load(flags) if err != nil { log.Error(err, "Failed to load watches.") return err @@ -107,7 +105,7 @@ func Run(flags *aoflags.AnsibleOperatorFlags) error { GVK: w.GroupVersionKind, Runner: runner, ManageStatus: w.ManageStatus, - MaxWorkers: getMaxWorkers(w.GroupVersionKind, flags.MaxWorkers), + MaxWorkers: w.MaxWorkers, ReconcilePeriod: w.ReconcilePeriod, }) if ctr == nil { @@ -188,28 +186,3 @@ func Run(flags *aoflags.AnsibleOperatorFlags) error { log.Info("Exiting.") return nil } - -// if the WORKER_* environment variable is set, use that value. -// Otherwise, use the value from the CLI. 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 -// configuration for the operator. -func getMaxWorkers(gvk schema.GroupVersionKind, defValue int) int { - envVar := strings.ToUpper(strings.Replace( - fmt.Sprintf("WORKER_%s_%s", gvk.Kind, gvk.Group), - ".", - "_", - -1, - )) - switch maxWorkers, err := strconv.Atoi(os.Getenv(envVar)); { - case maxWorkers <= 1: - return defValue - case err != nil: - // we don't care why we couldn't parse it just use default - log.Info("Failed to parse %v from environment. Using default %v", envVar, defValue) - return defValue - default: - return maxWorkers - } -} diff --git a/pkg/ansible/run_test.go b/pkg/ansible/run_test.go index 37d9720259..cc9697c4f9 100644 --- a/pkg/ansible/run_test.go +++ b/pkg/ansible/run_test.go @@ -14,60 +14,4 @@ package ansible -import ( - "os" - "strconv" - "testing" - - "k8s.io/apimachinery/pkg/runtime/schema" - - "github.com/stretchr/testify/assert" -) - // TODO: add a test for the Run method - -func TestMaxWorkers(t *testing.T) { - testCases := []struct { - name string - gvk schema.GroupVersionKind - defvalue int - expected int - setenvvar bool - envvar string - }{ - { - name: "no env, use default value", - gvk: schema.GroupVersionKind{ - Group: "cache.example.com", - Version: "v1alpha1", - Kind: "MemCacheService", - }, - defvalue: 10, - expected: 10, - setenvvar: false, - envvar: "WORKER_MEMCACHESERVICE_CACHE_EXAMPLE_COM", - }, - { - name: "env set to 3, expect 3", - gvk: schema.GroupVersionKind{ - Group: "cache.example.com", - Version: "v1alpha1", - Kind: "MemCacheService", - }, - defvalue: 1, - expected: 3, - setenvvar: true, - envvar: "WORKER_MEMCACHESERVICE_CACHE_EXAMPLE_COM", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - os.Unsetenv(tc.envvar) - if tc.setenvvar { - os.Setenv(tc.envvar, strconv.Itoa(tc.expected)) - } - assert.Equal(t, tc.expected, getMaxWorkers(tc.gvk, tc.defvalue)) - }) - } -} diff --git a/pkg/ansible/runner/runner.go b/pkg/ansible/runner/runner.go index a031638a24..fa4dad7979 100644 --- a/pkg/ansible/runner/runner.go +++ b/pkg/ansible/runner/runner.go @@ -52,56 +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, @@ -119,7 +128,6 @@ type runner struct { } func (r *runner) Run(ident string, u *unstructured.Unstructured, kubeconfig string) (RunResult, error) { - timer := metrics.ReconcileTimer(r.GVK.String()) defer timer.ObserveDuration() diff --git a/pkg/ansible/runner/runner_test.go b/pkg/ansible/runner/runner_test.go index 41edf3c82d..3de05014f0 100644 --- a/pkg/ansible/runner/runner_test.go +++ b/pkg/ansible/runner/runner_test.go @@ -15,7 +15,7 @@ package runner import ( - "fmt" + "os" "os/exec" "path/filepath" "reflect" @@ -26,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) @@ -63,103 +51,75 @@ func checkCmdFunc(t *testing.T, cmdFunc cmdFuncType, role, playbook string, watc } func TestNew(t *testing.T) { + cwd, err := os.Getwd() + if err != nil { + t.Fatalf("Unable to get working director: %v", err) + } + validPlaybook := filepath.Join(cwd, "testdata", "playbook.yml") + validRole := filepath.Join(cwd, "testdata", "roles", "role") testCases := []struct { - name string - watch watches.Watch + name string + gvk schema.GroupVersionKind + playbook string + role string + finalizer *watches.Finalizer }{ { name: "basic runner with playbook", - watch: watches.Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Group: "operator.example.com", - Version: "v1alpha1", - Kind: "BasicPlaybook", - }, - Playbook: "/opt/example/playbook.yml", - MaxRunnerArtifacts: watches.MaxRunnerArtifactsDefault, - ReconcilePeriod: watches.ReconcilePeriodDurationDefault, - ManageStatus: watches.ManageStatusDefault, - WatchDependentResources: watches.WatchDependentResourcesDefault, - WatchClusterScopedResources: watches.WatchClusterScopedResourcesDefault, - Finalizer: nil, + gvk: schema.GroupVersionKind{ + Group: "operator.example.com", + Version: "v1alpha1", + Kind: "Example", }, + playbook: validPlaybook, }, { name: "basic runner with role", - watch: watches.Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Group: "operator.example.com", - Version: "v1alpha1", - Kind: "BasicPlaybook", - }, - Role: "/opt/example/roles/example_role", - MaxRunnerArtifacts: watches.MaxRunnerArtifactsDefault, - ReconcilePeriod: watches.ReconcilePeriodDurationDefault, - ManageStatus: watches.ManageStatusDefault, - WatchDependentResources: watches.WatchDependentResourcesDefault, - WatchClusterScopedResources: watches.WatchClusterScopedResourcesDefault, - Finalizer: nil, + gvk: schema.GroupVersionKind{ + Group: "operator.example.com", + Version: "v1alpha1", + Kind: "Example", }, + role: validRole, }, { name: "basic runner with playbook + finalizer playbook", - watch: watches.Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Group: "operator.example.com", - Version: "v1alpha1", - Kind: "BasicPlaybook", - }, - Playbook: "/opt/example/playbook.yml", - MaxRunnerArtifacts: watches.MaxRunnerArtifactsDefault, - ReconcilePeriod: watches.ReconcilePeriodDurationDefault, - ManageStatus: watches.ManageStatusDefault, - WatchDependentResources: watches.WatchDependentResourcesDefault, - WatchClusterScopedResources: watches.WatchClusterScopedResourcesDefault, - Finalizer: &watches.Finalizer{ - Name: "example.finalizer.com", - Playbook: "/opt/example/finalizer.yml", - }, + gvk: schema.GroupVersionKind{ + Group: "operator.example.com", + Version: "v1alpha1", + Kind: "Example", + }, + playbook: validPlaybook, + finalizer: &watches.Finalizer{ + Name: "example.finalizer.com", + Playbook: validPlaybook, }, }, { name: "basic runner with role + finalizer role", - watch: watches.Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Group: "operator.example.com", - Version: "v1alpha1", - Kind: "BasicPlaybook", - }, - Role: "/opt/example/roles/example_role", - MaxRunnerArtifacts: watches.MaxRunnerArtifactsDefault, - ReconcilePeriod: watches.ReconcilePeriodDurationDefault, - ManageStatus: watches.ManageStatusDefault, - WatchDependentResources: watches.WatchDependentResourcesDefault, - WatchClusterScopedResources: watches.WatchClusterScopedResourcesDefault, - Finalizer: &watches.Finalizer{ - Name: "example.finalizer.com", - Role: "/opt/example/roles/finalizer_role", - }, + gvk: schema.GroupVersionKind{ + Group: "operator.example.com", + Version: "v1alpha1", + Kind: "Example", + }, + role: validRole, + finalizer: &watches.Finalizer{ + Name: "example.finalizer.com", + Role: validRole, }, }, { name: "basic runner with playbook + finalizer vars", - watch: watches.Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Group: "operator.example.com", - Version: "v1alpha1", - Kind: "BasicPlaybook", - }, - Playbook: "/opt/example/playbook.yml", - MaxRunnerArtifacts: watches.MaxRunnerArtifactsDefault, - ReconcilePeriod: watches.ReconcilePeriodDurationDefault, - ManageStatus: watches.ManageStatusDefault, - WatchDependentResources: watches.WatchDependentResourcesDefault, - WatchClusterScopedResources: watches.WatchClusterScopedResourcesDefault, - Finalizer: &watches.Finalizer{ - Name: "example.finalizer.com", - Vars: map[string]interface{}{ - "state": "absent", - }, + gvk: schema.GroupVersionKind{ + Group: "operator.example.com", + Version: "v1alpha1", + Kind: "Example", + }, + playbook: validPlaybook, + finalizer: &watches.Finalizer{ + Name: "example.finalizer.com", + Vars: map[string]interface{}{ + "state": "absent", }, }, }, @@ -167,7 +127,12 @@ func TestNew(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - testRunner, err := New(tc.watch) + testWatch := watches.New(tc.gvk) + testWatch.Playbook = tc.playbook + testWatch.Role = tc.role + testWatch.Finalizer = tc.finalizer + + testRunner, err := New(*testWatch) if err != nil { t.Fatalf("Error occurred unexpectedly: %v", err) } @@ -177,45 +142,44 @@ func TestNew(t *testing.T) { } switch { - case tc.watch.Playbook != "": - if testRunnerStruct.Path != tc.watch.Playbook { - t.Fatalf("Unexpected path %v expected path %v", testRunnerStruct.Path, tc.watch.Playbook) + case testWatch.Playbook != "": + if testRunnerStruct.Path != testWatch.Playbook { + t.Fatalf("Unexpected path %v expected path %v", testRunnerStruct.Path, testWatch.Playbook) } - case tc.watch.Role != "": - if testRunnerStruct.Path != tc.watch.Role { - t.Fatalf("Unexpected path %v expected path %v", testRunnerStruct.Path, tc.watch.Role) + case testWatch.Role != "": + if testRunnerStruct.Path != testWatch.Role { + t.Fatalf("Unexpected path %v expected path %v", testRunnerStruct.Path, testWatch.Role) } } - if testRunnerStruct.GVK != tc.watch.GroupVersionKind { - t.Fatalf("Unexpected GVK %v expected GVK %v", testRunnerStruct.GVK, tc.watch.GroupVersionKind) + if testRunnerStruct.GVK != testWatch.GroupVersionKind { + t.Fatalf("Unexpected GVK %v expected GVK %v", testRunnerStruct.GVK, testWatch.GroupVersionKind) } - if testRunnerStruct.maxRunnerArtifacts != tc.watch.MaxRunnerArtifacts { - t.Fatalf("Unexpected maxRunnerArtifacts %v expected maxRunnerArtifacts %v", testRunnerStruct.maxRunnerArtifacts, tc.watch.MaxRunnerArtifacts) + if testRunnerStruct.maxRunnerArtifacts != testWatch.MaxRunnerArtifacts { + t.Fatalf("Unexpected maxRunnerArtifacts %v expected maxRunnerArtifacts %v", testRunnerStruct.maxRunnerArtifacts, testWatch.MaxRunnerArtifacts) } // Check the cmdFunc - checkCmdFunc(t, testRunnerStruct.cmdFunc, tc.watch.Role, tc.watch.Playbook, tc.watch.MaxRunnerArtifacts, testRunnerStruct.maxRunnerArtifacts) + checkCmdFunc(t, testRunnerStruct.cmdFunc, testWatch.Playbook, testWatch.Role, testWatch.AnsibleVerbosity) // Check finalizer - if testRunnerStruct.Finalizer != tc.watch.Finalizer { - t.Fatalf("Unexpected finalizer %v expected finalizer %v", testRunnerStruct.Finalizer, tc.watch.Finalizer) + if testRunnerStruct.Finalizer != testWatch.Finalizer { + t.Fatalf("Unexpected finalizer %v expected finalizer %v", testRunnerStruct.Finalizer, testWatch.Finalizer) } - if tc.watch.Finalizer != nil { - if testRunnerStruct.Finalizer.Name != tc.watch.Finalizer.Name { - t.Fatalf("Unexpected finalizer name %v expected finalizer name %v", testRunnerStruct.Finalizer.Name, tc.watch.Finalizer.Name) + if testWatch.Finalizer != nil { + if testRunnerStruct.Finalizer.Name != testWatch.Finalizer.Name { + t.Fatalf("Unexpected finalizer name %v expected finalizer name %v", testRunnerStruct.Finalizer.Name, testWatch.Finalizer.Name) } - if len(tc.watch.Finalizer.Vars) == 0 { - checkCmdFunc(t, testRunnerStruct.finalizerCmdFunc, tc.watch.Finalizer.Role, tc.watch.Finalizer.Playbook, tc.watch.MaxRunnerArtifacts, testRunnerStruct.maxRunnerArtifacts) + if len(testWatch.Finalizer.Vars) == 0 { + 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, tc.watch.Role, tc.watch.Playbook, tc.watch.MaxRunnerArtifacts, testRunnerStruct.maxRunnerArtifacts) + checkCmdFunc(t, testRunnerStruct.finalizerCmdFunc, testWatch.Playbook, testWatch.Role, testWatch.AnsibleVerbosity) } } - }) } } diff --git a/pkg/ansible/runner/testdata/playbook.yml b/pkg/ansible/runner/testdata/playbook.yml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/pkg/ansible/runner/testdata/roles/role/tasks.yaml b/pkg/ansible/runner/testdata/roles/role/tasks.yaml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/pkg/ansible/watches/testdata/valid.yaml.tmpl b/pkg/ansible/watches/testdata/valid.yaml.tmpl index ce6bdc557f..c463a01c4d 100644 --- a/pkg/ansible/watches/testdata/valid.yaml.tmpl +++ b/pkg/ansible/watches/testdata/valid.yaml.tmpl @@ -55,3 +55,13 @@ name: finalizer.app.example.com vars: sentinel: finalizer_running +- version: v1alpha1 + group: app.example.com + kind: MaxWorkers + role: {{ .ValidRole }} + maxWorkers: 3 +- version: v1alpha1 + group: app.example.com + kind: AnsibleVerbosity + role: {{ .ValidRole }} + ansibleVerbosity: 11 diff --git a/pkg/ansible/watches/watches.go b/pkg/ansible/watches/watches.go index c2742d13eb..2a577ddc65 100644 --- a/pkg/ansible/watches/watches.go +++ b/pkg/ansible/watches/watches.go @@ -20,10 +20,13 @@ import ( "io/ioutil" "os" "path/filepath" + "strconv" + "strings" "time" "k8s.io/apimachinery/pkg/runtime/schema" + ansibleflags "github.com/operator-framework/operator-sdk/pkg/ansible/flags" yaml "gopkg.in/yaml.v2" logf "sigs.k8s.io/controller-runtime/pkg/log" ) @@ -37,10 +40,12 @@ type Watch struct { Playbook string `yaml:"playbook"` Role string `yaml:"role"` MaxRunnerArtifacts int `yaml:"maxRunnerArtifacts"` + MaxWorkers int `yaml:"maxWorkers"` ReconcilePeriod time.Duration `yaml:"reconcilePeriod"` ManageStatus bool `yaml:"manageStatus"` WatchDependentResources bool `yaml:"watchDependentResources"` WatchClusterScopedResources bool `yaml:"watchClusterScopedResources"` + AnsibleVerbosity int `yaml:"ansibleVerbosity"` Finalizer *Finalizer `yaml:"finalizer"` } @@ -53,13 +58,14 @@ type Finalizer struct { } // Default values for optional fields on Watch -const ( - ManageStatusDefault bool = true - WatchDependentResourcesDefault bool = true - MaxRunnerArtifactsDefault int = 20 - ReconcilePeriodDefault string = "0s" - ReconcilePeriodDurationDefault time.Duration = time.Duration(0) - WatchClusterScopedResourcesDefault bool = false +var ( + maxRunnerArtifactsDefault = 20 + maxWorkersDefault = 1 //set by flags + reconcilePeriodDefault = "0s" + manageStatusDefault = true + watchDependentResourcesDefault = true + watchClusterScopedResourcesDefault = false + ansibleVerbosityDefault = 2 //set by flags ) // UnmarshalYAML - implements the yaml.Unmarshaler interface for Watch @@ -72,21 +78,25 @@ func (w *Watch) UnmarshalYAML(unmarshal func(interface{}) error) error { Playbook string `yaml:"playbook"` Role string `yaml:"role"` MaxRunnerArtifacts int `yaml:"maxRunnerArtifacts"` + MaxWorkers int `yaml:"maxWorkers"` ReconcilePeriod string `yaml:"reconcilePeriod"` ManageStatus bool `yaml:"manageStatus"` WatchDependentResources bool `yaml:"watchDependentResources"` WatchClusterScopedResources bool `yaml:"watchClusterScopedResources"` + AnsibleVerbosity int `yaml:"ansibleVerbosity"` Finalizer *Finalizer `yaml:"finalizer"` } var tmp alias // by default, the operator will manage status and watch dependent resources // The operator will not manage cluster scoped resources by default. - tmp.ManageStatus = ManageStatusDefault - tmp.WatchDependentResources = WatchDependentResourcesDefault - tmp.MaxRunnerArtifacts = MaxRunnerArtifactsDefault - tmp.ReconcilePeriod = ReconcilePeriodDefault - tmp.WatchClusterScopedResources = WatchClusterScopedResourcesDefault + tmp.ManageStatus = manageStatusDefault + tmp.WatchDependentResources = watchDependentResourcesDefault + tmp.MaxRunnerArtifacts = maxRunnerArtifactsDefault + tmp.MaxWorkers = maxWorkersDefault + tmp.ReconcilePeriod = reconcilePeriodDefault + tmp.WatchClusterScopedResources = watchClusterScopedResourcesDefault + tmp.AnsibleVerbosity = ansibleVerbosityDefault if err := unmarshal(&tmp); err != nil { return err @@ -112,18 +122,68 @@ func (w *Watch) UnmarshalYAML(unmarshal func(interface{}) error) error { w.Playbook = tmp.Playbook w.Role = tmp.Role w.MaxRunnerArtifacts = tmp.MaxRunnerArtifacts + w.MaxWorkers = getMaxWorkers(gvk, tmp.MaxWorkers) w.ReconcilePeriod = reconcilePeriod w.ManageStatus = tmp.ManageStatus w.WatchDependentResources = tmp.WatchDependentResources w.WatchClusterScopedResources = tmp.WatchClusterScopedResources w.Finalizer = tmp.Finalizer + w.AnsibleVerbosity = getAnsibleVerbosity(gvk, tmp.AnsibleVerbosity) return nil } +// Validate - ensures that a Watch is valid +// A Watch is considered valid if it: +// - Specifies a valid path to a Role||Playbook +// - If a Finalizer is non-nil, it must have a name + valid path to a Role||Playbook or Vars +func (w *Watch) Validate() error { + err := verifyAnsiblePath(w.Playbook, w.Role) + if err != nil { + log.Error(err, fmt.Sprintf("Invalid ansible path for GVK: %v", w.GroupVersionKind.String())) + return err + } + + if w.Finalizer != nil { + if w.Finalizer.Name == "" { + err = fmt.Errorf("finalizer must have name") + log.Error(err, fmt.Sprintf("Invalid finalizer for GVK: %v", w.GroupVersionKind.String())) + return err + } + // only fail if Vars not set + err = verifyAnsiblePath(w.Finalizer.Playbook, w.Finalizer.Role) + if err != nil && len(w.Finalizer.Vars) == 0 { + log.Error(err, fmt.Sprintf("Invalid ansible path on Finalizer for GVK: %v", w.GroupVersionKind.String())) + return err + } + } + + return nil +} + +// New - returns a Watch with sensible defaults. +// There is no guarantee that the returned Watch is valid (that Validate() will not return an error). +func New(gvk schema.GroupVersionKind) *Watch { + reconcilePeriod, _ := time.ParseDuration(reconcilePeriodDefault) + return &Watch{ + GroupVersionKind: gvk, + MaxRunnerArtifacts: maxRunnerArtifactsDefault, + MaxWorkers: maxWorkersDefault, + ReconcilePeriod: reconcilePeriod, + ManageStatus: manageStatusDefault, + WatchDependentResources: watchDependentResourcesDefault, + WatchClusterScopedResources: watchClusterScopedResourcesDefault, + AnsibleVerbosity: ansibleVerbosityDefault, + } +} + // Load - loads a slice of Watches from the watch file at `path`. -func Load(path string) ([]Watch, error) { - b, err := ioutil.ReadFile(path) +func Load(flags *ansibleflags.AnsibleOperatorFlags) ([]Watch, error) { + // set the defaults used when unmarshalling + maxWorkersDefault = flags.MaxWorkers + ansibleVerbosityDefault = flags.AnsibleVerbosity + + b, err := ioutil.ReadFile(flags.WatchesFile) if err != nil { log.Error(err, "Failed to get config file") return nil, err @@ -138,33 +198,17 @@ func Load(path string) ([]Watch, error) { watchesMap := make(map[schema.GroupVersionKind]bool) for _, watch := range watches { - // prevent dupes if _, ok := watchesMap[watch.GroupVersionKind]; ok { return nil, fmt.Errorf("duplicate GVK: %v", watch.GroupVersionKind.String()) } watchesMap[watch.GroupVersionKind] = true - err = verifyAnsiblePath(watch.Playbook, watch.Role) + err = watch.Validate() if err != nil { - log.Error(err, fmt.Sprintf("Invalid ansible path for GVK: %v", watch.GroupVersionKind.String())) + log.Error(err, fmt.Sprintf("Watch with GVK %v failed validation", watch.GroupVersionKind.String())) return nil, err } - - if watch.Finalizer != nil { - if watch.Finalizer.Name == "" { - err = fmt.Errorf("finalizer must have name") - log.Error(err, fmt.Sprintf("Invalid finalizer for GVK: %v", watch.GroupVersionKind.String())) - return nil, err - } - // only fail if Vars not set - err = verifyAnsiblePath(watch.Finalizer.Playbook, watch.Finalizer.Role) - if err != nil && len(watch.Finalizer.Vars) == 0 { - log.Error(err, fmt.Sprintf("Invalid ansible path on Finalizer for GVK: %v", watch.GroupVersionKind.String())) - return nil, err - } - } - } return watches, nil @@ -204,3 +248,49 @@ func verifyAnsiblePath(playbook string, role string) error { } return nil } + +// if the WORKER_* environment variable is set, use that value. +// Otherwise, use the value from the CLI. 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 +// configuration for the operator. +func getMaxWorkers(gvk schema.GroupVersionKind, defValue int) int { + envVar := strings.ToUpper(strings.Replace( + fmt.Sprintf("WORKER_%s_%s", gvk.Kind, gvk.Group), + ".", + "_", + -1, + )) + maxWorkers, err := strconv.Atoi(os.Getenv(envVar)) + if err != nil { + // we don't care why we couldn't parse it just use default + log.Info("Failed to parse %v from environment. Using default %v", envVar, defValue) + return defValue + } + + if maxWorkers <= 1 { + return 1 + } + return maxWorkers +} + +// this behaves sim +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 + } + + if ansibleVerbosity <= 0 { + return 0 + } + return ansibleVerbosity +} diff --git a/pkg/ansible/watches/watches_test.go b/pkg/ansible/watches/watches_test.go index 0ff02f7377..104ed60703 100644 --- a/pkg/ansible/watches/watches_test.go +++ b/pkg/ansible/watches/watches_test.go @@ -19,12 +19,56 @@ import ( "os" "path/filepath" "reflect" + "strconv" "testing" "time" "k8s.io/apimachinery/pkg/runtime/schema" + + ansibleflags "github.com/operator-framework/operator-sdk/pkg/ansible/flags" ) +func TestNew(t *testing.T) { + expectedGVK := schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "Example", + } + expectedReconcilePeriod, _ := time.ParseDuration(reconcilePeriodDefault) + + watch := New(expectedGVK) + if watch.GroupVersionKind != expectedGVK { + t.Fatalf("Unexpected GVK %v expected %v", watch.GroupVersionKind, expectedGVK) + } + if watch.MaxRunnerArtifacts != maxRunnerArtifactsDefault { + t.Fatalf("Unexpected maxRunnerArtifacts %v expected %v", watch.MaxRunnerArtifacts, maxRunnerArtifactsDefault) + } + if watch.MaxWorkers != maxWorkersDefault { + t.Fatalf("Unexpected maxWorkers %v expected %v", watch.MaxWorkers, maxWorkersDefault) + } + if watch.ReconcilePeriod != expectedReconcilePeriod { + t.Fatalf("Unexpected reconcilePeriod %v expected %v", watch.ReconcilePeriod, expectedReconcilePeriod) + } + if watch.ManageStatus != manageStatusDefault { + t.Fatalf("Unexpected manageStatus %v expected %v", watch.ManageStatus, manageStatusDefault) + } + if watch.WatchDependentResources != watchDependentResourcesDefault { + t.Fatalf("Unexpected watchDependentResources %v expected %v", watch.WatchDependentResources, watchDependentResourcesDefault) + } + if watch.WatchClusterScopedResources != watchClusterScopedResourcesDefault { + t.Fatalf("Unexpected watchClusterScopedResources %v expected %v", watch.WatchClusterScopedResources, watchClusterScopedResourcesDefault) + } + if watch.AnsibleVerbosity != ansibleVerbosityDefault { + t.Fatalf("Unexpected ansibleVerbosity %v expected %v", watch.AnsibleVerbosity, ansibleVerbosityDefault) + } + + // this should fail validate + err := watch.Validate() + if err == nil { + t.Fatalf("Watch %v should have failed validation", watch) + } +} + func TestLoad(t *testing.T) { cwd, err := os.Getwd() if err != nil { @@ -56,10 +100,15 @@ func TestLoad(t *testing.T) { zeroSeconds := time.Duration(0) twoSeconds := time.Second * 2 testCases := []struct { - name string - path string - expected []Watch - shouldError bool + name string + path string + maxWorkers int + ansibleVerbosity int + expected []Watch + setenvvar bool + envvar string + envvarValue int + shouldError bool }{ { name: "error duplicate GVK", @@ -112,8 +161,280 @@ func TestLoad(t *testing.T) { shouldError: true, }, { - name: "valid watches file", - path: "testdata/valid.yaml", + name: "valid watches file", + path: "testdata/valid.yaml", + maxWorkers: 1, + ansibleVerbosity: 2, + expected: []Watch{ + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "NoFinalizer", + }, + Playbook: validTemplate.ValidPlaybook, + ManageStatus: true, + ReconcilePeriod: twoSeconds, + WatchDependentResources: true, + WatchClusterScopedResources: false, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "Playbook", + }, + Playbook: validTemplate.ValidPlaybook, + ManageStatus: true, + WatchDependentResources: true, + WatchClusterScopedResources: false, + Finalizer: &Finalizer{ + Name: "finalizer.app.example.com", + Role: validTemplate.ValidRole, + Vars: map[string]interface{}{"sentinel": "finalizer_running"}, + }, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "WatchClusterScoped", + }, + Playbook: validTemplate.ValidPlaybook, + ReconcilePeriod: twoSeconds, + ManageStatus: true, + WatchDependentResources: true, + WatchClusterScopedResources: true, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "NoReconcile", + }, + Playbook: validTemplate.ValidPlaybook, + ReconcilePeriod: zeroSeconds, + ManageStatus: true, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "DefaultStatus", + }, + Playbook: validTemplate.ValidPlaybook, + ManageStatus: true, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "DisableStatus", + }, + Playbook: validTemplate.ValidPlaybook, + ManageStatus: false, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "EnableStatus", + }, + Playbook: validTemplate.ValidPlaybook, + ManageStatus: true, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "Role", + }, + Role: validTemplate.ValidRole, + ManageStatus: true, + Finalizer: &Finalizer{ + Name: "finalizer.app.example.com", + Playbook: validTemplate.ValidPlaybook, + Vars: map[string]interface{}{"sentinel": "finalizer_running"}, + }, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "FinalizerRole", + }, + Role: validTemplate.ValidRole, + ManageStatus: true, + Finalizer: &Finalizer{ + Name: "finalizer.app.example.com", + Vars: map[string]interface{}{"sentinel": "finalizer_running"}, + }, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "MaxWorkers", + }, + Role: validTemplate.ValidRole, + ManageStatus: true, + MaxWorkers: 3, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "AnsibleVerbosity", + }, + Role: validTemplate.ValidRole, + ManageStatus: true, + AnsibleVerbosity: 11, + }, + }, + }, + { + name: "valid watches file, worker env wins", + path: "testdata/valid.yaml", + maxWorkers: 1, + ansibleVerbosity: 2, + setenvvar: true, + envvar: "WORKER_MAXWORKERS_APP_EXAMPLE_COM", + envvarValue: 4, + expected: []Watch{ + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "NoFinalizer", + }, + Playbook: validTemplate.ValidPlaybook, + ManageStatus: true, + ReconcilePeriod: twoSeconds, + WatchDependentResources: true, + WatchClusterScopedResources: false, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "Playbook", + }, + Playbook: validTemplate.ValidPlaybook, + ManageStatus: true, + WatchDependentResources: true, + WatchClusterScopedResources: false, + Finalizer: &Finalizer{ + Name: "finalizer.app.example.com", + Role: validTemplate.ValidRole, + Vars: map[string]interface{}{"sentinel": "finalizer_running"}, + }, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "WatchClusterScoped", + }, + Playbook: validTemplate.ValidPlaybook, + ReconcilePeriod: twoSeconds, + ManageStatus: true, + WatchDependentResources: true, + WatchClusterScopedResources: true, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "NoReconcile", + }, + Playbook: validTemplate.ValidPlaybook, + ReconcilePeriod: zeroSeconds, + ManageStatus: true, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "DefaultStatus", + }, + Playbook: validTemplate.ValidPlaybook, + ManageStatus: true, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "DisableStatus", + }, + Playbook: validTemplate.ValidPlaybook, + ManageStatus: false, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "EnableStatus", + }, + Playbook: validTemplate.ValidPlaybook, + ManageStatus: true, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "Role", + }, + Role: validTemplate.ValidRole, + ManageStatus: true, + Finalizer: &Finalizer{ + Name: "finalizer.app.example.com", + Playbook: validTemplate.ValidPlaybook, + Vars: map[string]interface{}{"sentinel": "finalizer_running"}, + }, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "FinalizerRole", + }, + Role: validTemplate.ValidRole, + ManageStatus: true, + Finalizer: &Finalizer{ + Name: "finalizer.app.example.com", + Vars: map[string]interface{}{"sentinel": "finalizer_running"}, + }, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "MaxWorkers", + }, + Role: validTemplate.ValidRole, + ManageStatus: true, + MaxWorkers: 4, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "AnsibleVerbosity", + }, + Role: validTemplate.ValidRole, + ManageStatus: true, + AnsibleVerbosity: 11, + }, + }, + }, + { + name: "valid watches file, ansible env wins", + path: "testdata/valid.yaml", + maxWorkers: 1, + ansibleVerbosity: 2, + setenvvar: true, + envvar: "ANSIBLE_VERBOSITY_ANSIBLEVERBOSITY_APP_EXAMPLE_COM", + envvarValue: 4, expected: []Watch{ Watch{ GroupVersionKind: schema.GroupVersionKind{ @@ -206,30 +527,70 @@ func TestLoad(t *testing.T) { Vars: map[string]interface{}{"sentinel": "finalizer_running"}, }, }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "FinalizerRole", + }, + Role: validTemplate.ValidRole, + ManageStatus: true, + Finalizer: &Finalizer{ + Name: "finalizer.app.example.com", + Vars: map[string]interface{}{"sentinel": "finalizer_running"}, + }, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "MaxWorkers", + }, + Role: validTemplate.ValidRole, + ManageStatus: true, + MaxWorkers: 3, + }, + Watch{ + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1alpha1", + Group: "app.example.com", + Kind: "AnsibleVerbosity", + }, + Role: validTemplate.ValidRole, + ManageStatus: true, + AnsibleVerbosity: 4, + }, }, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - watchSlice, err := Load(tc.path) + if tc.setenvvar { + os.Setenv(tc.envvar, strconv.Itoa(tc.envvarValue)) + } + ansibleFlags := &ansibleflags.AnsibleOperatorFlags{ + MaxWorkers: tc.maxWorkers, + AnsibleVerbosity: tc.ansibleVerbosity, + } + ansibleFlags.WatchesFile = tc.path + watchSlice, err := Load(ansibleFlags) if err != nil && !tc.shouldError { t.Fatalf("Error occurred unexpectedly: %v", err) } if err != nil && tc.shouldError { return } + // meant to protect from adding test to valid without corresponding check + if len(tc.expected) != len(watchSlice) { + t.Fatalf("Unexpected watches length: %v expected: %v", len(watchSlice), len(tc.expected)) + } for idx, expectedWatch := range tc.expected { gvk := expectedWatch.GroupVersionKind gotWatch := watchSlice[idx] if gotWatch.GroupVersionKind != gvk { t.Fatalf("Unexpected GVK: \nunexpected GVK: %#v\nexpected GVK: %#v", gotWatch.GroupVersionKind, gvk) } - // TODO: not clear if this is needed or what the corollary would be without a struct behind an interface - // run, ok := r.(*runner) - // if !ok { - // t.Fatalf("Here: %#v", r) - // } if gotWatch.Role != expectedWatch.Role { t.Fatalf("The GVK: %v unexpected Role: %v expected Role: %v", gvk, gotWatch.Role, expectedWatch.Role) } @@ -247,6 +608,126 @@ func TestLoad(t *testing.T) { if gotWatch.ReconcilePeriod != expectedWatch.ReconcilePeriod { t.Fatalf("The GVK: %v unexpected reconcile period: %v expected reconcile period: %v", gvk, gotWatch.ReconcilePeriod, expectedWatch.ReconcilePeriod) } + + if expectedWatch.MaxWorkers == 0 { + if gotWatch.MaxWorkers != tc.maxWorkers { + t.Fatalf("Unexpected max workers: %v expected workers: %v", gotWatch.MaxWorkers, tc.maxWorkers) + } + } else { + if gotWatch.MaxWorkers != expectedWatch.MaxWorkers { + t.Fatalf("Unexpected max workers: %v expected workers: %v", gotWatch.MaxWorkers, expectedWatch.MaxWorkers) + } + } + if expectedWatch.AnsibleVerbosity == 0 { + if gotWatch.AnsibleVerbosity != tc.ansibleVerbosity { + t.Fatalf("Unexpected ansible verbosity: %v ansible verbosity: %v", gotWatch.AnsibleVerbosity, tc.ansibleVerbosity) + } + } else { + if gotWatch.AnsibleVerbosity != expectedWatch.AnsibleVerbosity { + t.Fatalf("Unexpected ansible verbosity: %v ansible verbosity: %v", gotWatch.AnsibleVerbosity, expectedWatch.AnsibleVerbosity) + } + } + } + if tc.setenvvar { + os.Unsetenv(tc.envvar) + } + }) + } +} + +func TestMaxWorkers(t *testing.T) { + testCases := []struct { + name string + gvk schema.GroupVersionKind + defvalue int + expected int + setenvvar bool + envvar string + }{ + { + name: "no env, use default value", + gvk: schema.GroupVersionKind{ + Group: "cache.example.com", + Version: "v1alpha1", + Kind: "MemCacheService", + }, + defvalue: 1, + expected: 1, + setenvvar: false, + envvar: "WORKER_MEMCACHESERVICE_CACHE_EXAMPLE_COM", + }, + { + name: "env set to 3, expect 3", + gvk: schema.GroupVersionKind{ + Group: "cache.example.com", + Version: "v1alpha1", + Kind: "MemCacheService", + }, + defvalue: 1, + expected: 3, + setenvvar: true, + envvar: "WORKER_MEMCACHESERVICE_CACHE_EXAMPLE_COM", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + os.Unsetenv(tc.envvar) + if tc.setenvvar { + os.Setenv(tc.envvar, strconv.Itoa(tc.expected)) + } + workers := getMaxWorkers(tc.gvk, tc.defvalue) + if tc.expected != workers { + t.Fatalf("Unexpected MaxWorkers: %v expected MaxWorkers: %v", workers, tc.expected) + } + }) + } +} + +func TestAnsibleVerbosity(t *testing.T) { + testCases := []struct { + name string + gvk schema.GroupVersionKind + defvalue int + expected int + setenvvar bool + envvar string + }{ + { + name: "no env, use default value", + gvk: schema.GroupVersionKind{ + Group: "cache.example.com", + Version: "v1alpha1", + Kind: "MemCacheService", + }, + defvalue: 1, + expected: 1, + setenvvar: false, + envvar: "ANSIBLE_VERBOSITY_MEMCACHESERVICE_CACHE_EXAMPLE_COM", + }, + { + name: "env set to 3, expect 3", + gvk: schema.GroupVersionKind{ + Group: "cache.example.com", + Version: "v1alpha1", + Kind: "MemCacheService", + }, + defvalue: 1, + expected: 3, + setenvvar: true, + envvar: "ANSIBLE_VERBOSITY_MEMCACHESERVICE_CACHE_EXAMPLE_COM", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + os.Unsetenv(tc.envvar) + if tc.setenvvar { + os.Setenv(tc.envvar, strconv.Itoa(tc.expected)) + } + verbosity := getAnsibleVerbosity(tc.gvk, tc.defvalue) + if tc.expected != verbosity { + t.Fatalf("Unexpected Verbosity: %v expected Verbosity: %v", verbosity, tc.expected) } }) } From 0d38e6b2436275ffc24476643e1f93c1ea430cbe Mon Sep 17 00:00:00 2001 From: David Zager Date: Wed, 21 Aug 2019 16:08:03 -0400 Subject: [PATCH 2/7] Add specific test for ansibleVerbosityString --- pkg/ansible/runner/runner_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/pkg/ansible/runner/runner_test.go b/pkg/ansible/runner/runner_test.go index 3de05014f0..a01dd9eb34 100644 --- a/pkg/ansible/runner/runner_test.go +++ b/pkg/ansible/runner/runner_test.go @@ -183,3 +183,22 @@ func TestNew(t *testing.T) { }) } } + +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"}, + } + + for _, tc := range testCases { + gotString := ansibleVerbosityString(tc.verbosity) + if tc.expectedString != gotString { + t.Fatalf("Unexpected string %v expected %v", gotString, tc.expectedString) + } + } +} From bc3a919957ee91a15bb8f34b47aa1aa57e7c7748 Mon Sep 17 00:00:00 2001 From: David Zager Date: Tue, 27 Aug 2019 09:38:34 -0400 Subject: [PATCH 3/7] Add timeout for metrics check --- ci/tests/e2e-ansible.sh | 2 +- hack/tests/e2e-ansible.sh | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ci/tests/e2e-ansible.sh b/ci/tests/e2e-ansible.sh index 3285fc9173..6ed7478ad5 100755 --- a/ci/tests/e2e-ansible.sh +++ b/ci/tests/e2e-ansible.sh @@ -104,7 +104,7 @@ test_operator() { fi # verify that metrics reflect cr creation - if ! bash -c -- "kubectl run --attach --rm --restart=Never test-metrics --image=$metrics_test_image -- curl http://memcached-operator-metrics:8686/metrics | grep example-memcached"; + if ! timeout 1m bash -c -- "until kubectl run -it --rm --restart=Never test-metrics --image=$metrics_test_image -- curl http://memcached-operator-metrics:8686/metrics | grep example-memcached; do sleep 1; done"; then echo "Failed to verify custom resource metrics" kubectl describe pods diff --git a/hack/tests/e2e-ansible.sh b/hack/tests/e2e-ansible.sh index e40d53b57e..c72823967c 100755 --- a/hack/tests/e2e-ansible.sh +++ b/hack/tests/e2e-ansible.sh @@ -57,7 +57,7 @@ test_operator() { if ! timeout 1m bash -c -- "until kubectl run --attach --rm --restart=Never test-metrics --image=$metrics_test_image -- curl -sfo /dev/null http://memcached-operator-metrics:8383/metrics; do sleep 1; done"; then echo "Failed to verify that metrics endpoint exists" - kubectl logs deployment/memcached-operator + kubectl logs deployment/memcached-operator -c operator exit 1 fi @@ -65,7 +65,7 @@ test_operator() { if ! timeout 1m bash -c -- "until kubectl run --attach --rm --restart=Never test-metrics --image=$metrics_test_image -- curl -sfo /dev/null http://memcached-operator-metrics:8686/metrics; do sleep 1; done"; then echo "Failed to verify that metrics endpoint exists" - kubectl logs deployment/memcached-operator + kubectl logs deployment/memcached-operator -c operator exit 1 fi @@ -80,10 +80,10 @@ test_operator() { fi # verify that metrics reflect cr creation - if ! bash -c -- "kubectl run --attach --rm --restart=Never test-metrics --image=$metrics_test_image -- curl http://memcached-operator-metrics:8686/metrics | grep example-memcached"; + if ! timeout 1m bash -c -- "until kubectl run -it --rm --restart=Never test-metrics --image=$metrics_test_image -- curl http://memcached-operator-metrics:8686/metrics | grep example-memcached; do sleep 1; done"; then echo "Failed to verify custom resource metrics" - kubectl logs deployment/memcached-operator + kubectl logs deployment/memcached-operator -c operator exit 1 fi memcached_deployment=$(kubectl get deployment -l app=memcached -o jsonpath="{..metadata.name}") From fb2c63fd331125ff06ecb5b07dd8bcdaed2f9e0b Mon Sep 17 00:00:00 2001 From: David Zager Date: Wed, 28 Aug 2019 16:13:45 -0400 Subject: [PATCH 4/7] No worker/verbosity config on watches yaml Prevent configuration of ansible verbosity or max worker from watches.yaml --- pkg/ansible/watches/testdata/valid.yaml.tmpl | 24 +- pkg/ansible/watches/watches.go | 22 +- pkg/ansible/watches/watches_test.go | 265 ++----------------- 3 files changed, 48 insertions(+), 263 deletions(-) diff --git a/pkg/ansible/watches/testdata/valid.yaml.tmpl b/pkg/ansible/watches/testdata/valid.yaml.tmpl index c463a01c4d..ba3628122b 100644 --- a/pkg/ansible/watches/testdata/valid.yaml.tmpl +++ b/pkg/ansible/watches/testdata/valid.yaml.tmpl @@ -57,11 +57,27 @@ sentinel: finalizer_running - version: v1alpha1 group: app.example.com - kind: MaxWorkers + kind: MaxWorkersDefault role: {{ .ValidRole }} - maxWorkers: 3 - version: v1alpha1 group: app.example.com - kind: AnsibleVerbosity + kind: MaxWorkersIgnored + role: {{ .ValidRole }} + maxWorkers: 5 +- version: v1alpha1 + 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 }} - ansibleVerbosity: 11 diff --git a/pkg/ansible/watches/watches.go b/pkg/ansible/watches/watches.go index 2a577ddc65..6452fda6c2 100644 --- a/pkg/ansible/watches/watches.go +++ b/pkg/ansible/watches/watches.go @@ -40,13 +40,15 @@ type Watch struct { Playbook string `yaml:"playbook"` Role string `yaml:"role"` MaxRunnerArtifacts int `yaml:"maxRunnerArtifacts"` - MaxWorkers int `yaml:"maxWorkers"` ReconcilePeriod time.Duration `yaml:"reconcilePeriod"` ManageStatus bool `yaml:"manageStatus"` WatchDependentResources bool `yaml:"watchDependentResources"` WatchClusterScopedResources bool `yaml:"watchClusterScopedResources"` - AnsibleVerbosity int `yaml:"ansibleVerbosity"` Finalizer *Finalizer `yaml:"finalizer"` + + // Not configurable via watches.yaml + MaxWorkers int `yaml:"maxWorkers"` + AnsibleVerbosity int `yaml:"ansibleVerbosity"` } // Finalizer - Expose finalizer to be used by a user. @@ -60,12 +62,14 @@ type Finalizer struct { // Default values for optional fields on Watch var ( maxRunnerArtifactsDefault = 20 - maxWorkersDefault = 1 //set by flags reconcilePeriodDefault = "0s" manageStatusDefault = true watchDependentResourcesDefault = true watchClusterScopedResourcesDefault = false - ansibleVerbosityDefault = 2 //set by flags + + // these are overridden by cmdline flags + maxWorkersDefault = 1 + ansibleVerbosityDefault = 2 ) // UnmarshalYAML - implements the yaml.Unmarshaler interface for Watch @@ -78,25 +82,21 @@ func (w *Watch) UnmarshalYAML(unmarshal func(interface{}) error) error { Playbook string `yaml:"playbook"` Role string `yaml:"role"` MaxRunnerArtifacts int `yaml:"maxRunnerArtifacts"` - MaxWorkers int `yaml:"maxWorkers"` ReconcilePeriod string `yaml:"reconcilePeriod"` ManageStatus bool `yaml:"manageStatus"` WatchDependentResources bool `yaml:"watchDependentResources"` WatchClusterScopedResources bool `yaml:"watchClusterScopedResources"` - AnsibleVerbosity int `yaml:"ansibleVerbosity"` Finalizer *Finalizer `yaml:"finalizer"` } var tmp alias // by default, the operator will manage status and watch dependent resources - // The operator will not manage cluster scoped resources by default. tmp.ManageStatus = manageStatusDefault + // the operator will not manage cluster scoped resources by default. tmp.WatchDependentResources = watchDependentResourcesDefault tmp.MaxRunnerArtifacts = maxRunnerArtifactsDefault - tmp.MaxWorkers = maxWorkersDefault tmp.ReconcilePeriod = reconcilePeriodDefault tmp.WatchClusterScopedResources = watchClusterScopedResourcesDefault - tmp.AnsibleVerbosity = ansibleVerbosityDefault if err := unmarshal(&tmp); err != nil { return err @@ -122,13 +122,13 @@ func (w *Watch) UnmarshalYAML(unmarshal func(interface{}) error) error { w.Playbook = tmp.Playbook w.Role = tmp.Role w.MaxRunnerArtifacts = tmp.MaxRunnerArtifacts - w.MaxWorkers = getMaxWorkers(gvk, tmp.MaxWorkers) + w.MaxWorkers = getMaxWorkers(gvk, maxWorkersDefault) w.ReconcilePeriod = reconcilePeriod w.ManageStatus = tmp.ManageStatus w.WatchDependentResources = tmp.WatchDependentResources w.WatchClusterScopedResources = tmp.WatchClusterScopedResources w.Finalizer = tmp.Finalizer - w.AnsibleVerbosity = getAnsibleVerbosity(gvk, tmp.AnsibleVerbosity) + w.AnsibleVerbosity = getAnsibleVerbosity(gvk, ansibleVerbosityDefault) return nil } diff --git a/pkg/ansible/watches/watches_test.go b/pkg/ansible/watches/watches_test.go index 104ed60703..c6945a54b4 100644 --- a/pkg/ansible/watches/watches_test.go +++ b/pkg/ansible/watches/watches_test.go @@ -274,142 +274,27 @@ func TestLoad(t *testing.T) { GroupVersionKind: schema.GroupVersionKind{ Version: "v1alpha1", Group: "app.example.com", - Kind: "MaxWorkers", + Kind: "MaxWorkersDefault", }, Role: validTemplate.ValidRole, ManageStatus: true, - MaxWorkers: 3, + MaxWorkers: 1, }, Watch{ GroupVersionKind: schema.GroupVersionKind{ Version: "v1alpha1", Group: "app.example.com", - Kind: "AnsibleVerbosity", - }, - Role: validTemplate.ValidRole, - ManageStatus: true, - AnsibleVerbosity: 11, - }, - }, - }, - { - name: "valid watches file, worker env wins", - path: "testdata/valid.yaml", - maxWorkers: 1, - ansibleVerbosity: 2, - setenvvar: true, - envvar: "WORKER_MAXWORKERS_APP_EXAMPLE_COM", - envvarValue: 4, - expected: []Watch{ - Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1alpha1", - Group: "app.example.com", - Kind: "NoFinalizer", - }, - Playbook: validTemplate.ValidPlaybook, - ManageStatus: true, - ReconcilePeriod: twoSeconds, - WatchDependentResources: true, - WatchClusterScopedResources: false, - }, - Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1alpha1", - Group: "app.example.com", - Kind: "Playbook", - }, - Playbook: validTemplate.ValidPlaybook, - ManageStatus: true, - WatchDependentResources: true, - WatchClusterScopedResources: false, - Finalizer: &Finalizer{ - Name: "finalizer.app.example.com", - Role: validTemplate.ValidRole, - Vars: map[string]interface{}{"sentinel": "finalizer_running"}, - }, - }, - Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1alpha1", - Group: "app.example.com", - Kind: "WatchClusterScoped", - }, - Playbook: validTemplate.ValidPlaybook, - ReconcilePeriod: twoSeconds, - ManageStatus: true, - WatchDependentResources: true, - WatchClusterScopedResources: true, - }, - Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1alpha1", - Group: "app.example.com", - Kind: "NoReconcile", - }, - Playbook: validTemplate.ValidPlaybook, - ReconcilePeriod: zeroSeconds, - ManageStatus: true, - }, - Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1alpha1", - Group: "app.example.com", - Kind: "DefaultStatus", - }, - Playbook: validTemplate.ValidPlaybook, - ManageStatus: true, - }, - Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1alpha1", - Group: "app.example.com", - Kind: "DisableStatus", - }, - Playbook: validTemplate.ValidPlaybook, - ManageStatus: false, - }, - Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1alpha1", - Group: "app.example.com", - Kind: "EnableStatus", - }, - Playbook: validTemplate.ValidPlaybook, - ManageStatus: true, - }, - Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1alpha1", - Group: "app.example.com", - Kind: "Role", + Kind: "MaxWorkersIgnored", }, Role: validTemplate.ValidRole, ManageStatus: true, - Finalizer: &Finalizer{ - Name: "finalizer.app.example.com", - Playbook: validTemplate.ValidPlaybook, - Vars: map[string]interface{}{"sentinel": "finalizer_running"}, - }, + MaxWorkers: 1, }, Watch{ GroupVersionKind: schema.GroupVersionKind{ Version: "v1alpha1", Group: "app.example.com", - Kind: "FinalizerRole", - }, - Role: validTemplate.ValidRole, - ManageStatus: true, - Finalizer: &Finalizer{ - Name: "finalizer.app.example.com", - Vars: map[string]interface{}{"sentinel": "finalizer_running"}, - }, - }, - Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1alpha1", - Group: "app.example.com", - Kind: "MaxWorkers", + Kind: "MaxWorkersEnv", }, Role: validTemplate.ValidRole, ManageStatus: true, @@ -419,142 +304,27 @@ func TestLoad(t *testing.T) { GroupVersionKind: schema.GroupVersionKind{ Version: "v1alpha1", Group: "app.example.com", - Kind: "AnsibleVerbosity", + Kind: "AnsibleVerbosityDefault", }, Role: validTemplate.ValidRole, ManageStatus: true, - AnsibleVerbosity: 11, - }, - }, - }, - { - name: "valid watches file, ansible env wins", - path: "testdata/valid.yaml", - maxWorkers: 1, - ansibleVerbosity: 2, - setenvvar: true, - envvar: "ANSIBLE_VERBOSITY_ANSIBLEVERBOSITY_APP_EXAMPLE_COM", - envvarValue: 4, - expected: []Watch{ - Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1alpha1", - Group: "app.example.com", - Kind: "NoFinalizer", - }, - Playbook: validTemplate.ValidPlaybook, - ManageStatus: true, - ReconcilePeriod: twoSeconds, - WatchDependentResources: true, - WatchClusterScopedResources: false, - }, - Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1alpha1", - Group: "app.example.com", - Kind: "Playbook", - }, - Playbook: validTemplate.ValidPlaybook, - ManageStatus: true, - WatchDependentResources: true, - WatchClusterScopedResources: false, - Finalizer: &Finalizer{ - Name: "finalizer.app.example.com", - Role: validTemplate.ValidRole, - Vars: map[string]interface{}{"sentinel": "finalizer_running"}, - }, + AnsibleVerbosity: 2, }, Watch{ GroupVersionKind: schema.GroupVersionKind{ Version: "v1alpha1", Group: "app.example.com", - Kind: "WatchClusterScoped", + Kind: "AnsibleVerbosityIgnored", }, - Playbook: validTemplate.ValidPlaybook, - ReconcilePeriod: twoSeconds, - ManageStatus: true, - WatchDependentResources: true, - WatchClusterScopedResources: true, - }, - Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1alpha1", - Group: "app.example.com", - Kind: "NoReconcile", - }, - Playbook: validTemplate.ValidPlaybook, - ReconcilePeriod: zeroSeconds, - ManageStatus: true, - }, - Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1alpha1", - Group: "app.example.com", - Kind: "DefaultStatus", - }, - Playbook: validTemplate.ValidPlaybook, - ManageStatus: true, - }, - Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1alpha1", - Group: "app.example.com", - Kind: "DisableStatus", - }, - Playbook: validTemplate.ValidPlaybook, - ManageStatus: false, - }, - Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1alpha1", - Group: "app.example.com", - Kind: "EnableStatus", - }, - Playbook: validTemplate.ValidPlaybook, - ManageStatus: true, - }, - Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1alpha1", - Group: "app.example.com", - Kind: "Role", - }, - Role: validTemplate.ValidRole, - ManageStatus: true, - Finalizer: &Finalizer{ - Name: "finalizer.app.example.com", - Playbook: validTemplate.ValidPlaybook, - Vars: map[string]interface{}{"sentinel": "finalizer_running"}, - }, - }, - Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1alpha1", - Group: "app.example.com", - Kind: "FinalizerRole", - }, - Role: validTemplate.ValidRole, - ManageStatus: true, - Finalizer: &Finalizer{ - Name: "finalizer.app.example.com", - Vars: map[string]interface{}{"sentinel": "finalizer_running"}, - }, - }, - Watch{ - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1alpha1", - Group: "app.example.com", - Kind: "MaxWorkers", - }, - Role: validTemplate.ValidRole, - ManageStatus: true, - MaxWorkers: 3, + Role: validTemplate.ValidRole, + ManageStatus: true, + AnsibleVerbosity: 2, }, Watch{ GroupVersionKind: schema.GroupVersionKind{ Version: "v1alpha1", Group: "app.example.com", - Kind: "AnsibleVerbosity", + Kind: "AnsibleVerbosityEnv", }, Role: validTemplate.ValidRole, ManageStatus: true, @@ -564,11 +334,13 @@ func TestLoad(t *testing.T) { }, } + os.Setenv("WORKER_MAXWORKERSENV_APP_EXAMPLE_COM", "4") + defer os.Unsetenv("WORKER_MAXWORKERSENV_APP_EXAMPLE_COM") + os.Setenv("ANSIBLE_VERBOSITY_ANSIBLEVERBOSITYENV_APP_EXAMPLE_COM", "4") + defer os.Unsetenv("ANSIBLE_VERBOSITY_ANSIBLEVERBOSITYENV_APP_EXAMPLE_COM") + for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - if tc.setenvvar { - os.Setenv(tc.envvar, strconv.Itoa(tc.envvarValue)) - } ansibleFlags := &ansibleflags.AnsibleOperatorFlags{ MaxWorkers: tc.maxWorkers, AnsibleVerbosity: tc.ansibleVerbosity, @@ -628,9 +400,6 @@ func TestLoad(t *testing.T) { } } } - if tc.setenvvar { - os.Unsetenv(tc.envvar) - } }) } } From 18d1e6bfefc5d9a90be650ffb8f575b90d87ff36 Mon Sep 17 00:00:00 2001 From: David Zager Date: Mon, 23 Sep 2019 12:01:02 -0400 Subject: [PATCH 5/7] Remove ansible run_test Since there are no tests added to the file, remove the file so it's obvious there are no tests for the Ansible operator's run function. --- pkg/ansible/run_test.go | 17 ----------------- 1 file changed, 17 deletions(-) delete mode 100644 pkg/ansible/run_test.go diff --git a/pkg/ansible/run_test.go b/pkg/ansible/run_test.go deleted file mode 100644 index cc9697c4f9..0000000000 --- a/pkg/ansible/run_test.go +++ /dev/null @@ -1,17 +0,0 @@ -// Copyright 2018 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 ansible - -// TODO: add a test for the Run method From 1ab748ecd323a48d243ac754b614e4d4eb84ead2 Mon Sep 17 00:00:00 2001 From: David Zager Date: Thu, 10 Oct 2019 14:31:35 -0400 Subject: [PATCH 6/7] Don't allow ansible verbosity greater than 7 --- pkg/ansible/watches/watches.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/ansible/watches/watches.go b/pkg/ansible/watches/watches.go index 6452fda6c2..4ca3efcc7f 100644 --- a/pkg/ansible/watches/watches.go +++ b/pkg/ansible/watches/watches.go @@ -292,5 +292,9 @@ func getAnsibleVerbosity(gvk schema.GroupVersionKind, defValue int) int { if ansibleVerbosity <= 0 { return 0 } + // verbosity > 7 doesn't make sense + if ansibleVerbosity > 7 { + return 7 + } return ansibleVerbosity } From 532ebb705ce8856efc05ae64daba426c0a2d0ad8 Mon Sep 17 00:00:00 2001 From: David Zager Date: Thu, 10 Oct 2019 14:31:04 -0400 Subject: [PATCH 7/7] Add documentation --- CHANGELOG.md | 1 + pkg/ansible/watches/_doc.go | 3 +++ pkg/ansible/watches/watches.go | 20 +++++++++++++------- 3 files changed, 17 insertions(+), 7 deletions(-) create mode 100644 pkg/ansible/watches/_doc.go diff --git a/CHANGELOG.md b/CHANGELOG.md index b1c490f1b8..63499c8c72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Ansible based operators now gather and serve metrics about each custom resource on port 8686 of the metrics service. ([#1723](https://github.com/operator-framework/operator-sdk/pull/1723)) - Added the Go version, OS, and architecture to the output of `operator-sdk version` ([#1863](https://github.com/operator-framework/operator-sdk/pull/1863)) - Added support for `ppc64le-linux` for the `operator-sdk` binary and the Helm operator base image. ([#1533](https://github.com/operator-framework/operator-sdk/pull/1533)) +- Ansible based operators now support verbosity configuration via the `ansible-verbosity` flag at the command line. ([#1852](https://github.com/operator-framework/operator-sdk/pull/1852)) ### Changed diff --git a/pkg/ansible/watches/_doc.go b/pkg/ansible/watches/_doc.go new file mode 100644 index 0000000000..c1ef25f5d9 --- /dev/null +++ b/pkg/ansible/watches/_doc.go @@ -0,0 +1,3 @@ +// Package watches provides the structures and functions for mapping a +// GroupVersionKind to an Ansible playbook or role. +package watches diff --git a/pkg/ansible/watches/watches.go b/pkg/ansible/watches/watches.go index 4ca3efcc7f..3e2af2c0c2 100644 --- a/pkg/ansible/watches/watches.go +++ b/pkg/ansible/watches/watches.go @@ -72,7 +72,10 @@ var ( ansibleVerbosityDefault = 2 ) -// UnmarshalYAML - implements the yaml.Unmarshaler interface for Watch +// UnmarshalYAML - implements the yaml.Unmarshaler interface for Watch. +// This makes it possible to verify, when loading, that the GroupVersionKind +// specified for a given watch is valid as well as provide sensible defaults +// for values that were ommitted. func (w *Watch) UnmarshalYAML(unmarshal func(interface{}) error) error { // Use an alias struct to handle complex types type alias struct { @@ -162,7 +165,7 @@ func (w *Watch) Validate() error { } // New - returns a Watch with sensible defaults. -// There is no guarantee that the returned Watch is valid (that Validate() will not return an error). +// The returned Watch is not valid as it does not include a role or playbook. func New(gvk schema.GroupVersionKind) *Watch { reconcilePeriod, _ := time.ParseDuration(reconcilePeriodDefault) return &Watch{ @@ -177,7 +180,7 @@ func New(gvk schema.GroupVersionKind) *Watch { } } -// Load - loads a slice of Watches from the watch file at `path`. +// Load - loads a slice of Watches from the watches file from the CLI func Load(flags *ansibleflags.AnsibleOperatorFlags) ([]Watch, error) { // set the defaults used when unmarshalling maxWorkersDefault = flags.MaxWorkers @@ -214,10 +217,11 @@ func Load(flags *ansibleflags.AnsibleOperatorFlags) ([]Watch, error) { return watches, nil } +// verify that a given GroupVersionKind has a Version and Kind +// A GVK without a group is valid. Certain scenarios may cause a GVK +// without a group to fail in other ways later in the initialization +// process. func verifyGVK(gvk schema.GroupVersionKind) error { - // A GVK without a group is valid. Certain scenarios may cause a GVK - // without a group to fail in other ways later in the initialization - // process. if gvk.Version == "" { return errors.New("version must not be empty") } @@ -227,6 +231,7 @@ func verifyGVK(gvk schema.GroupVersionKind) error { return nil } +// verify that a valid path is specified for a given role or playbook func verifyAnsiblePath(playbook string, role string) error { switch { case playbook != "": @@ -275,7 +280,8 @@ func getMaxWorkers(gvk schema.GroupVersionKind, defValue int) int { return maxWorkers } -// this behaves sim +// if the WORKER_* environment variable is set, use that value. +// Otherwise, use the value from the CLI. func getAnsibleVerbosity(gvk schema.GroupVersionKind, defValue int) int { envVar := strings.ToUpper(strings.Replace( fmt.Sprintf("ANSIBLE_VERBOSITY_%s_%s", gvk.Kind, gvk.Group),