-
Notifications
You must be signed in to change notification settings - Fork 23
test(dns): default GEO conflict #826
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
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| def test_default_geo_conflict(dns_policy, dns_policy2): | ||
| """Verify that when two DNSPolicies have defaultGeo=True, both policies enter the await validation state""" | ||
| sleep(60) # wait a bit for records between two clusters to synchronize |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we replace the hardcoded sleep with something else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's not the best solution, I just needed to wait for an arbitrary reconciliation time for dns-operator dns records to synchronize between clusters. I'd expect this time to be longer when you have more clusters and dns policies in the dns-operator pool. Feel free to suggest if you have different solution on your mind. I'd implement testsuite-wide solution if we can come up with anything better
|
|
||
| assert dns_policy.wait_until( | ||
| has_record_condition("Ready", "False", "AwaitingValidation", "Awaiting validation"), timelimit=450 | ||
| ), f"DNSPolicy did not reach expected record status, instead it was: {dns_policy2.model.status.recordConditions}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this assertion is checking dns_policy so you should not use dns_policy2 in the error message
| for component in [gateway, gateway2, tls_policy, tls_policy2]: | ||
| component.wait_for_ready() | ||
| for component in [dns_policy, dns_policy2]: | ||
| component.wait_for_accepted() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would personally add dns_policy2 later inside test_... function so here it would be ok to wait_for_ready for dns_policy1. To me adding dns_policy2 to create a conflict is part of test scenario rather that the test setup.
For sure this does not block the merging
| sleep(60) # wait a bit for records between two clusters to synchronize | ||
|
|
||
| assert dns_policy.wait_until( | ||
| has_record_condition("Ready", "False", "AwaitingValidation", "Awaiting validation"), timelimit=450 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 450s time limit is quite long, is it really needed to be that long?
Signed-off-by: averevki <sandyverevkin@gmail.com>
18d1da8 to
945af03
Compare
Description
defaultGeoset toTruein a multicluster environmentCloses #623
Tests
testsuite/tests/multicluster/load_balanced/test_default_geo_conflict.pywith thetest_default_geo_conflicttestVerification steps
Run the test:
poetry run pytest -vv testsuite/tests/multicluster/load_balanced/test_default_geo_conflict.py # Note: This test requires a multicluster environment with DNS provider configuration.