Skip to content

Conversation

@azgabur
Copy link
Contributor

@azgabur azgabur commented Dec 1, 2025

Description

Work on #660

To introduce smarter waits for DNS related waiting. Not sure I covered all places in the codebase that would benefit from this util function. To benefit from Kuadrant/helm-charts-olm#60 this refactor was needed.

Changes

  • Added wait_for_dns util method which does dns query with retries and expected result.
  • Refactored two tests which had sleep(300) previously

Verification

Just two tests affected.

make testsuite/tests/multicluster/load_balanced/test_change_strategy.py testsuite/tests/multicluster/load_balanced/test_change_default_geo.py

Signed-off-by: Alex Zgabur <azgabur@redhat.com>
Copy link
Contributor

@fabikova fabikova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, LGTM. I added just a single comment about TTL logic. Could you please explain if this is a valid concern? I'm not 100% sure if the backoff handles this.


sleep(300) # wait for DNS propagation on providers
assert resolver.resolve(hostname.hostname)[0].address == gateway2.external_ip().split(":")[0]
answer = wait_for_dns(hostname.hostname, gateway2.external_ip().split(":")[0], resolver=resolver)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would this new function return after the unsuccessful backoff after 300 seconds? I suspect it will fail out on resolvement after the exception backoff is done.

Should we add something like assert answer is not None or similar after, so it is clear where test failed in case of dns timeout or something?

return False


def wait_for_dns(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should consider creating a class for our own dns resolver at this point

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.

3 participants