-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Use gradle and bundled jruby for acceptance tests orchestration #18536
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
base: main
Are you sure you want to change the base?
Conversation
🤖 GitHub commentsJust comment with:
|
|
run exhaustive tests |
|
This pull request does not have a backport label. Could you fix it @donoghuc? 🙏
|
|
run exhaustive tests |
andsel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a comment if we could avoid to compose command lines and use methods to run JRuby, plus a note on the usage of a deprecated API.
build.gradle
Outdated
| exec { | ||
| workingDir "${projectDir}/qa" | ||
| environment "BUNDLE_PATH", "${projectDir}/qa/vendor/bundle" | ||
| commandLine "${projectDir}/vendor/jruby/bin/jruby", "-S", "bundle", "install" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it's part of the problem that you try to avoid, but IIRC to run Ruby scripts inside Gradle we have a method named executeJruby in rubyUtils.grade
Line 190 in 9eeeb92
| Object executeJruby(File projectDir, File buildDir, Closure<?> /* Object*/ block) { |
Project.exec is deprecated since Gradle 8.11, I would avoid to use deprecated API usage, since that would add tech debt:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all, thanks for the pointer on the deprecated exec :)
Regarding why i'm shelling out here, I did a bit more of a deep dive in to WHY (and the reason may shock you)....
Backstory: I originally noticed this issue #18471 (comment) when building on test runners (notice how fpm wants to use file descriptors there to capture IO). At that time I just realized there was obviously a strageness there SO i decided i would just shell out early instead of trying to fight it.
With your comment here I decided it was time to do some investigation. Here is what i came up with:
As a minimal repro:
# build.gradle
tasks.register("testFdInheritance") {
dependsOn bootstrap
doLast {
rake(projectDir, buildDir, 'test:fd')
}
}
# fd_test.rake
task 'test:fd' do
r, w = IO.pipe
puts "Pipe FDs: r=#{r.fileno}, w=#{w.fileno}"
pid = Process.spawn("true", :out => w)
Process.wait(pid)
end
bash-4.4$ ./gradlew testFdInheritance
To honour the JVM settings for this build a single-use Daemon process will be forked. For more on this, please refer to https://docs.gradle.org/8.11.1/userguide/gradle_daemon.html#sec:disabling_the_daemon in the Gradle documentation.
Daemon will be stopped at the end of the build
> Task :downloadJRuby UP-TO-DATE
Download https://repo1.maven.org/maven2/org/jruby/jruby-dist/9.4.13.0/jruby-dist-9.4.13.0-bin.tar.gz
> Task :testFdInheritance FAILED
Pipe FDs: r=100003, w=100004
Rake task error: Errno::EBADF: Bad file descriptor - true
Backtrace: org/jruby/RubyProcess.java:1779:in `spawn'
/opt/buildkite-agent/logstash/rakelib/fd_test.rake:4:in `block in <main>'
/opt/buildkite-agent/logstash/vendor/bundle/jruby/3.1.0/gems/rake-13.3.1/lib/rake/task.rb:281:in `block in execute'
org/jruby/RubyArray.java:2009:in `each'
/opt/buildkite-agent/logstash/vendor/bundle/jruby/3.1.0/gems/rake-13.3.1/lib/rake/task.rb:281:in `execute'
/opt/buildkite-agent/logstash/vendor/bundle/jruby/3.1.0/gems/rake-13.3.1/lib/rake/task.rb:219:in `block in invoke_with_call_chain'
org/jruby/ext/monitor/Monitor.java:82:in `synchronize'
/opt/buildkite-agent/logstash/vendor/bundle/jruby/3.1.0/gems/rake-13.3.1/lib/rake/task.rb:199:in `invoke_with_call_chain'
/opt/buildkite-agent/logstash/vendor/bundle/jruby/3.1.0/gems/rake-13.3.1/lib/rake/task.rb:188:in `invoke'
<script>:6:in `<main>'
FAILURE: Build failed with an exception.
* Where:
Script '/opt/buildkite-agent/logstash/rubyUtils.gradle' line: 159
* What went wrong:
Execution failed for task ':testFdInheritance'.
> (null) Bad file descriptor - true
* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
> Get more help at https://help.gradle.org.
BUILD FAILED in 33s
39 actionable tasks: 4 executed, 35 up-to-date
As you can see somehow we get these crazy "high" file descriptor values r=100003, w=100004 which Process.spawn freaks out on. Something about how gradle/jruby interact there the file descriptors have some sort of virtual or proxy value that dont work when ruby wants to use them.
You can see that forking early "fixes" it (r=19, w=20)
# build.gradle
tasks.register("testFdStandalone", Exec) {
dependsOn bootstrap
workingDir projectDir
environment "GEM_HOME", "${projectDir}/vendor/bundle/jruby/3.1.0"
environment "GEM_PATH", "${projectDir}/vendor/bundle/jruby/3.1.0"
commandLine "${projectDir}/vendor/jruby/bin/jruby", "-S", "rake", "test:fd"
}
bash-4.4$ ./gradlew testFdStandalone
To honour the JVM settings for this build a single-use Daemon process will be forked. For more on this, please refer to https://docs.gradle.org/8.11.1/userguide/gradle_daemon.html#sec:disabling_the_daemon in the Gradle documentation.
Daemon will be stopped at the end of the build
> Task :downloadJRuby UP-TO-DATE
Download https://repo1.maven.org/maven2/org/jruby/jruby-dist/9.4.13.0/jruby-dist-9.4.13.0-bin.tar.gz
> Task :testFdStandalone
Pipe FDs: r=19, w=20
BUILD SUCCESSFUL in 43s
39 actionable tasks: 4 executed, 35 up-to-date
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this jruby/jruby#8555
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it looks like there is a workaround other than shelling out early...
From jruby we see how the "fake" file descriptors are managed: https://github.com/jruby/jruby/blob/bc973881b293b1fade48399c4b9b62e21d962351/core/src/main/java/org/jruby/util/io/FilenoUtil.java#L231
We miss an important warning message when that cant be found:
namely this part:
LOG.warn("Native IO integration requires open access to the JDK IO subsystem\n" +
"Pass '--add-opens java.base/sun.nio.ch=" + moduleName + " --add-opens java.base/java.io=" + moduleName + "' to enable.");
I verified that actually fixes the problem:
bash-4.4$ ./gradlew testFdInheritance -Dorg.gradle.jvmargs="--add-opens=java.base/sun.nio.ch=ALL-UNNAMED --add-opens=java.base/java.io=ALL-UNNAMED"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So to invoke Gradle and do not have the problem do we need to open the access to modules java.base/sun.nio.ch. and java.base/java.io ?
I would avoid to pass those string to CLI, we need to update a lot of scripts that uses Gradle. Did you tried adding those statements to org.gradle.jvmargs contained in gradle.properties? It would require a good explanation and link to the JRuby issue, for future us to remove or understand why :-(
However, nice work in digging into.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On my side, if it's simpler to shell out (with a good explanation) do that.
|
run exhaustive tests |
ed30fbf to
175b7e8
Compare
|
run exhaustive tests |
|
run exhaustive tests |
💚 Build Succeeded
History
|
Release notes
[rn:skip]
What does this PR do?
As a follow up to #18471 use gradle as the entrypoint in CI for running acceptance tests. Note that this still uses rake, but it explicitly uses the bundled jruby to invoke it. I did play around a bit with trying to not shell out and instead use jruby through gradle, but I ran in to an issue I have seen before with file descriptors when the ruby code goes to itself shell out (and capture output). Similarly, i looked at using rspec directly and removing rake, but the current rake file does some
requiresthat the rspec code needs. I think leaving this as a rake task is fine and this is a step forward that serves 1. ensuring gradle is "the" interface for orchestration tasks and 2. We use gradle to manage a ruby environment instead of assuming one is on system using the task orchestrator.