Conversation
|
Java Action had few test run failures, this were caused because of the docker image update and existing test cases were expecting empty STDOUT but updated docker image logs the java command and flags. The following logs were captured during those failed test runs, it will be useful to understand what changes were done to JavaContainerTests and why |
- rebased docker image with fabric8 java openjdk8 - Main class will be inferred from MANIFEST.MF if not provided via `--main` Signed-off-by: Kamesh Sampath <kamesh.sampath@hotmail.com>
kameshsampath
force-pushed
the
java-improvements/issue-18
branch
from
6951a58 to
44a6c4c
Compare
|
@csantanapr - can you please review and let me know what you think ? |
rabbah
left a comment
rabbah
left a comment
There was a problem hiding this comment.
Looks OK. Why the change to the logging? Can that be avoided?
Which one you refer to ? the logging from Proxy/JarLoader or via docker image startup |
| import static org.junit.Assert.fail; | ||
|
|
||
| /** | ||
| * @author kameshs |
There was a problem hiding this comment.
We generally don’t include authors in code.
There was a problem hiding this comment.
thats a template generated ;) will remove it
|
|
||
|
|
||
| private String encodeJarFile(String path) throws URISyntaxException, IOException { | ||
| /** START - This is to Simulation how it done via Proxy **/ |
There was a problem hiding this comment.
Grammar: this is to simulate how it’s done ...
Same above.
There was a problem hiding this comment.
am gonna remove those comments
| env.take(1).toMap) | ||
|
|
||
| out.trim shouldBe empty | ||
| out.trim should fullyMatch regex """^exec -a 'javaAction-all.jar'(.*) -jar /deployments/javaAction-all.jar$""" |
There was a problem hiding this comment.
these logs are from base docker image - just to easily debug whats the JVM options its started with. I would prefer to leave that logging it has been helpful in many of my previous cases when want to quickly find the JVM related args of a container
There was a problem hiding this comment.
I suppose that’s useful - it counts against the users log limit for an action. How long is the output I don’t have a chance to run this atm.
There was a problem hiding this comment.
its just at the start of the container and hardly max 2 lines
Signed-off-by: Kamesh Sampath <kamesh.sampath@hotmail.com>
kameshsampath
force-pushed
the
java-improvements/issue-18
branch
from
ba3f886 to
b69bdf9
Compare
csantanapr
left a comment
csantanapr
left a comment
There was a problem hiding this comment.
We should not move default image that users are currently extending and using to use ubuntu.
I think we should keep using ubuntu even if switching from oracle jdk to openjdk.
I prefered this approach #24
| @@ -1,38 +1,10 @@ | |||
| FROM buildpack-deps:trusty-curl | |||
| FROM docker.io/fabric8/java-jboss-openjdk8-jdk | |||
There was a problem hiding this comment.
I disagree to move to centos base image
|
@Param-S could you help review this PR? |
|
Closing this PR since we are now based on "adoptopenjdk/openjdk8-openj9" which is the base image we agreed to be on. |
4 participants
rebased docker image with fabric8 java openjdk8
Main class will be inferred from MANIFEST.MF if not provided via
--mainThis "Main class" implementation has two cascading changes via
* apache/openwhisk#3365
* apache/openwhisk-cli#233
Signed-off-by: Kamesh Sampath kamesh.sampath@hotmail.com