-
Notifications
You must be signed in to change notification settings - Fork 29
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
tickets/DM-48511: Fix redis values configuration #4364
base: main
Are you sure you want to change the base?
Conversation
2c296bf
to
49b9c74
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating a KafkaTopic resource for backpack is the right move.
I've made a few suggestions to improve the configuration.
@@ -110,3 +110,10 @@ redis: | |||
|
|||
# -- Affinity rules for the Redis pod | |||
affinity: {} | |||
|
|||
cluster: | |||
name: backpack |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the name of the Kafka cluster that backpack uses, i.e "sasquatch"
name: backpack | ||
|
||
strimzi: | ||
enabled: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would just list the Kafka topics you want to create here, assuming that Strimzi is deployed.
|
||
strimzi: | ||
enabled: true | ||
topics: [lsst.backpack.usgs-earthquake-data] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest you group all configuration related to Kafka for example:
kafka:
cluster:
name: sasquatch
topics:
- lsst.backpack.usgs-earthquake-data
@@ -0,0 +1,15 @@ | |||
{{- if .Values.strimzi.enabled }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can't really test this effectively here, so just assume Strimzi is deployed and that backpack depends on it.
@@ -0,0 +1,15 @@ | |||
{{- if .Values.strimzi.enabled }} | |||
{{- $cluster := .Values.cluster.name }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the suggested configuration below, this would become:
{{- $cluster := .Values.kafka.cluster.name }}
@@ -0,0 +1,15 @@ | |||
{{- if .Values.strimzi.enabled }} | |||
{{- $cluster := .Values.cluster.name }} | |||
{{- range $topic := .Values.strimzi.topics }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And this would become:
{{- range $topic := .Values.kafka.topics }}
replicas: 3 | ||
partitions: 1 | ||
{{- end }} | ||
{{- end }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove the extra end too.
No description provided.