Skip to content

Commit

Permalink
ansible/helm: Rename max-workers to max-concurrent-reconciles
Browse files Browse the repository at this point in the history
This commit:
- Adds --max-concurrent-reconciles flag to helm binary
- Rename MaxWorkers to MaxConcurrentReconciles in ansible/helm
  implementations.
  • Loading branch information
varshaprasad96 committed Jul 17, 2020
1 parent 3402111 commit 2198a3a
Show file tree
Hide file tree
Showing 9 changed files with 60 additions and 54 deletions.
4 changes: 4 additions & 0 deletions changelog/fragments/replace-max-workers.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
entries:
- description: >
Add "--max-concurrent-reconciles" flag to helm binary.
kind: "addition"
14 changes: 7 additions & 7 deletions cmd/ansible-operator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,13 @@ func main() {
}

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
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
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
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
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
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")

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
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
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.",
)
}

0 comments on commit 2198a3a

Please sign in to comment.