fix: correct Helm hook configuration for geoip-install-job and PVC #1527

patsevanton · 2024-10-09T11:06:29Z

This PR addresses two issues related to the GeoIP volume and Helm hook configuration in the Sentry Helm chart:

Add GeoIP Volume to Sentry Deployments:
- Previously, the data-sentry-geoip volume was not being added to several Sentry deployment manifests, leading to errors during installation. This PR adds the data-sentry-geoip volume to the following deployment manifests:
  - deployment-relay.yaml
  - deployment-sentry-web.yaml
  - deployment-sentry-worker-events.yaml
  - deployment-sentry-worker-transactions.yaml
  - deployment-sentry-worker.yaml
Fix Helm Hook Configuration for GeoIP Installation Job:
- The GeoIP installation job was previously configured to run before the chart installation, which could lead to errors since the Persistent Volume Claim (PVC) was not yet created. This PR changes the Helm hook configuration for the GeoIP installation job to run after the chart installation and ensures the PVC is created before the GeoIP installation job runs.

Changes

deployment-relay.yaml:
- Added the data-sentry-geoip volume to the volumes section.
deployment-sentry-web.yaml:
- Added the data-sentry-geoip volume to the volumes section.
deployment-sentry-worker-events.yaml:
- Added the data-sentry-geoip volume to the volumes section.
deployment-sentry-worker-transactions.yaml:
- Added the data-sentry-geoip volume to the volumes section.
deployment-sentry-worker.yaml:
- Added the data-sentry-geoip volume to the volumes section.
values.yaml:
- Updated comments in the data-sentry-geoip section to provide examples for storageClass, volumeName, mountPath, and path.
deployment-geoip-job.yaml:
- Changed the helm.sh/hook annotation value from pre-install to post-install.
- Changed the helm.sh/hook-weight annotation value from 3 to 9 to ensure the job runs after the main resources are installed.
pvc-geoip.yaml:
- Added helm.sh/hook annotation with the value pre-install.
- Added helm.sh/hook-weight annotation with the value -1 to ensure the PVC is created before the GeoIP installation job runs.

Related Pull Requests:

Feel free to review and provide feedback. Thank you!

patsevanton · 2024-10-09T13:09:13Z

@swade1987 @sdernbach-ionos Can you review?

How i tested:

Create S3 bucket
install csi-s3

helm install csi-s3 oci://cr.yandex/yc-marketplace/yandex-cloud/csi-s3/csi-s3 --namespace test --set secret.accessKey=accessKey --set secret.secretKey=secretKey

clone repo https://github.com/patsevanton/sentry-kubernetes-charts-fork.git
checkout branch geoip-install-job
change values.yaml

geodata:
  accountID: "xxxx"
  licenseKey: "xxxxx"
  editionIDs: "xxx"
  persistence:
    ## If defined, storageClassName: <storageClass>
    ## If undefined (the default) or set to null, no storageClassName spec is
    ##   set, choosing the default provisioner.  (gp2 on AWS, standard on
    ##   GKE, AWS & OpenStack)
    storageClass: "csi-s3"
    size: 1Gi
  volumeName: "data-sentry-geoip"
  # mountPath of the volume containing the database
  mountPath: "/usr/share/GeoIP"
  # path to the geoip database inside the volumemount
  path: "/usr/share/GeoIP/GeoLite2-City.mmdb"

helm install -n test --wait sentry . --values values.yaml --timeout=1000s

swade1987 · 2024-10-09T14:25:27Z

I personally think that the geodata configuration should be able to be provided as a secret with the following data:

accountID:
licenseKey:
editionIDs:

The company I work for (and I would assume a lot more) won't be happy having the license key in plain text

sdernbach-ionos · 2024-10-09T20:56:21Z

@patsevanton when I try to install I get:

Error: INSTALLATION FAILED: 1 error occurred:
	* Deployment.apps "sentry-web" is invalid: spec.template.spec.volumes[3].name: Duplicate value: "data-sentry-geoip"

When I checked the templates I see that it is defined multiple times, eg. the web deployment:

{{- if and .Values.geodata.volumeName .Values.geodata.accountID }}
      - name: {{ .Values.geodata.volumeName }}
        persistentVolumeClaim:
          claimName: data-sentry-geoip
      {{- end }}
{{- if .Values.sentry.web.volumes }}
{{ toYaml .Values.sentry.web.volumes | indent 6 }}
{{- end }}
      {{- if .Values.geodata.volumeName }}
      - name: {{ .Values.geodata.volumeName }}
        persistentVolumeClaim:
          claimName: {{ .Values.geodata.volumeName }}
      {{- end }}
{{- end }}

I guess you have to remove the first part that you added

{{- if and .Values.geodata.volumeName .Values.geodata.accountID }}
      - name: {{ .Values.geodata.volumeName }}
        persistentVolumeClaim:
          claimName: data-sentry-geoip
      {{- end }}

I think there is no reason to have the if and condition and I would just keep the below one that uses the configurable volumeName. What do you think?

If I remove one of the both in all files I can successfully install it, but I can not see a geo ip db in the bucket or in /usr/share/GeoIP (eg. in web container), but the update job succeeded

patsevanton · 2024-10-10T00:34:45Z

@sdernbach-ionos I deleted duplicate. Could you try again?
Could you run after helm install:

k get pvc -A | grep RWX
k get pvc -A | grep data-sentry-geoip

sdernbach-ionos · 2024-10-10T08:04:03Z

@patsevanton I get the following output:

k get pvc -A | grep RWX

container-registry   registry-claim                               Bound    pvc-0ebe07f3-4ed5-45b8-9d33-cc80e2376171   20Gi       RWX            microk8s-hostpath   <unset>                 25d
test                 data-sentry-geoip                            Bound    pvc-e2b56463-d1b9-4693-91fd-a54ad1574027   1Gi        RWX            csi-s3              <unset>                 12m

just to have both, but for sure already included in the one above:

k get pvc -A | grep data-sentry-geoip

test                 data-sentry-geoip                            Bound    pvc-e2b56463-d1b9-4693-91fd-a54ad1574027   1Gi        RWX            csi-s3              <unset>                 13m

So installation works with the new changes, but still can not see any files in the bucket nor in the folder of GeoIP:

root@sentry-web-5797df7fff-vscpk:~# ls -la /usr/share/GeoIP
total 8
drwxr-xr-x 2 root root 4096 Oct 10 08:02 .
drwxr-xr-x 1 root root 4096 Oct 10 07:46 ..

Strange thing is, even I create a txt file in the folder it is not visible in the bucket (would have expected that I can see it online)

charts/sentry/templates/relay/deployment-relay.yaml

patsevanton · 2024-10-10T08:35:57Z

@sdernbach-ionos
How was the installation of helm chart sentry?
Please run

kubectl get pod -n namespace-sentry

sdernbach-ionos · 2024-10-10T08:38:36Z

@patsevanton everything successfull:

helm install -n test --wait sentry . --values values.yaml --timeout=1000s

NAME: sentry
LAST DEPLOYED: Thu Oct 10 09:45:45 2024
NAMESPACE: test
STATUS: deployed
REVISION: 1
TEST SUITE: None
NOTES:
* When running upgrades, make sure to give back the `system.secretKey` value.

kubectl -n test get configmap sentry-sentry -o json | grep -m1 -Po '(?<=system.secret-key: )[^\\]*'

kubectl get pod -n test

NAME                                                              READY   STATUS      RESTARTS      AGE
csi-attacher-s3-0                                                 1/1     Running     0             52m
csi-provisioner-s3-0                                              2/2     Running     0             52m
csi-s3-qmjkf                                                      2/2     Running     0             52m
geoip-install-job-d6tj2                                           0/1     Completed   0             43m
sentry-billing-metrics-consumer-68d95496fb-kftnd                  1/1     Running     0             43m
sentry-clickhouse-0                                               1/1     Running     0             52m
sentry-cron-77875b77d9-sdr2n                                      1/1     Running     2 (52m ago)   52m
sentry-generic-metrics-consumer-68cf4fd568-67pzc                  1/1     Running     0             43m
sentry-ingest-consumer-attachments-bd465ffd-jtvv4                 1/1     Running     0             43m
sentry-ingest-consumer-events-5dd9f47959-xrhbv                    1/1     Running     0             43m
sentry-ingest-consumer-transactions-84b677689f-qkftj              1/1     Running     0             43m
sentry-ingest-monitors-856b74fdf4-g8rbz                           1/1     Running     0             43m
sentry-ingest-occurrences-6986c956b6-86k9p                        1/1     Running     0             43m
sentry-ingest-replay-recordings-df4bcc666-8sltv                   1/1     Running     0             43m
sentry-issue-occurrence-consumer-6cb8b6f9bd-x4jpq                 1/1     Running     0             42m
sentry-kafka-controller-0                                         1/1     Running     0             52m
sentry-kafka-controller-1                                         1/1     Running     0             52m
sentry-kafka-controller-2                                         1/1     Running     0             52m
sentry-metrics-consumer-5644f6496-szsns                           1/1     Running     0             43m
sentry-nginx-5c6dc8cf8-f7pqs                                      1/1     Running     0             52m
sentry-post-process-forward-errors-657695d8ff-vptfj               1/1     Running     0             43m
sentry-post-process-forward-issue-platform-cd74485fb-n7zhk        1/1     Running     0             43m
sentry-post-process-forward-transactions-7f9f958fb6-r9pr6         1/1     Running     0             43m
sentry-rabbitmq-0                                                 1/1     Running     0             52m
sentry-relay-848bd4847d-5pws9                                     1/1     Running     0             42m
sentry-sentry-postgresql-0                                        1/1     Running     0             52m
sentry-sentry-redis-master-0                                      1/1     Running     0             52m
sentry-sentry-redis-replicas-0                                    1/1     Running     0             52m
sentry-snuba-api-79446bd7c5-6vxtp                                 1/1     Running     0             52m
sentry-snuba-consumer-6896f96954-6cd6w                            1/1     Running     0             43m
sentry-snuba-generic-metrics-counters-consumer-845db75c5d-89tjn   1/1     Running     0             43m
sentry-snuba-generic-metrics-distributions-consumer-56ff76pdxr4   1/1     Running     0             43m
sentry-snuba-generic-metrics-sets-consumer-586dc6cf46-njn8k       1/1     Running     0             43m
sentry-snuba-group-attributes-consumer-f89f785c6-5p9cn            1/1     Running     0             43m
sentry-snuba-metrics-consumer-759d669bdc-kk64x                    1/1     Running     0             43m
sentry-snuba-outcomes-billing-consumer-868b88f68-dwqsm            1/1     Running     0             43m
sentry-snuba-outcomes-consumer-67ffbf5cc9-vv4p5                   1/1     Running     0             42m
sentry-snuba-replacer-b9d845fbc-srk2n                             1/1     Running     0             42m
sentry-snuba-replays-consumer-54bd765cfc-jz5gs                    1/1     Running     0             43m
sentry-snuba-spans-consumer-6895974784-7zj8z                      1/1     Running     0             42m
sentry-snuba-subscription-consumer-events-7ff6f64cf5-nxnxj        1/1     Running     0             42m
sentry-snuba-subscription-consumer-metrics-8594985f58-n8ck2       1/1     Running     0             42m
sentry-snuba-subscription-consumer-transactions-6d6fd6ff77pbpzv   1/1     Running     0             42m
sentry-snuba-transactions-consumer-5598f4cb6-7xcdr                1/1     Running     0             42m
sentry-subscription-consumer-events-6bc7dfbb76-59pl2              1/1     Running     0             43m
sentry-subscription-consumer-generic-metrics-59bfd68485-b4xnr     1/1     Running     0             43m
sentry-subscription-consumer-metrics-698489cfbf-4fwtw             1/1     Running     0             43m
sentry-subscription-consumer-transactions-8595f78c56-x2wl4        1/1     Running     0             43m
sentry-web-5797df7fff-vscpk                                       1/1     Running     2 (51m ago)   52m
sentry-worker-6757867cf4-qstm7                                    1/1     Running     2 (51m ago)   52m
sentry-zookeeper-clickhouse-0                                     1/1     Running     0             52m

patsevanton · 2024-10-10T08:42:15Z

Could you run and delete credentials:

stern -n test -l job-name=geoip-install-job

For example:

+ geoip-install-job-zsclt › geoipupdate
+ geoip-install-job-zsclt › init-geoip-conf
geoip-install-job-zsclt geoipupdate # STATE: Running geoipupdate
geoip-install-job-zsclt geoipupdate Error loading configuration: invalid account ID format
- geoip-install-job-zsclt › geoipupdate
- geoip-install-job-zsclt › init-geoip-conf

Now i can`t install helm sentry chart because i get error "Error loading configuration: invalid account ID format"

sdernbach-ionos · 2024-10-10T09:18:25Z

@patsevanton not sure if I got you right, but if I just execute it:

stern -n test -l job-name=geoip-install-job

+ geoip-install-job-d6tj2 › geoipupdate
+ geoip-install-job-d6tj2 › init-geoip-conf
geoip-install-job-d6tj2 geoipupdate # STATE: Running geoipupdate
- geoip-install-job-d6tj2 › geoipupdate
- geoip-install-job-d6tj2 › init-geoip-conf

(have to CTRL+C to cancel it)

If I delete the secret (sentry-geoip-env) before executing (same result):

stern -n test -l job-name=geoip-install-job

+ geoip-install-job-d6tj2 › geoipupdate
+ geoip-install-job-d6tj2 › init-geoip-conf
geoip-install-job-d6tj2 geoipupdate # STATE: Running geoipupdate
- geoip-install-job-d6tj2 › init-geoip-conf
- geoip-install-job-d6tj2 › geoipupdate

patsevanton · 2024-10-10T09:35:13Z

Could you use https://github.com/clbx/kubectl-browse-pvc ?

kubectl browse-pvc -n test data-sentry-geoip

sdernbach-ionos · 2024-10-10T09:38:11Z

@patsevanton same result as in the bucket and in the web pod:

kubectl browse-pvc -n test data-sentry-geoip

✓ Attached to browse-data-sentry-geoip-wg9lk
/mnt # ls
/mnt # ls -la
total 8
drwxr-xr-x    2 root     root          4096 Oct 10 09:37 .
drwxr-xr-x    1 root     root          4096 Oct 10 09:37 ..

patsevanton · 2024-10-10T09:54:46Z

kubectl browse-pvc -n test data-sentry-geoip
cd to root

cd /
ls -la
ls -la /usr/share/GeoIP

sdernbach-ionos · 2024-10-10T10:34:08Z

@patsevanton

kubectl browse-pvc -n test data-sentry-geoip

✓ Attached to browse-data-sentry-geoip-5snsr
/mnt # cd /

/ # ls -la
total 64
drwxr-xr-x    1 root     root          4096 Oct 10 10:33 .
drwxr-xr-x    1 root     root          4096 Oct 10 10:33 ..
drwxr-xr-x    2 root     root          4096 Sep  6 11:34 bin
drwxr-xr-x    5 root     root           360 Oct 10 10:33 dev
drwxr-xr-x    1 root     root          4096 Oct 10 10:33 etc
drwxr-xr-x    2 root     root          4096 Sep  6 11:34 home
drwxr-xr-x    6 root     root          4096 Sep  6 11:34 lib
drwxr-xr-x    5 root     root          4096 Sep  6 11:34 media
drwxr-xr-x    2 root     root          4096 Oct 10 10:33 mnt
drwxr-xr-x    2 root     root          4096 Sep  6 11:34 opt
dr-xr-xr-x  624 root     root             0 Oct 10 10:33 proc
drwx------    1 root     root          4096 Oct 10 10:33 root
drwxr-xr-x    1 root     root          4096 Oct 10 10:33 run
drwxr-xr-x    2 root     root          4096 Sep  6 11:34 sbin
drwxr-xr-x    2 root     root          4096 Sep  6 11:34 srv
dr-xr-xr-x   13 root     root             0 Oct 10 10:33 sys
drwxrwxrwt    2 root     root          4096 Sep  6 11:34 tmp
drwxr-xr-x    7 root     root          4096 Sep  6 11:34 usr
drwxr-xr-x   12 root     root          4096 Sep  6 11:34 var

/ # ls -la /usr/share/GeoIP
ls: /usr/share/GeoIP: No such file or directory

sdernbach-ionos · 2024-10-10T10:35:30Z

What is also a bit strange to me, the command in the deployment should write the config file to /usr/share/GeoIP if I got it right? I can also not see this file there as well as no info for the update container that it should pick up the file from there (as default location is another one according to the documentation).

patsevanton · 2024-10-10T10:40:12Z

I added code for create directory mkdir -p /usr/share/GeoIP
Try uninstall helm chart, deployment, pvc, namespace
and create namespace and install again

sdernbach-ionos · 2024-10-10T10:56:25Z

@patsevanton sadly same result (also browsing into pvc does not show the folder) but I can see your changes so I'm on the newest version. Only change within install process (even it succeeded) there was this error (but the geo ip job was not yet there):

W1010 12:46:03.629656   60773 reflector.go:484] k8s.io/[email protected]/tools/cache/reflector.go:243: watch of *unstructured.Unstructured ended with: an error on the server ("unable to decode an event from the watch stream: local error: tls: bad record MAC") has prevented the request from succeeding

patsevanton · 2024-10-10T11:05:07Z

where do these logs come from?

sdernbach-ionos · 2024-10-10T11:08:19Z

@patsevanton it is printed while during installation:

helm install -n test --wait sentry . --values values.yaml --timeout=1000s

WARNING: Kubernetes configuration file is group-readable. This is insecure. Location: /Users/sdernbach/.kube/config
WARNING: Kubernetes configuration file is world-readable. This is insecure. Location: /Users/sdernbach/.kube/config
coalesce.go:237: warning: skipped value for kafka.config: Not a table.
W1010 12:46:03.629656   60773 reflector.go:484] k8s.io/[email protected]/tools/cache/reflector.go:243: watch of *unstructured.Unstructured ended with: an error on the server ("unable to decode an event from the watch stream: local error: tls: bad record MAC") has prevented the request from succeeding

NAME: sentry
LAST DEPLOYED: Thu Oct 10 12:44:15 2024
NAMESPACE: test
STATUS: deployed
REVISION: 1
TEST SUITE: None
NOTES:
* When running upgrades, make sure to give back the `system.secretKey` value.

kubectl -n test get configmap sentry-sentry -o json | grep -m1 -Po '(?<=system.secret-key: )[^\\]*'

patsevanton · 2024-10-10T11:35:02Z

I think it's a problem with either kubernetes or the network
pterodactyl/panel#2523
kubernetes/minikube#7313

@Mokto is it possible to merge the pull request? no helm chart sentry installation errors. perhaps these are network errors.

sdernbach-ionos · 2024-10-10T12:02:35Z

@patsevanton ok so it is related to the s3 thing ... just tested without s3 storage class and my normal host storage and it works 🥳 I can see database files and config also within the web pod /usr/share/GeoIP

patsevanton · 2024-10-10T16:16:00Z

Tomorrow, I will create new and clean pull request based on this pull request

patsevanton · 2024-10-11T05:40:39Z

Close by #1529

fix: correct Helm hook configuration for geoip-install-job and PVC

560a657

patsevanton marked this pull request as draft October 9, 2024 11:55

fix: correct Helm hook configuration for geoip-install-job and PVC

96813e2

patsevanton marked this pull request as ready for review October 9, 2024 13:07

fix: correct Helm hook configuration for geoip-install-job and PVC

9c7cd6f

fix: correct Helm hook configuration for geoip-install-job and PVC

cb87270

fix: correct Helm hook configuration for geoip-install-job and PVC

77259d0

sdernbach-ionos reviewed Oct 10, 2024

View reviewed changes

charts/sentry/templates/relay/deployment-relay.yaml Show resolved Hide resolved

fix: correct Helm hook configuration for geoip-install-job and PVC

d081275

sdernbach-ionos approved these changes Oct 10, 2024

View reviewed changes

patsevanton mentioned this pull request Oct 11, 2024

fix: update geoip job hooks and volume handling #1529

Merged

patsevanton closed this Oct 11, 2024

patsevanton deleted the geoip-install-job branch October 13, 2024 13:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: correct Helm hook configuration for geoip-install-job and PVC #1527

fix: correct Helm hook configuration for geoip-install-job and PVC #1527

patsevanton commented Oct 9, 2024 •

edited

Loading

patsevanton commented Oct 9, 2024 •

edited

Loading

swade1987 commented Oct 9, 2024

sdernbach-ionos commented Oct 9, 2024 •

edited

Loading

patsevanton commented Oct 10, 2024 •

edited

Loading

sdernbach-ionos commented Oct 10, 2024

patsevanton commented Oct 10, 2024

sdernbach-ionos commented Oct 10, 2024

patsevanton commented Oct 10, 2024 •

edited

Loading

sdernbach-ionos commented Oct 10, 2024

patsevanton commented Oct 10, 2024

sdernbach-ionos commented Oct 10, 2024

patsevanton commented Oct 10, 2024 •

edited

Loading

sdernbach-ionos commented Oct 10, 2024

sdernbach-ionos commented Oct 10, 2024

patsevanton commented Oct 10, 2024

sdernbach-ionos commented Oct 10, 2024

patsevanton commented Oct 10, 2024

sdernbach-ionos commented Oct 10, 2024

patsevanton commented Oct 10, 2024 •

edited

Loading

sdernbach-ionos commented Oct 10, 2024

patsevanton commented Oct 10, 2024

patsevanton commented Oct 11, 2024

fix: correct Helm hook configuration for geoip-install-job and PVC #1527

fix: correct Helm hook configuration for geoip-install-job and PVC #1527

Conversation

patsevanton commented Oct 9, 2024 • edited Loading

Changes

patsevanton commented Oct 9, 2024 • edited Loading

swade1987 commented Oct 9, 2024

sdernbach-ionos commented Oct 9, 2024 • edited Loading

patsevanton commented Oct 10, 2024 • edited Loading

sdernbach-ionos commented Oct 10, 2024

patsevanton commented Oct 10, 2024

sdernbach-ionos commented Oct 10, 2024

patsevanton commented Oct 10, 2024 • edited Loading

sdernbach-ionos commented Oct 10, 2024

patsevanton commented Oct 10, 2024

sdernbach-ionos commented Oct 10, 2024

patsevanton commented Oct 10, 2024 • edited Loading

sdernbach-ionos commented Oct 10, 2024

sdernbach-ionos commented Oct 10, 2024

patsevanton commented Oct 10, 2024

sdernbach-ionos commented Oct 10, 2024

patsevanton commented Oct 10, 2024

sdernbach-ionos commented Oct 10, 2024

patsevanton commented Oct 10, 2024 • edited Loading

sdernbach-ionos commented Oct 10, 2024

patsevanton commented Oct 10, 2024

patsevanton commented Oct 11, 2024

patsevanton commented Oct 9, 2024 •

edited

Loading

patsevanton commented Oct 9, 2024 •

edited

Loading

sdernbach-ionos commented Oct 9, 2024 •

edited

Loading

patsevanton commented Oct 10, 2024 •

edited

Loading

patsevanton commented Oct 10, 2024 •

edited

Loading

patsevanton commented Oct 10, 2024 •

edited

Loading

patsevanton commented Oct 10, 2024 •

edited

Loading