pkg/alertmanager: Use lower value for --cluster.reconnect-timeout

[hwoarang]

Alertmanager in cluster mode resolves the DNS name of each peer and
caches its IP address which uses on regular intervals to 'refresh'
the connection.

In high-dynamic environment like kubernetes, it's possible that
alertmanager pods come and go on frequent intervals. The default timeout
value of 6h is not suitable in that case as alertmanager will keep
trying to reconnect to a non-existing pod over and over until it gives
up and remove that peer from the member list. During this period of
time, the cluster is reported to be in a degraded state due to the
missing member.

As such, it's best to use a lower value which will allow the
alertmanager to remove the pod from the list of peers soon
after it disappears.

Related: prometheus/alertmanager#2250

[simonpasquier]

The default timeout value of 6h is not suitable in that case as alertmanager will keep
trying to reconnect to a non-existing pod over and over until it gives
up and goes through another DNS resolution process.

To be clear Alertmanager will continuously resolve the peer addresses irrespective of the --cluster.reconnect-timeout value. The flag defines how long Alertmanager will try to reconnect to an IP address it has been connected before. I do agree that in dynamic environments such as Kubernetes, it makes sense to lower the default value.

[hwoarang]

@simonpasquier Thank you for the clarification. I can update the commit message if you want

[simonpasquier]

Yes please, update the comment too. Thanks!

[hwoarang]

Yes please, update the comment too. Thanks!

@simonpasquier I have updated both. I hope it's better now

[simonpasquier]

The CI fails because of Go formatting, please run make go-fmt and commit the changes.

[simonpasquier]

LGTM

[s-urbaniak]

LGTM, but potentially a follow-up question: Does it make sense to configure this setting? My impression is that it shouldn't as it is an implementation detail of how we reconcile alertmanager instances. We could add that rationale though at least as a comment, wdyt?

[hwoarang]

LGTM, but potentially a follow-up question: Does it make sense to configure this setting? My impression is that it shouldn't as it is an implementation detail of how we reconcile alertmanager instances. We could add that rationale though at least as a comment, wdyt?

I agree that configuring this option shouldn't be necessary. I think the existing comment explains the reason to change the default value. Do you think it needs more details?

[s-urbaniak]

I think we're good, merging, thank you!