Skip to content
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.

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

feat: Add maximum jobs per template configuration #196

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arsiesys
Copy link

@arsiesys arsiesys commented Jun 5, 2023

Description:
This pull request adds the feature to configure the maximum number of concurrent jobs per template in the Nomad plugin. It allows users to set a limit on the number of jobs that can be provisioned using a specific template.
This can be usefull in case of you nomad pool is shared across differents jenkins AND you do not have the chance to be able to use the premium quota feature.. :)

Changes:

  • Added a new parameter, maxConcurrentJobs, to the NomadWorkerTemplate class.
  • Modified the NomadCloud class to include a check for the maximum number of jobs when provisioning new Jenkins workers.
  • Introduced a new method, checkExcessJobs(), in the NomadCloud class to determine if the maximum configured jobs for a template have been reached during provisioning.
  • Updated the Jenkins configuration UI (config.jelly) for the NomadWorkerTemplate to include the new maxConcurrentJobs field.
  • Update existing unit tests in NomadCloudTest to do not break

I consider that the parameter with the value -1 is unlimited.
In my test, upgrade to the plugin have one issue. Each existing template will the value "0" until the user change it. Is there a way that I could set it to -1 so the default value would be unlimited so it doesn't break existing users configurations ? Thanks for your recommandations and reviews !

@aarnaud
Copy link

aarnaud commented Jun 6, 2023

nice feature, thanks. I hope it will be merged soon.

@multani
Copy link

multani commented Jun 6, 2023

Thanks for the PR @arsiesys !

I won't have time to look in details before end of June though.

In my test, upgrade to the plugin have one issue. Each existing template will the value "0" until the user change it. Is there a way that I could set it to -1 so the default value would be unlimited so it doesn't break existing users configurations?

Not sure how to do that to be honest, but that would be an issue yes (especially if the default value prevents from creating new jobs).

There's a migration helper in MigrationHelper.java, maybe that could inspire you?

@arsiesys
Copy link
Author

arsiesys commented Jun 8, 2023

Thanks for the suggestion !
I indeed checked MigrationHelper.java already but it use the "already" instantiated object "NomadWorkerTemplate" which mean it already wrote a 0 :(. I would need to do it "before" the plugin is init and object "loaded" to inject in the config.xml..

@Kamilcuk
Copy link

Kamilcuk commented Jun 29, 2023

Hi, please change the default of maximum jobs to some value, like 100. When upgrading, the default is set to 0, which makes it all stuck until manual action to change the value.

Hmmm, I see default=-1 in the source code, need to investigate, please ignore me.

@multani
Copy link

multani commented Nov 14, 2023

@arsiesys I'm not working on the plugin anymore :(

As a last suggestion for your question, I'd suggest to contact the developers mailing list on https://groups.google.com/g/jenkinsci-dev, maybe you will get some help.

Thanks again for the change, and good luck!

@gildor7
Copy link

gildor7 commented Apr 16, 2024

Hi, can you please merge this change? It is very useful and would solve various resource exhaustion issues!
@multani you say you no longer work on the plugin, but it doesn't seem abandoned.
Can anyone help?

@multani
Copy link

multani commented Apr 16, 2024

I removed myself several months, please check with @j3t.

Also, feel free to contact the Jenkins project itself via the developers mailing list if you want to participate and move this project forward!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants