Misbehavior of the controller config in manager options

[zqzten]

Currently some of the options in Controller of manager.Options does not take effect as a global configuration for all controllers of the manager, such as MaxConcurrentReconciles and NeedLeaderElection.

For example, if we do the below configuration on new manager:

mgr, err := ctrl.NewManager(cfg, ctrl.Options{
    // other options omitted...
    Controller: config.Controller{
        MaxConcurrentReconciles: 8,
    },
})

We would expect the max concurrent reconciles of every controller managed by this manager would be 8 if not specified in GroupKindConcurrency. But in reality, this configuration takes no effect and the max concurrent reconciles of controllers are still defaulted to 1.

After some code digging, I found the global configuration of controllers under the manager is somehow confusing - some of them are set in controller.NewUnmanaged (such as RecoverPanic), some are set in doController of builder.Builder (such as GroupKindConcurrency and CacheSyncTimeout), others are just ignored, as the MaxConcurrentReconciles and NeedLeaderElection mentioned above.

I think that we should unify the setting of global controller configurations, either in controller.NewUnmanaged or doController, or maybe other place that is more suitable. WDYT? @vincepri

[vincepri]

hmm, can we get a test case in place to show the behavior? The global controller behavior should be as you describe

[sbueringer]

This might have been changed or fixed via #2307

[zqzten]

This might have been changed or fixed via #2307

Cool, it is.

/close

[k8s-ci-robot]

@zqzten: Closing this issue.

In response to this:

This might have been changed or fixed via #2307

Cool, it is.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.