Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Review DeploymentManager and ScanningAppProvider #10437

Open
sbordet opened this issue Aug 30, 2023 · 2 comments · May be fixed by #12583
Open

Review DeploymentManager and ScanningAppProvider #10437

sbordet opened this issue Aug 30, 2023 · 2 comments · May be fixed by #12583
Assignees
@joakime
Labels
Bug For general bugs on Jetty side

Comments

@sbordet
Copy link
Contributor

sbordet commented Aug 30, 2023

Jetty version(s)
12

Description
Now that we have multiple environments, each ScanningAppProvider scans the webapps/ directory, and tries to deploy what's in there.

This is not optimal: we should have one scanner only, and offer what's been scanned to the environment deployers to deploy.
In this way we can detect if there are applications that do not have a deployer and won't be deployed.

We don't want to end up in situations such as:

2023-08-30 07:50:38.847:INFO :oejdp.ScanningAppProvider:main: Deployment monitor core in [file:///srv/www/websites/webapps/] at intervals 0s
2023-08-30 07:50:38.862:WARN :oejdp.ScanningAppProvider:main: class org.eclipse.jetty.deploy.providers.ContextProvider@781f10f2[file:///srv/www/websites/webapps/] no environment for App@3e14c16d[ee10,null,/srv/www/websites/webapps/download.cometd.org.xml], ignoring
2023-08-30 07:50:38.870:INFO :oejdp.ScanningAppProvider:main: Deployment monitor ee10 in [file:///srv/www/websites/webapps/] at intervals 0s
2023-08-30 07:50:38.874:INFO :oejd.DeploymentManager:main: addApp: App@1ae67cad[ee10,null,/srv/www/websites/webapps/download.cometd.org.xml]
2023-08-30 07:50:39.242:INFO :oejsh.ContextHandler:main: Started oejsh.ContextHandler@f9b7332{download.cometd.org,/,b=file:///var/www/download.cometd.org/,a=AVAILABLE,vh=[download.cometd.org],h=oejsh.ResourceHandler@6bab2585{STARTED}}

The logs above show that the core deployer started, scanned, found an app with explicit *.properties file indicating the ee10 environment, and emitting a warning that it cannot deploy it.
But then, the ee10 deployer starts, scans (again), finds the app, and deploys it.

If the app was an ee9 app, neither core nor ee10 would have been able to deploy it, with either a double warning, or no warning (if the warning message is demoted to DEBUG level).
@sbordet sbordet added the Bug For general bugs on Jetty side label Aug 30, 2023
sbordet added a commit that referenced this issue Aug 30, 2023
@sbordet
Reverted #10321.
4289529 
The warning message was causing confusion in case of multiple deployers.
See also #10437.

Signed-off-by: Simone Bordet <[email protected]>
@sbordet sbordet mentioned this issue Aug 30, 2023
Closed
@sbordet
Copy link
Contributor Author

sbordet commented Sep 29, 2023

Also, currently the scanner reports the entries to deploy in random order (by using a HashMap.keySet()).

Would be great if this could be normalized to alphabetical order, so that it would be possible to rely on a consistent deployment order for those cases where webapp B depends on webapp A.

@joakime joakime removed this from Jetty 12.0.3 Oct 11, 2023
@joakime joakime self-assigned this Nov 1, 2023
@joakime
Copy link
Contributor

joakime commented Mar 20, 2024

See also tasks in #5748

@joakime joakime mentioned this issue Mar 20, 2024
Closed
@sbordet sbordet mentioned this issue Sep 10, 2024
Closed
@joakime joakime linked a pull request Nov 26, 2024 that will close this issue
Issue #10437 - Unify Deployer ContextProvider #12583
Open
@joakime joakime moved this to 🏗 In progress in Jetty 12.1.0 Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@joakime joakime

Labels
Bug For general bugs on Jetty side
Projects
Jetty 12.1.0
Status: 🏗 In progress
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Issue #10437 - Unify Deployer ContextProvider jetty/jetty.project
2 participants
@joakime @sbordet