You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository was archived by the owner on Sep 16, 2021. It is now read-only.
Repository navigation
This repository was archived by the owner on Sep 16, 2021. It is now read-only.
Async support: create a distro in a tar gz file #140
We want to support both run and deploy (i.e create good staging content with correct quickstart web.xml) for the latest Jetty 9.3.x async branch.
We will need to create a distro we can make available as a downloadable file in this github project, that also will be (for now) bundled in the next Cloud SDK when possible.
We want to keep the 2 directories jetty home and jetty base as 2 top level separated directories.
we want a single main entry that will just generate a correct quickstart web.xml in the web-inf dir of a given web app directory archive.
Today, for this we created the quickstartgenerator.jar that has this main (passing a web directory) and has a deps on all correct *.jar are relative path in the jar manifest path. If there is a better way to either do this jar, or a way with java jar start.jar with correct flags to only generate the quick xml, let's do that and we will modify the staging phase that calls it (See current call at: https://code.google.com/p/googleappengine/source/browse/trunk/java/src/main/com/google/appengine/tools/admin/Application.java lines 717 calling line 1342). The long term plan is to move this code in a new SDK open source project. But for now (january 2016) I'll do the necessary changes in our google code, and bundle the correct distro in a new Cloud SDK deliverable.
We sill want to bundle the old jetty bundle in the Cloud SDK for a short transition period and it will be deleted later in Q1.
(current distro is in
~/google-cloud-sdk/platform/google_appengine/google/appengine/tools/java/lib/jetty-base-sdk
and
~/google-cloud-sdk/platform/google_appengine/google/appengine/tools/java/lib/java-managed-vm
with the main generator in ~/google-cloud-sdk/platform/google_appengine/google/appengine/tools/java/lib/java-managed-vm/appengine-java-vmruntime/quickstartgenerator.jar
@ludoch Sorry for the delay on this. It is taking me a bit longer that I thought.
There are two issues making this difficult:
The jetty-base in the images is generated by running start.jar --add-to-startd from the Dockerfile
The jetty-base is built up as an overlay from the one in the image in the gke-jetty image
I can move the add-to-startd command to a mvn exec plugin configuration, but the issue then is do we generate the full jetty-base or just the overlay to apply to the image. I don't think the overlay works, as it will not be usable on it's own (which is the whole point of creating this artefact).
So we want the full jetty-base, in which case what is it based on:
The gke-jetty base extracted from the docker image? ugh hard work but keeps the ancestry pure.
Create an gke-jetty-base tar.gz artefact. Still a bit artificial but keeps the ancestry
Create a new base with just the modules we want. This is easiest but will require the existing base to be removed from the base docker image. However we are already removing some files, so that is probably OK
I'll go ahead with 3. for now as that is simplest and see how it looks.
@ludoch next issue is that the configuration wants to log to /var/log/app_engine
I'll change that to $JETTY_BASE/logs and then when we make the docker image we can symlink that directory to /var/log/app_engine
For the logs:
In the sample https://github.com/GoogleCloudPlatform/appengine-java-vm-guestbook-extras using a custom_entryproint to do a local exectution we use the property -Dcom.google.apphosting.vmruntime.VmRuntimeFileLogHandler.pattern=/SOMEWRITABLEDIR/log
locally. It defaults to the correct var log app_engine in the image.
The PR #147 contains the first big step for this. It produces both a zip and tgz containing a jetty base directory, that has a webapps/root.xml deployed that expects a webapps/root directory containing the webapp.
Next step is to work out how use this to run with the SDK and/or maven plugin. We could parameterise the root.xml so a specific absolute webapp directory could be passed in and the image run with something like:
@ludoch if you can confirm this is in the direction you intended and which of the above you prefer, then I'll update the PR.
Not that this + async + #146 + #147 represents a it of a big renaming motza ball hanging out there. We need to merge a few of these soon else we will be in a world of merge pain.
Thanks!
Symlink on windows not so nice?
We have 3 use cases:
1/ CLI to call the quickstart transform step on such a root.webapp before a
run
2/ CLI to locally execute this app (windows and linux compliant)
3/ What is used to run the app in the docker image
I prefer a solution to pass a root.webapp=/var/lib/myapp with absolute
path (linux+windows). If no root.webapp is passed, we can default to the
default expected location in the jetty base.
Will it work for quickstart generation step as well (ie which exact cli
command to use?)
I looked at the PR. Overall looks good, and I've 5 questions:
1/ we are mixing directory renames with this PR, so it is difficult to
review.
2/ not sure why we do not keep the logging library as separate (might be
useful for other runtimes). No big deal for now
3/ we are removing the jarjar repackaging of dep jars: can we guarantee
that these JARs will not be visible to user applications and that users
could define their own versions in user land? I guess yes but I need
confirmation
4/ the PR is on master. What is really needed for now is this work to be
apply to the async branch. The master will be replaced entirely with async
content once we are happy with it.
5/ I see 2 extra *.pom files in the lig/gae directory
I looked at the PR. Overall looks good, and I've 5 questions:
1/ we are mixing directory renames with this PR, so it is difficult to
review.
2/ not sure why we do not keep the logging library as separate (might be
useful for other runtimes). No big deal for now
3/ we are removing the jarjar repackaging of dep jars: can we guarantee
that these JARs will not be visible to user applications and that users
could define their own versions in user land? I guess yes but I need
confirmation
4/ the PR is on master. What is really needed for now is this work to be
apply to the async branch. The master will be replaced entirely with async
content once we are happy with it.
5/ I see 2 extra *.pom files in the lig/gae directory
1/ Yes, that is difficult. But as I was creating new modules it needed some level of renaming anyway. I'll where the pain of this when merging async-support branch :(
2/ Was a simplification to avoid needing more names/jars/modules for only 2 classes. It can be reused from the runtime jar, which is not that large.
3/ The Jetty context loader does provide a level of protection, but it needs to be well configured in order to give a guarantee. It is configured to some extent, but more rigour needs to be applied to have any level of a guarantee. Note that the jar jar plugin itself will not actually hide the classes, but just put them in different name space. Rigour is still needed to ensure they are hidden from the context. I suggest that we open another separate issue to review class hiding and to perhaps re-introduce jarjar if its renaming makes hiding simpler.
4/ Ah OK. I note your renaming PR is on async as well. It may be simplest if that is merged into async first and then I can recreate a PR against the renamed async branch? Can you confirm if that is the approach you want to take?
5/ Ah mvn dependency plugin does that. Will configure to remove.
1/ Yes, that is difficult. But as I was creating new modules it needed
some level of renaming anyway. I'll where the pain of this when merging
async-support branch :(
2/ Was a simplification to avoid needing more names/jars/modules for only
2 classes. It can be reused from the runtime jar, which is not that large.
3/ The Jetty context loader does provide a level of protection, but it
needs to be well configured in order to give a guarantee. It is configured
to some extent, but more rigour needs to be applied to have any level of a
guarantee. Note that the jar jar plugin itself will not actually hide the
classes, but just put them in different name space. Rigour is still needed
to ensure they are hidden from the context. I suggest that we open another
separate issue to review class hiding and to perhaps re-introduce jarjar if
its renaming makes hiding simpler.
4/ Ah OK. I note your renaming PR is on async as well. It may be simplest
if that is merged into async first and then I can recreate a PR against the
renamed async branch? Can you confirm if that is the approach you want to
take?
5/ Ah mvn dependency plugin does that. Will configure to remove.
—
Reply to this email directly or view it on GitHub #140 (comment)
.
Activity
ludoch commentedon Dec 20, 2015
As soon as we have this in place, we will merge the async branch to master and that will be our default target.
gregw commentedon Dec 20, 2015
eta wednesday
gregw commentedon Dec 30, 2015
@ludoch Sorry for the delay on this. It is taking me a bit longer that I thought.
There are two issues making this difficult:
I can move the add-to-startd command to a mvn exec plugin configuration, but the issue then is do we generate the full jetty-base or just the overlay to apply to the image. I don't think the overlay works, as it will not be usable on it's own (which is the whole point of creating this artefact).
So we want the full jetty-base, in which case what is it based on:
I'll go ahead with 3. for now as that is simplest and see how it looks.
gregw commentedon Dec 30, 2015
@ludoch next issue is that the configuration wants to log to /var/log/app_engine
I'll change that to $JETTY_BASE/logs and then when we make the docker image we can symlink that directory to /var/log/app_engine
ludoch commentedon Dec 30, 2015
For the logs:
In the sample https://github.com/GoogleCloudPlatform/appengine-java-vm-guestbook-extras using a custom_entryproint to do a local exectution we use the property -Dcom.google.apphosting.vmruntime.VmRuntimeFileLogHandler.pattern=/SOMEWRITABLEDIR/log
locally. It defaults to the correct var log app_engine in the image.
Refactor modules for GoogleCloudPlatform#140 to have a JETTY_BASE in …
gregw commentedon Jan 5, 2016
The PR #147 contains the first big step for this. It produces both a zip and tgz containing a jetty base directory, that has a webapps/root.xml deployed that expects a webapps/root directory containing the webapp.
Next step is to work out how use this to run with the SDK and/or maven plugin. We could parameterise the root.xml so a specific absolute webapp directory could be passed in and the image run with something like:
Alternately the directory could be relative to the JETTY_BASE
Alternately it could be fixed and a symlink created from webapps/root to the directory containing the app?
gregw commentedon Jan 5, 2016
@ludoch if you can confirm this is in the direction you intended and which of the above you prefer, then I'll update the PR.
Not that this + async + #146 + #147 represents a it of a big renaming motza ball hanging out there. We need to merge a few of these soon else we will be in a world of merge pain.
ludoch commentedon Jan 5, 2016
Thanks!
Symlink on windows not so nice?
We have 3 use cases:
1/ CLI to call the quickstart transform step on such a root.webapp before a
run
2/ CLI to locally execute this app (windows and linux compliant)
3/ What is used to run the app in the docker image
I prefer a solution to pass a root.webapp=/var/lib/myapp with absolute
path (linux+windows). If no root.webapp is passed, we can default to the
default expected location in the jetty base.
Will it work for quickstart generation step as well (ie which exact cli
command to use?)
On Mon, Jan 4, 2016 at 9:57 PM, Greg Wilkins notifications@github.com
wrote:
gregw commentedon Jan 5, 2016
@ludoch OK I will update to support
For generation of quickstart-web.xml I will provide something like:
ludoch commentedon Jan 5, 2016
I looked at the PR. Overall looks good, and I've 5 questions:
1/ we are mixing directory renames with this PR, so it is difficult to
review.
2/ not sure why we do not keep the logging library as separate (might be
useful for other runtimes). No big deal for now
3/ we are removing the jarjar repackaging of dep jars: can we guarantee
that these JARs will not be visible to user applications and that users
could define their own versions in user land? I guess yes but I need
confirmation
4/ the PR is on master. What is really needed for now is this work to be
apply to the async branch. The master will be replaced entirely with async
content once we are happy with it.
5/ I see 2 extra *.pom files in the lig/gae directory
On Tue, Jan 5, 2016 at 2:06 PM, Greg Wilkins notifications@github.com
wrote:
ludoch commentedon Jan 5, 2016
1 more: could we add a LICENSE (apache) file at the top?
On Tue, Jan 5, 2016 at 2:36 PM, Ludovic Champenois <
ludovic.champenois@gmail.com> wrote:
gregw commentedon Jan 5, 2016
@ludoch
1/ Yes, that is difficult. But as I was creating new modules it needed some level of renaming anyway. I'll where the pain of this when merging async-support branch :(
2/ Was a simplification to avoid needing more names/jars/modules for only 2 classes. It can be reused from the runtime jar, which is not that large.
3/ The Jetty context loader does provide a level of protection, but it needs to be well configured in order to give a guarantee. It is configured to some extent, but more rigour needs to be applied to have any level of a guarantee. Note that the jar jar plugin itself will not actually hide the classes, but just put them in different name space. Rigour is still needed to ensure they are hidden from the context. I suggest that we open another separate issue to review class hiding and to perhaps re-introduce jarjar if its renaming makes hiding simpler.
4/ Ah OK. I note your renaming PR is on async as well. It may be simplest if that is merged into async first and then I can recreate a PR against the renamed async branch? Can you confirm if that is the approach you want to take?
5/ Ah mvn dependency plugin does that. Will configure to remove.
ludoch commentedon Jan 5, 2016
1:ok
2:ok for now
3: will do now
4: yes
5: no big deal
On Tue, Jan 5, 2016 at 3:09 PM, Greg Wilkins notifications@github.com
wrote:
Created jetty9-compat-base to create jetty base for use by jetty9-com…
Updated archive naming, README and LICENSE for GoogleCloudPlatform#140
Added the ability to set quickstart_web_xml system property to genera…
Fixed incorrect log path GoogleCloudPlatform#140
Merge pull request #152 from jetty-project/issue-140
gregw commentedon Jan 8, 2016
Fixed with PR #150