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

Set priority group only for pools with multiple members. #257

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

velp
Copy link
Contributor

@velp velp commented Mar 5, 2024

This PR fixes priorityGroup set for a single member. This is related to a bug in F5 which we detected with Octavia Tempest. The fix was already tested and the number of fails decreased by 40%.

@BenjaminLudwigSAP
Copy link
Collaborator

What exactly does this change in the behavior of the code? At first I thought that it might work around any([]) == True, but after checking on a python3 console I found that any([]) == False, which already is the default value you're setting for enable_priority_group. So there should be no change in functionality of the code from this PR, as far as I can see.

Please also note that I deployed #153 to QA yesterday. I don't think that this made a difference for the tests, but just so you know.

@BenjaminLudwigSAP
Copy link
Collaborator

BenjaminLudwigSAP commented Mar 5, 2024

fixes priorityGroup set for a single member

So I assume that member.backup is not set when there is only one member? If so, please add this fact as a comment above the line you changed. Thank you.

@velp velp self-assigned this Mar 5, 2024
@velp
Copy link
Contributor Author

velp commented Mar 5, 2024

@BenjaminLudwigSAP I've already explained this in our meeting. This is a situation when we have at the same time:

  • disabled member (aka backup) - that's why any() returns True
  • single member in the pool
  • priorityGroup set

@velp
Copy link
Contributor Author

velp commented Mar 5, 2024

BTW: it was tested in qa-de-1 before and after your deployment and this fix worked as expected.

@BenjaminLudwigSAP BenjaminLudwigSAP merged commit 03fd782 into stable/yoga-m3 Mar 5, 2024
3 checks passed
@BenjaminLudwigSAP BenjaminLudwigSAP deleted the fix_priority_group branch March 5, 2024 16:42
@BenjaminLudwigSAP
Copy link
Collaborator

Thank you for explaining again. PRs should always explain what exactly they're fixing.

I added my requested comment to the code.

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