(Note: mainly bumping for the CI to not fail since I was never able to
see this test fail on my machine [takes ~17 with this change] anyway
other than by running in Docker and aggressively reduce IOPS to 40 using
Docker resource constraints.)
Since we now *actually* sync to disk, this isn't really a regression IMO.
From the very role of AtomicFileWriter, it looks more like a feature or
an improvement than an issue.
So I suspect that anyway 1) we won't see
a lot of bad impact in the wild on most setups (as we generally already
recommend SSDs and so on as per how Jenkins works) and 2) we win anyway
in term of integrity by making sure (well, raising the chances, let's
say) we do write to disk before renaming files.
* Use NIO for FilePath#chmod and IOUtils#mode
* Add tests for NIO mode and chmod implementations
* Add test, remove new method, and update JavaDoc
* Provide system property to use native implementations of chmod and mode
* Revert unrelated whitespace modification
* Don't remove exception from throws and put imports in original location
* Fix broken JavaDoc links
* Ignore file type bits (above 0o7777) in Util#modeToPermission
* Use octal for constants and don't include file type bits
* Revert unnecessary changes to TarArchiverTest
* Add assertion that non-permission bits are ignored by chmod
* Use NIO copy with StandardCopyOption.COPY_ATTRIBUTES in copyToWithPermissions where possible
* Catch InvalidPathException and convert it to IOException
* Create utility method for File#toPath and use File#createDirectories after review
* Remove useless calls to toAbsolutePath and getAbsoluteFile
* Fix typos and use octal for constant after review
* Add test for behavior of copyToWithPermission with special bits
* Add default implementations to deprecated methods of BuilableItem and Item.
Currently the interface requires the API user to implement already deprecated methods.
It does not make much sense, and the API could be simplified.
* Address comments from @jglick
Not that critical, but still can keep them as we should
generally be on FS that support atomic moves. Hence not
enter that catch.
If proved otherwise, we'll reconsider adjusting this verbosity.
It might be preferrable from a consistency standpoint,
but many CLI tests start failing when you force this.
So, as the previous behaviour was not forcing the sync, this seems
to show there could be unintended behavioural changes in effect.
Possibly, we'll want to address this later, but as the main goal
here was to make commit() atomic as much as possible, fixing
this is probably another story anyway.
Note: I was *never* able to reproduce those failures on my machine in
a normal env dev. It was only visible in CI.
I finally managed to make it reproducible (but still a bit randomly) by
using Docker resource constraints, using something like this:
* cd to jenkins local clone
* `docker run -ti -v m2repo:/root/.m2/repository -v $PWD:/work -v ~/.m2/settings.xml:/root/.m2/settings.xml:ro -v ~/.m2/settings-security.xml:/root/.m2/settings-security.xml:ro --device-write-iops /dev/mapper/fedora-home:40 --device-read-iops /dev/mapper/fedora-home:40 --device-write-bps /dev/mapper/fedora-home:10m --device-read-bps /dev/mapper/fedora-home:10m maven:3.5.2-jdk-8 bash`
* cd /work
* `mvn clean install -Dtest=WaitNodeOfflineCommandTest,RunRangeCommandTest -Dfindbugs.skip=true -DfailIfNoTests=false -Dskip.npm=true`
Failure example:
```
ERROR] Tests run: 6, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 55.68OfflineCommandTest
[ERROR] waitNodeOfflineShouldSucceedOnDisconnectingNode(hudson.cli.WaitNodeOf <<< FAILURE!
java.lang.AssertionError:
Expected: <true>
but: was <false>
at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:8)
at hudson.cli.WaitNodeOfflineCommandTest.waitNodeOfflineShouldSucceedndTest.java:128)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorIm
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAc
at java.lang.reflect.Method.invoke(Method.java:498)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(Framew
at org.junit.internal.runners.model.ReflectiveCallable.run(Reflective
at org.junit.runners.model.FrameworkMethod.invokeExplosively(Framewor
at org.junit.internal.runners.statements.InvokeMethod.evaluate(Invoke
at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefor
at org.jvnet.hudson.test.JenkinsRule$1.evaluate(JenkinsRule.java:542)
at org.junit.internal.runners.statements.FailOnTimeout$CallableStatem
at org.junit.internal.runners.statements.FailOnTimeout$CallableStatem
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at java.lang.Thread.run(Thread.java:748)
[ERROR] waitNodeOfflineShouldSucceedOnDisconnectedNode(hudson.cli.WaitNodeOff <<< FAILURE!
java.lang.AssertionError
```
* [JENKINS-48157] - Reproduce the issue in test
* [JENKINS-48157] - Annotate and document nullness conditions in MyViewsProperty and ViewGroupMixIn
* [FIXED JENKINS-48157] - Prevent NPEs when using public API and when using null primaryViewName
* [JENKINS-48157] - Fix typo in Javadoc
I see we enter that catch, and the `else` in a failing test.
But I'm somewhat stuck since I miss both the stack, and even at
least the exception being thrown and not being an
`AtomicMoveNotSupportedException`.
It's possibly an `IOException` or some subclass. But still having the
stack could help understand why the test fails, and provide more
data for diagnosis in the future in production use.
```
=== Starting hudson.cli.RunRangeCommandTest
0.007 [id=96] WARNING o.jvnet.hudson.test.JenkinsRule#before: Jenkins.theInstance was not cleared by a previous test, doing that now
0.018 [id=96] INFO o.jvnet.hudson.test.JenkinsRule#createWebServer: Running on http://localhost:45833/jenkins/
0.037 [id=15] WARNING jenkins.model.Jenkins#cleanUp: This instance is no longer the singleton, ignoring cleanUp()
0.345 [id=96] WARNING hudson.util.AtomicFileWriter#commit: Unable to move atomically, falling back to non-atomic move.
0.345 [id=96] SEVERE hudson.util.AtomicFileWriter#commit: Unable to move /home/jenkins/workspace/Core_jenkins_PR-2548-5VRJ4BBBSEGVOZEA7OVAL2YLTLCBL55RB4LDQIZN56WJZD4M6ZKA/test/target/jenkinsTests.tmp/jenkins333129509690162870test/atomic9184049448662376708tmp to /home/jenkins/workspace/Core_jenkins_PR-2548-5VRJ4BBBSEGVOZEA7OVAL2YLTLCBL55RB4LDQIZN56WJZD4M6ZKA/test/target/jenkinsTests.tmp/jenkins333129509690162870test/secret.key. Attempting to delete /home/jenkins/workspace/Core_jenkins_PR-2548-5VRJ4BBBSEGVOZEA7OVAL2YLTLCBL55RB4LDQIZN56WJZD4M6ZKA/test/target/jenkinsTests.tmp/jenkins333129509690162870test/atomic9184049448662376708tmp and abandoning.
```
```
WARNING] Rule 3: org.apache.maven.plugins.enforcer.RequireUpperBoundDeps fai
Failed while enforcing RequireUpperBoundDeps. The error(s) are [
Require upper bound dependencies error for org.objenesis:objenesis:2.1 paths
+-org.jenkins-ci.main:test:2.92-SNAPSHOT
+-org.mockito:mockito-core:1.10.19
+-org.objenesis:objenesis:2.1
and
+-org.jenkins-ci.main:test:2.92-SNAPSHOT
+-org.awaitility:awaitility:3.0.0
+-org.objenesis:objenesis:2.5.1
```
Also reduce the time 250 ms (instead of 1 second previously) between
retries to hopefully reduce the test duration on quick enough setups,
and still degrade nicely on less performing ones.
Currently Jenkins to fire WorkflowJob re-calculates the ConstistentHash
for entire cluster even if there is no nodes were updated. If cluster is
big enough (>100 nodes), it becomes a problem, because MD5 is quite
expensive itself plus all this logic comes with high memory footprint.
Using the knowledge that WorkflowJob can be executed only on Jenkins master,
we can create a shortcut that does not do expensive calculation and just
returns Runnable or `null`.
Files.createDirectories(dir) is said to not fail if directory already
exists.
*But* this will still fail if `dir` is actually a symlink to a
directory...
This is documented in the Javadoc indeed, even if probably not really
one would expect...
See https://bugs.openjdk.java.net/browse/JDK-8130464
Without this fix:
```
[INFO] Running hudson.util.SecretRewriterTest
Cycle detected: /home/tiste/dev/JENKINS/jenkins/core/target/junit7537647214741745549/t/c/symlink
Scanning /home/tiste/dev/JENKINS/jenkins/core/target/junit7537647214741745549/t/c/foo.xml
Rewritten /home/tiste/dev/JENKINS/jenkins/core/target/junit7537647214741745549/t/c/foo.xml
Rewritten /home/tiste/dev/JENKINS/jenkins/core/target/junit7537647214741745549/t/c/d/foo.xml
Rewritten /home/tiste/dev/JENKINS/jenkins/core/target/junit7537647214741745549/t/c/d/e/foo.xml
ERROR: Failed to rewrite /home/tiste/dev/JENKINS/jenkins/core/target/junit7537647214741745549/t/a/symlink/foo.xml
java.nio.file.FileAlreadyExistsException: /home/tiste/dev/JENKINS/jenkins/core/target/junit7537647214741745549/t/a/symlink
at sun.nio.fs.UnixException.translateToIOException(UnixException.java:88)
at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)
at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)
at sun.nio.fs.UnixFileSystemProvider.createDirectory(UnixFileSystemProvider.java:384)
at java.nio.file.Files.createDirectory(Files.java:674)
at java.nio.file.Files.createAndCheckIsDirectory(Files.java:781)
at java.nio.file.Files.createDirectories(Files.java:727)
at hudson.util.AtomicFileWriter.<init>(AtomicFileWriter.java:103)
Caused: java.io.IOException: Failed to create a temporary file in /home/tiste/dev/JENKINS/jenkins/core/target/junit7537647214741745549/t/a/symlink
at hudson.util.AtomicFileWriter.<init>(AtomicFileWriter.java:106)
at hudson.util.AtomicFileWriter.<init>(AtomicFileWriter.java:73)
at hudson.util.SecretRewriter.rewrite(SecretRewriter.java:85)
at hudson.util.SecretRewriter.rewriteRecursive(SecretRewriter.java:166)
at hudson.util.SecretRewriter.rewriteRecursive(SecretRewriter.java:176)
at hudson.util.SecretRewriter.rewriteRecursive(SecretRewriter.java:176)
at hudson.util.SecretRewriter.rewriteRecursive(SecretRewriter.java:141)
at hudson.util.SecretRewriter$rewriteRecursive$0.call(Unknown Source)
at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:48)
at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:113)
at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:133)
at hudson.util.SecretRewriterTest.recursionDetection(SecretRewriterTest.groovy:110)
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.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
at org.junit.rules.RunRules.evaluate(RunRules.java:20)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:365)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:272)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:236)
at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:386)
at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:323)
at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:143)
Rewritten /home/tiste/dev/JENKINS/jenkins/core/target/junit7537647214741745549/t/a/foo.xml
Cycle detected: /home/tiste/dev/JENKINS/jenkins/core/target/junit7537647214741745549/t/b/symlink
Rewritten /home/tiste/dev/JENKINS/jenkins/core/target/junit7537647214741745549/t/b/foo.xml
[ERROR] Tests run: 2, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 1.097 s <<< FAILURE! - in hudson.util.SecretRewriterTest
[ERROR] recursionDetection(hudson.util.SecretRewriterTest) Time elapsed: 0.176 s <<< FAILURE!
Assertion failed:
assert 6==sw.rewriteRecursive(t, st)
| | | | |
| | 5 | hudson.util.StreamTaskListener@34a2d6e0
| | /home/tiste/dev/JENKINS/jenkins/core/target/junit7537647214741745549/t
| hudson.util.SecretRewriter@2525a5b8
false
at org.codehaus.groovy.runtime.InvokerHelper.assertFailed(InvokerHelper.java:404)
at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.assertFailed(ScriptBytecodeAdapter.java:650)
at hudson.util.SecretRewriterTest.recursionDetection(SecretRewriterTest.groovy:110)
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.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
at org.junit.rules.RunRules.evaluate(RunRules.java:20)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:365)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:272)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:236)
at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:386)
at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:323)
at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:143)
[INFO]
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] SecretRewriterTest.recursionDetection:110 assert 6==sw.rewriteRecursive(t, st)
| | | | |
| | 5 | hudson.util.StreamTaskListener@34a2d6e0
| | /home/tiste/dev/JENKINS/jenkins/core/target/junit7537647214741745549/t
| hudson.util.SecretRewriter@2525a5b8
false
[INFO]
[ERROR] Tests run: 2, Failures: 1, Errors: 0, Skipped: 0
```
Every request that comes from Jelly is checked against Permissions.
As result it leads to a call of `getId` method that produces the new string.
Usually it's not a problem, but in case of stop-the-world pause user requests are accumulated.
So, once pause is finished, we forcibly allocated tons of strings for
every request. That leads to new stop-the-world pause. (And this cycle
can repeat multiple times)
Since we have the confirmed regression due to the binary compatibility change, I think we need to restore the compatibility.
OTOH, I restricted the class, so all users will be forced to stop using it when they updgrade the core.
Baptiste Mathus