fix(e2e): reconcile IP connect on the existing shared bastion - #9165
Conversation
#9149 enabled IP connect on the shared bastion, but only in the creation path. ensureSharedBastion returns early when the bastion already exists, so the long-lived shared bastion in the e2e subscription kept enableIpConnect unset and every SSH attempt kept failing with: failed to start bastion tunnel: error creating tunnel: 403 bastionssh.go connects through .../bh-hostConnect/<ip>, Bastion's IP-based connect flow, which is rejected unless IP connect is on. Reconcile the flag on the existing bastion so the fix actually reaches the deployed resource. The update is a no-op once the flag is set. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1ab1da0c-5f90-4ace-a154-0f9e7dbbbad8
Windows Unit Test Results 3 files 12 suites 51s ⏱️ Results for commit af7d626. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Pull request overview
This PR fixes a reliability issue in the e2e shared infrastructure reconciliation: the shared Azure Bastion can predate the introduction of EnableIPConnect, and the existing “bastion already exists” fast-path previously returned without ever applying the newer required property—leading to persistent 403 failures when establishing SSH tunnels via Bastion’s IP-based connect flow.
Changes:
- Add a reconciliation step to enable
EnableIPConnecton an existing shared bastion before returning its DNS name. - Keep
EnableIPConnect: truein the bastion creation properties, with clarifying comments tying it to thebh-hostConnect/<ip>flow used bybastionssh.go.
| func ensureBastionIPConnect(ctx context.Context, rg string, bastion armnetwork.BastionHost) error { | ||
| if bastion.Properties.EnableIPConnect != nil && *bastion.Properties.EnableIPConnect { | ||
| return nil | ||
| } | ||
| toolkit.Logf(ctx, "enabling IP connect on existing shared bastion %s", SharedBastionName) | ||
| bastion.Properties.EnableIPConnect = to.Ptr(true) | ||
| poller, err := config.Azure.BastionHosts.BeginCreateOrUpdate(ctx, rg, SharedBastionName, bastion, nil) | ||
| if err != nil { | ||
| return fmt.Errorf("enabling bastion IP connect: %w", err) | ||
| } | ||
| if _, err := poller.PollUntilDone(ctx, config.DefaultPollUntilDoneOptions); err != nil { | ||
| return fmt.Errorf("waiting for bastion IP connect: %w", err) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func ensureSharedBastion(ctx context.Context, rg, location string) (string, error) { | ||
| existing, err := config.Azure.BastionHosts.Get(ctx, rg, SharedBastionName, nil) | ||
| if err == nil { | ||
| if existing.Properties == nil || existing.Properties.DNSName == nil { | ||
| return "", fmt.Errorf("shared bastion %s exists but has no DNS name", SharedBastionName) | ||
| } | ||
| if err := ensureBastionIPConnect(ctx, rg, existing.BastionHost); err != nil { |
| // ensureBastionIPConnect turns on IP connect for a bastion that predates it being set at | ||
| // creation time. Without it the /api/tokens call in bastionssh.go returns 403 forever, since | ||
| // the bastion is only created once and is otherwise never updated. |
|
Confirmed working: GPU E2E build 175689787 on this branch shows 0 bastion 403s (baseline 175585765 on Note this unmasks a pre-existing failure — all 12 GPU scenario attempts now fail on |
|
Updated the branch onto The three red gates were all from build set 175689775 / 175689787 / 175689780 on 2026-08-07, which predates #9150 (merged 2026-08-11). Per my earlier note, the remaining GPU failures on that run were the The other two gates on that run were unrelated infra flake, for the record:
This PR itself remains validated: 0 bastion 403s on this branch vs 252 on the |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (2)
e2e/shared_infra.go:301
- The error context here is a bit too generic; if enabling IP connect fails, it will be hard to tell which bastion/RG was being updated from CI logs. Consider including the bastion name and resource group in both error messages to make failures actionable.
if err != nil {
return fmt.Errorf("enabling bastion IP connect: %w", err)
}
if _, err := poller.PollUntilDone(ctx, config.DefaultPollUntilDoneOptions); err != nil {
return fmt.Errorf("waiting for bastion IP connect: %w", err)
e2e/shared_infra.go:296
- This helper accepts a BastionHost argument but hardcodes SharedBastionName for logging and the CreateOrUpdate call. That makes the function name/signature misleading and easy to accidentally reuse incorrectly later. Consider either (a) renaming it to ensureSharedBastionIPConnect to reflect the hardcoded behavior, or (b) passing bastionName as a parameter and using that instead of SharedBastionName.
func ensureBastionIPConnect(ctx context.Context, rg string, bastion armnetwork.BastionHost) error {
if bastion.Properties.EnableIPConnect != nil && *bastion.Properties.EnableIPConnect {
return nil
}
toolkit.Logf(ctx, "enabling IP connect on existing shared bastion %s", SharedBastionName)
bastion.Properties.EnableIPConnect = to.Ptr(true)
poller, err := config.Azure.BastionHosts.BeginCreateOrUpdate(ctx, rg, SharedBastionName, bastion, nil)
d8d2250
into
main
What
ensureSharedBastionreconcilesenableIpConnecton the existing shared bastion, not just on the one it creates.Why
#9149 enabled IP connect, but only in the creation path.
ensureSharedBastionreturns early when the bastion already exists:The shared bastion in the e2e subscription was created before #9149, so it kept
enableIpConnectunset and #9149 never reached it. Confirmed on the live resource:Every SSH attempt therefore failed, 5/5 retries, on every scenario:
bastionssh.gobuilds.../providers/Microsoft.Network/bh-hostConnect/<ip>, which is Bastion's IP-based connect flow. Bastion rejects it with 403 unless IP connect is on. Bastion itself was healthy the whole time — the resource-based flow (az network bastion tunnel --target-resource-id ...) connected fine.Verification
I set the flag out of band on the shared bastion and re-ran
Test_Ubuntu2204_GPUA100locally. SSH succeeded on attempt 1/5 instead of failing 5/5, and validation proceeded through the kubelet, NIC, kernel and waagent validators:This change makes that self-healing so the fix survives without manual intervention. The update is a no-op once the flag is set, so it costs nothing on subsequent runs.