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.

Sign up for GitHub

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

feat: enhance worker deployments with additional packages and CA certificates #1626

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
+84 −9
Open

feat: enhance worker deployments with additional packages and CA certificates #1626

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6b1dd1d
feat: enhance worker deployments with additional packages and CA cert…
patsevanton Dec 8, 2024
a15cae5
feat: enhance worker deployments with additional packages and CA cert…
patsevanton Dec 8, 2024
b50b337
feat: enhance worker deployments with additional packages and CA cert…
patsevanton Dec 8, 2024
432096e
feat: enhance worker deployments with additional packages and CA cert…
patsevanton Dec 8, 2024
2fb8c2c
feat: enhance worker deployments with additional packages and CA cert…
patsevanton Dec 8, 2024
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
25 changes: 22 additions & 3 deletions charts/sentry/templates/sentry/worker/deployment-sentry-worker-events.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,18 @@ spec:
- name: {{ .Chart.Name }}-worker
image: "{{ template "sentry.image" . }}"
imagePullPolicy: {{ default "IfNotPresent" .Values.images.sentry.pullPolicy }}
command: ["sentry"]
command: ["/bin/bash", "-c"]
Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to use init container and emptyDir. Нou will get the same result without changing the main container command

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What commands should I run in the init container?

Copy link
Contributor

Choose a reason for hiding this comment

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

Same commands, but in different container and mount emptyDir for /usr/local/share/ca-certificates/. Like init container in relay

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adonskoy could you create MR with you idea?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adonskoy there is no point in adding certificates to python storage using init container since python certificate storage in the main container is dynamic and can change from version to version

Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tried using the REQUESTS_CA_BUNDLE env? Should work for sentry

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also need to install additional python packages. They cannot be installed via init container

args:
- "run"
- "worker"
- |
{{- if .Values.sentry.worker.installAdditionalPackages }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Installing packages at runtime is not the best practice and will take much longer at startup compared to a custom image. Is this really necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I don't see any other options

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems to me that this can be solved in a more universal way: add the ability to set a custom command to launch the container. Then you can add what you need in your case

Copy link
Contributor Author

@patsevanton patsevanton Dec 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Can you create new MR with you idea?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adonskoy Can you create new MR with you idea?

pip install {{ range .Values.sentry.worker.installAdditionalPackages }}{{ . }} {{ end }}
{{- end }}
mkdir -p /usr/local/share/ca-certificates/
for c in $(ls -1 /usr/local/share/ca-certificates/); do
Copy link
Contributor

Choose a reason for hiding this comment

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

How about adding this to the template and adding it to all sentry workloads?

After all, CAs may be needed throughout the entire system, right?

Copy link
Contributor Author

@patsevanton patsevanton Dec 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Can you write the example code?

cat /usr/local/share/ca-certificates/$c >> $(python3 -m certifi) && echo >> $(python3 -m certifi)
done
update-ca-certificates
sentry run worker
- "-Q"
- {{ .Values.sentry.workerEvents.queues }}
{{- if .Values.sentry.workerEvents.concurrency }}
Expand Down Expand Up @@ -122,6 +130,11 @@ spec:
{{- if .Values.sentry.workerEvents.volumeMounts }}
{{ toYaml .Values.sentry.workerEvents.volumeMounts | indent 8 }}
{{- end }}
{{- if .Values.sentry.worker.caCertificatesSecret }}
- name: ca-certificates
mountPath: /usr/local/share/ca-certificates
readOnly: true
{{- end }}
{{- if .Values.sentry.workerEvents.livenessProbe.enabled }}
livenessProbe:
periodSeconds: {{ .Values.sentry.workerEvents.livenessProbe.periodSeconds }}
Expand Down Expand Up @@ -179,4 +192,10 @@ spec:
{{- if .Values.sentry.workerEvents.volumes }}
{{ toYaml .Values.sentry.workerEvents.volumes | indent 6 }}
{{- end }}
{{- if .Values.sentry.worker.caCertificatesSecret }}
- name: ca-certificates
secret:
secretName: {{ .Values.sentry.worker.caCertificatesSecret }}
defaultMode: 0644
{{- end }}
{{- end }}
25 changes: 22 additions & 3 deletions charts/sentry/templates/sentry/worker/deployment-sentry-worker-transactions.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,18 @@ spec:
- name: {{ .Chart.Name }}-worker
image: "{{ template "sentry.image" . }}"
imagePullPolicy: {{ default "IfNotPresent" .Values.images.sentry.pullPolicy }}
command: ["sentry"]
command: ["/bin/bash", "-c"]
args:
- "run"
- "worker"
- |
{{- if .Values.sentry.worker.installAdditionalPackages }}
pip install {{ range .Values.sentry.worker.installAdditionalPackages }}{{ . }} {{ end }}
{{- end }}
mkdir -p /usr/local/share/ca-certificates/
for c in $(ls -1 /usr/local/share/ca-certificates/); do
cat /usr/local/share/ca-certificates/$c >> $(python3 -m certifi) && echo >> $(python3 -m certifi)
done
update-ca-certificates
sentry run worker
- "-Q"
- {{ .Values.sentry.workerTransactions.queues }}
{{- if .Values.sentry.workerTransactions.concurrency }}
Expand Down Expand Up @@ -122,6 +130,11 @@ spec:
{{- if .Values.sentry.workerTransactions.volumeMounts }}
{{ toYaml .Values.sentry.workerTransactions.volumeMounts | indent 8 }}
{{- end }}
{{- if .Values.sentry.worker.caCertificatesSecret }}
- name: ca-certificates
mountPath: /usr/local/share/ca-certificates
readOnly: true
{{- end }}
{{- if .Values.sentry.workerTransactions.livenessProbe.enabled }}
livenessProbe:
periodSeconds: {{ .Values.sentry.workerTransactions.livenessProbe.periodSeconds }}
Expand Down Expand Up @@ -179,4 +192,10 @@ spec:
{{- if .Values.sentry.workerTransactions.volumes }}
{{ toYaml .Values.sentry.workerTransactions.volumes | indent 6 }}
{{- end }}
{{- if .Values.sentry.worker.caCertificatesSecret }}
- name: ca-certificates
secret:
secretName: {{ .Values.sentry.worker.caCertificatesSecret }}
defaultMode: 0644
{{- end }}
{{- end }}
25 changes: 22 additions & 3 deletions charts/sentry/templates/sentry/worker/deployment-sentry-worker.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,18 @@ spec:
- name: {{ .Chart.Name }}-worker
image: "{{ template "sentry.image" . }}"
imagePullPolicy: {{ default "IfNotPresent" .Values.images.sentry.pullPolicy }}
command: ["sentry"]
command: ["/bin/bash", "-c"]
args:
- "run"
- "worker"
- |
{{- if .Values.sentry.worker.installAdditionalPackages }}
pip install {{ range .Values.sentry.worker.installAdditionalPackages }}{{ . }} {{ end }}
{{- end }}
mkdir -p /usr/local/share/ca-certificates/
for c in $(ls -1 /usr/local/share/ca-certificates/); do
cat /usr/local/share/ca-certificates/$c >> $(python3 -m certifi) && echo >> $(python3 -m certifi)
done
update-ca-certificates
sentry run worker
{{- if .Values.sentry.worker.excludeQueues }}
- "--exclude-queues"
- "{{ .Values.sentry.worker.excludeQueues }}"
Expand Down Expand Up @@ -129,6 +137,11 @@ spec:
{{- if .Values.sentry.worker.volumeMounts }}
{{ toYaml .Values.sentry.worker.volumeMounts | indent 8 }}
{{- end }}
{{- if .Values.sentry.worker.caCertificatesSecret }}
- name: ca-certificates
mountPath: /usr/local/share/ca-certificates
readOnly: true
{{- end }}
{{- if .Values.sentry.worker.livenessProbe.enabled }}
livenessProbe:
periodSeconds: {{ .Values.sentry.worker.livenessProbe.periodSeconds }}
Expand Down Expand Up @@ -192,4 +205,10 @@ spec:
{{- if .Values.global.volumes }}
{{ toYaml .Values.global.volumes | indent 6 }}
{{- end }}
{{- if .Values.sentry.worker.caCertificatesSecret }}
- name: ca-certificates
secret:
secretName: {{ .Values.sentry.worker.caCertificatesSecret }}
defaultMode: 0644
{{- end }}
{{- end }}
18 changes: 18 additions & 0 deletions charts/sentry/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,12 @@ sentry:

worker:
enabled: true
# installAdditionalPackages:
# - django-multidb-router
# - sentry-nodestore-elastic
# - https://github.com/pavels/sentry-s3-nodestore/releases/download/v1.0.3/sentry-s3-nodestore-1.0.3.tar.gz
# kubectl create secret generic ca-certificates --from-file=you-certificates.crt=you-certificates.crt
# caCertificatesSecret: ca-certificates
# annotations: {}
replicas: 1
# concurrency: 4
Expand Down Expand Up @@ -291,6 +297,12 @@ sentry:
workerEvents:
## If the number of exceptions increases, it is recommended to enable workerEvents
enabled: false
# installAdditionalPackages:
# - django-multidb-router
# - sentry-nodestore-elastic
# - https://github.com/pavels/sentry-s3-nodestore/releases/download/v1.0.3/sentry-s3-nodestore-1.0.3.tar.gz
# kubectl create secret generic ca-certificates --from-file=you-certificates.crt=you-certificates.crt
# caCertificatesSecret: ca-certificates
# annotations: {}
queues: "events.save_event,post_process_errors"
## When increasing the number of exceptions and enabling workerEvents, it is recommended to increase the number of their replicas
Expand Down Expand Up @@ -328,6 +340,12 @@ sentry:
# allows to dedicate some workers to specific queues
workerTransactions:
enabled: false
# installAdditionalPackages:
# - django-multidb-router
# - sentry-nodestore-elastic
# - https://github.com/pavels/sentry-s3-nodestore/releases/download/v1.0.3/sentry-s3-nodestore-1.0.3.tar.gz
# kubectl create secret generic ca-certificates --from-file=you-certificates.crt=you-certificates.crt
# caCertificatesSecret: ca-certificates
# annotations: {}
queues: "events.save_event_transaction,post_process_transactions"
replicas: 1
Expand Down
Loading