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

Add a note on reloading from declarative pipelines #1227 #1675

Merged
merged 4 commits into from Oct 18, 2021
Merged

Add a note on reloading from declarative pipelines #1227 #1675

merged 4 commits into from Oct 18, 2021

Conversation

fugkco
Copy link
Contributor

@fugkco fugkco commented Aug 13, 2021

If JCasC is reloaded from a declarative pipeline, Jenkins throws an error due to permission issues. Subsequent reloads of JCasC fail regardless of the mechanisms used to do so, and regardless of user permissions.

This issue probably needs to be addressed correctly so that Jenkins isn't put in an invalid state, as it could potentially have other side effects that hasn't been noticed yet.

Your checklist for this pull request

🚨 Please review the guidelines for contributing to this repository.

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side) and not your master branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or in Jenkins JIRA
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Did you provide a test-case? That demonstrates feature works or fixes the issue.

Relates to #1227

If JCasC is reloaded from a declarative pipeline, Jenkins throws an error due to permission issues. Subsequent reloads of JCasC fail regardless of the mechanisms used to do so, and regardless of user permissions.

This issue probably needs to be addressed correctly so that Jenkins isn't put in an invalid state, as it could potentially have other side effects that hasn't been noticed yet.
@fugkco fugkco requested a review from a team as a code owner August 13, 2021 12:20
@@ -29,3 +29,4 @@ reload-jcasc-configuration
import io.jenkins.plugins.casc.ConfigurationAsCode;
ConfigurationAsCode.get().configure()
```
Note that running the above code in a declarative pipeline (either directly or via shared libraries) will put this plugin in a bad state where the configuration cannot be reload at all until Jenkins is restarted. See #1227 for more info.
Copy link
Member

Choose a reason for hiding this comment

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

is it better to just remove the above from the docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ultimately that is something for CloudBees/CJCasC maintainers to decide, but I don't believe so. So far we've been using this successfully as per the workaround posted in #1227, and this works as long as a) the script approvals allow it, b) it's a freestyle job, and c) it is run using systemGroovyCommand. The issue of course is when this is run through pipelineJob. Mind you, even if the above three predicates aren't met and the code is run in a freestyle job, Jenkins is still in a good state and the code above can be run e.g. within the script console fine.

@oleg-nenashev oleg-nenashev added the documentation A PR that adds to documentation - used by Release Drafter label Aug 14, 2021
@fugkco
Copy link
Contributor Author

fugkco commented Oct 17, 2021

@timja will this get approved/merged? If not, I'll close this PR. Thanks

@timja timja enabled auto-merge (squash) October 18, 2021 08:07
@codecov
Copy link

codecov bot commented Oct 18, 2021

Codecov Report

Merging #1675 (18d01ff) into master (d77a466) will increase coverage by 0.16%.
The diff coverage is n/a.

❗ Current head 18d01ff differs from pull request most recent head 5367924. Consider uploading reports for the commit 5367924 to get more accurate results

@@             Coverage Diff              @@
##             master    #1675      +/-   ##
============================================
+ Coverage     80.64%   80.80%   +0.16%     
- Complexity      820      835      +15     
============================================
  Files            66       71       +5     
  Lines          2402     2464      +62     
  Branches        340      345       +5     
============================================
+ Hits           1937     1991      +54     
- Misses          356      363       +7     
- Partials        109      110       +1     
Impacted Files Coverage Δ
...ators/GlobalConfigurationCategoryConfigurator.java 87.80% <0.00%> (-0.57%) ⬇️
...plugins/casc/impl/DefaultConfiguratorRegistry.java 74.00% <0.00%> (-0.51%) ⬇️
...casc/impl/configurators/DataBoundConfigurator.java 82.80% <0.00%> (-0.11%) ⬇️
...java/io/jenkins/plugins/casc/SchemaGeneration.java 93.90% <0.00%> (-0.08%) ⬇️
...ain/java/io/jenkins/plugins/casc/Configurator.java 79.06% <0.00%> (ø)
...ain/java/io/jenkins/plugins/casc/model/Scalar.java 88.67% <0.00%> (ø)
...java/io/jenkins/plugins/casc/BaseConfigurator.java 85.35% <0.00%> (ø)
...a/io/jenkins/plugins/casc/ConfigurationAsCode.java 80.31% <0.00%> (ø)
...kins/plugins/casc/yaml/StreamReaderWithSource.java 85.71% <0.00%> (ø)
...nkins/plugins/casc/misc/RoundTripAbstractTest.java 98.33% <0.00%> (ø)
... and 8 more

@timja timja merged commit 8f54e71 into jenkinsci:master Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation A PR that adds to documentation - used by Release Drafter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants