Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to use Jenkins label parser to allocate lockable-resources in more sophisticated way #475

Merged
merged 11 commits into from
Mar 27, 2023

Conversation

mPokornyETM
Copy link
Contributor

@mPokornyETM mPokornyETM commented Mar 23, 2023

Allow to use jenkins label parser to allocate lockbale-resources in more sofisticate way.

Example:

lock(label: 'os:Windows && ALM', variable : 'someVar') {
    echo 'os:Windows && ALM ' + env.someVar;
}

lock(label: 'osDetail:Debian[11] && arm64', variable : 'someVar') {
    echo 'osDetail:Debian[11] && arm64 ' + env.someVar;
}

lock(label: 'os:Windows || osDetail:Debian[11]', variable : 'someVar') {
    echo 'os:Windows || osDetail:Debian[11] ' + env.someVar;
}

lock(label: 'os:Windows || osDetail:Debian[11]', variable : 'someVar', quantity : 100) {
    echo 'os:Windows || osDetail:Debian[11] ' + env.someVar;
}

fix #428
close #309
close #474 107

see #341
see #455

Testing done

My local tests (exact with the example bellow) looks good.

Submitter checklist

  • The Jira / Github issue, if it exists, is well-described.
  • The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples).
    • The changelog generator for plugins uses the pull request title as the changelog entry.
    • Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during the upgrade.
  • There is automated testing or an explanation that explains why this change has no tests.
    - [ ] New public functions for internal use only are annotated with @NoExternalUse. In case it is used by non java code the Used by {@code <panel>.jelly} Javadocs are annotated.

- [ ] New or substantially changed JavaScript is not defined inline and does not call eval to ease the future introduction of Content Security Policy (CSP) directives (see documentation).
- [ ] For dependency updates, there are links to external changelogs and, if possible, full differentials.
- [ ] For new APIs and extension points, there is a link to at least one consumer.
- [ ] Any localizations are transferred to *.properties files.

  • Changes in the interface are documented also as examples.

Maintainer checklist

Before the changes are marked as ready-for-merge:

  • There is at least one (1) approval for the pull request and no outstanding requests for change.
  • Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
  • Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
  • Proper changelog labels are set so that the changelog can be generated automatically. See also release-drafter-labels.
  • If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
  • java code changes are tested by automated test.

@mPokornyETM mPokornyETM requested a review from a team as a code owner March 23, 2023 23:02
@mPokornyETM mPokornyETM added work-in-progress java Pull requests that update Java code lock Jenkins node Lockable resource interactions with Jenkins nodes. Need test Automated tests shall be extended. labels Mar 23, 2023
@mPokornyETM mPokornyETM added this to the Feature committed 2023 milestone Mar 23, 2023
@mPokornyETM mPokornyETM changed the title Use jenkins labels parser Allow to use Jenkins label parser to allocate lockbale-resources in more sofisticate way Mar 25, 2023
@mPokornyETM mPokornyETM added ready for review PR is ready for review and removed Need test Automated tests shall be extended. labels Mar 25, 2023
@mPokornyETM
Copy link
Contributor Author

@g3n35i5 do you want to provide review?

@g3n35i5
Copy link
Contributor

g3n35i5 commented Mar 27, 2023

@mPokornyETM The PR looks good to me so far, I have tested a few other examples at my local. What I don't understand are the labels in your test example. Did you give the lockable resources the labels "osDetail:Debian[11]" or "os:Linux" or does that refer to any agents configured in Jenkins?

README.md Outdated Show resolved Hide resolved
Co-authored-by: Jan-Frederik Schmidt <[email protected]>
@mPokornyETM
Copy link
Contributor Author

@mPokornyETM The PR looks good to me so far, I have tested a few other examples at my local. What I don't understand are the labels in your test example. Did you give the lockable resources the labels "osDetail:Debian[11]" or "os:Linux" or does that refer to any agents configured in Jenkins?

Good question: when I create mnew node, I create also new resource with exact same name and labels. Therefore it looks as node labels. I will change the example, to eliminate this mistunderstaning.

PS: Currently i work on automatic nodes-resources synchronisation. After them it will be really usefull.

@jimklimov jimklimov changed the title Allow to use Jenkins label parser to allocate lockbale-resources in more sofisticate way Allow to use Jenkins label parser to allocate lockable-resources in more sophisticated way Mar 27, 2023
Copy link
Contributor

@jimklimov jimklimov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks beautiful and elegant, but did not test yet.

@@ -4,10 +4,10 @@
*/
buildPlugin(useContainerAgent: true, configurations: [
// Test the common case (i.e., a recent LTS release) on both Linux and Windows.
[ platform: 'linux', jdk: '11', jenkins: '2.361.4' ],
[ platform: 'windows', jdk: '11', jenkins: '2.361.4' ],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"recent LTS release" was there for a reason ;)

Did something not work if you just bumped it, same way as below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"recent LTS release" was there for a reason ;)

Did something not work if you just bumped it, same way as below?

  1. yes there was compiling exceptions in java
  2. I need latest LTS (current one) to integrate some ideas rom data-tables-api.
  3. No body need to change jenkins version in 2 different files (this one + pom.xml) which leads to less mistakes.

Therefore I decide it will be fine to test it in this way also in future.

@mPokornyETM mPokornyETM merged commit b9f6b05 into jenkinsci:master Mar 27, 2023
@mPokornyETM mPokornyETM removed the ready for review PR is ready for review label Mar 27, 2023
@chickenkiller
Copy link

Thank you SO MUCH !!!! I can't test it yet but it is exactly what we needed.

@mPokornyETM
Copy link
Contributor Author

Thank you SO MUCH !!!! I can't test it yet but it is exactly what we needed.

Yes you can test it. But how? Curently it is merged into master, but not released. You build it manually. Or wait for official release.

@mPokornyETM mPokornyETM deleted the use-jenkins-labels-parser branch March 27, 2023 13:25
@mPokornyETM
Copy link
Contributor Author

@chickenkiller
Copy link

Thank you SO MUCH !!!! I can't test it yet but it is exactly what we needed.

Yes you can test it. But how? Curently it is merged into master, but not released. You build it manually. Or wait for official release.

I meant that I don't have time to test it right now, and I only have a production instance to test it so I'll rather wait for the official release. But thanks for the work and the links !

@cbolson1
Copy link

Thank you for this! When is the next release? I'm trying to figure out if I want to wait for that or install this from master.

@mPokornyETM
Copy link
Contributor Author

Th release interwal es every week, when something changed. The last one was brokne ( I think 1 sporadic failed testcase is the reason).
So when it works. This week shall be the plugin released.
Buiut when you test it before official release, it will be great. I does not see any possible sideeffects now. But nobody know. So one core change ... you know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation enhancement java Pull requests that update Java code lock Jenkins node Lockable resource interactions with Jenkins nodes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use the label parser from build agent selection.
5 participants