fix: provisionning limits counter should be init without num executors

[heurtematte]

The concurrency limit set in the cloud configuration is not well interpreted.

e.g: if a limit is set up to 2, only one executor will be able to run at a time. Currently the limit is a kind of n-1.

This is due to the fact that numExecutors is used twice in class KubernetesProvisioningLimits:

• In the initInstance method with node.getNumExecutors()
• In the register method with param numExecutors

initInstance method:

• https://github.com/jenkinsci/kubernetes-plugin/blob/master/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesProvisioningLimits.java#L52
• https://github.com/jenkinsci/kubernetes-plugin/blob/master/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesProvisioningLimits.java#L53

register method:

• https://github.com/jenkinsci/kubernetes-plugin/blob/master/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesProvisioningLimits.java#L79
• https://github.com/jenkinsci/kubernetes-plugin/blob/master/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesProvisioningLimits.java#L81

[Vlatombe]

Hello,

I would advise you to try writing a test that demonstrates the problem you are facing. Your current patch is removing count that occurs at startup, which means any agent that was existing before startup won't be taken into account by the limit logic.

[pieterjanpintens]

We also see weird behavior with this setting. It feels that the internal counter it is not always in sync with reality.
We have a limit set of 20 concurrent nodes. Our master claims that there are indeed 20 nodes up, while not even 10 are actually connected. This blocks the jobs as their executors cannot find nodes to run on.

[jglick]

the internal counter it is not always in sync with reality

FWIW I would not advise using this feature of the plugin at all. Use K8s namespace quotas instead.