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

reconcile premature on probes nil -> healthy #317

Merged
merged 1 commit into from
Nov 26, 2024
Merged

reconcile premature on probes nil -> healthy #317

merged 1 commit into from
Nov 26, 2024

Conversation

maksymvavilov
Copy link
Contributor

@maksymvavilov maksymvavilov commented Nov 26, 2024

addresses 2 bugs:

  1. When probe status is nil we would never write unhealthy status.
  2. When the probe is nil into healthy the watch does not trigger premature reconciliation. Refactored the watch on probes to consult DNSRecord instead of on-cluster copy of the probe

Also, integration suite proves to give false positives when it comes to the premature reconciliation logic. We cannot fail those tests confidently so I'm removing them. The premature logic to be tested manually and, probably in the e2e suite #282

Copy link
Collaborator

@maleck13 maleck13 left a comment

Choose a reason for hiding this comment

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

checked the following

  • new gateway no status on healthchecks while dns propigating then becomes healthy (records not published until health passes, dnspolicy shows correct status)
  • new gateway no status on healthchecks while dns propigating then stays unhealthy (records not published dnspolicy shows correct status)
  • make the health check pass after failing for sometime (records published and dnspolicy shows correct status)
  • make healthcheck fail after publish (records remain published dnspolicy shows correct status)

@maleck13 maleck13 added this pull request to the merge queue Nov 26, 2024
Merged via the queue into main with commit 57ea203 Nov 26, 2024
15 checks passed
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.

2 participants