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

Improve the docker image #923

Closed
lmsurpre opened this issue Apr 13, 2020 · 4 comments
Closed

Improve the docker image #923

lmsurpre opened this issue Apr 13, 2020 · 4 comments
Assignees
@lmsurpre
Labels
automation automation
Milestone
Sprint 2021-05

Comments

@lmsurpre
Copy link
Member

lmsurpre commented Apr 13, 2020 •
edited
Loading

Follow-on from #837

Currently, we install our own server.xml and our own server, then delete the defaultServer.
However, now that we're based on OpenLiberty, we should follow the guidance at https://github.com/OpenLiberty/ci.docker#building-an-application-image

Here's at least 3 things that I think we could clean up/take advantage of:

  1. The ibm-fhir-server container prints this warning during startup:
[AUDIT   ] CWWKG0102I: Found conflicting settings for defaultKeyStore instance of keyStore configuration.
  Property password has conflicting values:
    Secure value is set in file:/opt/ol/wlp/usr/servers/fhir-server/configDropins/defaults/keystore.xml.
    Secure value is set in file:/opt/ol/wlp/usr/servers/fhir-server/server.xml.
  Property password will be set to the value defined in file:/opt/ol/wlp/usr/servers/fhir-server/server.xml.

We might be able to workaround that by convincing Liberty not to mixin their own keystore.xml, but a better option would be to just let liberty generate its own keystore instead of pre-packaging ours.
For more info, see https://github.com/OpenLiberty/ci.docker/blob/master/SECURITY.md#providing-a-custom-keystore

  1. Annotation caching defined at https://developers.redhat.com/blog/2020/01/28/use-red-hat-openshifts-built-in-oauth-server-as-an-authentication-provider-in-open-liberty/

This should help us with the startup performance, which I feel is noticably slower since we moved to OpenLiberty 20.0.0.3 (not sure if its about OpenLiberty vs WebSphere Liberty or about 19.0.0.12 vs 20.0.0.3)

  1. Class caching (when using OpenJ9) as defined at https://github.com/OpenLiberty/ci.docker#openj9-shared-class-cache-scc
lmsurpre added a commit that referenced this issue Apr 20, 2020
@lmsurpre
issue #923 - split keystore config into separate config file
d1ea295 
This tells the openliberty docker.ci process not to bother with creating
a separate keystore, thereby removing the conflicting values warning:
```
  Property password has conflicting values:
    Secure value is set in
file:/opt/ol/wlp/usr/servers/fhir-server/configDropins/defaults/keystore.xml.
    Secure value is set in
file:/opt/ol/wlp/usr/servers/fhir-server/server.xml.
```

I also added a version tag (20.0.0.3) to our Dockerfile to make our
builds more consistent...this should be kept in-sync with our
liberty-runtime dependency version in `fhir-parent/pom.xml`.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Apr 20, 2020
@lmsurpre
issue #923 - split keystore config into separate config file
bcbb2ea 
This tells the openliberty docker.ci process not to bother with creating
a separate keystore, thereby removing the conflicting values warning:
```
  Property password has conflicting values:
    Secure value is set in
file:/opt/ol/wlp/usr/servers/fhir-server/configDropins/defaults/keystore.xml.
    Secure value is set in
file:/opt/ol/wlp/usr/servers/fhir-server/server.xml.
```

I also added a version tag (20.0.0.3) to our Dockerfile to make our
builds more consistent...this should be kept in-sync with our
liberty-runtime dependency version in `fhir-parent/pom.xml`.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre lmsurpre mentioned this issue Apr 20, 2020
Merged
lmsurpre added a commit that referenced this issue Apr 22, 2020
@lmsurpre
issue #923 - split keystore config into separate config file
794aafe 
This tells the openliberty docker.ci process not to bother with creating
a separate keystore, thereby removing the conflicting values warning:
```
  Property password has conflicting values:
    Secure value is set in
file:/opt/ol/wlp/usr/servers/fhir-server/configDropins/defaults/keystore.xml.
    Secure value is set in
file:/opt/ol/wlp/usr/servers/fhir-server/server.xml.
```

I also added a version tag (20.0.0.3) to our Dockerfile to make our
builds more consistent...this should be kept in-sync with our
liberty-runtime dependency version in `fhir-parent/pom.xml`.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre
Copy link
Member Author

lmsurpre commented May 1, 2020

Currently we use the "full" image.
There's a open issue with startup time on this image:
OpenLiberty/ci.docker#174

However, it looks like this should be improved in future versions: OpenLiberty/ci.docker#175

To move to the "kernel" version, we need to move our server name from fhir-server to defaultServer (or ask liberty to make their configure.sh script more flexible).

@prb112 prb112 mentioned this issue May 5, 2020
Closed
@prb112 prb112 added the automation automation label Oct 9, 2020
@lmsurpre lmsurpre mentioned this issue Nov 11, 2020
Closed
@lmsurpre lmsurpre self-assigned this Mar 31, 2021
@lmsurpre lmsurpre added this to the Sprint 2021-05 milestone Mar 31, 2021
lmsurpre added a commit that referenced this issue Apr 2, 2021
@lmsurpre
issue #923 - docker image updates
278be95 
1. Dockerfile now extends liberty `kernel-slim` instead of `full`; this
required us to use the defaultServer server instead of fhir-server, but
let us take advantage of OpenJ9's Shared Class Cache for faster start
times (at the expense of longer docker build times)
2. Added logging environment variables to get basic log and trace
messages sent to console by default; everything is done via environment
variable so that it can be overridden (at first I used a
bootstrap.properties file but I found out that takes precedent over the
environment variables set at runtime)
3. Removed the configResources library from server.xml and bulkdata.xml;
it turns out we weren't loading our config files from the classpath and
so we didn't need this.


Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Apr 3, 2021
@lmsurpre
issue #923 - docker image updates
fa0a2e3 
1. Dockerfile now extends liberty `kernel-slim` instead of `full`; this
required us to use the defaultServer server instead of fhir-server, but
let us take advantage of OpenJ9's Shared Class Cache for faster start
times (at the expense of longer docker build times)
2. Added logging environment variables to get basic log and trace
messages sent to console by default; everything is done via environment
variable so that it can be overridden (at first I used a
bootstrap.properties file but I found out that takes precedent over the
environment variables set at runtime)
3. Removed the configResources library from server.xml and bulkdata.xml;
it turns out we weren't loading our config files from the classpath and
so we didn't need this.


Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Apr 3, 2021
@lmsurpre
issue #923 - docker image updates
838124c 
1. Dockerfile now extends liberty `kernel-slim` instead of `full`; this
required us to use the defaultServer server instead of fhir-server, but
let us take advantage of OpenJ9's Shared Class Cache for faster start
times (at the expense of longer docker build times)
2. Added logging environment variables to get basic log and trace
messages sent to console by default; everything is done via environment
variable so that it can be overridden (at first I used a
bootstrap.properties file but I found out that takes precedent over the
environment variables set at runtime)
3. Removed the configResources library from server.xml and bulkdata.xml;
it turns out we weren't loading our config files from the classpath and
so we didn't need this.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Apr 3, 2021
@lmsurpre
issue #923 - docker image updates
43eee48 
1. Dockerfile now extends liberty `kernel-slim` instead of `full`; this
required us to use the defaultServer server instead of fhir-server, but
let us take advantage of OpenJ9's Shared Class Cache for faster start
times (at the expense of longer docker build times)
2. Added logging environment variables to get basic log and trace
messages sent to console by default; everything is done via environment
variable so that it can be overridden (at first I used a
bootstrap.properties file but I found out that takes precedent over the
environment variables set at runtime)
3. Removed the configResources library from server.xml and bulkdata.xml;
it turns out we weren't loading our config files from the classpath and
so we didn't need this.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Apr 3, 2021
@lmsurpre
issue #923 - docker image updates
1161e58 
1. Dockerfile now extends liberty `kernel-slim` instead of `full`; this
required us to use the defaultServer server instead of fhir-server, but
let us take advantage of OpenJ9's Shared Class Cache for faster start
times (at the expense of longer docker build times)
2. Added logging environment variables to get basic log and trace
messages sent to console by default; everything is done via environment
variable so that it can be overridden (at first I used a
bootstrap.properties file but I found out that takes precedent over the
environment variables set at runtime)
3. Removed the configResources library from server.xml and bulkdata.xml;
it turns out we weren't loading our config files from the classpath and
so we didn't need this.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Apr 3, 2021
@lmsurpre
issue #923 - docker image updates
3345b10 
1. Dockerfile now extends liberty `kernel-slim` instead of `full`; this
required us to use the defaultServer server instead of fhir-server, but
let us take advantage of OpenJ9's Shared Class Cache for faster start
times (at the expense of longer docker build times)
2. Added logging environment variables to get basic log and trace
messages sent to console by default; everything is done via environment
variable so that it can be overridden (at first I used a
bootstrap.properties file but I found out that takes precedent over the
environment variables set at runtime)
3. Removed the configResources library from server.xml and bulkdata.xml;
it turns out we weren't loading our config files from the classpath and
so we didn't need this.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Apr 3, 2021
@lmsurpre
issue #923 - docker image updates
54e0688 
1. Dockerfile now extends liberty `kernel-slim` instead of `full`; this
required us to use the defaultServer server instead of fhir-server, but
let us take advantage of OpenJ9's Shared Class Cache for faster start
times (at the expense of longer docker build times)
2. Added logging environment variables to get basic log and trace
messages sent to console by default; everything is done via environment
variable so that it can be overridden (at first I used a
bootstrap.properties file but I found out that takes precedent over the
environment variables set at runtime)
3. Removed the configResources library from server.xml and bulkdata.xml;
it turns out we weren't loading our config files from the classpath and
so we didn't need this.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Apr 3, 2021
@lmsurpre
issue #923 - docker image updates
2dac1da 
1. Dockerfile now extends liberty `kernel-slim` instead of `full`; this
required us to use the defaultServer server instead of fhir-server, but
let us take advantage of OpenJ9's Shared Class Cache for faster start
times (at the expense of longer docker build times)
2. Added logging environment variables to get basic log and trace
messages sent to console by default; everything is done via environment
variable so that it can be overridden (at first I used a
bootstrap.properties file but I found out that takes precedent over the
environment variables set at runtime)
3. Removed the configResources library from server.xml and bulkdata.xml;
it turns out we weren't loading our config files from the classpath and
so we didn't need this.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Apr 3, 2021
@lmsurpre
issue #923 - docker image updates
f3ebc46 
1. Dockerfile now extends liberty `kernel-slim` instead of `full`; this
required us to use the defaultServer server instead of fhir-server, but
let us take advantage of OpenJ9's Shared Class Cache for faster start
times (at the expense of longer docker build times)
2. Added logging environment variables to get basic log and trace
messages sent to console by default; everything is done via environment
variable so that it can be overridden (at first I used a
bootstrap.properties file but I found out that takes precedent over the
environment variables set at runtime)
3. Removed the configResources library from server.xml and bulkdata.xml;
it turns out we weren't loading our config files from the classpath and
so we didn't need this.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Apr 4, 2021
@lmsurpre
issue #923 - docker image updates
6b5e844 
1. Dockerfile now extends liberty `kernel-slim` instead of `full`; this
required us to use the defaultServer server instead of fhir-server, but
let us take advantage of OpenJ9's Shared Class Cache for faster start
times (at the expense of longer docker build times)
2. Added logging environment variables to get basic log and trace
messages sent to console by default; everything is done via environment
variable so that it can be overridden (at first I used a
bootstrap.properties file but I found out that takes precedent over the
environment variables set at runtime)
3. Removed the configResources library from server.xml and bulkdata.xml;
it turns out we weren't loading our config files from the classpath and
so we didn't need this.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre
Copy link
Member Author

lmsurpre commented Apr 5, 2021

#2196 addresses points number 2 and 3 from the description by extending kernel-slim instead of full and using the Liberty-managed features.sh and configure.sh to install the minimal set of required features and set up the shared class cache (respectively).

On the first point, I don't think we see this warning any more. However, we continue to use our own self-signed example certificate (rather than using a liberty-generated one on the fly)...mainly because we have all the trust relationships set up between our client and server certs and this would require significant release automation to replicate.

lmsurpre added a commit that referenced this issue Apr 5, 2021
@lmsurpre
issue #923 - run features.sh earlier to enable layer caching
2664bc6 
this change splits the COPY /opt/ol/wlp/usr /opt/ol/wlp/usr into two
steps:
* first we move just the server.xml and configDropins
* then, after installing the liberty features, we add the application
* finally, we add the bootstrap.sh script and related tools

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Apr 5, 2021
@lmsurpre
issue #923 - run features.sh earlier to enable layer caching
00f9a03 
this change splits the COPY /opt/ol/wlp/usr /opt/ol/wlp/usr into two
steps:
* first we move just the server.xml and configDropins
* then, after installing the liberty features, we add the application
* finally, we add the bootstrap.sh script and related tools

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Apr 5, 2021
@lmsurpre
Merge pull request #2200 from IBM/issue-923-b
e995efb 
issue #923 - run features.sh earlier to enable layer caching
@lmsurpre
Copy link
Member Author

lmsurpre commented Apr 6, 2021

I also moved us onto the java 11 version of liberty (from java 8)

@d0roppe
Copy link
Collaborator

d0roppe commented Apr 8, 2021

Deployed a new created docker image in the cloud and the the server seems to be up and running quite a bit faster than before. Marking this as closed.

@d0roppe d0roppe closed this as completed Apr 8, 2021
@lmsurpre lmsurpre mentioned this issue Mar 22, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lmsurpre lmsurpre

Labels
automation automation
Projects
None yet
Milestone
Sprint 2021-05
Development

No branches or pull requests
3 participants
@prb112 @lmsurpre @d0roppe