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.

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

Minor fixes on env vars and runtime for kubernetes #1389

Merged
merged 6 commits into from
Sep 1, 2024

Conversation

thgruiz
Copy link
Contributor

@thgruiz thgruiz commented Aug 28, 2024

First of all: this is an awesome project!

This commits changes the following:

  1. The vars on k8s deployment of the karavan-app
    As all variables were lowercase, quarkus did not see them, resulting in errors (like not finding the git repo's URL)

  2. Adds the JAVA_OPTS unused var to the command line used to run karavan-app
    The var was been defined on the Dockerfile, but not used on the final command
    With this change, It was replaced the docker 'exec form' with the 'shell form' so we can use the env var (or else we could not use it, as it has multiple values). I took caution to use the exec command to replace the shell by the java process (so it continues to receive OS signals)

  3. Adds a default runtime to the build.sh script for k8s, or else when creating an integration using only the karavan-app on k8s (did not tested with vscode extension) de build was failing with 'The runtime option must be specified'

There are more stuff that I wish to contribute (for example: there is an error on git checkout when the repo's HEAD does not exist, even if we would not use it ), but let's see if those simple ones here are accepted first :)

The vars used by quarkus need to be upper case
and dots replaced by underscore to work
Now the ENV will be used on the command line
(that was replaced from exec form to shell form of docker's CMD
as in the the devmode docker image)
Without this the build fails.
(was added too the possibility to configure other runtime with an env var)
Not needed anymore since Hazelcast was replaced by plain maps
@mgubaidullin
Copy link
Contributor

Thanks @thgruiz !

@mgubaidullin mgubaidullin merged commit 3a0eb24 into apache:main Sep 1, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants