Make ephemeral resource creation optional

[ayoubbergaoui]

Make ephemeral resource creation optional

Description

This pull request introduces a new configuration option in the Lockable Resources Plugin to control the creation of ephemeral resources. The goal is to give administrators more control over resource management by allowing them to prevent automatic creation of ephemeral resources.

Changes

• Added a new boolean field allowEphemeralResources in LockableResourcesManager (default: true)
• Updated the system configuration UI with a new checkbox option "Allow Ephemeral Resources"
• Enhanced error handling with clear user guidance:
  • For missing resources: Lists which resources are missing and how to fix it
  • For invalid labels: Shows which label doesn't match and possible solutions
• Updated documentation in README.md with the new feature
• Added configuration-as-code (JCasC) support

Implementation Details

The changes span across several files:

• LockableResourcesManager.java: Core logic for the new feature
• LockableResourcesStruct.java: Enhanced validation for resources and labels
• LockableResourcesQueueTaskDispatcher.java: Improved error messages
• config.jelly and config.properties: UI changes
• README.md: Documentation updates

Testing done

• Verified functionality with ephemeral resources enabled (default behavior)
• Tested with ephemeral resources disabled:
  • Missing resource case
  • Invalid label case
  • Multiple missing resources case
• Validated error messages clarity
• Confirmed JCasC compatibility

Proposed upgrade guidelines

N/A - This change is backward compatible. The feature is enabled by default, maintaining existing behavior.

Localizations

New strings added in config.properties:

• English

Notes

This change improves the plugin's flexibility by making ephemeral resource creation optional while maintaining backward compatibility.

[mPokornyETM]

@ayoubbergaoui Did this change fixed #651 ??

[ayoubbergaoui]

@mPokornyETM yes! it's the same need (even if I haven't seen that there have been other people who need this)

[ayoubbergaoui]

@mPokornyETM

Test Results: Non-Existing Resource/Label with Ephemeral Resources Disabled

Just sharing the results of the tests I ran locally.

When ephemeral resources are disabled and we specify a resource name that does not exist in the list of lockable resources, this is what happens:

Similarly, when specifying a label name that does not exist, we get:

[ayoubbergaoui]

@mPokornyETM can you review / validate the PR please?

[mPokornyETM]

@mPokornyETM

Test Results: Non-Existing Resource/Label with Ephemeral Resources Disabled

Just sharing the results of the tests I ran locally.

When ephemeral resources are disabled and we specify a resource name that does not exist in the list of lockable resources, this is what happens:

Similarly, when specifying a label name that does not exist, we get:

I mean automated tests. Sorry, I am very strict in this plugin, because it is used in many instances. It is complex and all touches are dangerous. Therefory I propose autimated tests for every code change, when possible

[ayoubbergaoui]

@mPokornyETM I just updated my commit with a set of tests that cover all my changes.

So all the lines I added are covered now by a test!

If you have time, can you validate the PR please?

Thanks in advance,

[ThomasFaivre]

Hi all,

Small change in documentation is needed (see review comment).

I feel like there are some cases that are not handled completely/correctly:

src/main/java/org/jenkins/plugins/lockableresources/LockableResourcesManager.java

    private boolean proceedNextContext() {
    ...
            List<LockableResource> requiredResourceForNextContext =
                this.fromNames(nextContext.candidates, /*create un-existent resources */ true);

I don't know what this proceedNextContext method is, but the true is ignored due to the check for the option in createResource. It feels confusing.

Same here:

    public List<LockableResource> getAvailableResources(
    ...
                available = fromNames(
                        getResourcesNames(requiredResources.required), /*create un-existent resources */ true);

These inconsistencies make me think that some tests are missing:

src/test/java/org/jenkins/plugins/lockableresources/LockStepWithRestartTest.java

    public void chaosOnRestart() throws Throwable {
    ...
                            + "    lock('ephemeral_' + env.BUILD_NUMBER + '_' + resourceName) {\n"
                            + "      lock(resourceName) {semaphore semId}\n"

src/test/java/org/jenkins/plugins/lockableresources/PressureTest.java

    public void pressure(final int resourcesCount) throws Exception {
    ...
                + "    lock('resource_ephemeral_' + stageName) {\n"
                // + "      echo \"*** locked resource_ephemeral_\" + stageName\n"
                + "    }\n"

The tests passed with current code because the ephemeral option is enabled by default.

Same idea with Configuration-as-Code: shouldn't the value of allowEphemeralResources be checked when false?

I'll let the maintainers way in on this.
Otherwise, LGTM (Disclaimer: I'm biased)

[ThomasFaivre]

According to your screenshots, the job will not actually fail and always block.