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

Adding 2.190.x line #110

Merged
merged 43 commits into from
Apr 8, 2020
Merged

Adding 2.190.x line #110

merged 43 commits into from
Apr 8, 2020

Conversation

jglick
Copy link
Member

@jglick jglick commented Sep 27, 2019

No description provided.

@jglick jglick added the enhancement New feature or request label Sep 27, 2019
…, need to work around jenkinsci/jenkins#4118 when declaring a new LTS baseline.
oleg-nenashev
oleg-nenashev previously approved these changes Sep 27, 2019
@jglick
Copy link
Member Author

jglick commented Sep 27, 2019

Hmm, there are test failures but they are not getting reported:

java.lang.NullPointerException
	at hudson.tasks.junit.TestResultAction.getDataFile(TestResultAction.java:132)
	at hudson.tasks.junit.TestResultAction.load(TestResultAction.java:208)
	at hudson.tasks.junit.TestResultAction.getResult(TestResultAction.java:138)
	at hudson.tasks.junit.TestResultAction.getResult(TestResultAction.java:63)
	at hudson.tasks.test.AbstractTestResultAction.findCorrespondingResult(AbstractTestResultAction.java:252)
	at hudson.tasks.test.TestResult.getPreviousResult(TestResult.java:146)
	at hudson.tasks.junit.SuiteResult.getPreviousResult(SuiteResult.java:416)
	at hudson.tasks.junit.CaseResult.getPreviousResult(CaseResult.java:483)
	at hudson.tasks.junit.CaseResult.recomputeFailedSinceIfNeeded(CaseResult.java:411)
	at hudson.tasks.junit.CaseResult.freeze(CaseResult.java:644)
	at hudson.tasks.junit.SuiteResult.freeze(SuiteResult.java:462)
	at hudson.tasks.junit.TestResult.freeze(TestResult.java:748)
	at hudson.tasks.junit.JUnitResultArchiver.parseAndAttach(JUnitResultArchiver.java:178)
	at hudson.tasks.junit.pipeline.JUnitResultsStepExecution.run(JUnitResultsStepExecution.java:52)
	at hudson.tasks.junit.pipeline.JUnitResultsStepExecution.run(JUnitResultsStepExecution.java:25)
	at org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution.lambda$start$0(SynchronousNonBlockingStepExecution.java:47)
	at …

which looks like TestResultAction.run == null for some reason. Not really sure what to make of that except something being seriously broken on the server. CC @olblak

@jglick
Copy link
Member Author

jglick commented Sep 27, 2019

I also saw

java.lang.NullPointerException
	at org.jenkinsci.plugins.workflow.cps.EnvActionImpl.getEnvironment(EnvActionImpl.java:88)

recently in this PR (build 2), again a case of an owner field not being updated from a RunAction2, suggesting a lower-level problem in Jenkins core or workflow-job or something. Do you see any warnings in the system log from Run.onLoad or anything like that?

@jglick
Copy link
Member Author

jglick commented Sep 30, 2019

Hoping the resolution of INFRA-2283 unblocks this.

@jglick
Copy link
Member Author

jglick commented Oct 1, 2019

Finally got in results, with a bunch of PCT failures.

@basil
Copy link
Member

basil commented Oct 1, 2019

Finally got in results, with a bunch of PCT failures.

May I suggest incorporating this type of plugin compatibility testing into the official Jenkins LTS release process? In other words, why not do this type of testing before an LTS line is released rather than after the fact? As an end user, I'd feel more comfortable upgrading to a given LTS release knowing that essential plugins (including Pipeline) have been tested with it.

@jglick
Copy link
Member Author

jglick commented Oct 1, 2019

May I suggest

Yes!

I could have filed a draft PR like this one as soon as the LTS line is announced, just building against the weekly baseline. Unfortunately the way the */pom.xml are set up means that if it were not merged promptly, every Dependabot PR would conflict with it, since Git only activates rename detection if the origin file no longer exists, and the way the BOM is set up, most of the content is always in the newest one. … I suppose I could just rename the directory for the latest line to be simply bom-latest, hmm. Will play with all this.

@basil
Copy link
Member

basil commented Oct 2, 2019

Finally got in results, with a bunch of PCT failures.

I wrote PipelineTest.globalDecoratorAnnotator, so I started looking into this failure. The 500 Server Error was caused by this stack trace:

Caused by: java.lang.NoClassDefFoundError: com/trilead/ssh2/crypto/Base64
	at org.jenkinsci.plugins.workflow.log.ConsoleAnnotators.setAnnotator(ConsoleAnnotators.java:101)
	at org.jenkinsci.plugins.workflow.log.FileLogStorage$1.writeHtmlTo(FileLogStorage.java:248)
	at hudson.model.Run.writeLogTo(Run.java:1488)
	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 org.apache.commons.jexl.util.introspection.UberspectImpl$VelMethodImpl.invoke(UberspectImpl.java:258)
	at org.apache.commons.jexl.parser.ASTMethod.execute(ASTMethod.java:104)
	at org.apache.commons.jexl.parser.ASTReference.execute(ASTReference.java:83)
	at org.apache.commons.jexl.parser.ASTReference.value(ASTReference.java:57)
	at org.apache.commons.jexl.parser.ASTReferenceExpression.value(ASTReferenceExpression.java:51)
	at org.apache.commons.jexl.ExpressionImpl.evaluate(ExpressionImpl.java:80)
	at hudson.ExpressionFactory2$JexlExpression.evaluate(ExpressionFactory2.java:74)
	at org.apache.commons.jelly.parser.EscapingExpression.evaluate(EscapingExpression.java:24)
	at org.apache.commons.jelly.impl.ExpressionScript.run(ExpressionScript.java:66)
	at org.apache.commons.jelly.TagSupport.invokeBody(TagSupport.java:161)
	at org.apache.commons.jelly.tags.core.WhitespaceTag.doTag(WhitespaceTag.java:48)
	at org.apache.commons.jelly.impl.TagScript.run(TagScript.java:269)
	... 115 more

Reading the changelog, I see the following:

Remove Trilead SSH library from Jenkins core and make it available in a new detached plugin. (issue 43610)

I'm not quite sure what to make of all of this yet, perhaps because I am unfamiliar with how split plugins work. I read this documentation, which led me to split-plugins.txt. Then I noticed that prep.sh is creating a split-plugins.txt whose contents are just the single line # nothing. Presumably this is to prevent DetachedPluginsUtil.getImpliedDependencies from returning any implied dependencies in order to simplify the implementation of the BOM somehow.

I set aside any latent questions about why split-plugins.txt is empty and decided to try adding trilead-api to the managed set explicitly, first at its latest version (1.0.5) and then with the version I'm currently using in production (1.0.3) with code like this:

diff --git a/bom-latest/pom.xml b/bom-latest/pom.xml
index a00b981..93a2ea6 100644
--- a/bom-latest/pom.xml
+++ b/bom-latest/pom.xml
@@ -253,6 +253,11 @@
                 <artifactId>timestamper</artifactId>
                 <version>1.10</version>
             </dependency>
+            <dependency>
+                <groupId>org.jenkins-ci.plugins</groupId>
+                <artifactId>trilead-api</artifactId>
+                <version>1.0.3</version>
+            </dependency>
             <dependency>
                 <groupId>org.jenkins-ci.plugins</groupId>
                 <artifactId>token-macro</artifactId>

This triggered the following error while building the sample project:

Managed dependencies of MavenProject: io.jenkins.tools.bom:sample:4-SNAPSHOT @ /home/basil/work/third-party/jenkinsci/bom/sample-plugin/target/sample-4-SNAPSHOT.pom: [com.github.spotbugs:spotbugs-annotations:jar, net.jcip:jcip-annotations:jar, org.jenkins-ci.main:jenkins-core:jar, org.jenkins-ci.main:jenkins-war:executable-war, org.jenkins-ci.main:jenkins-test-harness:jar, org.jenkins-ci:test-annotations:jar, junit:junit:jar, javax.servlet:javax.servlet-api:jar, org.codehaus.mojo:animal-sniffer-annotations:jar, org.codehaus.mojo.signature:java15:signature, org.codehaus.mojo.signature:java16:signature, org.codehaus.mojo.signature:java17:signature, org.codehaus.mojo.signature:java18:signature, commons-logging:commons-logging:jar, log4j:log4j:jar, org.mockito:mockito-core:jar, org.powermock:powermock-module-junit4:jar, org.powermock:powermock-api-mockito2:jar, org.objenesis:objenesis:jar, io.jenkins:configuration-as-code:jar, io.jenkins:configuration-as-code:jar:tests, org.jenkins-ci.plugins.workflow:workflow-api:jar, org.jenkins-ci.plugins.workflow:workflow-api:jar:tests, org.jenkins-ci.plugins.workflow:workflow-basic-steps:jar, org.jenkins-ci.plugins.workflow:workflow-cps:jar, org.jenkins-ci.plugins.workflow:workflow-cps:jar:tests, org.jenkins-ci.plugins.workflow:workflow-cps-global-lib:jar, org.jenkins-ci.plugins.workflow:workflow-durable-task-step:jar, org.jenkins-ci.plugins.workflow:workflow-job:jar, org.jenkins-ci.plugins.workflow:workflow-scm-step:jar, org.jenkins-ci.plugins.workflow:workflow-step-api:jar, org.jenkins-ci.plugins.workflow:workflow-step-api:jar:tests, org.jenkins-ci.plugins.workflow:workflow-support:jar, org.jenkins-ci.plugins.workflow:workflow-support:jar:tests, org.jenkins-ci.plugins:ansicolor:jar, org.jenkins-ci.plugins:apache-httpcomponents-client-4-api:jar, org.jenkins-ci.plugins:branch-api:jar, org.jenkins-ci.plugins:cloudbees-folder:jar, org.jenkins-ci.plugins:command-launcher:jar, org.jenkins-ci.plugins:credentials:jar, org.jenkins-ci.plugins:credentials-binding:jar, org.jenkins-ci.plugins:display-url-api:jar, org.jenkins-ci.plugins:durable-task:jar, org.jenkins-ci.plugins:git:jar, org.jenkins-ci.plugins:git:jar:tests, org.jenkins-ci.plugins:git-client:jar, org.jenkins-ci.plugins:git-server:jar, org.jenkins-ci.plugins:jdk-tool:jar, org.jenkins-ci.plugins:jquery:jar, org.jenkins-ci.plugins:jsch:jar, org.jenkins-ci.plugins:junit:jar, org.jenkins-ci.plugins:junit-attachments:jar, org.jenkins-ci.plugins:mailer:jar, org.jenkins-ci.plugins:matrix-project:jar, org.jenkins-ci.plugins:pipeline-build-step:jar, org.jenkins-ci.plugins:pipeline-input-step:jar, org.jenkins-ci.plugins:plain-credentials:jar, org.jenkins-ci.plugins:scm-api:jar, org.jenkins-ci.plugins:scm-api:jar:tests, org.jenkins-ci.plugins:script-security:jar, org.jenkins-ci.plugins:ssh-credentials:jar, org.jenkins-ci.plugins:ssh-slaves:jar, org.jenkins-ci.plugins:structs:jar, org.jenkins-ci.plugins:timestamper:jar, org.jenkins-ci.plugins:trilead-api:jar, org.jenkins-ci.plugins:token-macro:jar, org.jenkins-ci.plugins:variant:jar, org.jenkins-ci.ui:ace-editor:jar, org.jenkins-ci.ui:jquery-detached:jar, org.jenkins-ci:symbol-annotation:jar]
Do not see managed dependency org.codehaus.mojo.signature:java15
Do not see managed dependency org.codehaus.mojo.signature:java16
Do not see managed dependency org.codehaus.mojo.signature:java17
Do not see managed dependency org.codehaus.mojo.signature:java18
Do not see managed dependency org.mockito:mockito-core
Do not see managed dependency org.powermock:powermock-module-junit4
Do not see managed dependency org.powermock:powermock-api-mockito2
Do not see managed dependency org.objenesis:objenesis

Now here's where I am starting to get really confused. Running mvn dependency:tree in trilead-api shows that trilead-api doesn't consume any of the above libraries. The only reference I can find to them is in plugin-pom. In any case, they seem like test libraries and it doesn't seem right to add them to the managed set at first blush.

I decided to investigate whether my suspicion was correct, so I started reading sample-plugin/check.groovy. I see that we're failing in the first 18 lines of that file, but I wasn't able to quickly figure out the intention of the code in order to confirm whether it was doing what it was supposed to or not. In general, that file remains a bit opaque to me, since there aren't too many high-level comments and I haven't yet had the time to really dive into what it's doing.

Before I dive into sample-plugin/check.groovy, I was wondering if I'm on the right track here at all or if I should be going in some other direction. A quick check at this point would probably save me some time if I need to turn around.

@jglick
Copy link
Member Author

jglick commented Oct 2, 2019

I already started trying to deal with the Trilead split, for another plugin. Might not have pushed that yet. I have some ideas, but anyway CI is down again so this is on the back burner.

@jglick
Copy link
Member Author

jglick commented Oct 3, 2019

Also occurs to me that deleting all record of plugin splits as in

bom/prep.sh

Lines 31 to 34 in 6d7fa6e

echo '# nothing' > jenkins/split-plugins.txt
cp -r jenkins-for-test megawar-$LINE
jar uvf megawar-$LINE/WEB-INF/lib/jenkins-core-*.jar jenkins/split-plugins.txt
rm -rfv megawar-$LINE/WEB-INF/detached-plugins megawar-$LINE/META-INF/*.{RSA,SF}
may be unsustainable. I do want to have BOM-managed plugins override the ones bundled in WEB-INF/detached-plugins/*.hpi.

@basil
Copy link
Member

basil commented Oct 4, 2019

I do want to have BOM-managed plugins override the ones bundled in WEB-INF/detached-plugins/*.hpi

That makes sense to me. I suppose that points to an implementation where we remove from split-plugins.txt only the entries that are explicitly managed by the BOM (as opposed to all entries)?

@jetersen
Copy link
Member

jetersen commented Mar 5, 2020

Uhh a green build :octocat:

@timja
Copy link
Member

timja commented Mar 5, 2020

@jglick you able to take a look and check you're happy with the tests I ignored for this to work?

I'll adjust the PRs to the flakey components soon,

and I don't know how to fix the cloudbees-folder or trilead-api issues, but I moved them to ignored so we can hopefully move on with our lives

@jglick
Copy link
Member Author

jglick commented Mar 5, 2020

Huh, seems I cannot request myself as a reviewer on my own PR…

@jglick
Copy link
Member Author

jglick commented Mar 5, 2020

(I should have closed this and refiled as an origin branch PR as soon as I stopped active work on it, I guess.)

@timja
Copy link
Member

timja commented Mar 5, 2020

maybe that will do ^ otherwise I can re-submit if you want

@jglick
Copy link
Member Author

jglick commented Mar 5, 2020

I will try to deal with this soon.

@timja
Copy link
Member

timja commented Mar 10, 2020

I will try to deal with this soon.

ping 😄 😉

@bitwiseman
Copy link
Contributor

@jglick Any thing to do here?

@jglick
Copy link
Member Author

jglick commented Mar 17, 2020

Yes, I need to review it and push out a release.

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 vote for merging it to unblock further changes. Tests could be fixed later, BOM is a best-effort test coverage anyway

@timja timja closed this Apr 8, 2020
@timja timja reopened this Apr 8, 2020
@jglick
Copy link
Member Author

jglick commented Apr 8, 2020

Tests could be fixed later

No. Without green test runs we have no way of knowing what is or is not safe to merge here.

rm -fv pct-work/workflow-support/target/surefire-reports/TEST-org.jenkinsci.plugins.workflow.support.pickles.serialization.SerializationSecurityTest.xml
rm -fv pct-work/workflow-durable-task-step/target/surefire-reports/TEST-org.jenkinsci.plugins.workflow.steps.durable_task.ShellStepTest.xml
rm -fv pct-work/workflow-durable-task-step/target/surefire-reports/TEST-org.jenkinsci.plugins.workflow.support.steps.ExecutorStepTest.xml
rm -fv pct-work/workflow-cps/target/surefire-reports/TEST-org.jenkinsci.plugins.workflow.cps.FlowDurabilityTest.xml
Copy link
Member Author

Choose a reason for hiding this comment

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

@timja I do see flakes here but did you try to track this down?

Copy link
Member Author

Choose a reason for hiding this comment

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

FlowDurabilityTest.testCompleteAndLoadBuilds expected:<...ne] End of Pipeline
[]> but was:<...ne] End of Pipeline
[Finished: SUCCESS
]>

specifically

Copy link
Member

Choose a reason for hiding this comment

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

Not sure it’s been a long time since I looked at these flakes

@jglick jglick merged commit a86fff6 into jenkinsci:master Apr 8, 2020
@jglick jglick deleted the 2.190.x branch April 8, 2020 22:06
MarkEWaite added a commit to MarkEWaite/git-plugin that referenced this pull request Apr 10, 2020
Jesse Glick noted that some flaky pipeline tests in other plugins
could be resolved by switching from assertLogContains to
waitForMessage. Since the git plugin has shown a tendency to flaky
tests on Windows, let's use Jesse's recommendation as well.

See jenkinsci/bom#110 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants