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

[lbaas] Health monitors: Fix mapping of max retries to API parameter #1175

Closed
wants to merge 1 commit into from

Conversation

BenjaminLudwigSAP
Copy link
Contributor

max_retries has been split up in Octavia API into max_retries and max_retries_down.
max_retries_down specifies the amount of retries for a HM before the pool member is considered down, while max_retries specifies the amount of retries for a HM before the pool member is considered up again.
We're only interested in the max retries before a member is down.

@ArtieReus
Copy link
Collaborator

Hi @BenjaminLudwigSAP thank you for the notice and the PR. I just have seen, that it should be also changed in other parts of the application as the HM model or the edit view, etc. I will check it next week.

BenjaminLudwigSAP added a commit that referenced this pull request Jan 13, 2023
Probe Timeout (`timeout` on the API) is not honored by the Octavia
provider driver. This is intentional. I understand this might be
confusing, so an optional detailed technical explanation can be found
below.

PR #1175 might seem similar to this, but it's not; #1175 fixes a mapping
between Elektra parameter name and API parameter name, while this PR
removes an unused parameter entirely.

Optional detailed technical explanation:
Health monitors on the F5 backend only accept two parameters:
`interval` and `timeout`. While the `interval` backend parameter has
the same semantics as the `delay` Octavia API parameter (that is, the
amount of seconds between probes), the `timeout` backend parameter has
slightly different semantics than the Octavia API `timeout` parameter:
While the `timeout` Octavia API parameter denotes the timeout *of a
single probe*, the `timeout` backend parameter denotes the timeout
`over all probes`. Therefore the `timeout` Octavia API parameter
cannot be used here. Instead we calculate the `timeout` backend
parameter (that is, the overall timeout) as the value of the
`max_retries_down` Octavia API parameter (that is, the number of
failing probes before the member is set to OFFLINE) multiplied by the
`delay` Octavia API parameter (that is, the amount of seconds between
probes) plus one spare second to give the last probe a chance to not
run into a timeout. That calculation happens here:
https://github.com/sapcc/octavia-f5-provider-driver/blob/stable/yoga-m3/octavia_f5/restclient/as3objects/monitor.py#L122
max_retries has been split up in Octavia API into max_retries and
max_retries_down. max_retries_down specifies the amount of retries for
a HM before the pool member is considered down, while max_retries
specifies the amount of retries for a HM before the pool member is
considered up again. We're only interested in the max retries before a
member is down.
@BenjaminLudwigSAP
Copy link
Contributor Author

BenjaminLudwigSAP commented Jan 13, 2023

(the force-push was to remove the change of Max Retries maximum value from 10 to 9. Turns out 10 is indeed the maximum value accepted by the Octavia API, while 9 is the maximum number accepted by the CLI... m(
TLDR: 10 is already the correct value.

@ArtieReus
Copy link
Collaborator

ArtieReus commented Jan 24, 2023

@BenjaminLudwigSAP are you sure that you can submit a HM without max_retries. Exchanging the attribute to 'max_retries_down' would return following error:

Invalid input for field/attribute max_retries. Value: 'None'. Mandatory field missing.

The API description for the field max_retries doesn't say that it is optional.
https://docs.openstack.org/api-ref/load-balancer/v2/?expanded=create-health-monitor-detail#create-health-monitor

@BenjaminLudwigSAP
Copy link
Contributor Author

BenjaminLudwigSAP commented Jan 24, 2023

Thanks for pointing this out. I suggest always supplying a fixed value of 1 for max_retries. The value isn't honored by the Octavia F5 provider driver anyway.

@BenjaminLudwigSAP
Copy link
Contributor Author

Closed in favor of #1179

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants