-
Notifications
You must be signed in to change notification settings - Fork 108
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
[issue#211] Remove use of Jakarta Deployment and classes that use it (no replacement deployment interfaces added) #292
Conversation
import javax.enterprise.deploy.model.*; | ||
|
||
import java.io.*; | ||
import java.util.*; | ||
|
||
import javax.enterprise.deploy.shared.factories.*; |
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.
Are you planning to remove the pending occurrences of javax.enterprise.deploy also. Will that be done in another change ?
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.
@alwin-joseph
I thought I removed all of the references, I will add an additional commit to remove the missed ones.
I'll also remove the added wget of https://repo1.maven.org/maven2/javax/enterprise/deploy/javax.enterprise.deploy-api/1.7/javax.enterprise.deploy-api-1.7.jar in docker/build_jakartaeetck.sh, as we shouldn't reference those classes any more.
The same for docker/build_standalone-tcks.sh.
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.
@alwin-joseph according to jakartaee/src/main/jbake/content/commonappdeploy.adoc, there are additional tests that depend on Jakarta Deployment, however, we removed Jakarta Deployment, so these tests will not work.
|Directory Under `com/sun/ts/tests` |Directory Under `com/sun/ts/tests`
With Associated Common Applications
|`ejb/ee/tx/session` |`ejb/ee/tx/txbean`
|`ejb/ee/tx/entity/bmp` |`ejb/ee/tx/txEbean`
|`ejb/ee/tx/entity/cmp` |`ejb/ee/tx/txECMPbean`
|`ejb/ee/tx/entity/pm` |`ejb/ee/tx/txEPMbean`
|`connector/ee/localTx/msginflow` |`common/connector/whitebox`
|`connector/ee/mdb` |`connector/ee/localTx`
|`common/connector/whitebox` |`connector/ee/noTx`
|`common/connector/whitebox` |`connector/ee/xa`
|`common/connector/whitebox` |`connector/ee/connManager`
|`common/connector/whitebox` |`xa/ee`
|`compat13/connector/localTx` |`compat13/connector/whitebox`
|`compat13/connector/noTx` |`compat13/connector/whitebox`
|`compat13/connector/xa` |`compat13/connector/whitebox`
|`interop/tx/session` |`interop/tx/txbean`
|`interop/tx/entity` |`interop/tx/txEbean`
|`interop/tx/webclient` |`interop/tx/txbean`
|`tests/interop/csiv2` |`interop/csiv2/rionly`
|`ejb/ee/pm/ejbql` |`ejb/ee/pm/ejbql/schema`
|`ejb/ee/tx/session/stateful/bm/TxMDBMS_Direct`
|`ejb/ee/tx/session/stateful/bm/TxMDBMSBeans/BeanA`
|`ejb/ee/tx/session/stateful/bm/TxMDBMSBeans/BeanB`
|`ejb/ee/tx/session/stateful/bm/TxMDBMSBeans/BeanC`
|`tests/ejb/ee/tx/session/stateful/bm/TxMDBMS_Indirect`
|`ejb/ee/tx/session/stateful/bm/TxMDBMSBeans/BeanA`
|`ejb/ee/tx/session/stateful/bm/TxMDBMSBeans/BeanB`
|`ejb/ee/tx/session/stateful/bm/TxMDBMSBeans/BeanC`
|`ejb/ee/tx/session/stateful/bm/TxMDBSS_Direct`
|`ejb/ee/tx/session/stateful/bm/TxMDBSSBeans/BeanA`
|`ejb/ee/tx/session/stateful/bm/TxMDBSSBeans/BeanB`
|`tests/ejb/ee/tx/session/stateful/bm/TxMDBSSBeans/BeanC`
|`ejb/ee/tx/session/stateful/bm/TxMDBSS_Indirect`
|`ejb/ee/tx/session/stateful/bm/TxMDBSSBeans/BeanA`
|`ejb/ee/tx/session/stateful/bm/TxMDBSSBeans/BeanB`
|`ejb/ee/tx/session/stateful/bm/TxMDBSSBeans/BeanC`
src/com/sun/ts/lib/harness/commonarchives.properties seems to have a different list, so the above mentioned doc file may be out of 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.
I am thinking that the Common Applications Deployment will not work without Jakarta Deployment. If that is the case, we should remove the Common Applications Deployment tests, docs, support classes. In theory, we could introduce an alternative to Jakarta Deployment as discussed but we don't have that in place yet, so likely need to remove CAD.
Also, we already discussed removing Common Applications Deployment on the mailing list, so will do that next.
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.
Updated plan, to try to get through the build with failures, will remove Common Applications Deployment in a separate pull request.
Signed-off-by: Scott Marlow <smarlow@redhat.com>
3c16015
to
488fc98
Compare
Will merge this now, further feedback is still appreciated! As mentioned above we still expect to remove the Common Application Deployment in a separate pull request. |
deployManageruri.2=deployer:Sun:AppServer::${deployment_host.2}:${ri.admin.port} | ||
deployManageruname.2=${ri.admin.user} | ||
deployManagerpasswd.2=${ri.admin.passwd} | ||
#deployManagerJarFile.2 |
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.
Property fetching causing exception:
As of now there are exceptions while sample test run as below :
" com.sun.ts.lib.deliverable.PropertyNotSetException: No value found for deployManagerJarFile.1. Please check your jte file for an appropiate value " from ./src/com/sun/ts/lib/deliverable/AbstractPropertyManager.java
So looks like it does not allow null for these currently when fetched at src/com/sun/ts/lib/deliverable/cts/CTSPropertyManager.java
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.
Thanks @alwin-joseph, I will look at this.
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.
Pushed some possible changes to https://github.com/scottmarlow/jakartaee-tck/tree/moreDeploymentRemoval
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.
We are now getting execStatus=Failed. Illegal access to test\: java.lang.NullPointerException
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.
@alwin-joseph from the ./root/vi/glassfish6/glassfish/domains/domain1/logs/server.log from samples-results.tar.gz:
[2020-06-08T17:09:01.779+0000] [glassfish 6.0] [INFO] [NCLS-DEPLOYMENT-02027] [javax.enterprise.system.tools.deployment.autodeploy] [tid: _ThreadID=54 _ThreadName=AutoDeployer] [timeMillis: 1591636141779] [levelValue: 800] [[
Selecting file /root/vi/glassfish6/glassfish/domains/domain1/autodeploy/ejb_sam_Hello.ear for autodeployment]]
...
[2020-06-08T17:09:03.332+0000] [glassfish 6.0] [SEVERE] [NCLS-CORE-00029] [javax.enterprise.system.core] [tid: _ThreadID=54 _ThreadName=AutoDeployer] [timeMillis: 1591636143332] [levelValue: 1000] [[
Cannot start container ejb, exception: A MultiException has 7 exceptions. They are:
1. com.sun.enterprise.module.ResolveError: Failed to start OSGiModuleImpl:: Bundle = [org.glassfish.main.orb.iiop [218]], State = [NEW]
2. java.lang.IllegalStateException: Could not load descriptor SystemDescriptor(
implementation=org.glassfish.enterprise.iiop.impl.GlassFishORBFactoryImpl
contracts={org.glassfish.enterprise.iiop.impl.GlassFishORBFactoryImpl,org.glassfish.enterprise.iiop.api.GlassFishORBFactory}
scope=jakarta.inject.Singleton
qualifiers={}
descriptorType=CLASS
descriptorVisibility=NORMAL
metadata=Bundle-SymbolicName={org.glassfish.main.orb.iiop},Bundle-Version={6.0.0.SNAPSHOT}
rank=0
loader=OsgiPopulatorPostProcessor.HK2Loader(OSGiModuleImpl:: Bundle = [org.glassfish.main.orb.iiop [218]], State = [NEW],655682992)
proxiable=null
proxyForSameScope=null
analysisName=null
id=110
locatorId=0
identityHashCode=444705614
reified=false)
3. java.lang.IllegalStateException: Unable to perform operation: post construct on org.glassfish.enterprise.iiop.api.GlassFishORBHelper
4. java.lang.IllegalArgumentException: While attempting to resolve the dependencies of com.sun.ejb.containers.EjbContainerUtilImpl errors were found
5. java.lang.IllegalStateException: Unable to perform operation: resolve on com.sun.ejb.containers.EjbContainerUtilImpl
6. java.lang.IllegalArgumentException: While attempting to resolve the dependencies of org.glassfish.ejb.startup.EjbContainerStarter errors were found
7. java.lang.IllegalStateException: Unable to perform operation: resolve on org.glassfish.ejb.startup.EjbContainerStarter
NPE call stack is
[runcts] OUT => [javatest.batch] java.lang.NullPointerException
[runcts] OUT => [javatest.batch] at com.sun.ts.lib.harness.TSScript.run(TSScript.java:320)
[runcts] OUT => [javatest.batch] at com.sun.javatest.Script.run(Script.java:241)
[runcts] OUT => [javatest.batch] at com.sun.javatest.DefaultTestRunner.runTest(DefaultTestRunner.java:174)
[runcts] OUT => [javatest.batch] at com.sun.javatest.DefaultTestRunner.access$100(DefaultTestRunner.java:43)
[runcts] OUT => [javatest.batch] at com.sun.javatest.DefaultTestRunner$1.run(DefaultTestRunner.java:66)
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.
Created minor NPE fix via #311.
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.
Pushed some possible changes to https://github.com/scottmarlow/jakartaee-tck/tree/moreDeploymentRemoval
This issue is fixed now in the latest build. Thanks.
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.
Created minor NPE fix via #311.
Scott - From what I see the root problem is that the deployment fails due to some reason. I am not sure if the fix is addressing that. The deployment failures are there for several packages even for standalone tck runs and need to be looked into. We can use the tck bundles built and try them in local to check the server logs. It is also possible something is wrong with the deployment rar/war or with glassfish itself.
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.
From the server logs :
[2020-06-08T16:58:42.522+0000] [glassfish 6.0] [WARNING] [] [] [tid: _ThreadID=52 _ThreadName=AutoDeployer] [timeMillis: 1591635522522] [levelValue: 900] [[
Error occurred
org.xml.sax.SAXParseException; lineNumber: 52; columnNumber: 106; Deployment descriptor file META-INF/ra.xml in archive [whitebox-tx.rar]. cvc-enumeration-valid: Value 'jakarta.resource.spi.security.PasswordCredential' is not facet-valid with respect to enumeration '[javax.resource.spi.security.PasswordCredential, org.ietf.jgss.GSSCredential, javax.resource.spi.security.GenericCredential]'. It must be a value from the enumeration.
I see glassfish6/glassfish/lib/schemas/connector_1_7.xsd & glassfish6/glassfish/lib/dtds/connector_1_5.dtd having javax.resource.spi.security.PasswordCredential. I suspect these are to be changed to jakarta.* in glassfish ?
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.
Good catch, yes, I agree that glassfish should update these.
I did find eclipse-ee4j/glassfish#23080 which is for the ORB failure, that we also need fixed I think.
Links for updated XSDs:
https://jakarta.ee/xml/ns/jakartaee/connector_2_0.xsd
https://github.com/eclipse-ee4j/jakartaee-schemas/blob/master/jakartaee9/src/connector_2_0.xsds
#266 is for updating the Platform TCK to use EE 9 XSDs
https://github.com/eclipse-ee4j/jakartaee-tck/blob/master/src/com/sun/ts/tests/common/connector/whitebox/permissiondd/ra.xml is one of the files that do need to switch to the EE 9 XSDs.
#211