Skip to content
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

Add DNSPolicy health check tests #573

Merged
merged 1 commit into from
Nov 19, 2024

Conversation

averevki
Copy link
Contributor

@averevki averevki commented Nov 6, 2024

Test cases:

  • Healthy endpoint with TLS
  • Healthy endpoint without TLS
  • Unhealthy endpoint
  • Endpoint removal and return

Also added classes for the health check section of the DNSPolicy and for the DNSHealthCheckProbe CR

Part of the #572

@averevki averevki added Test case New test case DNS Issues for DNSOperator labels Nov 6, 2024
@averevki averevki requested a review from trepel November 6, 2024 18:28
@averevki averevki self-assigned this Nov 6, 2024
@trepel
Copy link
Contributor

trepel commented Nov 8, 2024

It looks good to me and it (more or less) worked yesterday for me, but today with a new Kuadrant nightly build it does not work. The reason is that DNSPolicy keeps being successfully enforced despite failing health_check. So this piece:

    assert dns_policy.wait_until(has_condition("Enforced", "False"))
    assert dns_policy.wait_until(
        has_record_condition("Ready", "False", "HealthChecksFailed", "None of the healthchecks succeeded")
    )

is never true. Not sure if there has been any changes related to this behavior recently and whether it is intended or not but it seems like a bug to me.

@trepel
Copy link
Contributor

trepel commented Nov 8, 2024

It happened to me twice that I got following error:

testsuite/kuadrant/policy/dns.py:99: in get_dns_health_probe
    dns_probe = self.get_owned("dnsrecords.kuadrant.io")[0].get_owned("DNSHealthCheckProbe")[0]
E   IndexError: list index out of range

As if the resource has not been created yet or ownership relationship has not been established quickly enough. I don't think this is something to fix in this PR - we'll see in nightlies whether this is worth investigating or not.

@averevki
Copy link
Contributor Author

averevki commented Nov 8, 2024

It happened to me twice that I got following error:

testsuite/kuadrant/policy/dns.py:99: in get_dns_health_probe
    dns_probe = self.get_owned("dnsrecords.kuadrant.io")[0].get_owned("DNSHealthCheckProbe")[0]
E   IndexError: list index out of range

As if the resource has not been created yet or ownership relationship has not been established quickly enough. I don't think this is something to fix in this PR - we'll see in nightlies whether this is worth investigating or not.

It's a bug where if any subdomain in the hostname starts with a number, the dnsoperator won't generate a health probe object for this endpoint. So if testsuite, for example, will generate hostname.123abc.app-services.net hostname for the this test, then test will fail. It is a known issue and I'm waiting for it to get resolved

jsmolar
jsmolar previously approved these changes Nov 14, 2024
@averevki
Copy link
Contributor Author

It looks good to me and it (more or less) worked yesterday for me, but today with a new Kuadrant nightly build it does not work. The reason is that DNSPolicy keeps being successfully enforced despite failing health_check. So this piece:

    assert dns_policy.wait_until(has_condition("Enforced", "False"))
    assert dns_policy.wait_until(
        has_record_condition("Ready", "False", "HealthChecksFailed", "None of the healthchecks succeeded")
    )

is never true. Not sure if there has been any changes related to this behavior recently and whether it is intended or not but it seems like a bug to me.

Thanks, there were indeed some changes in the dnspolicy. Adjusted now 👍

Copy link
Contributor

@trepel trepel left a comment

Choose a reason for hiding this comment

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

Code looks good to me. Tests passed against November 19 nightly build. Approving this one.

@jsmolar jsmolar merged commit f47d53f into Kuadrant:main Nov 19, 2024
3 checks passed
@averevki averevki deleted the dns-healthchecks-tests branch November 19, 2024 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DNS Issues for DNSOperator Test case New test case
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants