OCPBUGS-100184: Wait for Azure ignition DNS before creating workers - #9172
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@stbenjam: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Skipping CI for Draft Pull Request. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe NodePool reconciler now checks Azure Ignition external DNS resolution before Ignition CA secret validation. The resolver can be injected for tests. Resolution failures set Sequence Diagram(s)sequenceDiagram
participant NodePoolReconciler
participant ignitionEndpointAvailableCondition
participant DNSResolver
NodePoolReconciler->>ignitionEndpointAvailableCondition: evaluate endpoint availability
ignitionEndpointAvailableCondition->>DNSResolver: resolve Azure external DNS hostname
DNSResolver-->>ignitionEndpointAvailableCondition: resolution result
ignitionEndpointAvailableCondition-->>NodePoolReconciler: condition status and optional 10-second requeue
Suggested reviewers: 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9172 +/- ##
==========================================
+ Coverage 44.90% 44.91% +0.01%
==========================================
Files 775 775
Lines 97267 97285 +18
==========================================
+ Hits 43673 43696 +23
+ Misses 50581 50573 -8
- Partials 3013 3016 +3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
@stbenjam: This pull request references Jira Issue OCPBUGS-100184, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Prevent Azure machines from starting their provisioning deadline before the public ignition hostname resolves. Surface DNS readiness on the NodePool and retry automatically. Keep failure details in NodePool status without logging the customer hostname. Signed-off-by: Stephen Benjamin <stephen@redhat.com>
7c6daf4 to
21fc24b
Compare
|
/pipeline required |
|
Scheduling tests matching the |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@stbenjam: This pull request references Jira Issue OCPBUGS-100184, which is valid. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
TRT Review — Payload Infrastructure ResilienceAssessment: High value. This addresses the primary root cause of the AKS conformance failures identified by the payload agent analysis. Context from payload agent analysisThe payload agent classified the
This PR breaks the causal chain at the first link — the race between DNS propagation and VM provisioning. Code reviewThe implementation is well-scoped and follows HyperShift patterns:
CodeRabbit's sensitive-data flagCodeRabbit flagged the log line ImpactThis addresses the initiating cause in the payload agent's failure chain. Without this fix, Azure DNS throttling creates a race: VMs are provisioned before their ignition endpoint is resolvable, leading to Together, these two PRs form a defense-in-depth pair: this one prevents the bad state from occurring, and #199 makes recovery work when it does. Recommendation: Merge-worthy. Clear, well-tested fix for a documented Azure infrastructure timing issue. The HyperShift team should review the interaction with their reconciler requeue patterns, but the approach is sound. |
Test Resultse2e-aws
e2e-aks
|
|
/retest-required CI failures are Azure/AWS infrastructure issues (TLS handshake timeouts, EOF, DNS |
|
/retest-required |
Question: Should the DNS gate narrow to ARO HCP instead of all Azure?A question came up during review — this PR gates on How the gate scopes todayThe check calls
Effective behavior:
SuggestionThere's an existing if netutil.IsAroHCPByHC(hcluster) {Arguments for narrowing:
Arguments for keeping
Bottom lineThe Posted on behalf of a reviewer from the #forum-ocp-hypershift discussion. |
|
Thanks for the thorough analysis — great question. I think keeping the broader
So self-managed Azure clusters without external DNS are already completely unaffected — the only clusters that hit this code path are ones that have explicitly configured an external DNS hostname for ignition, on a public Azure topology. The root cause is Azure DNS API throttling (429s on the Azure DNS API), which is not ARO HCP-specific — it affects any Azure DNS zone. A self-managed Azure cluster that configures external DNS for ignition would be equally vulnerable to this race, so the broader scope provides defense-in-depth at no cost to the unaffected path. Happy to add an inline comment explaining this rationale if that would be helpful. |
|
Added an inline comment explaining the broader |
Address review: document why the DNS readiness gate uses AzurePlatform rather than narrowing to ARO HCP. Azure DNS API throttling (429s) affects all Azure DNS zones, and ServiceExternalDNSHostnameByHC already scopes the check to public clusters with an explicit external DNS hostname. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
bc0901e to
9772cd1
Compare
…pools-on-ignition-dns
Signed-off-by: Stephen Benjamin <stephen@redhat.com>
|
Addressed the Test Structure And Quality warning in c201e41 by adding a diagnostic message to the ignitionEndpointAvailableCondition error assertion. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox, stbenjam The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
|
Scheduling tests matching the |
|
/verified by CI |
|
@stbenjam: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/retest |
|
/retest-required |
|
/test e2e-kubevirt-aws-ovn-reduced |
|
/override "ci/prow/e2e-kubevirt-aws-ovn-reduced" This was failing due to an outside reason outside this PR |
|
@bryan-cox: Overrode contexts on behalf of bryan-cox: ci/prow/e2e-kubevirt-aws-ovn-reduced DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@stbenjam: Jira Issue Verification Checks: Jira Issue OCPBUGS-100184 Jira Issue OCPBUGS-100184 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@stbenjam: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Fix included in release 5.0.0-0.nightly-2026-08-05-033721 |
These bugs and fixes were automatically generated by a payload-agent experiment to improve resilience and diagnostics for infrastructure failures. Please review the PR and either shepherd it to merge or close it. If the work is incorrect or unhelpful, a brief comment would help us improve. Thanks, and apologies if we missed the mark.
Bug: OCPBUGS-100184
What this PR does / why we need it:
On public Azure HostedClusters with an explicit ignition DNS hostname, wait for that hostname to resolve before continuing NodePool reconciliation into CAPI machine creation.
Azure starts a finite VM provisioning deadline when the machine is created. In this AKS conformance run, Azure DNS API throttling delayed the ignition A record until after that deadline, leading to
OSProvisioningTimedOut. The NodePool now reportsIgnitionEndpointAvailable=Falsewith reasonExternalDNSHostNotReachable, requeues after ten seconds, and automatically proceeds when DNS becomes available.The gate is limited to Azure and to explicit public external-DNS ignition hostnames. Private Azure and other platform paths are unchanged.
Which issue(s) this PR fixes:
Fixes: OCPBUGS-100184
Special notes for your reviewer:
The resolver is injected only to keep the condition logic deterministic in unit tests. Tests cover unresolvable and resolvable Azure hostnames, private Azure, non-Azure, missing external DNS configuration, and a missing ignition endpoint.
Checklist:
Summary by CodeRabbit