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

Update docker-entrypoint.sh #48

merged 2 commits into from
Nov 21, 2018

Conversation

lukasmrtvy
Copy link
Contributor

Support for REDIS_URL environment variable in docker-entrypoint.sh

@lukasmrtvy
Copy link
Contributor Author

lukasmrtvy commented Nov 12, 2018

There is also problem with SSL, if you want to access REDIS with SSL enabled, it does not work.

EDIT:
Proper SSLredis URL according IANA is:
rediss://[:password]@[host]:[port]/[db] -> its working

@lukasmrtvy lukasmrtvy reopened this Nov 12, 2018
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.

@eli-darkly eli-darkly merged commit 1435de3 into launchdarkly:v5 Nov 21, 2018
@eli-darkly
Copy link
Contributor

This will be in the next release. Thanks!

@eli-darkly eli-darkly mentioned this pull request Nov 27, 2018
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.

2 participants