Skip to content

ansible/helm: Rename max-workers to max-concurrent-reconciles #3452

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
varshaprasad96 merged 3 commits into from
Jul 20, 2020
Merged

ansible/helm: Rename max-workers to max-concurrent-reconciles #3452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d9f5e0a
ansible/helm: Rename max-workers to max-concurrent-reconciles
varshaprasad96 Jul 17, 2020
60c9ca2
Modify changelog fragment
varshaprasad96 Jul 17, 2020
8625124
update relevant helm doc
varshaprasad96 Jul 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions changelog/fragments/rename-max-workers.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
entries:
- description: >
**(Valid only for Ansible/Helm-based Operators)**. The flag `--max-workers` is no longer supported.
Use `--max-concurrent-reconciles` instead.
kind: "change"
breaking: true
migration:
header: Flag `max-workers` is no longer supported.
body: |
The flag `--max-workers` is no longer supported with ansible/helm binaries. It has been
renamed to `--max-concurrent-reconciles`.

14 changes: 7 additions & 7 deletions cmd/ansible-operator/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,13 @@ func main() {
}
Copy link
Copy Markdown
Contributor

@camilamacedo86 camilamacedo86 Jul 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing update the helm doc: https://github.com/operator-framework/operator-sdk/blob/master/website/content/en/docs/building-operators/helm/reference/advanced_features.md#changing-the-concurrent-worker-count

Also, could you please replace its example for to use the new layout? We have no longer the exec-entrypoint.

to use it we need to update the config/manager/manager.yaml with or run $ helm-operator --max-concurrent-reconciles=3

 spec:
      containers:
      - image: {{ .Image }}
        args:
        - "--enable-leader-election"
        - "--max-concurrent-reconciles=3"
        name: manager

Copy link
Copy Markdown
Member

@joelanford joelanford Jul 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@varshaprasad96 @camilamacedo86

This section was pre-emptively updated and wordsmith-ed here:

https://github.com/operator-framework/operator-sdk/pull/3326/files#diff-47162e7ecac88f3c6f2c91e49dcd7757R91-R105

It would be better to make that update in this PR, so feel free to steal that section.

Copy link
Copy Markdown
Contributor

@camilamacedo86 camilamacedo86 Jul 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merged the helm doc to help its updated.
PS.: I update the doc and forgot :-) tks

Copy link
Copy Markdown
Member Author

@varshaprasad96 varshaprasad96 Jul 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made a small addition to the existing doc.


ctr := controller.Add(mgr, controller.Options{
GVK: w.GroupVersionKind,
Runner: runner,
ManageStatus: w.ManageStatus,
AnsibleDebugLogs: getAnsibleDebugLog(),
MaxWorkers: w.MaxWorkers,
ReconcilePeriod: w.ReconcilePeriod,
Selector: w.Selector,
GVK: w.GroupVersionKind,
Runner: runner,
ManageStatus: w.ManageStatus,
AnsibleDebugLogs: getAnsibleDebugLog(),
MaxConcurrentReconciles: w.MaxConcurrentReconciles,
ReconcilePeriod: w.ReconcilePeriod,
Selector: w.Selector,
})
if ctr == nil {
log.Error(fmt.Errorf("failed to add controller for GVK %v", w.GroupVersionKind.String()), "")
Expand Down
2 changes: 1 addition & 1 deletion cmd/helm-operator/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func main() {
ReconcilePeriod: f.ReconcilePeriod,
WatchDependentResources: *w.WatchDependentResources,
OverrideValues: w.OverrideValues,
MaxWorkers: f.MaxWorkers,
MaxConcurrentReconciles: f.MaxConcurrentReconciles,
})
if err != nil {
log.Error(err, "Failed to add manager factory to controller.")
Expand Down
4 changes: 2 additions & 2 deletions pkg/ansible/controller/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ type Options struct {
AnsibleDebugLogs bool
WatchDependentResources bool
WatchClusterScopedResources bool
MaxWorkers int
MaxConcurrentReconciles int
Selector metav1.LabelSelector
}

Expand Down Expand Up @@ -90,7 +90,7 @@ func Add(mgr manager.Manager, options Options) *controller.Controller {
c, err := controller.New(fmt.Sprintf("%v-controller", strings.ToLower(options.GVK.Kind)), mgr,
controller.Options{
Reconciler: aor,
MaxConcurrentReconciles: options.MaxWorkers,
MaxConcurrentReconciles: options.MaxConcurrentReconciles,
})
if err != nil {
log.Error(err, "")
Expand Down
6 changes: 3 additions & 3 deletions pkg/ansible/watches/testdata/valid.yaml.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,16 @@
sentinel: finalizer_running
- version: v1alpha1
group: app.example.com
kind: MaxWorkersDefault
kind: MaxConcurrentReconcilesDefault
role: {{ .ValidRole }}
- version: v1alpha1
group: app.example.com
kind: MaxWorkersIgnored
kind: MaxConcurrentReconcilesIgnored
role: {{ .ValidRole }}
maxWorkers: 5
- version: v1alpha1
group: app.example.com
kind: MaxWorkersEnv
kind: MaxConcurrentReconcilesEnv
role: {{ .ValidRole }}
- version: v1alpha1
group: app.example.com
Expand Down
8 changes: 4 additions & 4 deletions pkg/ansible/watches/watches.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ type Watch struct {
Selector metav1.LabelSelector `yaml:"selector"`

// Not configurable via watches.yaml
MaxWorkers int `yaml:"-"`
AnsibleVerbosity int `yaml:"-"`
MaxConcurrentReconciles int `yaml:"-"`
AnsibleVerbosity int `yaml:"-"`
}

// Finalizer - Expose finalizer to be used by a user.
Expand Down Expand Up @@ -179,7 +179,7 @@ func (w *Watch) setValuesFromAlias(tmp alias) error {
w.Role = tmp.Role
w.Vars = tmp.Vars
w.MaxRunnerArtifacts = tmp.MaxRunnerArtifacts
w.MaxWorkers = getMaxConcurrentReconciles(gvk, maxConcurrentReconcilesDefault)
w.MaxConcurrentReconciles = getMaxConcurrentReconciles(gvk, maxConcurrentReconcilesDefault)
w.ReconcilePeriod = tmp.ReconcilePeriod.Duration
w.ManageStatus = *tmp.ManageStatus
w.WatchDependentResources = *tmp.WatchDependentResources
Expand Down Expand Up @@ -306,7 +306,7 @@ func New(gvk schema.GroupVersionKind, role, playbook string, vars map[string]int
Role: role,
Vars: vars,
MaxRunnerArtifacts: maxRunnerArtifactsDefault,
MaxWorkers: maxConcurrentReconcilesDefault,
MaxConcurrentReconciles: maxConcurrentReconcilesDefault,
ReconcilePeriod: reconcilePeriodDefault.Duration,
ManageStatus: manageStatusDefault,
WatchDependentResources: watchDependentResourcesDefault,
Expand Down
60 changes: 31 additions & 29 deletions pkg/ansible/watches/watches_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,9 @@ func TestNew(t *testing.T) {
t.Fatalf("Unexpected maxRunnerArtifacts %v expected %v", watch.MaxRunnerArtifacts,
maxRunnerArtifactsDefault)
}
if watch.MaxWorkers != maxConcurrentReconcilesDefault {
t.Fatalf("Unexpected maxWorkers %v expected %v", watch.MaxWorkers, maxConcurrentReconcilesDefault)
if watch.MaxConcurrentReconciles != maxConcurrentReconcilesDefault {
t.Fatalf("Unexpected maxConcurrentReconciles %v expected %v", watch.MaxConcurrentReconciles,
maxConcurrentReconcilesDefault)
}
if watch.ReconcilePeriod != expectedReconcilePeriod {
t.Fatalf("Unexpected reconcilePeriod %v expected %v", watch.ReconcilePeriod,
Expand Down Expand Up @@ -246,31 +247,31 @@ func TestLoad(t *testing.T) {
GroupVersionKind: schema.GroupVersionKind{
Version: "v1alpha1",
Group: "app.example.com",
Kind: "MaxWorkersDefault",
Kind: "MaxConcurrentReconcilesDefault",
},
Role: validTemplate.ValidRole,
ManageStatus: true,
MaxWorkers: 1,
Role: validTemplate.ValidRole,
ManageStatus: true,
MaxConcurrentReconciles: 1,
},
Watch{
GroupVersionKind: schema.GroupVersionKind{
Version: "v1alpha1",
Group: "app.example.com",
Kind: "MaxWorkersIgnored",
Kind: "MaxConcurrentReconcilesIgnored",
},
Role: validTemplate.ValidRole,
ManageStatus: true,
MaxWorkers: 1,
Role: validTemplate.ValidRole,
ManageStatus: true,
MaxConcurrentReconciles: 1,
},
Watch{
GroupVersionKind: schema.GroupVersionKind{
Version: "v1alpha1",
Group: "app.example.com",
Kind: "MaxWorkersEnv",
Kind: "MaxConcurrentReconcilesEnv",
},
Role: validTemplate.ValidRole,
ManageStatus: true,
MaxWorkers: 4,
Role: validTemplate.ValidRole,
ManageStatus: true,
MaxConcurrentReconciles: 4,
},
Watch{
GroupVersionKind: schema.GroupVersionKind{
Expand Down Expand Up @@ -373,7 +374,7 @@ func TestLoad(t *testing.T) {
testCases := []struct {
name string
path string
maxWorkers int
maxConcurrentReconciles int
ansibleVerbosity int
expected []Watch
shouldError bool
Expand Down Expand Up @@ -453,24 +454,24 @@ func TestLoad(t *testing.T) {
{
name: "valid watches file",
path: "testdata/valid.yaml",
maxWorkers: 1,
maxConcurrentReconciles: 1,
ansibleVerbosity: 2,
shouldSetAnsibleCollectionPathEnvVar: true,
expected: validWatches,
},
{
name: "should load file successfully with ANSIBLE ROLES PATH ENV VAR set",
path: "testdata/valid.yaml",
maxWorkers: 1,
maxConcurrentReconciles: 1,
ansibleVerbosity: 2,
shouldSetAnsibleRolePathEnvVar: true,
shouldSetAnsibleCollectionPathEnvVar: true,
expected: validWatches,
},
}

os.Setenv("WORKER_MAXWORKERSENV_APP_EXAMPLE_COM", "4")
defer os.Unsetenv("WORKER_MAXWORKERSENV_APP_EXAMPLE_COM")
os.Setenv("WORKER_MAXCONCURRENTRECONCILESENV_APP_EXAMPLE_COM", "4")
defer os.Unsetenv("WORKER_MAXCONCURRENTRECONCILESENV_APP_EXAMPLE_COM")
os.Setenv("ANSIBLE_VERBOSITY_ANSIBLEVERBOSITYENV_APP_EXAMPLE_COM", "4")
defer os.Unsetenv("ANSIBLE_VERBOSITY_ANSIBLEVERBOSITYENV_APP_EXAMPLE_COM")
Comment thread
joelanford marked this conversation as resolved.

Expand All @@ -490,7 +491,7 @@ func TestLoad(t *testing.T) {
defer os.Unsetenv("ANSIBLE_COLLECTIONS_PATH")
}

watchSlice, err := Load(tc.path, tc.maxWorkers, tc.ansibleVerbosity)
watchSlice, err := Load(tc.path, tc.maxConcurrentReconciles, tc.ansibleVerbosity)
if err != nil && !tc.shouldError {
t.Fatalf("Error occurred unexpectedly: %v", err)
}
Expand Down Expand Up @@ -546,23 +547,23 @@ func TestLoad(t *testing.T) {
gotWatch.Selector, expectedWatch.Selector)
}

if expectedWatch.MaxWorkers == 0 {
if gotWatch.MaxWorkers != tc.maxWorkers {
t.Fatalf("Unexpected max workers: %v expected workers: %v", gotWatch.MaxWorkers,
tc.maxWorkers)
if expectedWatch.MaxConcurrentReconciles == 0 {
if gotWatch.MaxConcurrentReconciles != tc.maxConcurrentReconciles {
t.Fatalf("Unexpected max workers: %v expected workers: %v", gotWatch.MaxConcurrentReconciles,
tc.maxConcurrentReconciles)
}
} else {
if gotWatch.MaxWorkers != expectedWatch.MaxWorkers {
t.Fatalf("Unexpected max workers: %v expected workers: %v", gotWatch.MaxWorkers,
expectedWatch.MaxWorkers)
if gotWatch.MaxConcurrentReconciles != expectedWatch.MaxConcurrentReconciles {
t.Fatalf("Unexpected max workers: %v expected workers: %v", gotWatch.MaxConcurrentReconciles,
expectedWatch.MaxConcurrentReconciles)
}
}
}
})
}
}

func TestMaxWorkers(t *testing.T) {
func TestMaxConcurrentReconciles(t *testing.T) {
testCases := []struct {
name string
gvk schema.GroupVersionKind
Expand Down Expand Up @@ -654,7 +655,8 @@ func TestMaxWorkers(t *testing.T) {
}
workers := getMaxConcurrentReconciles(tc.gvk, tc.defValue)
if tc.expectedValue != workers {
t.Fatalf("Unexpected MaxWorkers: %v expected MaxWorkers: %v", workers, tc.expectedValue)
t.Fatalf("Unexpected MaxConcurrentReconciles: %v expected MaxConcurrentReconciles: %v",
workers, tc.expectedValue)
}
})
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/helm/controller/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ type WatchOptions struct {
ReconcilePeriod time.Duration
WatchDependentResources bool
OverrideValues map[string]string
MaxWorkers int
MaxConcurrentReconciles int
}

// Add creates a new helm operator controller and adds it to the manager
Expand All @@ -71,7 +71,7 @@ func Add(mgr manager.Manager, options WatchOptions) error {

c, err := controller.New(controllerName, mgr, controller.Options{
Reconciler: r,
MaxConcurrentReconciles: options.MaxWorkers,
MaxConcurrentReconciles: options.MaxConcurrentReconciles,
})
if err != nil {
return err
Expand Down
12 changes: 6 additions & 6 deletions pkg/helm/flags/flag.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ import (
type Flags struct {
ReconcilePeriod time.Duration
WatchesFile string
MaxWorkers int
MetricsAddress string
EnableLeaderElection bool
LeaderElectionID string
LeaderElectionNamespace string
MaxConcurrentReconciles int
}

// AddTo - Add the helm operator flags to the the flagset
Expand All @@ -47,11 +47,6 @@ func (f *Flags) AddTo(flagSet *pflag.FlagSet) {
"./watches.yaml",
"Path to the watches file to use",
)
flagSet.IntVar(&f.MaxWorkers,
"max-workers",
runtime.NumCPU(),
"Maximum number of workers to use",
)
flagSet.StringVar(&f.MetricsAddress,
"metrics-addr",
":8080",
Expand All @@ -72,4 +67,9 @@ func (f *Flags) AddTo(flagSet *pflag.FlagSet) {
"",
"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",
runtime.NumCPU(),
"Maximum number of concurrent reconciles for controllers.",
)
}
5 changes: 5 additions & 0 deletions website/content/en/docs/building-operators/helm/reference/advanced_features.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ $ cat config/manager/manager.yaml
...
```

While running locally, this flag can also be added to the helm binary. For example, running `helm-operator` binary with the above mentioned flag would give us a similar result:
```
helm-operator --max-concurrent-reconciles=10
```

**NOTE**: If you're using the default scaffolding, it is necessary to also apply this change to the `config/default/manager_auth_proxy_patch.yaml` file. This file is a `kustomize` patch to the operator deployment that configures [kube-rbac-proxy][kube-rbac-proxy] to require authorization for accessing your operator metrics. When `kustomize` applies this patch, it overrides the args defined in `config/manager/manager.yaml`

## Use `helm upgrade --force` for deployment
Expand Down