Unexpected Setting Concurrency as 1 in Binding Status Controller

[lxtywypc]

What happened:
Because of missing forType in binding-status controller, the concurrency of the controller would be always set as 1.

What you expected to happen:
The concurrency of binding-status controller could inherit from the concurrency of binding declared in global options.

How to reproduce it (as minimally and precisely as possible):

Anything else we need to know?:
If a controller is built WITHOUT concurrency options, AND it HAS a forType, the concurrency of the controller would inherit from the concurrency for forType declared in global options.

karmada/vendor/sigs.k8s.io/controller-runtime/pkg/builder/controller.go

Lines 354 to 370 in 01b086d

	hasGVK := blder.forInput.object != nil
	if hasGVK {
	var err error
	gvk, err = getGvk(blder.forInput.object, blder.mgr.GetScheme())
	if err != nil {
	return err
	}
	}
	// Setup concurrency.
	if ctrlOptions.MaxConcurrentReconciles == 0 && hasGVK {
	groupKind := gvk.GroupKind().String()
	if concurrency, ok := globalOpts.GroupKindConcurrency[groupKind]; ok && concurrency > 0 {
	ctrlOptions.MaxConcurrentReconciles = concurrency
	}
	}

Up to now, all the concurrency of controllers in Karmada inherit from the global options. And all controllers except binding-status controller has a forType. So only binding-status controller would has a concurrency as 1.

There are two ways to solve the problem:

1. Add a forType with a fake predicate to inherit concurrency from global options and keep the same performance at the same time.
2. Add concurrency options when building the binding-status controller.

This problem occurs after we seprate binding-status controller from binding controller. On the latest master branch, this problem is fixed after the #4138 coincidentally, but in the earlier version it still exists.

It might be hard to cherry-pick #4138 straightly because it seems a fix for feature hpaReplicasSyncer, that might not necessary in earlier version.

I would try to raise a PR to solve it in first way soon. But it would bring differences on commit history between master branch and other branches of eralier version, I wonder if it is acceptable, or what the recommend way is.

Environment:

• Karmada version:
• kubectl-karmada or karmadactl version (the result of kubectl-karmada version or karmadactl version):
• Others:

[lxtywypc]

/cc @XiShanYongYe-Chang @RainbowMango

[RainbowMango]

This problem occurs after we seprate binding-status controller from binding controller. On the latest master branch, this problem is fixed after the #4138 coincidentally, but in the earlier version it still exists.

Thank you for your patience. I understand the issue is the concurrency of resource-binding-status-controller and cluster-resource-binding-status-controller default to 1 in release branches, the concurrency is expected to inherit from the global configuration:

karmada/cmd/controller-manager/app/controllermanager.go

Lines 164 to 169 in 01b086d

	GroupKindConcurrency: map[string]int{
	workv1alpha1.SchemeGroupVersion.WithKind("Work").GroupKind().String(): opts.ConcurrentWorkSyncs,
	workv1alpha2.SchemeGroupVersion.WithKind("ResourceBinding").GroupKind().String(): opts.ConcurrentResourceBindingSyncs,
	workv1alpha2.SchemeGroupVersion.WithKind("ClusterResourceBinding").GroupKind().String(): opts.ConcurrentClusterResourceBindingSyncs,
	clusterv1alpha1.SchemeGroupVersion.WithKind("Cluster").GroupKind().String(): opts.ConcurrentClusterSyncs,
	schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Namespace"}.GroupKind().String(): opts.ConcurrentNamespaceSyncs,

Nice catch!

Please remind me if not the case. I'll get back to this later.

[XiShanYongYe-Chang]

In favor of #4340
/assign @lxtywypc

[XiShanYongYe-Chang]

#4340 has been merged.
/close

[karmada-bot]

@XiShanYongYe-Chang: Closing this issue.

In response to this:

#4340 has been merged.
/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.