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

Update docker-entrypoint.sh #48

Merged
merged 2 commits into from
Nov 21, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 21 additions & 13 deletions docker-entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,28 @@ heartbeatIntervalSecs = ${HEARTBEAT_INTERVAL:-15}
" > /ldr/ld-relay.conf

if [ "$USE_REDIS" = 1 ]; then
if echo "$REDIS_PORT" | grep -q 'tcp://'; then
# REDIS_PORT gets set to tcp://$docker_ip:6379 when linking to a redis container
# default to using those values if they exist
REDIS_HOST_PART="${REDIS_PORT%:*}"
REDIS_HOST="${REDIS_HOST_PART##*/}"
REDIS_PORT="${REDIS_PORT##*:}"
fi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fi shouldn't have been deleted— it goes with the if on line 20. Also, the lines inside this if block shouldn't have been unindented.

Copy link
Contributor Author

@lukasmrtvy lukasmrtvy Nov 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont know what You mean...
Is:

if [ "$USE_REDIS" = 1 ]; then
        if [ -z "${REDIS_HOST}" ] && [ -z "${REDIS_URL}" ]; then echo "Choose REDIS_HOST and REDIS_PORT or REDIS_URL"; exit 1; fi

        if echo "$REDIS_PORT" | grep -q 'tcp://'; then
                # REDIS_PORT gets set to tcp://$docker_ip:6379 when linking to a redis container
                # default to using those values if they exist
                REDIS_HOST_PART="${REDIS_PORT%:*}"
                REDIS_HOST="${REDIS_HOST_PART##*/}"
                REDIS_PORT="${REDIS_PORT##*:}"
                cat <<-EOF >> /ldr/ld-relay.conf
                [redis]
                host = "${REDIS_HOST:-redis}"
                port = "${REDIS_PORT:-6379}"
                EOF
        elif [ -n "${REDIS_URL+x}" ]; then
                cat <<-EOF >> /ldr/ld-relay.conf
                [redis]
                url = "${REDIS_URL:-rediss://:password@redis:6380/0}"
                EOF
        fi
        echo "localTtl = ${REDIS_TTL:-30000}" >> /ldr/ld-relay.conf
fi

more suitable for You?

Copy link
Contributor

@eli-darkly eli-darkly Nov 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, there's still a problem there. The problem is that you have made the entire section from # REDIS_PORT gets set to tcp://$docker_ip:6379 through port = "${REDIS_PORT:-6379}" \n EOF conditional on REDIS_PORT matching that special Docker URL pattern. That means that if REDIS_HOST is set, and REDIS_PORT is just a port number, it will not put the host and port in the config file.

The logic should be:

  • Write [redis] to the config file no matter what (I mean, if USE_REDIS is set).
  • If REDIS_HOST or REDIS_PORT is set to anything, then we are going to write host= and port= to the config file. The default values for those are REDIS_HOST and REDIS_PORT; however, if REDIS_PORT starts with tcp:// then we will parse it into a host and port.

So I think that would end up like this:

if [ "$USE_REDIS" = 1 ]; then
        if [ -z "${REDIS_HOST}" ] && [ -z "${REDIS_URL}" ]; then echo "Choose REDIS_HOST and REDIS_PORT or REDIS_URL"; exit 1; fi

        echo "[redis]" >> /ldr/ld-relay.conf

        if [ -n "$REDIS_HOST" || -n "$REDIS_PORT" ]; then
                if echo "$REDIS_PORT" | grep -q 'tcp://'; then
                        # REDIS_PORT gets set to tcp://$docker_ip:6379 when linking to a redis container
                        # default to using those values if they exist
                        REDIS_HOST_PART="${REDIS_PORT%:*}"
                        REDIS_HOST="${REDIS_HOST_PART##*/}"
                        REDIS_PORT="${REDIS_PORT##*:}"
                fi
                cat <<-EOF >> /ldr/ld-relay.conf
                host = "${REDIS_HOST:-redis}"
                port = "${REDIS_PORT:-6379}"
                EOF
        elif [ -n "${REDIS_URL+x}" ]; then
                cat <<-EOF >> /ldr/ld-relay.conf
                url = "${REDIS_URL}"
                EOF
        fi
        echo "localTtl = ${REDIS_TTL:-30000}" >> /ldr/ld-relay.conf
fi

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I added a fix for it.

if [ -z "${REDIS_HOST}" ] && [ -z "${REDIS_URL}" ]; then echo "Choose REDIS_HOST and REDIS_PORT or REDIS_URL"; exit 1; fi

echo "
[redis]
host = \"${REDIS_HOST:-redis}\"
port = ${REDIS_PORT:-6379}
localTtl = ${REDIS_TTL:-30000}
" >> /ldr/ld-relay.conf
echo "[redis]" >> /ldr/ld-relay.conf

if [ -n "$REDIS_HOST" || -n "$REDIS_PORT" ]; then
if echo "$REDIS_PORT" | grep -q 'tcp://'; then
# REDIS_PORT gets set to tcp://$docker_ip:6379 when linking to a redis container
# default to using those values if they exist
REDIS_HOST_PART="${REDIS_PORT%:*}"
REDIS_HOST="${REDIS_HOST_PART##*/}"
REDIS_PORT="${REDIS_PORT##*:}"
fi
cat <<-EOF >> /ldr/ld-relay.conf
host = "${REDIS_HOST:-redis}"
port = "${REDIS_PORT:-6379}"
EOF
elif [ -n "${REDIS_URL+x}" ]; then
cat <<-EOF >> /ldr/ld-relay.conf
url = "${REDIS_URL}"
EOF
fi
echo "localTtl = ${REDIS_TTL:-30000}" >> /ldr/ld-relay.conf
fi

if [ "$USE_EVENTS" = 1 ]; then
Expand Down