feat(dns): Add smarter wait for changing dns zone asserts#822
Open
azgabur wants to merge 1 commit intoKuadrant:mainfrom
Open
feat(dns): Add smarter wait for changing dns zone asserts#822azgabur wants to merge 1 commit intoKuadrant:mainfrom
azgabur wants to merge 1 commit intoKuadrant:mainfrom
Conversation
Signed-off-by: Alex Zgabur <azgabur@redhat.com>
fabikova
reviewed
Dec 2, 2025
Contributor
fabikova
left a comment
fabikova
left a comment
There was a problem hiding this comment.
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.
averevki
reviewed
Dec 8, 2025
|
|
||
| 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) |
Contributor
There was a problem hiding this comment.
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( |
Contributor
There was a problem hiding this comment.
We should consider creating a class for our own dns resolver at this point
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
wait_for_dnsutil method which does dns query with retries and expected result.sleep(300)previouslyVerification
Just two tests affected.