jenkinsci / lockable-resources-plugin

Lock resources against concurrent use
https://plugins.jenkins.io/lockable-resources
MIT License
87 stars 183 forks source link

Upgrade to 1102 erased all resource labels #434

Closed krachynski closed 1 year ago

krachynski commented 1 year ago

Jenkins and plugins versions report

Environment ```text Jenkins: 2.375.1 OS: Linux - 3.10.0-1160.80.1.el7.x86_64 --- Office-365-Connector:4.18.0 ace-editor:1.1 active-directory:2.29 amazon-ecr:1.107.ve50d37906739 analysis-model-api:10.21.0 ansible:1.1 ant:481.v7b_09e538fcca antisamy-markup-formatter:155.v795fb_8702324 apache-httpcomponents-client-4-api:4.5.13-138.v4e7d9a_7b_a_e61 authentication-tokens:1.4 authorize-project:1.4.0 aws-credentials:191.vcb_f183ce58b_9 aws-java-sdk:1.12.287-357.vf82d85a_6eefd aws-java-sdk-cloudformation:1.12.287-357.vf82d85a_6eefd aws-java-sdk-codebuild:1.12.287-357.vf82d85a_6eefd aws-java-sdk-ec2:1.12.287-357.vf82d85a_6eefd aws-java-sdk-ecr:1.12.287-357.vf82d85a_6eefd aws-java-sdk-ecs:1.12.287-357.vf82d85a_6eefd aws-java-sdk-efs:1.12.287-357.vf82d85a_6eefd aws-java-sdk-elasticbeanstalk:1.12.287-357.vf82d85a_6eefd aws-java-sdk-iam:1.12.287-357.vf82d85a_6eefd aws-java-sdk-logs:1.12.287-357.vf82d85a_6eefd aws-java-sdk-minimal:1.12.287-357.vf82d85a_6eefd aws-java-sdk-sns:1.12.287-357.vf82d85a_6eefd aws-java-sdk-sqs:1.12.287-357.vf82d85a_6eefd aws-java-sdk-ssm:1.12.287-357.vf82d85a_6eefd bootstrap5-api:5.2.1-3 bouncycastle-api:2.26 branch-api:2.1051.v9985666b_f6cc build-timeout:1.25 caffeine-api:2.9.3-65.v6a_47d0f4d1fe checks-api:1.8.1 cloudbees-folder:6.758.vfd75d09eea_a_1 command-launcher:90.v669d7ccb_7c31 commons-httpclient3-api:3.1-3 commons-lang3-api:3.12.0-36.vd97de6465d5b_ commons-text-api:1.10.0-27.vb_fa_3896786a_7 conditional-buildstep:1.4.2 config-file-provider:3.11.1 configuration-as-code:1569.vb_72405b_80249 copyartifact:1.48 credentials:1189.vf61b_a_5e2f62e credentials-binding:523.vd859a_4b_122e6 dark-theme:262.v0202a_4c8fb_6a data-tables-api:1.12.1-4 display-url-api:2.3.7 docker-commons:1.21 docker-java-api:3.2.13-37.vf3411c9828b9 docker-plugin:1.2.10 docker-workflow:563.vd5d2e5c4007f dtkit-api:3.0.2 durable-task:503.v57154d18d478 ec2:2.0.4 echarts-api:5.4.0-1 email-ext:2.92 embeddable-build-status:312.vf2de01b_051d0 favorite:2.4.1 font-awesome-api:6.2.1-1 forensics-api:1.17.0 git:4.14.3 git-client:3.13.1 github:1.36.0 github-api:1.303-400.v35c2d8258028 github-branch-source:1696.v3a_7603564d04 github-checks:1.0.19 gradle:2.2 handlebars:3.0.8 handy-uri-templates-2-api:2.1.8-22.v77d5b_75e6953 htmlpublisher:1.31 instance-identity:142.v04572ca_5b_265 ionicons-api:31.v4757b_6987003 jackson2-api:2.14.1-313.v504cdd45c18b jacoco:3.3.2 jakarta-activation-api:2.0.1-2 jakarta-mail-api:2.0.1-2 javadoc:226.v71211feb_e7e9 javax-activation-api:1.2.0-5 javax-mail-api:1.6.2-8 jaxb:2.3.7-1 jdk-tool:63.v62d2fd4b_4793 jersey2-api:2.37-1 jira:3.8 jjwt-api:0.11.5-77.v646c772fddb_0 jobConfigHistory:1176.v1b_4290db_41a_5 jquery:1.12.4-1 jquery-detached:1.2.1 jquery3-api:3.6.1-2 jsch:0.1.55.61.va_e9ee26616e7 junit:1166.va_436e268e972 ldap:2.12 lockable-resources:1102.vde5663d777cf mailer:438.v02c7f0a_12fa_4 mapdb-api:1.0.9-28.vf251ce40855d material-theme:0.5.2-rc100.6121925fe229 matrix-auth:3.1.6 matrix-project:785.v06b_7f47b_c631 maven-plugin:3.20 metrics:4.2.13-420.vea_2f17932dd6 mina-sshd-api-common:2.9.2-50.va_0e1f42659a_a mina-sshd-api-core:2.9.2-50.va_0e1f42659a_a momentjs:1.1.1 msbuild:1.30 node-iterator-api:49.v58a_8b_35f8363 okhttp-api:4.9.3-108.v0feda04578cf pam-auth:1.10 parameterized-trigger:2.45 pipeline-aws:1.43 pipeline-build-step:2.18 pipeline-github-lib:38.v445716ea_edda_ pipeline-graph-analysis:195.v5812d95a_a_2f9 pipeline-groovy-lib:621.vb_44ce045b_582 pipeline-input-step:466.v6d0a_5df34f81 pipeline-milestone-step:101.vd572fef9d926 pipeline-model-api:2.2118.v31fd5b_9944b_5 pipeline-model-definition:2.2118.v31fd5b_9944b_5 pipeline-model-extensions:2.2118.v31fd5b_9944b_5 pipeline-rest-api:2.28 pipeline-stage-step:296.v5f6908f017a_5 pipeline-stage-tags-metadata:2.2118.v31fd5b_9944b_5 pipeline-stage-view:2.28 plain-credentials:139.ved2b_9cf7587b plugin-util-api:2.20.0 popper-api:1.16.1-3 popper2-api:2.11.6-2 prism-api:1.29.0-2 publish-over:0.22 publish-over-cifs:0.16 pubsub-light:1.17 resource-disposer:0.20 run-condition:1.5 scm-api:631.v9143df5b_e4a_a script-security:1228.vd93135a_2fb_25 shelve-project-plugin:3.2 snakeyaml-api:1.33-90.v80dcb_3814d35 solarized-theme:0.1 sse-gateway:1.26 ssh-credentials:305.v8f4381501156 ssh-slaves:2.854.v7fd446b_337c9 sshd:3.275.v9e17c10f2571 structs:324.va_f5d6774f3a_d subversion:2.16.0 theme-manager:1.6 timestamper:1.21 token-macro:321.vd7cc1f2a_52c8 trilead-api:2.84.v72119de229b_7 variant:59.vf075fe829ccb warnings-ng:9.22.0 workflow-aggregator:590.v6a_d052e5a_a_b_5 workflow-api:1200.v8005c684b_a_c6 workflow-basic-steps:994.vd57e3ca_46d24 workflow-cps:3583.v4f58de0d78d5 workflow-durable-task-step:1217.v38306d8fa_b_5c workflow-job:1254.v3f64639b_11dd workflow-multibranch:716.vc692a_e52371b_ workflow-scm-step:400.v6b_89a_1317c9a_ workflow-step-api:639.v6eca_cd8c04a_a_ workflow-support:839.v35e2736cfd5c ws-cleanup:0.44 xcode-plugin:2.0.17-565.v1c48051d46ef xunit:3.1.2 ```

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

Linux on controller (Docker container) Windows on most agents needing the locked resources

Reproduction steps

  1. Install version 1069
  2. Create lockable resources with labels
  3. Upgrade to version 1102
  4. Verify resources

Expected Results

I expected my labels to still be available after upgrading the plugin.

Actual Results

The labels on all of my lockable resources were empty.

Anything else?

No response

mPokornyETM commented 1 year ago

Ouch. That not good . I have tested it with JCaC and on my production jenkins too. And has works as well. Do you use JCaC or classic configuration via /manage page? Just to simulate it assap

krachynski commented 1 year ago

I am managing classically, still. I need to eventually convert to casc.

On the whole it wasn't too bad since I had downloaded my casc configuration and could look that up to reapply it.

mPokornyETM commented 1 year ago

Thx for answer. I will try to reproduce it. When it is not critical, it might take a while, because my vacation starts today.

mPokornyETM commented 1 year ago

I thing I find it. When you look into org.jenkins.plugins.lockableresources.LockableResourcesManager.xml file you will find for each resource a a xml tag like this <labels>__oaSetup__ __intern__</labels> But when you save config in now it looks like `

__oaSetup__ __intern__

` So as workaround will works to remove all the and . But it is really strange. I will try to fix it now.

mPokornyETM commented 1 year ago

Sorry, I am wrong. It must be used the 'new' wrong format with the tags. To explain. The release 1102 need a list of labels (due better performance) and before this release it was stored as string with spaces. I will try to fix it in the backward compatibility step. Sorry for this mistake. This will not happen when you use JCaC.

krachynski commented 1 year ago

Ah, that's good to know. All the more reason for me to work on upgrading to a proper configuration as code setup, eh?

mPokornyETM commented 1 year ago

How to explain it. Configuration as code is perfect feature. But it is definitely a bug. So it must be fixed by our side. But yes I personally will propose to use JCaC. it helps you to manage many things in jenkins. I personally like it because our Configuration is in git repository. That means it is transparent who and why make the changes.

rollingstones62 commented 1 year ago

Hi to all, just to inform, it happened to me too. We need to improve ourselves in doing that conf via code, but we are not there yet. Fortunately we updated only our test env, so we will wait for the fix to update the pdn env.

Thanks for doing this super plugin!! for us is really useful!

mPokornyETM commented 1 year ago

Thx for feedback. I am currently on the vacation. I will try to fix it at January. I have the fix now. But it is not checked in, because I don't like not tested last checkins. So we hear us in next year.

moritzboth commented 1 year ago

It occurred to us, too. Thanks for your work on this useful plugin and for the heads up regarding CaC.

koan00 commented 1 year ago

Came here to report this issue as well. Thanks for having it fixed already!

andreyakostovsap commented 1 year ago

Hi, after the upgrade to 1102, I manually restored all missing labels. It looks like the new version erased them again.

rollingstones62 commented 1 year ago

Hi, after the upgrade to 1102, I manually restored all missing labels. It looks like the new version erased them again.

same to me.. I will wait for next release. I'm testing in UAT. This time it seams that in the xml the labels were there, but jenkins did not show them or was taking in consideration on the gui and use. I opened the xml and copy the labels from there to restore uat.

I think first time, were even erased from xml.

Thanks

mPokornyETM commented 1 year ago

Can somebody poste here your configs please. This is strange behavior. I test it in few scenarios and found only one failure. When you save your configs in the version 1102 from global-config page, it will erase all labels. But in the latest 1106 release it must works.

@rollingstones62 when I understand correct it, this is what you did:

  1. some config with labels
  2. upgrade to 1102 - and labels are missing
  3. upgrade to 1106 - and labels are still missing (in the UI) right?
andreyakostovsap commented 1 year ago

Our scenario is the following:

  1. Some config with labels
  2. Upgrade to 1102 - labels are missing
  3. Re-create them using backed up Jenkins configuration
  4. Upgrade to 1106 - labels are missing from UI, but the <labels> are still present in the xml - I believe this is what @rollingstones62 meant.

After 1106, I once again re-created the labels. This is an excerpt from the diff of the backed up Jenkins configuration after I clicked "Save":

     <org.jenkins.plugins.lockableresources.LockableResource>
       <name>tests-integration-installation-0</name>
       <description>Lock for synchronizing integration test installations</description>
-      <labels>
+      <labelsAsList>
         <string>tests-integration-installation</string>
-      </labels>
+      </labelsAsList>
       <note></note>
       <stolen>false</stolen>
       <ephemeral>false</ephemeral>
       <queueItemId>0</queueItemId>
       <queuingStarted>0</queuingStarted>
       <queuedContexts/>
     </org.jenkins.plugins.lockableresources.LockableResource>
mPokornyETM commented 1 year ago

Strange, let me check it. The config looks good.

rollingstones62 commented 1 year ago

i confirm that same steps were done by me! thanks @andreyakostovsap and @mPokornyETM for the help

mPokornyETM commented 1 year ago

I got it now. It is little hard to explain, but I will try to do my best: The release 1102 use List<String> instead of String for the property labels. As describe bellow, the reason is performance. So, what happens now.

Config before 1102 will looks like:

<labels>tests-integration-installation</labels>

This is simply string in xml config.

The release 1102 expect list-of-string like this:

<labels>
  <string>tests-integration-installation</string>
</labels>

That was original issue.

In the release 1106 I use new property 'labelsAsList' and convert automatically all string labels to the list. That fix the issue as well and do not slow down the plugin.

Pretty nice BUT When you has "fixed" (or just saved) your labels in the 1102 it will produce this config as well in the correct format for 1102

<labels>
  <string>tests-integration-installation</string>
</labels>

It looks to be good and work for the release 1102.

And the fun starts now :-(

When you has config from 1102, it is not possible to read it in 1106, because the property labels is string again. Or more detailed. The de-serialization from "file" to java object will ignore it, because List<String> does not match with String (I am still wondering, why there are no exceptions, warnings ...? but is other one thing) (I tries to explain it short and the 'terminus technicus' is not really correct)

It is really not possible, to do it programmatically or better I have no solution for that :-(. I am so sorry, but yeah, we can not jump back to the history. Only one solution is to remove the <string> tags from your org.jenkins.plugins.lockableresources.LockableResourcesManager.xml config file, before you upgrade to 1106. ex: this one

<labels>
  <string>tests-integration-installation</string>
</labels>

to

<labels>
  tests-integration-installation
</labels>

(currently tested by me and it works)

or install JCaC plugin export current properties, make a update and import it again. (just a idea, not tested by me, but I do not see any reason why it shall fails)

What we shall definitely to do, is to update 'Proposed upgrade guidelines' in the 1106 and documentation as well.

Again sorry for that.

mPokornyETM commented 1 year ago

Please can somebody check linked PR https://github.com/jenkinsci/lockable-resources-plugin/pull/440 Because my English is not the best one.