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.

Sign up for GitHub

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

Jump to bottom

Support configuration as code #262

Merged
merged 28 commits into from
Jun 10, 2023
Merged

Support configuration as code #262

merged 28 commits into from
Jun 10, 2023

Conversation

Kamyki
Copy link
Contributor

@Kamyki Kamyki commented May 31, 2023

  • 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

This PR implements the needed changes to support the configuration of this plugin
with the configuration-as-code-plugin. It is my rework of previous work PR-62 by @fr123k . I submit it as a separate PR for my convenience, but if necessary I can apply patch on the original.

Initial pull request had problem with exporting configuration by jenkins/manage/configuration-as-code/viewExport, causing serialisation errors. To mitigate that, following this comment I had added DataBoundConstructor and DataBoundSetter annotation to some classes.

I was aiming to preserve format of xml configuration for plugin, thus I haven't edited class hierarchy, but it results in slightly uglier casc configuration format - see tests.

This PR also implements the following jira ticket with the title Problem with jcasc compatibility plugin and job priority in Jenkins.
JENKINS-54571

fr123k and others added 18 commits March 20, 2021 20:01
@fr123k
Add CasC support for the PrioritySorterConfiguration class
775194f 
The PrioritySorterConfiguration class couldn't be configured with CasC
(Configuration as Code) jenkins plugin.
@fr123k
Add support for configuration as code plugin usage
9404fb5 
The configuration for the priority sorter plugin can be defined with
the [configuration as code plugin](https://github.com/jenkinsci/configuration-as-code-plugin).

This is the example from the junit test resource file `PriorityConfiguration.yaml`
```
unclassified:
  priorityConfiguration:
    jobGroups:
      - id: 0
        priority: 1
        description: "Complex"
        runExclusive: true
        usePriorityStrategies: true
        priorityStrategies:
          - userIdCauseStrategy:
              priority: 1
          - upstreamCauseStrategy
          - userIdCauseStrategy:
              priority: 3
          - cLICauseStrategy:
              priority: 4
          - jobPropertyStrategy
          - buildParameterStrategy:
              parameterName: priority
        jobGroupStrategy:
          folderBased:
            folderName: "Jenkins"
      - id: 1
        priority: 2
        description: "Simple"
        runExclusive: false
        usePriorityStrategies: false
        jobGroupStrategy: allJobs
```
@fr123k
Add test assertion for the casc priorityStrategies configuration
c63bb2c 
Extend the ConfigurationAsCodeTest test case to also assert the casc
defined priorityStrategies configurations.

The test casc configuration resource files for the ConfigurationAsCodeTest
test case class are at a similar location then all the other configuration
test files.
@MarkEWaite
Merge branch 'master' into add_casc_support
3ffc89a
@MarkEWaite
Merge branch 'master' into add_casc_support
47c29f8
@MarkEWaite
Merge branch 'master' into add_casc_support
0c447fa
@MarkEWaite
Exclude commons-lang3 from test dependency
1aa0ac9
@MarkEWaite
Merge branch 'master' into add_casc_support
e9d3571
@MarkEWaite
Format current files
66e52b0
@MarkEWaite
Merge branch 'master' into add_casc_support
81b7274
@MarkEWaite
Fix merge error
6f5907a
@MarkEWaite
Format source files
1e5c6c4
@MarkEWaite
Exclude one new spotbugs warning
88ee776
@Kamyki
Use DataBoundConstructor in JobGroup
7327042
@Kamyki
Merge branch 'add_casc_support' into use_data_bound_builder
81f4463 
# Conflicts:
#	src/main/java/jenkins/advancedqueue/JobGroup.java
@Kamyki
Fix errors when holder contains array
46f8dd9
@Kamyki
Resolve errors when exporting configuration using casc
8f3bebc
@Kamyki
Merge remote-tracking branch 'kamyki/master' into add_casc_support_an…
c996878 
…d_use_data_bound_constructor
@github-actions github-actions bot added dependencies Dependency related change test Automated test addition or improvement labels May 31, 2023
@MarkEWaite MarkEWaite mentioned this pull request Jun 2, 2023
Closed
6 tasks
MarkEWaite added a commit to MarkEWaite/docker-lfs that referenced this pull request Jun 3, 2023
@MarkEWaite
Use PrioritySorter plugin 4.1.1-rc667.28ddb_f715a_40 incrementals
0496268 
Includes configuration as code support from
jenkinsci/priority-sorter-plugin#262
@MarkEWaite MarkEWaite changed the title Add casc support and use data bound constructor in JobGroup Support configuration as code Jun 3, 2023
@MarkEWaite MarkEWaite added enhancement Improvement or new feature and removed dependencies Dependency related change test Automated test addition or improvement labels Jun 3, 2023
@github-actions github-actions bot added the dependencies Dependency related change label Jun 3, 2023
@github-actions github-actions bot added the test Automated test addition or improvement label Jun 3, 2023
MarkEWaite added a commit to MarkEWaite/docker-lfs that referenced this pull request Jun 3, 2023
@MarkEWaite
Use +PrioritySorter plugin 4.1.1-rc668.29a_cc42d49f5 incremental
b9658c0 
jenkinsci/priority-sorter-plugin#262

Configuration as code while trying to not lose the improvements from the
master branch.  Needs documentation of configuration as code sample
configurations and needs compatibility testing.
@MarkEWaite
Copy link
Contributor

Thanks very much for the pull request. I've added one of the changes from the master branch back into the pull request so that the new icon renders correctly on the pages.

I've started testing the incremental build of this pull request in my configuration as code environment and it looks very good.

The documentation in the README file needs a section that describes configuration as code examples. Would you be willing to add that documentation?

@julianxhokaxhiu you mentioned in #62 (comment) that you were waiting for the next iteration. This is the next iteration. I'd love to have your help testing it to confirm that compatibility is retained and that configuration as code works as expected.

@easybe you had tested the earlier pull request ( #62 ). It would be very nice if you could help with the testing of this pull request.

@easybe
Copy link

easybe commented Jun 3, 2023 •
edited
Loading

@easybe you had tested the earlier pull request ( #62 ). It would be very nice if you could help with the testing of this pull request.

Unfortunately, we do not use Jenkins in the company I work for now. But, I reached out to my former co-workers.

Thanks for finally wrapping this up!

@julianxhokaxhiu
Copy link

Unfortunately, we do not use Jenkins in the company I work for now. But, I reached out to my former co-workers.

Thanks for finally wrapping this up!

Same here, hopefully someone will reach out soon :) Nevertheless, thanks for making this possible!

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Jun 9, 2023
@Kamyki
Copy link
Contributor Author

Kamyki commented Jun 9, 2023

I added README section with sample casc configuration. Is there something else I can do to speed up acceptance of this PR ? Are we just waiting for more people to test it?

@MarkEWaite
Copy link
Contributor

I added README section with sample casc configuration. Is there something else I can do to speed up acceptance of this PR ? Are we just waiting for more people to test it?

That was what I was seeking, since there were two users during the life of the pull request who had expressed interest in configuration as code and seemed like good candidates to test it. However, since they are both unavailable to test it, I think your testing and my testing are enough to declare it sufficient to release.

@MarkEWaite MarkEWaite removed dependencies Dependency related change documentation Improvements or additions to documentation test Automated test addition or improvement labels Jun 10, 2023
@github-actions github-actions bot added dependencies Dependency related change documentation Improvements or additions to documentation test Automated test addition or improvement labels Jun 10, 2023
@MarkEWaite MarkEWaite removed dependencies Dependency related change documentation Improvements or additions to documentation test Automated test addition or improvement labels Jun 10, 2023
MarkEWaite
MarkEWaite approved these changes Jun 10, 2023
View reviewed changes
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Completed my interactive testing, confirmed that the configuration as code works in my installation, and implemented two additional tests to be sure.

@MarkEWaite MarkEWaite merged commit e90616b into jenkinsci:master Jun 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarkEWaite MarkEWaite MarkEWaite approved these changes

Assignees
No one assigned
Labels
enhancement Improvement or new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Kamyki @MarkEWaite @easybe @julianxhokaxhiu @fr123k