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

[JENKINS-51469] Add Essentials configuration to CasC tests #216

Merged
merged 2 commits into from
May 23, 2018
Merged

[JENKINS-51469] Add Essentials configuration to CasC tests #216

merged 2 commits into from
May 23, 2018

Conversation

batmat
Copy link
Member

@batmat batmat commented May 22, 2018 •
edited
Loading

This PR is based off configuration-as-code-0.5-alpha on purpose.
On my machine, mvn clean verify is successful.

But if I update (i.e. merge or rebase) on latest master it fails.

It fails with the following error:

[SEVERE][2018-05-22 08:37:32] Failed ConfigurationAsCode.init (from jenkins.InitReactorRunner$1 onTaskFailed)
java.lang.Error: java.lang.reflect.InvocationTargetException
	at hudson.init.TaskMethodFinder.invoke(TaskMethodFinder.java:110)
	at hudson.init.TaskMethodFinder$TaskImpl.run(TaskMethodFinder.java:175)
	at org.jvnet.hudson.reactor.Reactor.runTask(Reactor.java:296)
	at jenkins.model.Jenkins$5.runTask(Jenkins.java:1068)
	at org.jvnet.hudson.reactor.Reactor$2.run(Reactor.java:214)
	at org.jvnet.hudson.reactor.Reactor$Node.run(Reactor.java:117)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.reflect.InvocationTargetException
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at hudson.init.TaskMethodFinder.invoke(TaskMethodFinder.java:104)
	... 8 more
Caused by: org.jenkinsci.plugins.casc.ConfiguratorException: jenkins: error configuring <jenkins> with <jenkins> configurator
	at org.jenkinsci.plugins.casc.ConfigurationAsCode.configureWith(ConfigurationAsCode.java:339)
	at org.jenkinsci.plugins.casc.ConfigurationAsCode.configureWith(ConfigurationAsCode.java:280)
	at org.jenkinsci.plugins.casc.ConfigurationAsCode.configure(ConfigurationAsCode.java:252)
	at org.jenkinsci.plugins.casc.ConfigurationAsCode.configure(ConfigurationAsCode.java:155)
	at org.jenkinsci.plugins.casc.ConfigurationAsCode.init(ConfigurationAsCode.java:134)
	... 13 more
Caused by: org.jenkinsci.plugins.casc.ConfiguratorException: Invalid configuration elements for type class hudson.model.Hudson : metricsaccesskey
	at org.jenkinsci.plugins.casc.BaseConfigurator.configure(BaseConfigurator.java:200)
	at org.jenkinsci.plugins.casc.JenkinsConfigurator.configure(JenkinsConfigurator.java:44)
	at org.jenkinsci.plugins.casc.JenkinsConfigurator.configure(JenkinsConfigurator.java:24)
	at org.jenkinsci.plugins.casc.ConfigurationAsCode.configureWith(ConfigurationAsCode.java:335)
	... 17 more

oleg-nenashev
oleg-nenashev approved these changes May 22, 2018
View reviewed changes
Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

I would just split it to "MetricsPluginTest" and "SystemMessage" test. The provided test does not really verify that metrics are loaded correctly tho

@batmat
Copy link
Member Author

batmat commented May 22, 2018

@oleg-nenashev that is right, the test does not check everything that could be, I filed it as-is for now since it does fail locally for me though. Failing the exact same way as in essentials tests currently on the master branch. So I figured it would be enough as a first stab and help for CasC devs to figure out what got changed.

@batmat
Copy link
Member Author

batmat commented May 22, 2018

I bisected the issue:

git bisect log                                                                                                                                           37e4cbb
git bisect start
# good: [8921814751e0465ba31b853b9adec18bb557f0ff] [maven-release-plugin] prepare release configuration-as-code-0.5-alpha
git bisect good 8921814751e0465ba31b853b9adec18bb557f0ff
# bad: [4055b206726c069579cf38543bc274ca4d740657] Avoid calling as sequence on a null object (#215)
git bisect bad 4055b206726c069579cf38543bc274ca4d740657
# bad: [8de6b0b94a56968100180bd7f9ae96818257d7e4] Shortcut notation to programmatically set scalar values
git bisect bad 8de6b0b94a56968100180bd7f9ae96818257d7e4
# good: [7d2a93d1ce2d716de41b4f82e31d427e1b55e9eb] Run configuration before jobs are loaded.
git bisect good 7d2a93d1ce2d716de41b4f82e31d427e1b55e9eb
# good: [533a77e49d95d144806d3d73b1983bb710d1a49e] link to export live instance as Yaml
git bisect good 533a77e49d95d144806d3d73b1983bb710d1a49e
# bad: [d9e751109a444cd24c9070ab067261e8acfbaed1] fix ActiveDirectoryTest (jenkinsInternalUser was ignored)
git bisect bad d9e751109a444cd24c9070ab067261e8acfbaed1
# bad: [37e4cbb3803c4f70450a1ad69a3b564863773739] introduce a model for configuration tree so we abstract away fro snakeyaml API (and could later switch to another lib) and can restrict supported model to Map|List|Scalars
git bisect bad 37e4cbb3803c4f70450a1ad69a3b564863773739
# first bad commit: [37e4cbb3803c4f70450a1ad69a3b564863773739] introduce a model for configuration tree so we abstract away fro snakeyaml API (and could later switch to another lib) and can restrict supported model to Map|List|Scalars

gentle ping @ndeloof @ewelinawilkosz thanks!

@ewelinawilkosz
Copy link
Contributor

just from the top of my head - a lot of configurators that were under jenkins root element are moved to unclassified now, maybe you need to fix your yaml?

@batmat
Copy link
Member Author

batmat commented May 22, 2018

FTR, fails the expected way:
image

@batmat
Copy link
Member Author

batmat commented May 22, 2018

@ewelinawilkosz possibly. Checking the docs. That rings some bell somewhere, but I didn't follow closely your changes TBH indeed.

@batmat batmat mentioned this pull request May 22, 2018
Merged
@ewelinawilkosz
Copy link
Contributor

the docs... I should review the dosc and make sure everything is aligned. didn't do that before :(

@batmat batmat mentioned this pull request May 22, 2018
Merged
@batmat
Copy link
Member Author

batmat commented May 22, 2018

@ewelinawilkosz we can adjust the yaml if need be. I've just no idea as to what to do actually. cc @ndeloof

@ewelinawilkosz
Copy link
Contributor

sure,
so in ca82f27 @ndeloof moved all unclassified descriptors under separate root element - I assume most of the plugins to end up there
here is an example for mailer
https://github.com/jenkinsci/configuration-as-code-plugin/blob/1070b7da6cfbadffec0dc8bbe5ce4b78c1315337/src/test/resources/org/jenkinsci/plugins/casc/MailerTest.yml
I've created #217 to fix that

@ndeloof
Copy link
Contributor

ndeloof commented May 22, 2018

indeed, as part of the "export" feature development, as I can't compare current Jenkins instance state with a reference state (one can't build two Jenkins objects within the same JVM), it sounded bad we dump data for all unclassified descriptors. By the way this could help plugin authors adopt those categories (I've seen few of them being 'unclassified' but which should belong an existing category)

@batmat
Move metricsAccessKey to new unclassified root
f2982fb 
Also enrich tests heavily in the go.
@batmat batmat mentioned this pull request May 23, 2018
Merged
@batmat
Copy link
Member Author

batmat commented May 23, 2018 •
edited
Loading

@ndeloof @ewelinawilkosz adapted the PR to the new format. I believe, once the PR build is green, it would be valuable to merge this to more easily track compatibility with what we use on Essentials side. Thanks!

@batmat
Copy link
Member Author

batmat commented May 23, 2018

Failure here because of FindBugs failing on master branch...

@ndeloof ndeloof merged commit 13dd9ab into jenkinsci:master May 23, 2018
@batmat batmat deleted the JENKINS-51469 branch May 23, 2018 10:01
@batmat
Copy link
Member Author

batmat commented May 23, 2018

Merci Nicolas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oleg-nenashev oleg-nenashev oleg-nenashev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@batmat @ewelinawilkosz @ndeloof @oleg-nenashev