OCPBUGS-115005: add polling timeout for Azure NIC updates

[Ultimate-etamitlU]

Summary

• Add a 2-minute polling timeout (defaultAzurePollingTimeout) to waitForCompletion() for Azure NIC update operations
• Previously used context.TODO() with no timeout, allowing a single stalled operation to hold the per-node mutex indefinitely
• On clusters with many EgressIPs per node (e.g. 250+), this causes cascading delays of 10+ minutes during upgrades when IPs are reassigned

Details

The Azure SDK's PollUntilDone has no built-in timeout — it polls at a default 30s interval until a terminal state or context cancellation. The 2-minute timeout was validated against these SDK defaults: it allows ~4 poll cycles, which is generous for a single IP add/remove operation. This aligns with the AWS provider which uses a 1-minute timeout via PollUntilContextTimeout.

The timeout bounds how long the per-node mutex is held per operation. If the operation hasn't completed within 2 minutes, the controller returns an error and can retry, releasing the mutex for other queued IP operations on the same node.

Test plan

• Unit tests pass (go test ./...)
• No new unit tests added — waitForCompletion takes a real Azure SDK runtime.Poller and no Azure mock infrastructure exists in the repo. The change adds a stdlib context.WithTimeout to an existing call path.
• Validated timeout value against Azure SDK source: default poll frequency is 30s, 2 min allows ~4 cycles
• E2E validation requires ARO cluster with 250+ EgressIPs (customer environment)

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

[openshift-ci-robot]

@Ultimate-etamitlU: This pull request references Jira Issue OCPBUGS-115005, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (5.1.0) matches configured target version for branch (5.1.0)
• bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

No GitHub users were found matching the public email listed for the QA contact in Jira (core-networking-bot@redhat.com), skipping review request.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Summary

• Add a 2-minute polling timeout (defaultAzurePollingTimeout) to waitForCompletion() for Azure NIC update operations
• Previously used context.TODO() with no timeout, allowing a single stalled operation to hold the per-node mutex indefinitely
• On clusters with many EgressIPs per node (e.g. 250+), this causes cascading delays of 10+ minutes during upgrades when IPs are reassigned

Details

The Azure SDK's PollUntilDone has no built-in timeout — it polls at a default 30s interval until a terminal state or context cancellation. The 2-minute timeout was validated against these SDK defaults: it allows ~4 poll cycles, which is generous for a single IP add/remove operation. This aligns with the AWS provider which uses a 1-minute timeout via PollUntilContextTimeout.

The timeout bounds how long the per-node mutex is held per operation. If the operation hasn't completed within 2 minutes, the controller returns an error and can retry, releasing the mutex for other queued IP operations on the same node.

Test plan

• Unit tests pass (go test ./...)
• No new unit tests added — waitForCompletion takes a real Azure SDK runtime.Poller and no Azure mock infrastructure exists in the repo. The change adds a stdlib context.WithTimeout to an existing call path.
• Validated timeout value against Azure SDK source: default poll frequency is 30s, 2 min allows ~4 cycles
• E2E validation requires ARO cluster with 250+ EgressIPs (customer environment)

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.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Team

Run ID: 9f6cec61-50ff-42f7-b2db-9fee75e5543c

📥 Commits

Reviewing files that changed from the base of the PR and between 82bed43 and dea807e.

📒 Files selected for processing (1)

• pkg/cloudprovider/azure.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

Summary by CodeRabbit

• Bug Fixes
  • Added a two-minute timeout when waiting for Azure network interface updates.
  • Improved error reporting when updates fail or exceed the timeout.

Walkthrough

Azure NIC update polling now stops after two minutes. waitForCompletion returns contextual errors when polling fails or times out.

Changes

Azure NIC polling

Layer / File(s)	Summary
Bounded polling and error handling pkg/cloudprovider/azure.go	The code defines a two-minute polling timeout. waitForCompletion uses the timeout with PollUntilDone and wraps polling errors with context.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to dea80

The PR adds a bounded timeout to Azure NIC update polling without any identified merge-blocking risk; it is merge-ready after normal checks and review.

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: adding a polling timeout for Azure NIC updates. The Jira issue reference is relevant.
Description check	✅ Passed	The description accurately explains the timeout change, its purpose, expected behavior, validation, and testing status. It is directly related to the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	The pull request changes only pkg/cloudprovider/azure.go. The diff adds a polling timeout and error wrapping; it does not add or modify Ginkgo test titles. No dynamic test-name value or overly speci…
Test Structure And Quality	✅ Passed	PASS: The pull request changes only pkg/cloudprovider/azure.go; it adds no Ginkgo or other test code. The repository has no tests that exercise waitForCompletion, and the custom check applies to G…
Microshift Test Compatibility	✅ Passed	PASS: The pull request changes only pkg/cloudprovider/azure.go. The committed diff adds a timeout and error wrapping in waitForCompletion; it adds or modifies no *_test.go files and introduces n…
Single Node Openshift (Sno) Test Compatibility	✅ Passed	PASS: The pull request changes only pkg/cloudprovider/azure.go. The parent-to-HEAD diff adds a polling timeout and error wrapping in waitForCompletion; it adds no Ginkgo e2e tests or other test co…
Topology-Aware Scheduling Compatibility	✅ Passed	PASS: The pull request changes only pkg/cloudprovider/azure.go. It adds a timeout to Azure NIC polling and wraps polling errors. It does not modify deployment manifests, replica settings, affinity, …
Ote Binary Stdout Contract	✅ Passed	PASS: The pull request changes only pkg/cloudprovider/azure.go. The added code creates a context timeout and wraps an error. It adds no stdout write, logging setup, suite hook, main, init, or to…
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	PASS: The pull request changes only pkg/cloudprovider/azure.go (+17/-4) and adds no test files or Ginkgo constructs. Therefore, it introduces no IPv4 networking assumption or external connectivity r…
No-Weak-Crypto	✅ Passed	PASS: The pull request changes only pkg/cloudprovider/azure.go. The added code defines a timeout, creates a context, polls Azure, and wraps an error. It adds no MD5, SHA1, DES, RC4, 3DES, Blowfish, EC…
Container-Privileges	✅ Passed	PASS: The pull request changes only pkg/cloudprovider/azure.go. The diff adds a Go polling timeout and wrapped error handling. It does not add or change a container or Kubernetes manifest, privilege…
No-Sensitive-Data-In-Logs	✅ Passed	PASS: The pull request adds no logging statements and adds no sensitive values to error text. waitForCompletion only wraps the existing Azure SDK error with the static message `timed out or failed w…

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.

Full details: Stable And Deterministic Test Names

Explanation

The pull request changes only pkg/cloudprovider/azure.go. The diff adds a polling timeout and error wrapping; it does not add or modify Ginkgo test titles. No dynamic test-name value or overly specific test title was introduced.

Full details: Test Structure And Quality

Explanation

PASS: The pull request changes only pkg/cloudprovider/azure.go; it adds no Ginkgo or other test code. The repository has no tests that exercise waitForCompletion, and the custom check applies to Ginkgo test structure and quality. Therefore, no stated test-quality failure was introduced.

Full details: Microshift Test Compatibility

Explanation

PASS: The pull request changes only pkg/cloudprovider/azure.go. The committed diff adds a timeout and error wrapping in waitForCompletion; it adds or modifies no *_test.go files and introduces no Ginkgo e2e tests. The MicroShift compatibility check is therefore not applicable.

Full details: Single Node Openshift (Sno) Test Compatibility

Explanation

PASS: The pull request changes only pkg/cloudprovider/azure.go. The parent-to-HEAD diff adds a polling timeout and error wrapping in waitForCompletion; it adds no Ginkgo e2e tests or other test constructs. Therefore the SNO test compatibility check is not applicable.

Full details: Topology-Aware Scheduling Compatibility

Explanation

PASS: The pull request changes only pkg/cloudprovider/azure.go. It adds a timeout to Azure NIC polling and wraps polling errors. It does not modify deployment manifests, replica settings, affinity, topology spread, node selectors, tolerations, or PDBs. It introduces no topology-dependent scheduling constraint.

Full details: Ote Binary Stdout Contract

Explanation

PASS: The pull request changes only pkg/cloudprovider/azure.go. The added code creates a context timeout and wraps an error. It adds no stdout write, logging setup, suite hook, main, init, or top-level initializer. The process-level entry points are unchanged from the parent commit, so this pull request does not introduce an OTE binary stdout contract violation.

Full details: Ipv6 And Disconnected Network Test Compatibility

Explanation

PASS: The pull request changes only pkg/cloudprovider/azure.go (+17/-4) and adds no test files or Ginkgo constructs. Therefore, it introduces no IPv4 networking assumption or external connectivity requirement in a new Ginkgo e2e test.

Full details: No-Weak-Crypto

Explanation

PASS: The pull request changes only pkg/cloudprovider/azure.go. The added code defines a timeout, creates a context, polls Azure, and wraps an error. It adds no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom cryptography, or secret/token comparison.

Full details: Container-Privileges

Explanation

PASS: The pull request changes only pkg/cloudprovider/azure.go. The diff adds a Go polling timeout and wrapped error handling. It does not add or change a container or Kubernetes manifest, privilege flag, host namespace setting, SYS_ADMIN capability, root execution setting, or allowPrivilegeEscalation setting.

Full details: No-Sensitive-Data-In-Logs

Explanation

PASS: The pull request adds no logging statements and adds no sensitive values to error text. waitForCompletion only wraps the existing Azure SDK error with the static message timed out or failed waiting for Azure NIC update to complete; timeout errors contain no credentials or customer data. The controller already logged the returned error and stored its text in status before this change, so the pull request does not introduce a new sensitive-data logging path. Existing IP and node logging is unchanged.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Ultimate-etamitlU
Once this PR has been reviewed and has the lgtm label, please assign pperiyasamy for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-ci]

@Ultimate-etamitlU: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.