CNTRLPLANE-2511: refactor(cpo): move OAuth internal LB annotation into ReconcileService

[bryan-cox]

What this PR does / why we need it:

Addresses review feedback from @muraee on #8149:

1. Moves the Azure internal LoadBalancer annotation (service.beta.kubernetes.io/azure-load-balancer-internal) from infra.go into oauth.ReconcileService(), co-locating all OAuth service configuration in one place. This follows the same pattern used by kas.ReconcileService().

2. Reverts the unnecessary switch refactor on the strategy type check back to the original if serviceStrategy.Type != hyperv1.Route form, since the switch didn't change behavior.

3. Adds behavioral unit tests for the new isPrivate parameter:

   • Public Azure LB does NOT set the internal LB annotation (negative assertion)
   • Private Azure LB sets the internal LB annotation
   • Blank LoadBalancer ingress (neither hostname nor IP) returns appropriate message

Which issue(s) this PR fixes:

Fixes #8149 (review feedback)

Special notes for your reviewer:

Follow-up to #8149. The ReconcileService function gains an isPrivate bool parameter so the ILB annotation logic can live alongside all other service configuration rather than being split across infra.go and oauth/service.go.

Checklist:

• Subject and description added to both, commit and PR.
• Relevant issues have been referenced.
• This change includes docs.
• This change includes unit tests.

Summary by CodeRabbit

• Improvements
  • OAuth service reconciliation now correctly applies or removes Azure internal load balancer annotations based on private vs. public hosted control planes, and consistently sets hostname annotations when provided.
• Tests
  • Expanded OAuth service tests for Azure: private/public LB scenarios, removal of stale internal annotations, and additional service status edge cases.

[openshift-ci-robot]

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]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bryan-cox

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [bryan-cox]

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

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 76e7ba48-b1f8-4f83-b8b9-786acb17cb85

📥 Commits

Reviewing files that changed from the base of the PR and between f00054c and 5b79e4f.

📒 Files selected for processing (3)

• control-plane-operator/controllers/hostedcontrolplane/infra/infra.go
• control-plane-operator/controllers/hostedcontrolplane/oauth/service.go
• control-plane-operator/controllers/hostedcontrolplane/oauth/service_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• control-plane-operator/controllers/hostedcontrolplane/oauth/service.go

---

📝 Walkthrough

Walkthrough

The infra reconciler now passes a private-topology flag (util.IsPrivateHCP(hcp)) into oauth.ReconcileService. ReconcileService gained an isPrivate bool parameter and initializes the service annotations map, always sets the external DNS hostname annotation when provided, and sets or removes the Azure internal load balancer annotation based on the isPrivate flag. The infra reconciler's post-reconcile flow was simplified to early-return unless the publishing strategy is Route. Unit tests were extended to cover private/public Azure LoadBalancer annotation presence/removal and refined service-status assertions.

Sequence Diagram(s)

(Section intentionally omitted.)

Assessment against linked issues

Objective (issue refs)	Addressed	Explanation
Annotate OAuth LB service with Azure internal load balancer annotation for private topology (CNTRLPLANE-2511)	✅
Add LB code path in reconcileOAuthServerService (skip Route reconciliation for LB) (CNTRLPLANE-2511)	✅
Add LoadBalancer case to ReconcileService and ReconcileServiceStatus (CNTRLPLANE-2511)	❓	ReconcileService signature and LB annotation handling were added; diff does not show a corresponding implementation change to ReconcileServiceStatus (tests updated but no explicit status function change visible).
PLS Observer / PLS Controller / HO Azure controller / CLI changes (CNTRLPLANE-2511)	❌	No observer/controller/HO/CLI files or changes for PLS naming, observer registration, or CLI flags are present in this diff.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[openshift-ci-robot]

@bryan-cox: This pull request references CNTRLPLANE-2511 which is a valid jira issue.

Details

In response to this:

What this PR does / why we need it:

Addresses review feedback from @muraee on #8149:

1. Moves the Azure internal LoadBalancer annotation (service.beta.kubernetes.io/azure-load-balancer-internal) from infra.go into oauth.ReconcileService(), co-locating all OAuth service configuration in one place. This follows the same pattern used by kas.ReconcileService().

2. Reverts the unnecessary switch refactor on the strategy type check back to the original if serviceStrategy.Type != hyperv1.Route form, since the switch didn't change behavior.

3. Adds behavioral unit tests for the new isPrivate parameter:

• Public Azure LB does NOT set the internal LB annotation (negative assertion)
• Private Azure LB sets the internal LB annotation
• Blank LoadBalancer ingress (neither hostname nor IP) returns appropriate message

Which issue(s) this PR fixes:

Fixes CNTRLPLANE-2511

Special notes for your reviewer:

Follow-up to #8149. The ReconcileService function gains an isPrivate bool parameter so the ILB annotation logic can live alongside all other service configuration rather than being split across infra.go and oauth/service.go.

Checklist:

• Subject and description added to both, commit and PR.
• Relevant issues have been referenced.
• This change includes docs.
• This change includes unit tests.

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.

[openshift-ci-robot]

@bryan-cox: This pull request references CNTRLPLANE-2511 which is a valid jira issue.

Details

In response to this:

What this PR does / why we need it:

Addresses review feedback from @muraee on #8149:

1. Moves the Azure internal LoadBalancer annotation (service.beta.kubernetes.io/azure-load-balancer-internal) from infra.go into oauth.ReconcileService(), co-locating all OAuth service configuration in one place. This follows the same pattern used by kas.ReconcileService().

2. Reverts the unnecessary switch refactor on the strategy type check back to the original if serviceStrategy.Type != hyperv1.Route form, since the switch didn't change behavior.

3. Adds behavioral unit tests for the new isPrivate parameter:

• Public Azure LB does NOT set the internal LB annotation (negative assertion)
• Private Azure LB sets the internal LB annotation
• Blank LoadBalancer ingress (neither hostname nor IP) returns appropriate message

Which issue(s) this PR fixes:

Fixes #8149 (review feedback)

Special notes for your reviewer:

Follow-up to #8149. The ReconcileService function gains an isPrivate bool parameter so the ILB annotation logic can live alongside all other service configuration rather than being split across infra.go and oauth/service.go.

Checklist:

• Subject and description added to both, commit and PR.
• Relevant issues have been referenced.
• This change includes docs.
• This change includes unit tests.

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.

[openshift-ci-robot]

@bryan-cox: This pull request references CNTRLPLANE-2511 which is a valid jira issue.

Details

In response to this:

What this PR does / why we need it:

Addresses review feedback from @muraee on #8149:

1. Moves the Azure internal LoadBalancer annotation (service.beta.kubernetes.io/azure-load-balancer-internal) from infra.go into oauth.ReconcileService(), co-locating all OAuth service configuration in one place. This follows the same pattern used by kas.ReconcileService().

2. Reverts the unnecessary switch refactor on the strategy type check back to the original if serviceStrategy.Type != hyperv1.Route form, since the switch didn't change behavior.

3. Adds behavioral unit tests for the new isPrivate parameter:

• Public Azure LB does NOT set the internal LB annotation (negative assertion)
• Private Azure LB sets the internal LB annotation
• Blank LoadBalancer ingress (neither hostname nor IP) returns appropriate message

Which issue(s) this PR fixes:

Fixes #8149 (review feedback)

Special notes for your reviewer:

Follow-up to #8149. The ReconcileService function gains an isPrivate bool parameter so the ILB annotation logic can live alongside all other service configuration rather than being split across infra.go and oauth/service.go.

Checklist:

• Subject and description added to both, commit and PR.
• Relevant issues have been referenced.
• This change includes docs.
• This change includes unit tests.

Summary by CodeRabbit

Release Notes

• Improvements

• Corrected OAuth service configuration for private Azure hosted control planes to properly apply internal load balancer settings.

• Tests

• Enhanced test coverage for private Azure OAuth service scenarios, including validation of internal load balancer annotations and edge cases in service status handling.

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]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@control-plane-operator/controllers/hostedcontrolplane/oauth/service.go`:
- Around line 74-76: The code only sets
svc.Annotations[azureutil.InternalLoadBalancerAnnotation] when isPrivate is
true, so if a Service was previously private the annotation remains when
reconciling to public; update the reconcile logic in service.go (the block
around svc and isPrivate) to remove/delete
svc.Annotations[azureutil.InternalLoadBalancerAnnotation] when isPrivate is
false (i.e., clear the key instead of leaving it), ensuring the
azureutil.InternalLoadBalancerAnnotation is removed from svc.Annotations; also
add a unit/regression test that seeds a Service with the
azureutil.InternalLoadBalancerAnnotation and then calls the reconcile path with
isPrivate=false and asserts the annotation is absent afterward.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 64c50a63-0046-43d9-a251-f0f035a4bcd5

📥 Commits

Reviewing files that changed from the base of the PR and between c674483 and 2e7ba2e.

📒 Files selected for processing (3)

• control-plane-operator/controllers/hostedcontrolplane/infra/infra.go
• control-plane-operator/controllers/hostedcontrolplane/oauth/service.go
• control-plane-operator/controllers/hostedcontrolplane/oauth/service_test.go

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 33.12%. Comparing base (c674483) to head (5b79e4f).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #8185   +/-   ##
=======================================
  Coverage   33.11%   33.12%           
=======================================
  Files         768      768           
  Lines       93116    93111    -5     
=======================================
+ Hits        30840    30841    +1     
+ Misses      59665    59660    -5     
+ Partials     2611     2610    -1     

Files with missing lines	Coverage Δ
...ator/controllers/hostedcontrolplane/infra/infra.go	50.54% <100.00%> (-0.61%)	⬇️
...or/controllers/hostedcontrolplane/oauth/service.go	74.74% <100.00%> (+5.93%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[openshift-ci-robot]

@bryan-cox: This pull request references CNTRLPLANE-2511 which is a valid jira issue.

Details

In response to this:

What this PR does / why we need it:

Addresses review feedback from @muraee on #8149:

1. Moves the Azure internal LoadBalancer annotation (service.beta.kubernetes.io/azure-load-balancer-internal) from infra.go into oauth.ReconcileService(), co-locating all OAuth service configuration in one place. This follows the same pattern used by kas.ReconcileService().

2. Reverts the unnecessary switch refactor on the strategy type check back to the original if serviceStrategy.Type != hyperv1.Route form, since the switch didn't change behavior.

3. Adds behavioral unit tests for the new isPrivate parameter:

• Public Azure LB does NOT set the internal LB annotation (negative assertion)
• Private Azure LB sets the internal LB annotation
• Blank LoadBalancer ingress (neither hostname nor IP) returns appropriate message

Which issue(s) this PR fixes:

Fixes #8149 (review feedback)

Special notes for your reviewer:

Follow-up to #8149. The ReconcileService function gains an isPrivate bool parameter so the ILB annotation logic can live alongside all other service configuration rather than being split across infra.go and oauth/service.go.

Checklist:

• Subject and description added to both, commit and PR.
• Relevant issues have been referenced.
• This change includes docs.
• This change includes unit tests.

Summary by CodeRabbit

• Improvements
• OAuth service reconciliation now correctly applies or removes internal load balancer annotations for private vs public Azure hosted control planes and ensures hostname annotations are set when provided.
• Tests
• Expanded tests for Azure OAuth service: private/public LB scenarios, removal of stale internal annotations, and additional service status edge cases.

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.

[openshift-ci-robot]

@bryan-cox: This pull request references CNTRLPLANE-2511 which is a valid jira issue.

Details

In response to this:

What this PR does / why we need it:

Addresses review feedback from @muraee on #8149:

1. Moves the Azure internal LoadBalancer annotation (service.beta.kubernetes.io/azure-load-balancer-internal) from infra.go into oauth.ReconcileService(), co-locating all OAuth service configuration in one place. This follows the same pattern used by kas.ReconcileService().

2. Reverts the unnecessary switch refactor on the strategy type check back to the original if serviceStrategy.Type != hyperv1.Route form, since the switch didn't change behavior.

3. Adds behavioral unit tests for the new isPrivate parameter:

• Public Azure LB does NOT set the internal LB annotation (negative assertion)
• Private Azure LB sets the internal LB annotation
• Blank LoadBalancer ingress (neither hostname nor IP) returns appropriate message

Which issue(s) this PR fixes:

Fixes #8149 (review feedback)

Special notes for your reviewer:

Follow-up to #8149. The ReconcileService function gains an isPrivate bool parameter so the ILB annotation logic can live alongside all other service configuration rather than being split across infra.go and oauth/service.go.

Checklist:

• Subject and description added to both, commit and PR.
• Relevant issues have been referenced.
• This change includes docs.
• This change includes unit tests.

Summary by CodeRabbit

• Improvements
• OAuth service reconciliation now correctly applies or removes Azure internal load balancer annotations based on private vs. public hosted control planes, and consistently sets hostname annotations when provided.
• Tests
• Expanded OAuth service tests for Azure: private/public LB scenarios, removal of stale internal annotations, and additional service status edge cases.

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.

[muraee]

/lgtm

[openshift-ci-robot]

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-azure-self-managed
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws

[bryan-cox]

/verified later @xiuwang

This can be verified with the previous PR for CNTRLPLANE-2511

[openshift-ci-robot]

@bryan-cox: This PR has been marked to be verified later by @xiuwang.

Details

In response to this:

/verified later @xiuwang

This can be verified with the previous PR for CNTRLPLANE-2511

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.

[cwbotbot]

Test Results

e2e-aws

• Status: ✅ PASS
• Started: 2026-04-08T20:28:45Z
• View Job
• View Job History

e2e-aks

• Status: ✅ PASS
• Started: 2026-04-08T19:12:54Z
• View Job
• View Job History

[openshift-merge-bot]

/retest-required

Remaining retests: 0 against base HEAD c674483 and 2 for PR HEAD 5b79e4f in total

[bryan-cox]

/retest

[openshift-ci]

@bryan-cox: 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.