Kube-Series-2.1: Features/properties-makefile-dockerfile

[bygui86]

Refactored application properties system.

As this refactoring is pretty big, I decided to split it in different subsequent PRs:

1. improve application properties system
2. add Makefile to make development and operations easier
3. improve Dockerfile

After this I can complete Kubernetes YAML manifests.

[Crim]

changing these environment names will break backwards compatibility yea?

[bygui86]

I tested it manually and I don't see problems.
Unfortunately I was not able to test in Docker, because the build fail. It's not able to download from the artifact repo :(
As soon as I can, I will attach logs for this.

[Crim]

I think the issue is if someone has built a deployment process around using these environment variables, removing/renaming them would be a breaking change for them as they would need to update their deployment to use the new variable names.

I'd prefer to maintain backwards compatibility where possible to avoid causing version upgrade pains/difficulties for people.

[bygui86]

I know what you mean, but sometimes it's unavoidable to have some kind of disruptions after a while between two different versions. The main example is Java 8 to 9. Some disruptive changes were unavoidable and better for the future of Java in general.
Moreover if the documentation is written well, I think everyone can figure out easily what to change and how.

Analysing specifically these changes:

• HEAP_OPTS refers just to some specific Java configurations about the memory, what about other configurations? Like -noverify -server -XX:TieredStopAtLevel=1 these should not be included as this would mean misusing this specific environment variable. A variable called JVM_OPTS and JAVA_OPTS are more general purpose and usable in many different flexible ways.
• LOG_OPTS can be restored back, but it's kind of the same as HEAP_OPTS.
• The usage of SPRING_CONFIG_LOCATION is completely the same, I removed this as I already included everything in the two default files /resources/application.yaml and /resources/application-dev.yaml. This has 100% backward compatibility.

Moreover the definition of empty environment variables like

if [[ -z "$HEAP_OPTS" ]]; then
    export HEAP_OPTS=""
fi
if [[ -z "$LOG_OPTS" ]]; then
    export LOG_OPTS=""
fi

is useless and equivalent to not defining them, so they can be removed without backward compatibility problems anyway.

I missed to share with you a bigger vision on my series of PRs. In the last one, improving Dockerfile, I would remove the usage of start.sh in the Dockerfile, in favour to something like following:

FROM openjdk:8-jre-alpine

...

ENTRYPOINT exec java $JVM_OPTS $LOG_OPTS $JAVA_OPTS -jar kafka-webview-ui-*.jar

start.sh does nothing special/specific, so this could be avoided in favour to an easier Dockerfile and no startup scripts, replacing some useless complexity.

[Crim]

In regards to the start.sh changes. It may be better to have a check if the to be deprecated environment variables are used, and display an appropriate warning and migration information. Until removal, it should be easy to merge the previous variable into the new one.

I see no issues with the Dockerfile avoiding the start.sh entirely as you've laid above.

[Crim]

Got it.
Do you think that the Makefile (to be introduced by PR #200 ) is enough as alternative to start.sh?

Probably doesn't make sense to package a Makefile with a binary distribution that's already compiled.

RE: Docker failing to find the archive from sonatype... I'm not sure. It looks like all of the releases were removed from their maven repository, I'll need to look into why.

You should be able to swap out the URL for the copy hosted on github here: https://github.com/SourceLabOrg/kafka-webview/releases/download/v2.5.0/kafka-webview-ui-2.5.0-bin.zip

[bygui86]

Probably doesn't make sense to package a Makefile with a binary distribution that's already compiled.

Ah ok, you mean really deploy just the jar together with the script.sh to be deployed on prem. Ok got it. I will think of something like that ;)

You should be able to swap out the URL for the copy hosted on github here: https://github.com/SourceLabOrg/kafka-webview/releases/download/v2.5.0/kafka-webview-ui-2.5.0-bin.zip

I will prepare some different Dockerfile to have more options to build the project. For example a Dockerfile.local to take the jar from local, in order to be able to use a very custom version of Kafka-WebView and a Dockerfile.release to be used for an official release.

[bygui86]

Hi @Crim, sorry to let you wait, Xmas time here around.

I made the changes we discussed on the start.sh, please have a look at it!
After my changes in the last PR of this series, I would like to discuss how to deal with the start scripts. I don't understand why they are inside src/assembly for example.

I will put all changes to the Dockerfile in the last PR of this series.

[Crim]

So after reviewing this file, I'm not sure it works like you might expect it to. I've modified line 25 below to show what command would be run.

# Start application
echo java $JVM_OPTS $MEM_OPTS $JAVA_OPTS -jar kafka-webview-ui-*.jar

As written its impossible to set the new environment variables and have them be used. Example:

SMBP:tmp $ export JAVA_OPTS="-Xms1G -Xmx5G"
SMBP:tmp $ ./test.sh 
java -noverify -server -XX:TieredStopAtLevel=1 -Xms2G -Xmx2G -XX:MaxMetaspaceSize=300M -jar kafka-webview-ui-*.jar

I believe you'd need the if [[ -z .... checks to default the the values, instead of just hardcoding the environment variables?

[bygui86]

@Crim I'm really sorry! I made a big mistake.
I just pushed a new version of the script. I tested it more extensively with zsh and bash, it works as expected now. Please test it and let me know.

Big changes:

• removed the folder change, it's not necessary if we take care of find the directory of the script and put it in front of the jar file name
• removed exec from java execution, not necessary as java command is blocker anyway

[Crim]

Does it not make sense to have this use a profile as well to avoid having to redefine every property in this config?

Instead I'd like to be able to only surface the most relevant/common settings and hide away the complexities of configuring springboot.

[bygui86]

Does it not make sense to have this use a profile as well to avoid having to redefine every property in this config?

Spring Profiles are intended to make easier the customisation (add, update) of specific application properties. We don't have to redefine every time all properties in each configurations. The example is application-dev.yaml:

• server.servlet.session.persistent is set to true instead of the default false
• spring.jpa.show-sql = true is added
  Or did I understand wrong your question?

Instead I'd like to be able to only surface the most relevant/common settings and hide away the complexities of configuring springboot.

We can hide Spring Boot complexity defining some easier environment variables. But I don't think that changing completely the way configurations are set is an easier way, taking away some complexity. Instead you have to figure out a new way, more difficult and not really well documented compared to the standard Spring Boot way.
Instead we can think about something like this:

spring:
  application:
    name: ${APP_NAME:"Kafka Webview"}
management:
  server:
    port: ${MANAGEMENT_PORT:9090}

Already done by you before for the server.port:

server:
  port: ${PORT:8080}

[bygui86]

The way I described above about defining easier environment variables, we can decide which configuration should be easy to be overridden and provide documentation for that. If someone wants to override something else deeper in Spring Boot, it's his/her duty to understand how.

[bygui86]

So what do you think about my last comment?
Can we define a set of easy-to-configure properties like suggested above and the rest will remain up to the user?

[bygui86]

@Crim are you busy in this period?

[bygui86]

@Crim can I have an update on this PR?