Add LB tagged IP dual-frontend support with portal NSG and port binding fixes by preetisht · Pull Request #4890 · Azure/ARO-RP

Preetish Tripathi (preetisht) · 2026-06-15T09:29:37Z

Which issue this PR addresses:

Fixes https://redhat.atlassian.net/browse/ARO-22460

What this PR does / why we need it:

Adds dual-frontend Load Balancer support with FirstPartyUsage tagged IPs for RP and Portal services:

Adds tagged public IPs (rp-pip-tagged, portal-pip-tagged) with LB frontends and rules
Adds portal_in_tagged NSG rule (priority 143, port 8444) to allow laptop access to the tagged portal path
Fixes container port bindings: moves -p 8444:8444 from aro-rp to aro-portal, fixes SSH mapping -p 2223:2222
Adds VMSS public IP tagging for RP and Gateway (FirstPartyUsage on rp-vmss-pip, gateway-vmss-pip)
Adds orphaned tagged PIP cleanup logic during RP deployment
Adds forceNSGs predeploy mode to deploy NSG changes without triggering secret rotation/VMSS restart

Test plan for issue:

Unit tests pass (make unit-test-go)
Deployed to INT environment with tag v20260604.02-lb-tagged-ip-dual-frontend
LB health probes healthy for both tagged and untagged paths
Stage/Canary release and test is still pending

How do you know this will function as expected in production?

Successfully deployed and validated in INT environment. Dual-frontend design preserves untagged path as fallback — DNS cutover to tagged IPs is controlled separately via RP-Config.

Made with Cursor

Preetish Tripathi (preetisht) · 2026-06-15T09:36:13Z

I have createed this pull request so that folks can go through the implementation. Post Stage/Canary release and test, I will close this and create a new one . Also, this was created to check what co-pilot has to say .

Copilot

Pull request overview

This PR extends the RP load balancer and VMSS networking to support “tagged IP” scenarios (dual frontends + FirstPartyUsage tagging), adds related NSG/firewall/container port updates, and introduces deployment-time cleanup/controls intended to reduce disruption.

Changes:

Add dual-frontend LB support with new tagged public IPs and additional probes/rules/NSG allowances for tagged ingress paths.
Add conditional IP tag injection for RP/Gateway VMSS per-instance public IPs via template fixup + new config parameters.
Add cleanup for orphaned tagged LB PIPs and introduce a forceNSGs predeploy behavior (intended to deploy NSG changes without secret rotation/VMSS restart).

Reviewed changes

Copilot reviewed 21 out of 23 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
pkg/deploy/generator/templates.go	Adds regex/template fixups for LB/VMSS IP tags and NSG portal source prefixes.
pkg/deploy/deploy.go	Introduces `forceNSGs` deployer field (currently defaulted on).
pkg/deploy/predeploy.go	Adds `forceNSGs` behavior to force NSG deployment and skip secret config/restarts.
pkg/deploy/deploy_rp.go	Adds tagged-PIP cleanup + switches DNS resolution to tagged PIPs + passes VMSS tag params.
pkg/deploy/deploy_gateway.go	Passes gateway VMSS tag params.
pkg/deploy/config.go	Adds config fields for LB/VMSS IP tags and disabled-region lists + `IPTag` struct.
pkg/deploy/generator/resources.go	Adds helper for tagged LB public IP resources (with placeholder tags).
pkg/deploy/generator/resources_rp.go	Adds tagged LB frontends/rules/probes and NSG rules; adds VMSS PIP IPTags anchor.
pkg/deploy/generator/resources_gateway.go	Adds VMSS PIP IPTags anchor for gateway.
pkg/deploy/generator/templates_rp.go	Adds new parameters/resources for tagged LB PIPs + NSG rule resource.
pkg/deploy/generator/templates_gateway.go	Adds new parameters for gateway VMSS PIP tags.
pkg/deploy/generator/scripts/util-services.sh	Updates podman port mappings for additional tagged frontends and SSH mapping.
pkg/deploy/generator/scripts/rpVMSS.sh	Opens firewall ports for tagged frontends/SSH.
pkg/deploy/deploy_test.go	Adds unit tests for orphaned tagged PIP cleanup logic.
pkg/deploy/assets/*	Updates generated ARM templates/parameters for the above infrastructure changes.
docs/vmss-ip-tagged.md	Documents VMSS public IP tagging feature and rollout notes.
docs/load-balancer-tagged-ip.md	Documents LB tagged IP dual-frontend approach and rollout notes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

 		config:      config,
 		version:     version,
 		vmssCleaner: vmsscleaner.New(_env.LoggerForComponent("vmsscleaner"), vmssClient),
+		forceNSGs:   true,


+	if d.forceNSGs {
+		d.log.Info("forceNSGs set, skipping secret configuration and VMSS restart")
+		return nil


+	parameters.Parameters["rpVmssIpTags"] = &arm.ParametersParameter{
+		Value: d.config.Configuration.RPVmssIpTags,
+	}
+	parameters.Parameters["rpVmssIpTagsDisabledRegions"] = &arm.ParametersParameter{
+		Value: d.config.Configuration.RPVmssIpTagsDisabledRegions,
+	}


+	parameters.Parameters["gwyVmssIpTags"] = &arm.ParametersParameter{
+		Value: d.config.Configuration.GwyVmssIpTags,
+	}
+	parameters.Parameters["gwyVmssIpTagsDisabledRegions"] = &arm.ParametersParameter{
+		Value: d.config.Configuration.GwyVmssIpTagsDisabledRegions,
+	}


 func (d *deployer) configureDNS(ctx context.Context) error {
-	rpPIP, err := d.publicipaddresses.Get(ctx, d.config.RPResourceGroupName, "rp-pip", nil)
+	rpPIP, err := d.publicipaddresses.Get(ctx, d.config.RPResourceGroupName, "rp-pip-tagged", nil)
 	if err != nil {
 		return err
 	}


+			return err
+		}
+
+		if pip.Properties != nil && pip.Properties.IPConfiguration == nil {


+	RPLbIpTags                        []interface{}          `json:"rpLbIpTags,omitempty"`
+	PortalLbIpTags                    []interface{}          `json:"portalLbIpTags,omitempty"`
+	LbIpTagsDisabledRegions           []string               `json:"lbIpTagsDisabledRegions,omitempty"`


 	}
 }

+func (g *generator) publicLBIPAddressTagged(name string, ipTagsParam string) *arm.Resource {


+	// IP tags conditional logic: RP and Gateway VMSS each need different FirstPartyUsage tags for inbound vs outbound traffic
+	// We use the VMSS pip name as context to determine which parameter set to apply
+	// For RP VMSS (rp-vmss-pip) - applies inbound FirstPartyUsage tag
+	if bytes.Contains(b, []byte(`"name": "rp-vmss-pip"`)) {
+		b = bytes.ReplaceAll(b, []byte(`"ipTags": []`), []byte(`"ipTags": "[if(or(contains(parameters('rpVmssIpTagsDisabledRegions'), resourceGroup().location), equals(length(parameters('rpVmssIpTags')), 0)), createArray(), createArray(createObject('ipTagType', parameters('rpVmssIpTags')[0].type, 'tag', parameters('rpVmssIpTags')[0].value)))]"`))
+	}
+	// For Gateway VMSS (gateway-vmss-pip) - applies outbound FirstPartyUsage tag
+	if bytes.Contains(b, []byte(`"name": "gateway-vmss-pip"`)) {
+		b = bytes.ReplaceAll(b, []byte(`"ipTags": []`), []byte(`"ipTags": "[if(or(contains(parameters('gwyVmssIpTagsDisabledRegions'), resourceGroup().location), equals(length(parameters('gwyVmssIpTags')), 0)), createArray(), createArray(createObject('ipTagType', parameters('gwyVmssIpTags')[0].type, 'tag', parameters('gwyVmssIpTags')[0].value)))]"`))
+	}


github-actions · 2026-06-15T14:47:13Z

Please rebase pull request.

… support Introduces rp-pip-tagged and portal-pip-tagged public IPs with configurable IP tags, adds dual frontends and LB rules on ports 8443/8444, separate health probes, NSG rules for the new ports, and updates DNS to use the tagged IPs. Untagged IPs remain as a rollback path.

- Rename portal-probe-tagged to portal-probe-https-tagged for naming consistency (portal has both https and ssh probe types) - Add portal-lbrule-ssh-tagged: maps portal-frontend-tagged:22 to backend:2223 for SSH over the tagged IP path - Add portal-probe-ssh-tagged: TCP health probe on port 2223 - Open firewall port 2223/tcp on VMSS - Add -p 2223:2223 port mapping to the portal container - Update docs with motivation, container routing, cross-repo dependency, and port convention sections

Cherry-pick of core fix from hawkowl/ARO-25186 (commit 4b86d33). The lastSubscriptionChangefeed atomic.Value was never written to after the changefeed refactor (d345c3d, Feb 9), causing checkReady() to always return false and suppressing monitor.heartbeat emission. Replaces mon.lastSubscriptionChangefeed.Load() with mon.subs.GetLastProcessed() which reads the actual subscription changefeed timestamp. Also adds the two missing duration constants (defaultMonitorReadinessDelay, defaultChangefeedReadinessInterval) that the cherry-picked commit references.

Introduces configurable IP tags for VMSS per-instance public IPs on both the RP and Gateway scale sets. Tags are conditionally applied at ARM template generation time using a regex-based fixup in templates.go. - Add RPVmssIpTags, RPVmssIpTagsDisabledRegions, GwyVmssIpTags, and GwyVmssIpTagsDisabledRegions config fields with IPTag struct - Add empty IPTags placeholder to rp-vmss-pip and gateway-vmss-pip - Inject conditional ARM expressions to apply tags based on config - Pass config values as ARM parameters in deploy_rp.go and deploy_gateway.go - Support per-region disable list to skip tagging in specific regions

Document the VMSS IP tag feature covering motivation, architecture, configuration parameters, template fixup logic, rollback strategy, and comparison with the LB tagged IP migration.

- cleanupOrphanedTaggedPIPs in deploy_rp.go: deletes orphaned rp-pip-tagged and portal-pip-tagged before ARM deployment to avoid IPTagsCannotBeModified errors after rollback - Unit tests for cleanupOrphanedTaggedPIPs in deploy_test.go - forceNSGs in predeploy.go: forces NSG deployment and skips secret rotation / VMSS restart to prevent restartOldScalesets hangs during NSG-only updates Made-with: Cursor

- Move forceNSGs from hardcoded local variable to deployer struct field, set to true in New(). Tests default to false (zero value) so existing TestPreDeploy cases continue to exercise the normal code path. - Fix TestCleanupOrphanedTaggedPIPs wantErr: azcore.ResponseError without RawResponse produces "Missing RawResponse", not "GET ". Made-with: Cursor

Made-with: Cursor

…ndings The tagged Portal IP was unreachable from laptops because: 1. No NSG rule existed for port 8444 (tagged portal backend port) 2. Host port 8444 was bound to aro-rp container instead of aro-portal 3. SSH tagged mapping sent traffic to container port 2223 where nothing listens Fix by adding portal_in_tagged NSG rule (priority 143, port 8444, same source prefixes as portal_in), moving -p 8444:8444 from aro-rp to aro-portal, and correcting -p 2223:2223 to -p 2223:2222. Co-authored-by: Cursor <cursoragent@cursor.com>

Co-authored-by: Cursor <cursoragent@cursor.com>

The previous bytes.ReplaceAll approach for VMSS PIP ipTags injection replaced ALL "ipTags": [] occurrences in the template when a VMSS PIP name was detected. This clobbered LB PIP resources (rp-pip-tagged, portal-pip-tagged), causing them to incorrectly use rpVmssIpTags parameters instead of their own rpLbIpTags/portalLbIpTags parameters. Switch to targeted regex (same pattern used for LB PIPs) so each resource only has its own ipTags replaced within its JSON context. Co-authored-by: Cursor <cursoragent@cursor.com>

Replace the hardcoded `forceNSGs: true` with a config-driven approach using the new `ForceNSGsOnly` field. When `forceNSGsOnly: true` is set in the deploy config YAML, NSG deployment is forced while secret rotation and VMSS restart are skipped. When absent or false, normal deploy behavior applies. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Pull request overview

Copilot reviewed 21 out of 23 changed files in this pull request and generated 7 comments.

 	}
 }

+func (g *generator) publicLBIPAddressTagged(name string, ipTagsParam string) *arm.Resource {


 func (d *deployer) configureDNS(ctx context.Context) error {
-	rpPIP, err := d.publicipaddresses.Get(ctx, d.config.RPResourceGroupName, "rp-pip", nil)
+	rpPIP, err := d.publicipaddresses.Get(ctx, d.config.RPResourceGroupName, "rp-pip-tagged", nil)
 	if err != nil {
 		return err
 	}

-	portalPIP, err := d.publicipaddresses.Get(ctx, d.config.RPResourceGroupName, "portal-pip", nil)
+	portalPIP, err := d.publicipaddresses.Get(ctx, d.config.RPResourceGroupName, "portal-pip-tagged", nil)
 	if err != nil {


+Released to INT on **2026-02-17**. Deployment completed successfully.
+[INT Release Pipeline (v2)](https://msazure.visualstudio.com/AzureRedHatOpenShift/_build/results?buildId=153402072&view=results)
+
+Changes in this release: renamed `portal-probe-tagged` to `portal-probe-https-tagged`, added `portal-lbrule-ssh-tagged` (port 22 → 2223), `portal-probe-ssh-tagged` (TCP/2223), firewall port 2223, and portal container `-p 2223:2223`.


+### 4. VMSS and Firewall Changes
+
+- **Firewall ports opened:** `8443/tcp`, `8444/tcp`, and `2223/tcp` added to the RP VMSS firewall allow list.
+- **Container port mapping:** RP container gets `-p 8443:8443` and `-p 8444:8444` in addition to the existing `-p 443:8443`. Portal container gets `-p 2223:2223` in addition to the existing `-p 2222:2222`.


+| **aro-rp**       | `aro-rp.service`                 | `443:8443`, `8443:8443`, `8444:8444`       | RP API (ARM, Geneva)      |
+| **aro-portal**   | `aro-portal.service`             | `444:8444`, `2222:2222`, `2223:2223`       | Portal HTTPS and SSH      |


+| `rp_in_arm_tagged`     | 121      | 8443 | `AzureResourceManager` |
+| `rp_in_geneva_tagged`  | 131      | 8443 | `GenevaActions`        |
+
+Additionally, NSG deployment is now **forced on every predeploy** (not only on initial creation) to ensure new rules are always applied.


+| **Azure/ARO-RP** | ARM templates with the dual-frontend LB, tagged IP resources, probes, rules, NSG rules, firewall/container config, and DNS logic. |
+| **Azure/RP-Config** | Per-environment configuration values: the actual IP tag objects (`rpLbIpTags`, `portalLbIpTags`) and the disabled regions list (`lbIpTagsDisabledRegions`). Without these values, the tagged IPs are created but have no tags applied. |
+
+**Deployment order:** RP-Config should be deployed first (or simultaneously) so that the IP tag values are available when the ARO-RP ARM template is evaluated. If ARO-RP is deployed before RP-Config provides the tag values, the tagged IPs will be created without tags (safe, but defeats the purpose).


Remove the ForceNSGsOnly config-driven approach (Copilot #1/#2) and revert to hardcoded forceNSGs=true. Critically, remove the early return that skipped configureServiceSecrets when forceNSGs was set — this caused RP VMSS instances to never become healthy during deployment. Phase 1 of two-phase deploy: force NSG rules for port 8443 while allowing the normal secret configuration and VMSS health flow to proceed.

github-actions · 2026-07-15T09:37:05Z

Please rebase pull request.

openshift-ci · 2026-07-22T02:40:22Z

Preetish Tripathi (@preetisht): The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-images	`b7f949c`	link	true	`/test e2e-images`

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.

Copilot AI review requested due to automatic review settings June 15, 2026 09:29

Preetish Tripathi (preetisht) requested a review from Ben Vesel (bennerv) as a code owner June 15, 2026 09:29

Preetish Tripathi (preetisht) added the work-in-progress label Jun 15, 2026

Copilot started reviewing on behalf of Preetish Tripathi (preetisht) June 15, 2026 09:30 View session

Copilot AI reviewed Jun 15, 2026

View reviewed changes

github-actions Bot added the needs-rebase branch needs a rebase label Jun 15, 2026

Preetish Tripathi (preetisht) added 6 commits July 1, 2026 12:09

Fix missing regexp import in deploy generator templates

b2c9979

Add INT release pipeline link to LB tagged IP docs

c0da370

Add second INT release status to LB tagged IP docs

191fe2c

Preetish Tripathi (preetisht) and others added 13 commits July 1, 2026 12:09

Regenerate deploy assets after cherry-pick onto latest master

84966fe

Add documentation for VMSS public IP tagging

5dcd700

Document the VMSS IP tag feature covering motivation, architecture, configuration parameters, template fixup logic, rollback strategy, and comparison with the LB tagged IP migration.

Fix whitespace in templates_rp.go after make generate

9a56702

Update docs with correct commit ID and tag

276eeb5

Add INT release status to VMSS IP tagged docs

c0e701f

Regenerate deploy assets after VMSS IP tagging cherry-pick

f161d4c

Fix TestCleanupOrphanedTaggedPIPs: use plain error for non-404 case

664cd11

Made-with: Cursor

Regenerate deploy assets on latest master base

751d1e4

Co-authored-by: Cursor <cursoragent@cursor.com>

Regenerate deploy assets after rebase onto latest master

6a7c830

Co-authored-by: Cursor <cursoragent@cursor.com>

Preetish Tripathi (preetisht) force-pushed the preetisht/ARO-20087-04062026 branch from 9c68855 to 6a7c830 Compare July 1, 2026 06:55

github-actions Bot removed the needs-rebase branch needs a rebase label Jul 1, 2026

Preetish Tripathi (preetisht) and others added 2 commits July 2, 2026 12:27

Copilot AI review requested due to automatic review settings July 2, 2026 07:37

Copilot started reviewing on behalf of Preetish Tripathi (preetisht) July 2, 2026 07:37 View session

Copilot AI reviewed Jul 2, 2026

View reviewed changes

github-actions Bot added the needs-rebase branch needs a rebase label Jul 15, 2026

Navigation Menu

Uh oh!

Add LB tagged IP dual-frontend support with portal NSG and port binding fixes - #4890

Add LB tagged IP dual-frontend support with portal NSG and port binding fixes#4890
Preetish Tripathi (preetisht) wants to merge 22 commits into
masterfrom
preetisht/ARO-20087-04062026

Preetish Tripathi (preetisht) commented Jun 15, 2026 •

edited

Loading

Uh oh!

Preetish Tripathi (preetisht) commented Jun 15, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented Jun 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented Jul 15, 2026

Uh oh!

openshift-ci Bot commented Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		\| aro-rp \| `aro-rp.service` \| `443:8443`, `8443:8443`, `8444:8444` \| RP API (ARM, Geneva) \|
		\| aro-portal \| `aro-portal.service` \| `444:8444`, `2222:2222`, `2223:2223` \| Portal HTTPS and SSH \|

Uh oh!

Conversation

Preetish Tripathi (preetisht) commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue this PR addresses:

What this PR does / why we need it:

Test plan for issue:

How do you know this will function as expected in production?

Uh oh!

Preetish Tripathi (preetisht) commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

github-actions Bot commented Jun 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

github-actions Bot commented Jul 15, 2026

Uh oh!

openshift-ci Bot commented Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Preetish Tripathi (preetisht) commented Jun 15, 2026 •

edited

Loading

Preetish Tripathi (preetisht) commented Jun 15, 2026 •

edited

Loading