fix(sentry/sentry/helper): Redis URL not set when using externalRedis.existingSecret or redis.auth.existingSecret

[jiriks74]

Fixes: #1870

[jiriks74]

Hello, seems like this might've been missed. Can I do something to push this further?

[patsevanton]

@jiriks74 could you share minimal values.yaml for reproduce?

[jiriks74]

redis:
  enabled: false
externalRedis:
  host: "my-redis"
  port: 6379
  existingSecret: redis-secret
  existingSecretKey: REDIS_PASSWORD
  db: 0
  ssl: false

[jiriks74]

The issue can be seen in the if statement..

1. Case: RabbitMQ
2. Case: Password defined in values

   {{- else if $redisPass }}

3. Case: No password

   {{- else if and (not .Values.externalRedis.existingSecret) (not .Values.redis.auth.existingSecret)}}

There's a missing case for when either externalRedis.existingSecret or redis.auth.existingSecret is set. And because the 3rd case is an if and on those values, once you set at least one of those the broker URL completely disappears and the workers try to connect to localhost instead.

[jiriks74]

@patsevanton
Anything I can do to push this further? This should solve the problem and the solution takes advantage of variables set up by other templates so it's even not that big of a change. Is there something I'm missing?

[patsevanton]

Need someone to check your MR. I'll try to take the time for that.

[patsevanton]

i tested issue #1870 and not reproducible

[jiriks74]

I'll look into this again in the coming days.

[patsevanton]

@jiriks74 could you update fork?

[jiriks74]

If you want me to rebase, I'm on it. I can't see any conflicts though.

[jiriks74]

Rebase done. Sorry for the bit of mess, I'm on a machine where I didn't set this repo up properly.

[patsevanton]

@Mokto confirm.

kubectl create namespace test
kubectl create secret generic redis-secret   --namespace test   --from-literal=REDIS_PASSWORD=password

values:

rabbitmq:
  enabled: false

redis:
  enabled: false

externalRedis:
  host: "c-c9qepvb2e3gcv46ht57p.rw.mdb.cloud.net"
  port: 6379
  existingSecret: redis-secret
  existingSecretKey: REDIS_PASSWORD

get config:

kubectl get configmap sentry-sentry -n test -o jsonpath='{.data.sentry\.conf\.py}' | python3 -c "import sys; print(sys.stdin.read().replace('\\\\n', '\n').replace('\\\\\"', '\"'));"


BROKER_URL = os.environ.get("BROKER_URL", f"redis://:{os.environ.get("REDIS_PASSWORD")}@c-c9qepvb2e3gcv46ht57p.rw.mdb.cloud.net:6379/0")

no error by port 6379

stern  -n test  --max-log-requests 500 . | grep 6379

get pods:

k get pod -n test
NAME                                                              READY   STATUS    RESTARTS   AGE
sentry-billing-metrics-consumer-547cbdc85c-qjzvp                  1/1     Running   0          2m24s
sentry-clickhouse-0                                               1/1     Running   0          21m
sentry-cron-5f6d8c889b-xhgtx                                      1/1     Running   0          21m
sentry-generic-metrics-consumer-6fb94c87d7-4sgdp                  1/1     Running   0          2m23s
sentry-ingest-consumer-attachments-66c8d9dd9-6cks8                1/1     Running   0          2m22s
sentry-ingest-consumer-events-85bf84d59-htf9p                     1/1     Running   0          2m21s
sentry-ingest-consumer-transactions-5459b48b87-gqlgz              1/1     Running   0          2m20s
sentry-ingest-monitors-656bbbbfb9-8mkhn                           1/1     Running   0          2m20s
sentry-ingest-occurrences-666c9c978b-srmjb                        1/1     Running   0          2m19s
sentry-ingest-replay-recordings-6df98c4948-n2mrr                  1/1     Running   0          2m18s
sentry-issue-occurrence-consumer-7b8d7bb878-2qcp9                 1/1     Running   0          2m
sentry-kafka-controller-0                                         1/1     Running   0          21m
sentry-kafka-controller-1                                         1/1     Running   0          21m
sentry-kafka-controller-2                                         1/1     Running   0          21m
sentry-metrics-consumer-6988fbc79f-95trp                          1/1     Running   0          2m18s
sentry-nginx-76b877d65c-46gdd                                     1/1     Running   0          21m
sentry-post-process-forward-errors-7d7d589f64-hpb2n               1/1     Running   0          2m17s
sentry-post-process-forward-issue-platform-bd64fb657-9tvml        1/1     Running   0          2m16s
sentry-post-process-forward-transactions-7f9745ff89-6vnzv         1/1     Running   0          2m15s
sentry-process-segments-74455fdc56-r2jfm                          1/1     Running   0          2m14s
sentry-process-spans-7f46d79547-d5w48                             1/1     Running   0          2m14s
sentry-relay-8458fdb788-pwnzf                                     1/1     Running   0          112s
sentry-sentry-postgresql-0                                        1/1     Running   0          21m
sentry-snuba-api-84969c954c-t9r9t                                 1/1     Running   0          21m
sentry-snuba-consumer-6f67877d64-wfddx                            1/1     Running   0          2m13s
sentry-snuba-eap-items-consumer-5967bffbbd-gfbz2                  1/1     Running   0          2m8s
sentry-snuba-generic-metrics-counters-consumer-bf668dcd-nfh8f     1/1     Running   0          2m8s
sentry-snuba-generic-metrics-distributions-consumer-698496z7rpq   1/1     Running   0          2m7s
sentry-snuba-generic-metrics-gauges-consumer-b897bc4fb-qqnnq      1/1     Running   0          2m6s
sentry-snuba-generic-metrics-sets-consumer-64d4974c9d-fmwlv       1/1     Running   0          2m5s
sentry-snuba-group-attributes-consumer-74c97cc9fd-rgdkl           1/1     Running   0          2m4s
sentry-snuba-metrics-consumer-568554bdfc-v6htk                    1/1     Running   0          2m4s
sentry-snuba-outcomes-billing-consumer-76f47f9bf5-v64cd           1/1     Running   0          2m3s
sentry-snuba-outcomes-consumer-7fd985f55-62827                    1/1     Running   0          119s
sentry-snuba-replacer-68864bfd8f-vklhn                            1/1     Running   0          118s
sentry-snuba-replays-consumer-7586c7d48d-btvbf                    1/1     Running   0          2m2s
sentry-snuba-spans-consumer-78d999549b-8dpgz                      1/1     Running   0          2m2s
sentry-snuba-subscription-consumer-eap-items-7cc68c8d88-csp6z     1/1     Running   0          118s
sentry-snuba-subscription-consumer-events-b6484f7b4-ksjk8         1/1     Running   0          117s
sentry-snuba-subscription-consumer-generic-metrics-counterqv2j4   1/1     Running   0          116s
sentry-snuba-subscription-consumer-generic-metrics-distribr2w66   1/1     Running   0          116s
sentry-snuba-subscription-consumer-generic-metrics-gauges-lwdw8   1/1     Running   0          115s
sentry-snuba-subscription-consumer-generic-metrics-sets-fbknw54   1/1     Running   0          114s
sentry-snuba-subscription-consumer-metrics-7bf8799b67-6skgx       1/1     Running   0          114s
sentry-snuba-subscription-consumer-transactions-7b895fdd84bmqk5   1/1     Running   0          113s
sentry-snuba-transactions-consumer-646df44866-w5k8c               1/1     Running   0          2m1s
sentry-snuba-uptime-results-consumer-5bbf569684-4q4b9             1/1     Running   0          2m
sentry-subscription-consumer-events-599bf6b469-lnllj              1/1     Running   0          2m12s
sentry-subscription-consumer-generic-metrics-5bfb9b75c4-sbjbp     1/1     Running   0          2m12s
sentry-subscription-consumer-metrics-68d9bf4c58-rb7rz             1/1     Running   0          2m11s
sentry-subscription-consumer-results-eap-items-7b9bf8755f-4gxh5   1/1     Running   0          2m10s
sentry-subscription-consumer-transactions-59cd48c649-895wx        1/1     Running   0          2m10s
sentry-uptime-results-59c6f99d75-tgj79                            1/1     Running   0          2m9s
sentry-web-d79fb9476-zm5qv                                        1/1     Running   0          21m
sentry-worker-74cfc8456c-94cxp                                    1/1     Running   0          21m
sentry-zookeeper-clickhouse-0                                     1/1     Running   0          21m

[jiriks74]

Thanks for the review, I'm glad we got to the bottom of this.

[patsevanton]

@Mokto Could you review pr?

[patsevanton]

@Mokto Could you review pr?

[tyr1k]

merge please

[vashian]

Hi — I’m also running into the same bug described here: when using externalRedis.existingSecret or redis.auth.existingSecret with the Sentry Helm chart I see that BROKER_URL is not set in the configmap (and pods still try to connect to 127.0.0.1:6379).

I believe the proposed fix, would resolve this for me too. Would it be possible to merge the PR and release a new chart version when convenient?

Thanks for all your work on the chart!

[jiriks74]

Hi,
I don't like doing these kinds of things but this has been stale for a while and I think that the patch is ready.

If there's something missing or you'd like changes feel free to comment and I'll incorporate them.

It would be really helpful if this was released by the of the week if you have time to make one.

[Mokto]

sorry somehow missed that

[jiriks74]

No worries, thanks for merging.