-
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| """Test that dns-operator correctly handles situation when more than one DNSPolicy has defaultGeo set to True""" | ||
|
|
||
| from time import sleep | ||
| import pytest | ||
|
|
||
| from testsuite.kuadrant.policy.dns import DNSPolicy, LoadBalancing, has_record_condition | ||
|
|
||
| pytestmark = [pytest.mark.multicluster] | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| def dns_policy2(blame, cluster2, gateway2, dns_server2, module_label, dns_provider_secret): | ||
| """Second cluster DNSPolicy with defaultGeo also set to True""" | ||
| lb = LoadBalancing(defaultGeo=True, geo=dns_server2["geo_code"]) | ||
| return DNSPolicy.create_instance( | ||
| cluster2, blame("dns"), gateway2, dns_provider_secret, load_balancing=lb, labels={"app": module_label} | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module", autouse=True) | ||
| def commit( | ||
| request, routes, gateway, gateway2, dns_policy, dns_policy2, tls_policy, tls_policy2 | ||
| ): # pylint: disable=unused-argument | ||
| """Commits gateways and all policies for the test. Default geo conflict is expected to occur with dns policies""" | ||
| for component in [gateway, gateway2, dns_policy, dns_policy2, tls_policy, tls_policy2]: | ||
| request.addfinalizer(component.delete) | ||
| component.commit() | ||
|
|
||
| for component in [gateway, gateway2, tls_policy, tls_policy2]: | ||
| component.wait_for_ready() | ||
| for component in [dns_policy, dns_policy2]: | ||
| component.wait_for_accepted() | ||
|
|
||
|
|
||
| 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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we replace the hardcoded sleep with something else?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? |
||
| ), f"DNSPolicy did not reach expected record status, instead it was: {dns_policy2.model.status.recordConditions}" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this assertion is checking |
||
| assert dns_policy2.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.
I would personally add
dns_policy2later insidetest_...function so here it would be ok towait_for_readyfordns_policy1. To me addingdns_policy2to create a conflict is part of test scenario rather that the test setup.For sure this does not block the merging