[Bug][Worker] Replace jre with jdk #15764
Conversation
Signed-off-by: Gallardot <[email protected]>
Gallardot
requested review from
EricGao888,
caishunfeng,
SbloodyS and
ruanwenjun
as code owners
| @@ -15,7 +15,7 @@ | |||
| # limitations under the License. | |||
| # | |||
|
|
|||
| FROM eclipse-temurin:8-jre | |||
| FROM eclipse-temurin:8-jdk | |||
There was a problem hiding this comment.
It's better to replace all Dockerfile to jdk.
There was a problem hiding this comment.
I think it's better to keep things the way they are. Because only Worker Server and Standalone Server require JDK, JRE is sufficient for other components. Unless other components need it as well.
There was a problem hiding this comment.
You are right. So only Standalone needs to be added too.
There was a problem hiding this comment.
If the task that relies on JAVA_HOME uses an environment to import the JAVA_HOME, do we still need to change the Docker file?
There was a problem hiding this comment.
If the task that relies on JAVA_HOME uses an environment to import the JAVA_HOME, do we still need to change the Docker file?
@ruanwenjun
Yes, because the JAVA task requires javac to compile java code, and this command requires the JDK to be installed, which is not available when only the JRE is installed.
There was a problem hiding this comment.
If the task that relies on JAVA_HOME uses an environment to import the JAVA_HOME, do we still need to change the Docker file?
@ruanwenjun
If the worker server does not provide a basic working environment, users will have to install jdk and rebuild the docker image themselves, which can avoid some security issues, but it is not very user friendly.
There was a problem hiding this comment.
Please add this change to Standalone together.
There was a problem hiding this comment.
Please add this change to
Standalonetogether.
Standalone Server already uses a docker image with JDK
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #15764 +/- ##
============================================
- Coverage 39.11% 39.09% -0.03%
+ Complexity 4854 4849 -5
============================================
Files 1316 1316
Lines 44936 44936
Branches 4797 4797
============================================
- Hits 17577 17566 -11
- Misses 25467 25478 +11
Partials 1892 1892 ☔ View full report in Codecov by Sentry. |
|
Purpose of the pull request
Brief change log
Verify this pull request
This pull request is code cleanup without any test coverage.
(or)
This pull request is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(or)
If your pull request contain incompatible change, you should also add it to
docs/docs/en/guide/upgrede/incompatible.md