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.

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

Add ttl to alertmanagerconfigs resource #6360

Open
onedr0p opened this issue Mar 2, 2024 · 11 comments · May be fixed by #6515
Open

Add ttl to alertmanagerconfigs resource #6360

onedr0p opened this issue Mar 2, 2024 · 11 comments · May be fixed by #6515

Comments

@onedr0p
Copy link

onedr0p commented Mar 2, 2024

Component(s)

AlertManagerConfig

What is missing? Please describe.

A new field was added to the pushover_configs called ttl, it would be nice if the operator supported this new field

Refs:

Describe alternatives you've considered.

No response

Environment Information.

Environment

Kubernetes Version: v1.29.2
Prometheus-Operator Version: 0.71.2

@onedr0p onedr0p added kind/feature needs-triage Issues that haven't been triaged yet labels Mar 2, 2024
@slashpai
Copy link
Contributor

slashpai commented Mar 4, 2024

IIUC this needs alertmanager version 0.27.0

@slashpai slashpai added help wanted good first issue and removed needs-triage Issues that haven't been triaged yet labels Mar 4, 2024
@slashpai
Copy link
Contributor

slashpai commented Mar 4, 2024

Do you want to contribute to the change?

@aerosouund
Copy link

Hello @slashpai
I would be interested in taking this, in case @onedr0p isn't already working on it

@onedr0p
Copy link
Author

onedr0p commented Mar 4, 2024

Go for it, thanks @aerosouund !

@slashpai
Copy link
Contributor

slashpai commented Mar 5, 2024

@aerosouund sure go ahead :)

@slashpai
Copy link
Contributor

slashpai commented Mar 5, 2024

@aerosouund aerosouund mentioned this issue Mar 6, 2024
5 tasks
@aerosouund
Copy link

Hello @slashpai

I have created a PR that adds the field, although i'd like to ask you where to add the functionality that uses this field.
i didn't find that in the prometheus operator there's an implementation of the Notify interface.

where do you think this functionality belongs ? or where do i start looking ?
Thanks in advance

@onedr0p
Copy link
Author

onedr0p commented Mar 7, 2024

@aerosouund Alertmanager will be able to pick up the config and use, see the PR that added the functionality to AM here

prometheus/alertmanager#3474

@slashpai
Copy link
Contributor

slashpai commented Mar 7, 2024

You need to add the code to include this change in pkg/alertmanager/amcfg.go, unit test to pkg/alertmanager/amcfg_test.go

See the example PR I linked in previous comment

@afzal442
Copy link

hi @aerosouund just check in! 😄 Are you still following up with your PR? I wanted to take this up.

@aerosouund
Copy link

aerosouund commented Apr 12, 2024

Hey @afzal442
No, feel free to take this up
I already have a PR, you can build on that

@afzal442 afzal442 linked a pull request Apr 15, 2024 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants