Fetch GlobalIngressIP from API server to avoid stale cache

[tpantelis]

When the service controller re-queues a GlobalIngressIP immediately after initial allocation, the syncer's cache may not yet reflect the updated status with the allocated IP. This causes the controller to attempt re-allocation instead of updating the internal service.

Fix by always fetching the latest GlobalIngressIP from the API server at the start of the Create operation, before trimming status conditions or processing. This ensures we have fresh data even when the cache is stale.

Fixes: #4122

Summary by CodeRabbit

• Bug Fixes
  • Improved GlobalIngressIP processing by retrieving the latest resource state before handling status updates.
  • Added automatic retry handling when the latest resource cannot be retrieved or converted.

[submariner-bot]

🤖 Created branch: z_pr4123/tpantelis/gip-allocate-ips-race
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

[coderabbitai]

Walkthrough

The controller now stores a GlobalIngressIP resource client, refreshes the resource from the API server during create processing, and requeues when retrieval or conversion fails.

Changes

GlobalIngressIP refresh

Layer / File(s)	Summary
Resource client wiring pkg/globalnet/controllers/types.go, pkg/globalnet/controllers/global_ingressip_controller.go	The controller stores the globalIngressIPs resource interface for API-server reads.
Create refresh flow pkg/globalnet/controllers/global_ingressip_controller.go	Create processing refreshes the namespaced GlobalIngressIP before status handling and creation logic. Retrieval or conversion errors are logged and requeued.

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

Sequence Diagram(s)

sequenceDiagram
  participant CreateProcessing
  participant globalIngressIPController
  participant KubernetesAPIServer
  CreateProcessing->>globalIngressIPController: Refresh GlobalIngressIP
  globalIngressIPController->>KubernetesAPIServer: Retrieve namespaced resource
  KubernetesAPIServer-->>globalIngressIPController: Return current GlobalIngressIP
  globalIngressIPController-->>CreateProcessing: Return converted resource or error
  CreateProcessing->>CreateProcessing: Requeue on refresh error

Loading

Suggested labels: ready-to-test

Suggested reviewers: yboaron, skitt, oats87

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: fetching the latest GlobalIngressIP from the API server to avoid stale cache data.
Linked Issues check	✅ Passed	The controller fetches the latest GlobalIngressIP before Create processing, directly addressing stale-cache races and duplicate allocations in issue #4122.
Out of Scope Changes check	✅ Passed	The changes remain focused on GlobalIngressIP retrieval and controller state needed to prevent stale-cache allocation behavior.
Actionable Comments Resolved	✅ Passed	The provided PR context contains no actionable review comments; the changed files add only explanatory comments and no TODO, FIXME, or review-action markers.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

pkg/globalnet/controllers/global_ingressip_controller.go (1)

171-178: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy lift

Add a regression test for the stale-cache Create path.

Use a queued GlobalIngressIP with an empty Status.AllocatedIP while the API-server object already has an allocated IP. Assert that the controller reuses the live IP, updates the internal Service, and does not allocate a second IP. Also verify that refresh failures requeue the operation.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/globalnet/controllers/global_ingressip_controller.go` around lines 171 -
178, Add regression coverage for the GlobalIngressIP create/reconcile path
around fetchLatestFromAPIServer: queue a stale object with empty
Status.AllocatedIP while the API-server version has an allocated IP, then assert
the live IP is reused, the internal Service is updated, and no second allocation
occurs. Add a refresh-failure case asserting the operation is requeued.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@pkg/globalnet/controllers/global_ingressip_controller.go`:
- Around line 171-178: Add regression coverage for the GlobalIngressIP
create/reconcile path around fetchLatestFromAPIServer: queue a stale object with
empty Status.AllocatedIP while the API-server version has an allocated IP, then
assert the live IP is reused, the internal Service is updated, and no second
allocation occurs. Add a refresh-failure case asserting the operation is
requeued.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1a88f302-c15a-48b5-9d96-a5e0955566eb

📥 Commits

Reviewing files that changed from the base of the PR and between 7cb77d4 and 37c5f1f.

📒 Files selected for processing (2)

• pkg/globalnet/controllers/global_ingressip_controller.go
• pkg/globalnet/controllers/types.go

[submariner-bot]

🤖 Closed branches: [z_pr4123/tpantelis/gip-allocate-ips-race]