Add check to see if druid is running in kubernetes #17697
+20
−4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gianm
reviewed
Feb 4, 2025
There was a problem hiding this comment.
Hmm, why change the name of the variable?
I see that #16386 changed it from DRUID_SET_HOST to DRUID_SET_HOST_IP. I don't see discussion in that PR about why, but changing it back and forth makes it tougher for users to understand what version uses what variable name. IMO better to stick with DRUID_SET_HOST_IP given it was already released that way in Druid 31.
gianm
reviewed
Feb 4, 2025
distribution/docker/druid.sh
Outdated
| DRUID_SET_HOST=${DRUID_SET_HOST:-1} | ||
| else | ||
| # Running in kubernetes | ||
| DRUID_SET_HOST=${DRUID_SET_HOST:-0} |
There was a problem hiding this comment.
Please include a comment about why the default is different inside and outside of k8s.
distribution/docker/druid.sh
Outdated
| @@ -138,8 +138,15 @@ then | |||
| setKey _common druid.zk.service.host "${ZOOKEEPER}" | |||
| fi | |||
|
|
|||
| DRUID_SET_HOST_IP=${DRUID_SET_HOST_IP:-0} | |||
| if [ "${DRUID_SET_HOST_IP}" = "1" ] | |||
| if [ -z ${KUBERNETES_SERVICE_HOST} ] | |||
There was a problem hiding this comment.
Use [ -z "${KUBERNETES_SERVICE_HOST}" ] instead, it will work better if the variable has whitespace in it. Unlikely but still a good practice with shell scripting.
cryptoe
approved these changes
Feb 11, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The PR #16386 changed the docker file to use the canonical hostname instead of ip by default. This was an attempt to fix
druid.indexer.task.restoreTasksOnRestart, as it does not work on kubernetes without the canonical hostname.This change is not backward compatible, as switching to canonical names would not work with docker. This PR attempts to fix this by adding a check to see if the deployment is in a kubernetes environment by checking
${KUBERNETES_SERVICE_HOST}and using canonical hostnames instead of IP addresses only in that case.This PR has: