test(dns): Verify DNS Endpoint Provider aggregation logic #830
Conversation
Signed-off-by: Martina Fabikova <mfabikov@redhat.com>
fabikova
force-pushed
the
feature/issue-811-dns-endpoint-provider-tests
branch
from
ef5e5db to
25cd7db
Compare
| @pytest.fixture(scope="module") | ||
| def aws_provider_secret(): | ||
| """Returns the name of the AWS provider secret""" | ||
| return "aws-credentials" |
There was a problem hiding this comment.
Can you make this test dns provider agnostic? Use the default dns_provider_secret fixture instead.
Hardcoding secret names like this would mean that this test requires a secret named aws-credentials to be present on the cluster. Config-related things like this one should be configurable from the testsuite settings file
| @pytest.fixture(scope="module") | ||
| def endpoint_provider_secret(): | ||
| """Returns the name of the endpoint provider secret""" | ||
| return "dns-provider-credentials-endpoint" |
There was a problem hiding this comment.
Same with the hardcoded name here. Is this a secret you expect to be present on a cluster before test is executed?
| @pytest.fixture(scope="module") | ||
| def shared_hostname(base_domain, blame): | ||
| """Returns the shared hostname used for aggregation""" | ||
| return f"{blame('app')}.{base_domain}" |
There was a problem hiding this comment.
Try reusing the hostname fixture. We usually expose hostnames in the said fixture to not overwrite hostname for every test
| request.addfinalizer(destination_dnsrecord.delete) | ||
| destination_dnsrecord.commit() | ||
| destination_dnsrecord.wait_for_ready() | ||
|
|
||
| for record in source_dnsrecords: | ||
| request.addfinalizer(record.delete) | ||
| record.commit() |
There was a problem hiding this comment.
I think you can merge these 2 blocks under a single loop. Why isn't there a wait-for-ready for source records?
| def test_endpoint_provider_configuration(destination_dnsrecord, source_dnsrecords, endpoint_provider_secret): | ||
| """Verify configuration and labels""" | ||
| destination_dnsrecord.refresh() | ||
| assert destination_dnsrecord.model.metadata.labels.get("kuadrant.io/zone-record") == "true" | ||
|
|
||
| for record in source_dnsrecords: | ||
| record.refresh() | ||
| assert record.model.spec.providerRef.name == endpoint_provider_secret | ||
| assert record.model.spec.rootHost == destination_dnsrecord.model.spec.rootHost |
There was a problem hiding this comment.
We usually don't check resource object fields in our tests, including values added by the kuadrant. We verify if something works or not with actual http requests instead
testsuite/kuadrant/policy/dns.py
Outdated
| answers = resolver.resolve(hostname, "A") | ||
| found_ips = {ip.to_text() for ip in answers} | ||
| return expected_ip in found_ips | ||
| except Exception: # pylint: disable=broad-exception-caught |
There was a problem hiding this comment.
If pylint giving you warnings, it might be an indication to look for different solution than do disable this warning 😁
Look up why you shouldn't use broad exceptions in python
| def test_records_accessible(destination_dnsrecord, shared_hostname): | ||
| """Verify that endpoints are merged and accessible via DNS""" | ||
| # 1. Verify Merge | ||
| destination_dnsrecord.wait_for_endpoints_merged({SOURCE_IP1, SOURCE_IP2}) |
There was a problem hiding this comment.
How long does it take for endpoints to merge that you need custom wait here?
| destination_dnsrecord.wait_until_resolves(f"src1.{shared_hostname}", SOURCE_IP1) | ||
| destination_dnsrecord.wait_until_resolves(f"src2.{shared_hostname}", SOURCE_IP2) |
There was a problem hiding this comment.
Same thing about wait-for-ready I've written on your other PR. How long does it take until endpoints resolve after dnsrecords report them as ready?
Signed-off-by: Martina Fabikova <mfabikov@redhat.com>
2 participants
Description
This PR implements the test scenarios defined in issue #811 to verify the new DNS Endpoint Provider functionality.
It validates the aggregation workflow where multiple "Source" DNSRecords (acting as endpoint feeders) merge their endpoints into a single central "Destination" DNSRecord (acting as a Zone), which then propagates the records to an upstream provider (e.g., AWS).
Scenarios covered:
endpointprovider.Closes #811
Changes
testsuite/tests/singlecluster/gateway/dnspolicy/dns_records/test_dns_endpoint_provider.pywhich implements the test cases specified in the issue.DNSRecordclass intestsuite/kuadrant/policy/dns.pywith:wait_for_endpoints_mergedwait_until_resolves