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 Redis connection settings #776

Merged

Conversation

bmclaughlin
Copy link
Contributor

Update to only populate PULP_REDIS_URL if the relevant PULP_REDIS_* environment variables are set.

Issue: AAH-581

@bmclaughlin bmclaughlin requested a review from cutwater May 21, 2021 18:14
else
protocol="redis://"
fi
if [[ ! -z "${PULP_REDIS_HOST-}" && ! -z "${PULP_REDIS_PASSWORD-}" &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if [[ ! -z "${PULP_REDIS_HOST-}" && ! -z "${PULP_REDIS_PASSWORD-}" &&
if [[ ! -z "${PULP_REDIS_HOST:-}" && ! -z "${PULP_REDIS_PASSWORD:-}" &&

@bmclaughlin bmclaughlin force-pushed the update-redis-connection-hack branch from 61707bb to 983eff3 Compare June 1, 2021 15:38
Copy link
Contributor

@cutwater cutwater left a comment

Choose a reason for hiding this comment

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

In bash you can check for non-empty string by using -n:

if [[ -n "${redis_host}" ...

Shouldn't it be OR condition?

Copy link
Contributor

@cutwater cutwater left a comment

Choose a reason for hiding this comment

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

See my comments above.

Comment on lines 105 to 119
redis_host="${PULP_REDIS_HOST:-}"
redis_password="${PULP_REDIS_PASSWORD:-}"
redis_port="${PULP_REDIS_PORT:-}"
redis_ssl="${PULP_REDIS_SSL:-}"

if [[ "${redis_host}" != "" && "${redis_port}" != "" &&
"${redis_password}" != "" && "${redis_ssl}" != "" ]]; then
if [[ ${redis_ssl} == "true" ]]; then
protocol="rediss://"
else
protocol="redis://"
fi

if [[ -z "${PULP_REDIS_PASSWORD:-}" ]]; then
password=""
else
password=":${PULP_REDIS_PASSWORD}@"
PULP_REDIS_URL="${protocol}:${password}@${redis_host}:${redis_port}/0"
unset PULP_REDIS_HOST PULP_REDIS_PORT PULP_REDIS_PASSWORD
export PULP_REDIS_URL
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

redis_connection_hack() {
    redis_host="${PULP_REDIS_HOST:-}"
    redis_password="${PULP_REDIS_PASSWORD:-}"
    redis_port="${PULP_REDIS_PORT:-}"
    redis_ssl="${PULP_REDIS_SSL:-}"

    if [[ -z "${redis_host}" && -z "${redis_port}" 
          && -z "${redis_password}" && -z "${redis_ssl}" ]]; then
        return
    fi

    if [[ "${redis_ssl}" == "true" ]]; then
        protocol="rediss://"
    else
        protocol="redis://"
    fi

    PULP_REDIS_URL="${protocol}:${redis_password}@${redis_host:-localhost}:${redis_port:-6379}/0"
    unset PULP_REDIS_HOST PULP_REDIS_PORT PULP_REDIS_PASSWORD PULP_REDIS_SSL
    export PULP_REDIS_URL
}

password=""
else
password=":${PULP_REDIS_PASSWORD}@"
PULP_REDIS_URL="${protocol}:${password}@${redis_host}:${redis_port}/0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Must be redis_password

@bmclaughlin bmclaughlin force-pushed the update-redis-connection-hack branch from 983eff3 to ebf9a59 Compare June 2, 2021 14:50
@bmclaughlin
Copy link
Contributor Author

@cutwater, updated. Successfully tested against dev environment and ephemeral templates.

Issue: AAH-581
@bmclaughlin bmclaughlin force-pushed the update-redis-connection-hack branch from ebf9a59 to c16efd9 Compare June 2, 2021 15:51
Copy link
Contributor

@cutwater cutwater left a comment

Choose a reason for hiding this comment

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

lgtm

@bmclaughlin bmclaughlin merged commit 2763ef5 into ansible:master Jun 2, 2021
@bmclaughlin bmclaughlin deleted the update-redis-connection-hack branch June 2, 2021 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants