search

OpenLiberty / ci.maven

Maven plugins for managing Liberty profile servers #devops
Apache License 2.0
126 stars 90 forks source link

Confusing when user provides app config but plugin generates one anyway because of location mismatch #1780

Open scottkurz opened 9 months ago

scottkurz commented 9 months ago

BACKGROUND

The context behind the issue is outlined here: https://stackoverflow.com/questions/77548432/open-liberty-context-root-not-found-error-classcastexception-cwwkz0013e/77548433#77548433 which I wrote up as a Q&A after seeing a couple people internally stumble over related aspects of this issue.

As mentioned there, an easy way to trip over this is to configure a location like:

 <application location="demo.war"...

without configuring the non-default final name in pom.xml <finalName>${project.artifactId}</finalName> (the default final name includes the version).

Though there are clues to what's gone wrong:

[WARNING] CWWKM2179W: The application is not defined in the server configuration but the POM configuration indicates it should be installed in the apps folder. Application configuration is being added to the target server configuration dropins folder by the plug-in.
...
[INFO] [ERROR   ] CWWKZ0013E: It is not possible to start two applications called intro.

it might take too much of a trained eye to make sense of these.

PROPOSAL

Suggestion, maybe we should "fail fast" and abort the execution with an error.

IIUC, the match is currently done based on the 'location' attr of the app element. Perhaps we could do a check like: if either the 'name' or the 'id' LMP would generate for the app deployment matches something already in use, then just fail the deploy.

This might be better than allowing the app to start but without the desired config...leading to the wrong ctx root, ClassCastExc, as mentioned.

Probably could use another quick round of design review before finalizing whatever implementation we come up with.

Note from the issue history there have been a few now overlapping this area: https://github.com/OpenLiberty/ci.maven/issues?q=is%3Aissue+CWWKZ0013E (not that that experience argues 100% for doing what I'm proposing here..but at least argues it's an area worthy of further consideration).

Perhaps we'd want a similar ci.gradle issue?

cherylking commented 9 months ago

Just noting here that we have to be careful with this change as someone could be depending on the current behavior. We have to be confident that the change to fail fast only occurs for a situation where the app will not function correctly because of mismatched config.

scottkurz commented 9 months ago

Just noting here that we have to be careful with this change as someone could be depending on the current behavior. We have to be confident that the change to fail fast only occurs for a situation where the app will not function correctly because of mismatched config.

And/or, we could create a new config parm to disable the new behavior. Guess we'd have to see what the tough cases would look like.