WASdev / ci.docker.websphere-traditional

Dockerfiles for WebSphere Application Server traditional
Apache License 2.0
171 stars 192 forks source link

Properties creation on Build and Runtime #127

Closed DSTOLF closed 5 years ago

DSTOLF commented 5 years ago

Hi,

I have two suggestions about config-properties.

applyConfigs(){
  if [ ! -z "$(ls /etc/websphere)" ]; then
    echo "+ Found config-files under /etc/websphere. Executing..."
    for i in $(ls /etc/websphere/*props); do
    /work/applyConfig.sh $i
    done
  fi
}

This way, we could name the files like below and be sure there'll be no error (example, creating a connection pool before creating JDBC Provider, AuthData and DataSource)

01-JDBCProvider.props
02-JAASAuthData.conf
03-DataSource.conf
04-ConnectionPool.conf
DSTOLF commented 5 years ago

My second suggestion is about build time.

I think it would be convenient if build time and run time properties were more consistent.

At build time, applyConfig.sh just applies the one property file (was-config.props). If we have several properties to create, it can get pretty unwieldy pretty fast.

So, my suggestion for applyConfig.sh would be:

#!/bin/bash
if [ ! -z "$1" ]; then
        /opt/IBM/WebSphere/AppServer/bin/wsadmin.sh -conntype None -f /work/applyConfig.py $1
        exit 0
elif [ $( ls -larth /work/config/*.props| wc -l) -gt 1  ]; then
         for i in $(ls /work/config/*props); do
           /work/applyConfig.sh $i
         done
fi

This way, we can have multiple, easier to edit property files, with names preceded by numbers that imply the orders in which they should be applied.

DSTOLF commented 5 years ago

As I mentioned earlier this year, we're using tWAS in my company, but we're using a custom built image we keep in our private registry.

I think that, with this two modifications I suggested, we would be ready to use the official image, or, at the very least, just mirror the official repo to ours.

DSTOLF commented 5 years ago

If you'd like, I can submit a PR.

shareauto commented 5 years ago

@DSTOLF If you have the below config files handy, would you please share with us by removing your environment specific details. It will be more helpful. 01-JDBCProvider.props 02-JAASAuthData.conf 03-DataSource.conf 04-ConnectionPool.conf

DSTOLF commented 5 years ago

I tried my best to remove environment specific details without breaking anyhing, but I haven't tested them after that due to time constraints.

JAASAuthData and DataSource are templates we use in our CI, so some fields have placeholders on them: Ex.

password="__DB2_PASSWORD__" #required
userId="__DB2_USER__" #required
serverName=__DB2_HOST__ #String
portNumber=__DB2_PORT_NUMBER__ #integer
databaseName=__DB2_DB_NAME__ #String,required

Here are the gists:

000_JDBCProvider.props https://gist.github.com/DSTOLF/481edb47eb65297c1313bf29d752494e

001_JAASAuthData.props https://gist.github.com/DSTOLF/1822723a3462524134509a72e0497046

002_DataSource.props https://gist.github.com/DSTOLF/b45e6c188a15b538bc09d6d69d5d3a1d

003_ConnectionPool.props https://gist.github.com/DSTOLF/8b1cbd6ef342db2480c5ac1f990bdfc8

DSTOLF commented 5 years ago

Reasoning behind breaking everything into smaller parts: 1- Some of this props can be applied in build-time and never be touched again, like JDBC Provider or Connection Pools. All we have to worry about is overwriting AuthData and DataSources. Also, if we have multiple DataSources, we won't re-apply the JDBC Provider over and over again. With this approach, we can speed up startup time a bit; 2- It's easier to handle. If we have multiple DataSources pointing to many different hosts, we don't have to worry about creating different HOSTS and DB_NAME template variables for each of them.

DSTOLF commented 5 years ago

The idea of using the same approach for build and run times is being able to use the same templates on build and deploy steps, testing or editing each one individually and not worrying about appending multiple properties into one single file. If anything goes wrong in build time, we know exactl where the error was.

arthurdm commented 5 years ago

hey @DSTOLF - that sounds awesome. If you could create a PR that's greatly appreciated. Will be good to enable you guys to pickup from the official image.

@arturdzm fyi.

DSTOLF commented 5 years ago

@arthurdm sorry for the delay, got caught up on some project tasks.

I've sent a PR with the proposed changes #143 for your review. If there's any thing I should change, please let mw know.