Skip to content

fix(e2e): reconcile IP connect on the existing shared bastion - #9165

Open
Ganeshkumar Ashokavardhanan (ganeshkumarashok) wants to merge 1 commit into
mainfrom
ganesh/e2e-bastion-ip-connect
Open

fix(e2e): reconcile IP connect on the existing shared bastion#9165
Ganeshkumar Ashokavardhanan (ganeshkumarashok) wants to merge 1 commit into
mainfrom
ganesh/e2e-bastion-ip-connect

Conversation

@ganeshkumarashok

Copy link
Copy Markdown
Contributor

What

ensureSharedBastion reconciles enableIpConnect on the existing shared bastion, not just on the one it creates.

Why

#9149 enabled IP connect, but only in the creation path. ensureSharedBastion returns early when the bastion already exists:

existing, err := config.Azure.BastionHosts.Get(ctx, rg, SharedBastionName, nil)
if err == nil {
    ...
    return *existing.Properties.DNSName, nil   // <- creation properties never applied
}

The shared bastion in the e2e subscription was created before #9149, so it kept enableIpConnect unset and #9149 never reached it. Confirmed on the live resource:

$ az network bastion show -n abe2e-shared-bastion -g abe2e-westus2 \
    --query "{sku:sku.name,tunnel:enableTunneling,ipconnect:enableIpConnect}"
{ "sku": "Standard", "tunnel": true, "ipconnect": null }

Every SSH attempt therefore failed, 5/5 retries, on every scenario:

failed to start bastion tunnel: error creating tunnel: 403

bastionssh.go builds .../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_GPUA100 locally. SSH succeeded on attempt 1/5 instead of failing 5/5, and validation proceeded through the kubelet, NIC, kernel and waagent validators:

bastionssh.go:304: Attempt 1/5 establishing SSH over bastion to 10.99.32.5
test_helpers.go:1032: SSH connectivity to 10.99.32.5 verified successfully

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.

#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
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Windows Unit Test Results

  3 files   12 suites   49s ⏱️
389 tests 389 ✅ 0 💤 0 ❌
392 runs  392 ✅ 0 💤 0 ❌

Results for commit 5d3098d.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 EnableIPConnect on an existing shared bastion before returning its DNS name.
  • Keep EnableIPConnect: true in the bastion creation properties, with clarifying comments tying it to the bh-hostConnect/<ip> flow used by bastionssh.go.

Comment thread e2e/shared_infra.go
Comment on lines +290 to +312
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 {
Comment thread e2e/shared_infra.go
Comment on lines +287 to +289
// 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.
@ganeshkumarashok

Copy link
Copy Markdown
Contributor Author

Confirmed working: GPU E2E build 175689787 on this branch shows 0 bastion 403s (baseline 175585765 on main had 252).

Note this unmasks a pre-existing failure — all 12 GPU scenario attempts now fail on [nvidia-cdi-refresh.path nvidia-cdi-refresh.service] entering a failed state, with 0 VMExtensionProvisioningError. The validator simply could not reach the node before. That is fixed by #9150, not by this PR; the two should land together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants