[JENKINS-31437] Assign properties to lockable resources and retrieve them in environment variables

[gaspardpetit]

(This replaces #295 with a simplified commit history)

This is pretty much #20 updated for HEAD - most of the credits go to @McFoggy

I recommend reviewing this after #294 since it will simplify the complexity of this PR. #294 adds an environment variable for each acquired locks when unlocking multiple ones, and this PR builds on this change.

The lock properties can be set on the lock definition in the System Configuration;
When setting a variable, the name of the variable is used as a prefix to also set an environment variable for each of the lockable resource's properties. For example:

lock(label: 'some_resource', variable: 'LOCKED_RESOURCE', quantity: 2) {
  // comma separated names of all acquired locks
  echo env.LOCKED_RESOURCE

  // first lock
  echo env.LOCKED_RESOURCE0
  echo env.LOCKED_RESOURCE0_PROP_ABC

  // second lock
  echo env.LOCKED_RESOURCE1
  echo env.LOCKED_RESOURCE0_PROP_ABC
}

We plan on using these properties to store information such as the IP, URL or an account to use with the lockable resource obtained.

• 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

[gaspardpetit]

Please hold - this change exacerbates the randomness of the lock ordering in the env variable; I will fix the current issue in master first and fix here after

[gaspardpetit]

Ok - fixed. This change would have caused regression against #301

[McFoggy]

You desserve the revival or wakeup the deads badge ;-)

Congrats.

[jimklimov]

@gaspardpetit : given the comments in #301, is this PR good to merge already (e.g. for #303/#302 to build upon, if suitable) or better delay until you complete the bigger idea here first?

[gaspardpetit]

This one is ready to go, it's a nice improvement to the plugin already :)

[gaspardpetit]

If #307 makes it to the master branch, I am now wondering if we need to output properties to environment variables - we could instead make it easy to findLocks on the locked resources, ex. findLocks() could return the build's locks by default and then we'd have the lock details in groovy objects instead of environment variables. Any opinion on this? It can still be convenient to set environment variables from locks, but can easily be done by the user, ex.

lock(label:'color_printer', quantity:1) {
  selectedPrinter = findLocks()[0]
  withEnv(["PRINTER_IP=${selectedPrinter.properties.ip}"]) {
    echo '${env.PRINTER_IP}'
  }
}

This would soft of make the variable not necessary anymore; this approach would also correctly handle the extra locks

[jimklimov]

Sounds cool, but I think one downside could be generally about scripted vs declarative pipelines. Although in this particular use-case, you "declare" that you want a selectedPrinter variable...

@abayer : if we might bother someone more keen on pure-declarative code for an opinion? ;)

[gaspardpetit]

The most problematic flaw I see with the existing design is that the result is unpredictable.

If you ask for a single lock, or N of a single lock label, it's fine. But when you mix the extra in the lot it gets messy. For example, if you ask for 1 resource matching label A and 1 extra resource matching label B, you may end up with 2 locks, one matching each requirement, or a single lock matching both.

This makes using environment variables for the result almost impossible because you can't predict how many locks you will actually get.

The findLocks approach is much easier since you can ask to find the lock that matched A and later find the lock that matched B - and perhaps you will get the same resource each time, maybe not... but you don't have to worry, each findLock will give you what you expect.

If we do continue with environment variables, we will need to define very well how the extra should be handled, especially when they request the same variable name...

[basil]

Hi @gaspardpetit, in case you couldn't tell, I'm more of an interim maintainer than a maintainer for this plugin. If you want commit access to be able to merge and release PRs for this plugin, feel free to file a repository-permissions-updater and I'll approve it.

[jimklimov]

Sorry about mostly falling out of the loop, recent months were hectic :\ Hope to catch up in the coming weeks...

[gaspardpetit]

@basil no worries, I understand - I find this plugin useful and I have java experience, so I'd be happy to give a hand on reviewing and approving some of the PR; Sorry for asking, I have never before asked for such permission, can you explain how I can request "repository-permissions-updater" permission on github?

[basil]

Hi @gaspardpetit, I highly appreciate your contributions to this plugin. This page documents the procedure; namely, submit a PR on the "Repository Permission Updater" (RPU) repository. Feel free to at-mention me on the PR and I will approve it. Once you have commit access, you will be able to merge PRs and perform releases. Feel free to reach out to me or any other members of the Jenkins core development team if you have any questions about Jenkins plugin development that are not answered in the Jenkins developer reference.

[basil]

Hi @gaspardpetit, are you still interested in adopting this plugin?

[jimklimov]

+1 from me, hectic time here switching from one job to another to another... too long without a laptop of my own :\

[mPokornyETM]

@gaspardpetit are you still interested to conrtibut?

[gaspardpetit]

Yes, even though my time will be limited I'm happy the help. I still haven't figured out how to request the role though 😅

[mPokornyETM]

I am one of the owners now. And very interested to improve this plugin (now).
It will be fine, when we can merge some of your PRs. There are some great ideas. But I am not sure what shall be the best way.

• Maybe you can allows to update your PRs by other developers. So I can remerge it with current master branch. (like this try to resolve merge conflicts eidosmontreal/lockable-resources-plugin#1)
• When you want, I can add you (or request it for you) to be also a owner of this repo.
• Or I bring your changes via new PRs like this Assign properties to lockable resources and retrieve them in environment variables #392 and can add you as reviewer.
• Also it will be great when you looks sometimes into open PRs.

I try to do my best, but I am still missing some other developers here. Approve own changes without review makes no sense.

I think when we merge currently opened PRs and solve some issues, the maintenance here will be fine. It looks terrible at the moment.

[gaspardpetit]

Thank you for dedicating time to this plugin. I will update my pending reviews during the week and also consider the feedback left on them.