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

Icinga2Client#Do(): call parent method, re-try on HTTP 503 300 times #29

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

Al2Klimov
Copy link
Member

@Al2Klimov Al2Klimov commented Mar 11, 2024

During an Icinga 2 reload, there are phases where the API can't perform config changes and replies with an HTTP 503 error instead (see Icinga/icinga2#9445 for example) which means that the client should retry the operation at a later time. The PR retries such errors for approximately 5 minutes (plus response times).

refs Icinga/icingadb#685

@cla-bot cla-bot bot added the cla/signed label Mar 11, 2024
@Al2Klimov
Copy link
Member Author

Al2Klimov commented Mar 11, 2024

utils/icinga2_client.go Outdated Show resolved Hide resolved
utils/icinga2_client.go Outdated Show resolved Hide resolved
@Al2Klimov Al2Klimov marked this pull request as draft March 12, 2024 10:55

for attempt := 1; ; attempt++ {
response, err := c.Client.Do(req)
if err == nil && response.StatusCode == http.StatusServiceUnavailable && req.GetBody != nil && attempt < 300 {
Copy link
Member

Choose a reason for hiding this comment

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

attempt < 300 with attempt starting at 1 results in performing c.Client.Do() 300 times, not re-trying 300 times.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The number of attempts isn't even the most relevant part here. It retries for roughly 5 minutes, which should be plenty for our tests.

@Al2Klimov Al2Klimov marked this pull request as ready for review March 14, 2024 08:57
Copy link
Collaborator

@julianbrost julianbrost left a comment

Choose a reason for hiding this comment

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

Fixed the PR description for you.

@julianbrost julianbrost merged commit 143a0bd into main Mar 14, 2024
4 checks passed
@Al2Klimov Al2Klimov deleted the Icinga503 branch March 14, 2024 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants