search

OpenLiberty / open-liberty

Open Liberty is a highly composable, fast to start, dynamic application server runtime environment
https://openliberty.io
Eclipse Public License 2.0
1.15k stars 591 forks source link

Read Kubernetes config in as Liberty config variables #9786

Closed Emily-Jiang closed 2 years ago

Emily-Jiang commented 4 years ago

We need to inject a property defined in a mounted file. This is an important scenario in a k8s environment, where credentials from a secret are mounted (volumeMount) into the container, rather than bound to env vars. Maybe we should opt in some folders where any key/values defined there would be available for injection.

@tevans78: (18th Feb 2020) @Emily-Jiang and I have been mulling over a design for this and I think we have concluded that the feature can be broadened out to include multiple different types of properties files. Initially we will go for just Java properties files and Kubernetes secrets files. In the future we may be able to also include json files, yaml files, xml files etc etc. We are also going to include the ability to read multiple files by specifying a directory rather than just a specific file.

@tevans78: Old UFO removed

@tevans78: (17th March 2020) Further discussion suggests that reading/loading Kubernetes configuration should be solved more generally for Liberty, not just for MP Config. Much of the processing would either be done by an Operator before Liberty starts OR it would be done at the kernel level as Liberty starts. Either way, the K8 properties would most likely end up as Liberty variables. MP Config would not be involved.

The generic file Config Source suggested above is something that has been asked for before so we may spin that off into a separate feature.

@brenthdaniel: (13th July 2020) New UFO: https://ibm.box.com/s/6xg3tvkodxr2qwth7drgvnb60k2zmefs FTS: https://github.com/OpenLiberty/open-liberty/issues/14768

arthurdm commented 4 years ago

+1

To match the secret-mounting-behaviour from k8s, a proposal is to define a folder within OL (perhaps configurable), such that each filename in that folder maps to a MP Config key and the contents of that file map to that key's value.

For example, if we have a secret called couchDBSecret with the following items:

couchdb.username: arthur
couchdb.password: xzy123

Then during deployment the user has two options:

  1. Mounting the entire secret to a folder.
    This is done via: 
    ...
volumeMounts:
- name: couchDB
  mountPath: "/etc/couchdb"
  readOnly: true
volumes:
- name: couchDB
secret:
  secretName: couchDBSecret
...

    Then inside our container we will have a file with path /etc/couchdb/couchdb.username, with value arthur, and another file with path /etc/couchdb/couchdb.password, with value xyz123.

In this scenario our Java app would have:

   @Inject @ConfigProperty(name="couchdb.username") Optional<String> dbUsername;
   @Inject @ConfigProperty(name="couchdb.password") Optional<String> dbPassword;
  1. The user can pick which filename and option they use For example, let's say only the password is needed, you could do: 
    ...
volumeMounts:
- name: couchDB
  mountPath: "/etc/couchdb"
  readOnly: true
volumes:
- name: couchDB
secret:
  secretName: couchDBSecret
  items:
  - key: password
    path: myPassword
...

    In this case the only file created is at /etc/couchdb/myPassword, with its value being xyz123.

In this scenario our Java app would have:

   @Inject @ConfigProperty(name="myPassword") Optional<String> dbPassword;
Emily-Jiang commented 4 years ago

@arthurdm Thanks for the detailed explanation! For scenario 2., why does - key: username need to be mentioned?

arthurdm commented 4 years ago

@Emily-Jiang - good catch, that should have been key : password. I have updated the comment now. :)

hutchig commented 4 years ago

For OpenShift it would be great if we can firm up on a convention for a solid service binding information 'root' location - so that we can write server Java code, or third-parties can, for example, write MicroProfile Reactive Messaging Connector Java code that can go and look in the default location (either directly of via MP Config) so that service binding can be resolved automatically on behalf of the user - for example for EventStreams. If we can not get a convention on a root location for mounting then we should push for some sort of convention for a sign-post that points to the varying bindings root location.

(The above to be advocated via https://groups.google.com/forum/#!topic/operator-framework/ScQXAsGCFeg probably by @arthurdm and /Chris Bailey.

tevans78 commented 4 years ago

The mounting binding information is proposed here: https://github.com/application-stacks/service-binding-specification/tree/updateBar#mounting-binding-information

yeekangc commented 4 years ago

We should consider if we need a new guide to cover this. Currently, we have https://openliberty.io/guides/kubernetes-microprofile-config.html.

If we believe a new guide is warranted, please file an issue here: https://github.com/OpenLiberty/guides-common/issues

Cc @gkwan-ibm.

brenthdaniel commented 4 years ago

@yeekangc At the moment the UFO lists a guide as a possible socialization, but I was planning on soliciting feedback on that. With the current design, the function is most relevant in an OpenShift/CP4A environment with the service binding operator. It could be used with just OL but would require simulating the work that the service binding operator is doing.

Emily-Jiang commented 4 years ago

@yeekangc we do need a guide for this to demonstrate how to make Kubernetes sceret into application running on Open Liberty without asking customers to wire multiple paths.

brenthdaniel commented 4 years ago

List of Steps to complete or get approvals / sign-offs for Onboarding to the Liberty release (GM date)

Instructions:

TARGET COMPLETION DATE Before Development Starts or 8 weeks before Onboarding

Prerequisites

yeekangc commented 4 years ago

I might have missed some discussions. @brenthdaniel, did we come to a consensus on guide for this? If it's more relevant to OpenShift and the operator there, I can see this as an update to the Deploying to OpenShift using Operator guide, which is in progress.

brenthdaniel commented 4 years ago

@yeekangc The overwhelming response was that we needed a guide. I thought I had opened an issue already, but it looks like I didn't. https://github.com/OpenLiberty/guides-common/issues/511 id open now.

donbourne commented 3 years ago

Serviceability Approval Comment - Please answer the following questions for serviceability approval:

  1. UFO -- does the UFO identify the most likely problems customers will see and identify how the feature will enable them to diagnose and solve those problems without resorting to raising a PMR? Have these issues been addressed in the implementation?

Yes... Error messages have been added where appropriate.

  1. Test and Demo -- As part of the serviceability process we're asking feature teams to test and analyze common problem paths for serviceability and demo those problem paths to someone not involved in the development of the feature (eg. L2, test team, or another development team).
    a) What problem paths were tested and demonstrated? b) Who did you demo to? c) Do the people you demo'd to agree that the serviceability of the demonstrated problem scenarios is sufficient to avoid PMRs for any problems customers are likely to encounter, or that L2 should be able to quickly address those problems without need to engage L3?

a) Loading a binary file b) kernel team (not involved in development) c) yes

  1. SVT -- SVT team is often the first team to try new features and often encounters problems setting up and using them. Note that we're not expecting SVT to do full serviceability testing -- just to sign-off on the serviceability of the problem paths they encountered. a) Who conducted SVT tests for this feature? b) Do they agree that the serviceability of the problems they encountered is sufficient to avoid PMRs, or that L2 should be able to quickly address those problems without need to engage L3?

N/A

  1. Which L2 / L3 queues will handle PMRs for this feature? Ensure they are present in the contact reference file and in the queue contact summary, and that the respective L2/L3 teams know they are supporting it. Ask Don Bourne if you need links or more info.

L3 Liberty Kernel

  1. Does this feature add any new metrics or emit any new JSON events? If yes, have you updated the JMX metrics reference list / Metrics reference list / JSON log events reference list in the Open Liberty docs?

N/A

mhldr commented 3 years ago

We'll need some information before Fat Approval.

brenthdaniel commented 3 years ago

FAT test summary: https://github.com/OpenLiberty/open-liberty/issues/14768

skasund commented 3 years ago

L2 has requested STE slides for this feature. The STE template can be found at the links below. You can use either one to create the education.

Slide Template: https://ibm.box.com/s/1an42g7zdgmaj84w7dft0indqfgi8ffm

Github Template: https://pages.github.ibm.com/WASL3/site/STE/about

Please upload the completed slides to the same 'STE Archive' BOX folder or provide me the Github link. Thanks!

brenthdaniel commented 3 years ago

STE: https://ibm.box.com/s/lg3dmlhnpdyligw9se38qyiucqd6bp2g

skasund commented 3 years ago

@brenthdaniel Thanks for slides. I've approved the feature.

jhanders34 commented 3 years ago

@jdmcclur took a look at the performance and did not see any red flags related to this feature. He found a performance issue in mpConfig that he is looking at making a PR to resolve. But since it isn't caused by by this feature, I am marking it approved.

donbourne commented 3 years ago

@brenthdaniel , what will happen in case where file in config directory has wrong ownership or mode such that it can't be read by Liberty?

brenthdaniel commented 3 years ago

@donbourne Any error reading the file will result in the same error message: error.bad.variable.file=CWWKG0106E: The {0} file can not be read. error.bad.variable.file.explanation=The file can not be loaded as a variable because it can not be read. error.bad.variable.file.useraction=Verify that the file contains text content and check the operating system permissions for the file.

chirp1 commented 3 years ago

Brent opened the ID issue at https://github.com/OpenLiberty/docs/issues/4567 and included the information to be documented. The docs are done. Brent has approved the doc updates. I'm adding ID approved to this epic.