add mailgun and alerting alerting to Redis #257
Conversation
| @@ -509,6 +509,12 @@ local composition = | |||
| controlNamespace: params.services.controlNamespace, | |||
| restoreSA: 'redisrestoreserviceaccount', | |||
| quotasEnabled: std.toString(params.services.vshn.quotasEnabled), | |||
| emailAlertingEnabled: std.toString(params.services.vshn.emailAlerting.enabled), | |||
There was a problem hiding this comment.
I know I start to sound like a broken record in all my reviews, but maybe a function to generate these configs will avoid copy/pasting in the future.
There was a problem hiding this comment.
Can you clarify what exactly you mean? How does a function help here? It's only passing a parameter to the configMap for the xfn container
There was a problem hiding this comment.
We have the same emailAlerting* lines repeated over and over for each service. Each time copy pasted, this is bad practice for various reasons:
- Copy paste errors happen
- We would need to change all files again if there are changes
If we instead just have a jsonnet function where we pass params.services.vshn.emailAlerting and it returns all those lines, we would make it much less error prone to add a new service with email alerting. The we just need to call the function here and we're done. If there are any changes in the future we'd only have to adjust the function and all services will get the changes.
There was a problem hiding this comment.
Ah I understand now. Refactored the code accordingly. Please check again.
TheBigLee
force-pushed
the
redis_user_alerting
branch
from
7fd1d79 to
94d476f
Compare
TheBigLee
force-pushed
the
redis_user_alerting
branch
2 times, most recently
from
009cb79 to
08152ce
Compare
TheBigLee
force-pushed
the
redis_user_alerting
branch
from
08152ce to
3edb433
Compare
Simply adding mailgun alerting and custom Alertmanager configs to Redis
Checklist
changelog.
The PR has a meaningful description that sums up the change. It will be
linked in the changelog.
bug,enhancement,documentation,change,breaking,dependencyas they show up in the changelog.