egeria icon indicating copy to clipboard operation
egeria copied to clipboard

Correct container environment for ui-server-chassis

Open planetf1 opened this issue 2 years ago • 5 comments

When testing https://github.com/odpi/egeria/pull/5848 it was found that the ui server chassis would crash if LOADER_PATH was set to include the egeria server-chassis libraries. The failure was in loading a Cassandra health check, probably due to spring component scanning finding libraries to initialize.

However for the ui server chassis this loader path should not be set - the ui chassis is very different to the server chassis, and operates in a different environment

Currently the Dockerfile for egeria explicitly defaults LOADER_PATH to include the server/lib directory of the distribution. The egeria container docs also refer to this path

This is also used as a default (no override) in the egeria-base and odpi-egeria-lab helm charts

Whilst a workaround is in place (cassandra health check disabled), the environment should be corrected. Possibilities include

  • Do not set a default loader path in the base egeria image

Whilst this is clean, it could cause more problems since the out of the box experience would require the value to be set even to launch server chassis - or at least to get any useful function (eventually, as we decompose more)

  • Override LOADER_PATH when running the UI

Can be encoded in the helm charts Would need adding to docs on launching the ui chassis Does not impact on the server chassis Simple to implement

  • Add logic to dynamically set loader-path based on what is launched

Adds complexity

  • Create new docker image, specifically for egeria-ui

Clean, two distinct images to run - but adds more process, time, space (even if container layers are cached)

In addition

  • Check docs for the docker image, and ensure the value can be overriden, and how to do this is clearly documented

planetf1 avatar Nov 09 '21 17:11 planetf1

I'm inclined to go with overriding the loader path when launching the UI (which will be tested) & ensuring the docs explain this.

planetf1 avatar Nov 09 '21 17:11 planetf1

+1 to override the LOADER_PATH during runtime for now. The original image is designed for server-chassis and I think it should remain with its default parameters. At later stage we might consolidate UI backend and then we can think of new dedicated image.

lpalashevski avatar Nov 10 '21 08:11 lpalashevski

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Mar 29 '22 00:03 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar May 31 '22 00:05 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Aug 21 '22 00:08 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Oct 25 '22 01:10 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Dec 31 '22 00:12 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Mar 05 '23 00:03 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar May 08 '23 00:05 github-actions[bot]

@lpalashevski over to you to decide if any followup is needed on this

planetf1 avatar May 23 '23 12:05 planetf1

FYI @mandy-chessell - the UI chassis picking up the (new) application.properties is a related issue. Having unique containers, or somehow specifying the right context would be needed.

planetf1 avatar May 31 '23 13:05 planetf1

I analyzed the two problems up close, in summary, we have limited options if we stick to the single docker image:

  • In regards to LOADER_PATH, I've set the evn var to non-existent user-interfaces/lib location following the pattern for server-chassis that is server/lib -- ui-chassis-spring now will not see and load extra libraries. I think this is good for isolation while sharing the image.

  • External configuration, in the current docker environment based on redhat ubi9/openjdk-17 image we cannot do much since JAVA_APP_DIR cannot be customized from outside (this is running directory for all apps in our case spring boot starter). With this image this app dir is always /deployments -- I know this because I inspected the image and looked at the java lunch scripts (I can provide more details if anyone is curious) -- bottom line if we place application.properties as we do now in /deployments (which is fine for server-chassis-spring entry-point) we have to control/override the spring configuration from the application (ui-spring-chassis) if we re-use the same image. Or second option is to move the application.properties per application dir and configure spring to look it up there (this is now done only for ui-chassis-spring, see the PR)

Both changes are in this PR https://github.com/odpi/egeria-charts/pull/269/files

Configuration becomes simpler if we have two docker images but we need to residing and have more consistent way while packaging/creating the assembly. This is more work and I decided to go with the intermediate change as good enough solution.

@planetf1 @mandy-chessell

lpalashevski avatar Jun 06 '23 15:06 lpalashevski

Agree with the analysis and actions.

The redhat image we use is fairly flexible, and does allow both the directory (JAVA_APP_DIR) and a variety of classpath related settings to be specified - for example see the list here (slightly different image).

In our Dockerfile we do set a value for JAVA_APP_JAR and JAVA_OPTS_APPEND, but these can both be overriden within our chart's yaml (and from there, could be configurable via values)

So in future, it should be possible to specify a different directory if needed, or different classpaths/app jar within the same directory. Though it's still also worth thinking about whether to have a separate image, and that really makes sense with a very different assembly setup for that image

planetf1 avatar Jun 07 '23 08:06 planetf1

Yes this is the theory but in practice setting JAVA_APP_DIR does not work at lease not for ubi9/openjdk-17 - I went over the S2I base image docs and it is indeed mentioned but test did not work. Inspecting the image endpoint and the startup scripts I found out that in the context of the startup sequence we are using and passing JAVA_APP_JAR, the JAVA_APP_DIR is hard-coded to /deployments.

image

image This looks like a bug, for the time being decided not troubleshoot any further.

lpalashevski avatar Jun 07 '23 09:06 lpalashevski

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Aug 07 '23 00:08 github-actions[bot]

Prompted by the cloud native work, alternatives to the combined open-metadata-assembly are being created (see #7766) to create specific images for each runtime.

mandy-chessell avatar Aug 28 '23 07:08 mandy-chessell