Reserved resources available again after reloading CasC configuration

[meeusen]

Jenkins and plugins versions report

Environment

Jenkins: 2.411
OS: Linux - 6.3.8-100.fc37.x86_64
Java: 11.0.19 - Eclipse Adoptium (OpenJDK 64-Bit Server VM)
---
apache-httpcomponents-client-4-api:4.5.14-150.v7a_b_9d17134a_5
bootstrap5-api:5.3.0-1
bouncycastle-api:2.28
branch-api:2.1109.vdf225489a_16d
caffeine-api:3.1.6-115.vb_8b_b_328e59d8
checks-api:2.0.0
cloudbees-folder:6.815.v0dd5a_cb_40e0e
commons-lang3-api:3.12.0-36.vd97de6465d5b_
commons-text-api:1.10.0-36.vc008c8fcda_7b_
credentials:1254.vb_96f366e7b_a_d
credentials-binding:604.vb_64480b_c56ca_
data-tables-api:1.13.4-3
display-url-api:2.3.7
durable-task:507.v050055d0cb_dd
echarts-api:5.4.0-5
font-awesome-api:6.4.0-1
git:5.1.0
git-client:4.4.0
instance-identity:173.va_37c494ec4e5
ionicons-api:56.v1b_1c8c49374e
jackson2-api:2.15.2-350.v0c2f3f8fc595
jakarta-activation-api:2.0.1-3
jakarta-mail-api:2.0.1-3
javax-activation-api:1.2.0-6
javax-mail-api:1.6.2-9
jaxb:2.3.8-1
jquery3-api:3.7.0-1
junit:1214.va_2f9db_3e6de0
lockable-resources:1172.v4b_8fc8eed570
mailer:457.v3f72cb_e015e5
matrix-project:789.v57a_725b_63c79
mina-sshd-api-common:2.10.0-69.v28e3e36d18eb_
mina-sshd-api-core:2.10.0-69.v28e3e36d18eb_
pipeline-build-step:496.v2449a_9a_221f2
pipeline-groovy-lib:656.va_a_ceeb_6ffb_f7
pipeline-input-step:468.va_5db_051498a_4
pipeline-milestone-step:111.v449306f708b_7
pipeline-model-api:2.2141.v5402e818a_779
pipeline-model-definition:2.2141.v5402e818a_779
pipeline-model-extensions:2.2141.v5402e818a_779
pipeline-stage-step:305.ve96d0205c1c6
pipeline-stage-tags-metadata:2.2141.v5402e818a_779
plain-credentials:143.v1b_df8b_d3b_e48
plugin-util-api:3.3.0
scm-api:676.v886669a_199a_a_
script-security:1251.vfe552ed55f8d
snakeyaml-api:1.33-95.va_b_a_e3e47b_fa_4
ssh-credentials:305.v8f4381501156
structs:324.va_f5d6774f3a_d
trilead-api:2.84.v72119de229b_7
variant:59.vf075fe829ccb
workflow-aggregator:596.v8c21c963d92d
workflow-api:1215.v2b_ee3e1b_dd39
workflow-basic-steps:1017.vb_45b_302f0cea_
workflow-cps:3691.v28b_14c465a_b_b_
workflow-durable-task-step:1247.v7f9dfea_b_4fd0
workflow-job:1308.v58d48a_763b_31
workflow-multibranch:756.v891d88f2cd46
workflow-scm-step:415.v434365564324
workflow-step-api:639.v6eca_cd8c04a_a_
workflow-support:839.v35e2736cfd5c

What Operating System are you using (both controller, and any agents involved in the problem)?

Fedora 37

Reproduction steps

1. Add two lockable resources using "Configuration as Code" (without any note/reservedBy fields)
2. Reserve one resource
3. Add a note to the other resource
4. Reload the existing "Configuration as Code" configuration

Expected Results

The first resource should still be reserved, the other should still have the note.

Actual Results

The first resource is available again and the note of the other resource has been removed.

Anything else?

As the status (reserved/free...), timestamp and the note or not really configuration properties, but more state properties, should they not be kept when reloading the CasC configuration? For the classic configuration in Jenkins (Manage Jenkins -> System), this works fine when e.g. adding resources (or just changing something else in the System settings). I think this is done with a call to copyUnconfigurableProperties() in https://github.com/jenkinsci/lockable-resources-plugin/blob/master/src/main/java/org/jenkins/plugins/lockableresources/LockableResourcesManager.java#L980.

[mPokornyETM]

This is not a bug. This is how the JCaC works. It will reload stored configuration. Independed of the status. I do not know, how can be changed (and if we really want).
For ex. when you configure your agents in the JCaC yaml, and you set new labels manually and then you relaod the configuration, all manually configured labes are gone. Exact this scenario we use in my company.

What you can do, is to set the reserved status in the yaml. Then it will be reserved also after reload. But I am not sure, if you want it.

And it might be dangarous. JCaC his helpfull when you start your Jenkins in docker image, or you want to have transparent configuration (using with some source code management ...) And change like this, might broke Jenkins reboots.

[roded]

I'd like to throw in my agreement with @meeusen

It makes sense for some aspects of locks to be controlled via CasC: the existence or not of the lock itself, descriptions, labels, etc. But it makes no sense to keep transient the state of the locks (locked or not) in the CasC.

In our experience, the fact that this plugin loads the lock states from the CasC files makes it incompatible with Jenkins CasC which is a pain for shops relying on CasC for persistent configuration of Jenkins.

Please consider not keeping transient information about locks in the CasC files.

[meeusen]

Basically, if you have configured your Jenkins with JCasC configuration, then you lose the reservations and the notes whenever Jenkins restarts or you reload the config. I think it comes down to that.

I'm only just starting out with JCasC, but I see it as a replacement of the normal configuration in the UI, where it would behave in the same way.

When you add a new resource in the UI, other resources that were reserved, are still reserved, and there is specific code in the plugin to make this possible.

When you add a new resource to a JCasC YAML file and reload the config, I would expect the same to happen. That would be, for us, the main benefit of using JCasC, to make it easier to add/manage resources (which is cumbersome through the UI if you have many, especially if you want to order them in a specific way), while not interfering with users that have reserved a resource at that time.

If a user wants to unreserve all resources when reloading the config, maybe they can explicitly set an empty "reservedBy" in their YAML file. On the other hand, I would prefer for the "reservedBy" to be removed from the UI settings, as it can interfere when adding new resources, basically what is described in #143. But that's more because of a problem with the UI configuration page, so that could be a reason to differ from the JCasC configuration behaviour.

With your example about the Jenkins agents. The labels will be updated, but the executors that are running on that agent, will still keep running when you reload the config (I suppose).

[mPokornyETM]

I'd like to throw in my agreement with @meeusen

It makes sense for some aspects of locks to be controlled via CasC: the existence or not of the lock itself, descriptions, labels, etc. But it makes no sense to keep transient the state of the locks (locked or not) in the CasC.

In our experience, the fact that this plugin loads the lock states from the CasC files makes it incompatible with Jenkins CasC which is a pain for shops relying on CasC for persistent configuration of Jenkins.

Please consider not keeping transient information about locks in the CasC files.

FYI: This was done a long time before I start with this pulgin.
Removing transient variables will break classic .xml config concept. ( I think, not 100% sure) Because it must be stored there, wenn jenkins reboots (or crashed). But let me see how it is done in other plugins / core. "The community will helps you" ;-)

[imarkov-storpool]

We encountered the same issue as @meeusen just now. While I agree that recreating the resources without the locks (and other post-CasC attributes) is technically what is supposed to happen, it can be quite disruptive.

Would you consider an additional flag in the JCasC configuration to preserve locked/reserved resources acceptable?
Could be per-resource, global for all resources, or global with per-resource override. That is, of course, it this is at all possible - I haven't looked into how JCasC works behind the scenes, but I could probably dedicate some time to submitting a PR with such a fix.

[mPokornyETM]

I am not sure if the flag is necessary.
General I does not see any reason, why the "state" shall be configurable. It like you will configure "the agant is online".
That means, maybe we can "remove" that transient part.

@imarkov-storpool do you want to try that?

[imarkov-storpool]

Just to confirm, do you mean I should make it so that locked/reserved builds are not touched at all by CasC? The point of having a flag is to preserve the current behaviour (since it is technically correct) by default.

[mPokornyETM]

I mean, we can set the property reserved by as transient and remove it from the config page as well. Because it leads to buggy scenarios.
From my side, here is no backward compatibility necessary. We just need write 'correct' upgrade quidelines afterword

[manosnoam]

+1 for this feature request.
Is there any workaround to preserve lockable resources status after reloading Jenkins JCaC ?
Maybe an option (in API) to backup the statuses into an object which can be saved into a yaml file, that can be triggered each time a resource is locked/released ?
In addition an option to restore the statuses from that yaml later (e.g. after Jenkins reload) ?

[mPokornyETM]

No, currently no workarround.
I still does not understand, why somebody need to set the reservation in JCaC. From my POV it makes no sense.

My whish scenario:
When you reserve a resource and you reboot the jenkins it is (dhall be) still reserved.
When you re-load JCaC, then you will set all the given options new. That means you will parametrized Jenkins. Why shall I set status on resource. It will be free, because it is free in POV from new system.

When there are no more open discussion, I will change it as describe here

[manosnoam]

A common scenario in our Jenkins env:
People work on reserved resources (test Clusters) and from time to time we fix jobs that require Jenkins reload.
If someone had a reserved cluster, then after the JCaC reload it's not reserved anymore...

[mPokornyETM]

A common scenario in our Jenkins env: People work on reserved resources (test Clusters) and from time to time we fix jobs that require Jenkins reload. If someone had a reserved cluster, then after the JCaC reload it's not reserved anymore...

exaclty, but you as admin, never set the reserved "property" in JCaC . And does not see any reason to do that. Therefore I will remvoe this possibility and that´s it ;-)

[GhostInThePotato]

I agree, I don't think the reservedBy or reservedTimestamp properties should be in the JCaC.

I would like to be able to create lockable resources in JCaC but only the name, label and description. Users/pipelines can reserve or unreserve these resources but if Jenkins was restarted or the JCaC reloaded then the lockable resources should stay in the same reserved/unreserved state as they were before the restart.

[mPokornyETM]

reopened - see also #650