Improving uniqueness of templateID string

[davidtlascelles]

The instance capacity help text says that it is "The maximum number of containers, based on this template, that this provider is allowed to run in total.".

With the current implementation, the instance capacity is based on the number of instances of an image, not of a template. This causes issues when multiple templates are made using the same image, but different names or labels are used to define the template.

This change makes the templateID more unique, so a user can define different templates that use the same image as long as the labels and name are unique.

Alternatively, this could be changed to template.hashCode().toString(), but that may cause other problems due to the kinds of values that contribute to the hash.

This may resolve the issue described in issue #655, as it seems to be the same problem I am having.

• Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
• Ensure that the pull request title represents the desired changelog entry
• Please describe what you did
• Link to relevant issues in GitHub or Jira
• Link to relevant pull requests, esp. upstream and downstream changes
• Ensure you have provided tests - that demonstrates feature works or fixes the issue

[pjdarton]

This issue requires a more nuanced solution than this...
(which is why it's remained unsolved for so long)

There are two requirements here, and any solution needs to solve both at once:

1. On the one hand, we want uniqueness so that different templates have different IDs.
2. On the other hand, we want "the same template" to keep the same template ID even if the Jenkins-admin changes a setting within the template, otherwise we can easily find ourselves in a situation where we run too many containers of a given specification.

e.g. consider the following scenario:

• we've got one cloud defined, giving us access to a small docker host
• we've defined two templates within that cloud
• the first template is for a container that has a tiny footprint, so small we can run loads of them, but we've set the instanceCap for that to 20
• containers made from the second template are huge and each consume nearly 20% of the hardware resources of that cloud we've set an instanceCap for the template to just 5 instances.
• There's a lot of builds queued up needing the heavyweight container
• The system is currently running at full capacity (i.e. 5 heavyweight builds are running)
• ... but the Jenkins admin edits the heavyweight template e.g. to increase the stopTimeout field (or any other minor change).
  What Jenkins MUST do is to apply the new setting to any newly-created heavyweight containers, but not run more than 5 at once (otherwise we'll blow the memory limits and everything will fail).
  BUT with the code change above, making even a minor change would alter the hashCode, which would alter the templateId, which would then mean that Jenkins would think it had zero instances of heavyweight template running, and so it would attempt to start 5 more containers ... and then all 10 builds fail because we run out of resources (e.g. out of memory, causing the oom-killer to start killing things).

i.e. the proposed change fixes one problem but (re)introduces a worse one (i.e. not honouring the instance caps).

What we really need is some form of "templateID" field that can be kept a constant between edits so that Jenkins can recognise "this new template is really the same as the old template" instead of thinking it's totally different ... while also recognising when it really is a different template.
This code change doesn't satisfy both of those criteria.

[davidtlascelles]

Thanks for the feedback! This explanation is really valuable. I haven't had the time to really delve into this problem, but it is a constant annoyance at work so whenever I get annoyed, I re-visit this for a few minutes and get nowhere.

Anyway, here's an idea for how I could try solving this:

• The "Name" field can be required, and must be unique.
• Making a new template would create a default name, e.g. "docker-###" (The current default behavior for an empty Name field is "docker", this would just append a unique value to the end, possibly an incrementing integer, maybe a hash, etc.)
• If the name field is altered after the template is created, display some red warning text about instance capacities. Alternatively, make the field immutable and grey it out after template creation, and have some explanation in the help text advising the user to create a new template with a new, immutable name, warning that the instance capacity of the new template will be independent of the current template.

If this sounds good to you, I'll try to bump this up my priority list. Maybe even convince my manager to put it on the backlog at work.

Also, let me know if you have some implementation suggestions. I'm not very familiar with Jenkins plugin development either, so figuring out how to do my suggestions in bullet point 3 is going to be a challenge.

[pjdarton]

I was thinking about maybe have a invisible-to-the-user field that was a plugin-assigned ID.
I'm not an expert on Jenkins' "Jelly" so I can't give much advice on how to do that (other than "I'm reasonably sure other plugins have use this"), but I suspect that this might be the best option.

I can certainly advise on how to add new fields, and how to handle the serialisation to/from XML etc, and help you figure out how to make it automatically populate if it's not been given a value.
...but I don't know how to make a field invisible on the configuration page, and we'd want it invisible and immutable by the user.

[basil]

@davidtlascelles Is there an interest in completing this PR? If not, I will close it due to inactivity.

[davidtlascelles]

@basil I really wish I had time to fix this, but I haven’t had an opportunity yet. I’ll close this and hopefully some day open a new PR with a more sophisticated fix.