Skip to content

iscsi+nvmeof: apply Tailscale-aware blkid budget to iSCSI too#3

Merged
fenio merged 1 commit into
mainfrom
iscsi-tailscale-timeout
Jun 4, 2026
Merged

iscsi+nvmeof: apply Tailscale-aware blkid budget to iSCSI too#3
fenio merged 1 commit into
mainfrom
iscsi-tailscale-timeout

Conversation

@fenio

@fenio fenio commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

NVMe-oF already gets a 30s per-blkid timeout + 6 retries because the developer noticed blkid was taking 10-20s per attempt over Tailscale and the original 5s budget killed it. iSCSI fell through to the default 5s timeout / 3 retries despite running on the same kind of Tailscale links — observed in the failing iSCSI E2E job where blkid against `/dev/sd[a-z]` devices was killed at the 5s deadline three attempts in a row, then the driver correctly refused to format an unknown device, and the test timed out waiting for a pod to become Ready.

Mea culpa on the diagnosis

Misdiagnosed twice in flight before finding this:

  1. First theory: dual-stack v6 portal advertisement was confusing iscsid. Engine PR #400 made the v6 default best-effort — didn't fix it. CSI PR iscsi: prune all discovered node entries except the StorageClass portal #2 pruned discovered-but-not-StorageClass portals — didn't fix it either. The v6 entry in the node DB was real but wasn't causing the blkid hang.
  2. Actual cause: the existing NVMe-oF Tailscale-budget heuristic was matched only by `/dev/nvme` path prefix. iSCSI devices (`/dev/sd*`) over the same link got the LAN-optimized 5s budget and lost.

The smoking gun was `Current Portal: 100.127.184.102:3260` in the session log — a 100.64.0.0/10 CGNAT IP, i.e. Tailscale.

Fix

  • Extract `isNetworkBlockDevice(devicePath)` matching both `/dev/nvme` and `/dev/sd`. In this driver's NodeStageVolume path, `/dev/sd` is always iSCSI-attached (local SCSI disks don't reach this code) and `/dev/nvme` is always NVMe-oF, so the false-positive rate for treating either as high-latency is zero.
  • Per-blkid `context.WithTimeout` now goes from 5s to 30s for iSCSI too. The 30s is a ceiling, not a wait — blkid still exits in milliseconds on a healthy LAN link, so LAN deployments pay nothing for the change.
  • Retry budget for iSCSI goes from 3 to 6, matching NVMe-oF. Combined with the per-attempt timeout bump this gives up to ~3 minutes of blkid budget under sustained slowness — still well under the typical 2-minute gRPC deadline thanks to exponential backoff and early-exit on first definitive result.

The non-network default branch (3 retries, 5s timeout) stays unchanged for loop devices, mapper / LVM devices, and anything else that isn't network-attached.

Tests

`TestIsNetworkBlockDevice` pins the classifier across the failure modes that drove this PR:

  • iSCSI `/dev/sd[a-z]` and partitions get the budget
  • NVMe-oF `/dev/nvme*` with namespaces and partitions get it
  • `/dev/loop*` (block subvolumes) and `/dev/mapper/*` (LVM) stay on the fast path

`go test ./pkg/...` green; `golangci-lint run` 0 issues.

Predicted effect

The iSCSI concurrent E2E and Shared E2E suites that have been the only E2E failures after engine PR #400 should finally go green.

Test plan

  • Merge → integration tests run → `E2E: iSCSI` and `E2E: Shared` pass
  • Confirm no regression in LAN deployments (blkid still completes near-instantly; 30s ceiling never reached)

NVMe-oF already gets a 30s per-blkid timeout + 6 retries because
the developer noticed blkid was taking 10-20s per attempt over
Tailscale links and the original 5s budget killed it. iSCSI fell
through to the default 5s timeout / 3 retries despite running on
the same kind of Tailscale links — observed in the iSCSI concurrent
E2E tests where blkid against /dev/sd[a-z] devices was killed at
the 5s deadline three attempts in a row, then the driver correctly
refused to format an unknown device, and the test timed out
waiting for a pod to become Ready.

The root cause was misdiagnosed twice in flight:
  1. First theory: dual-stack v6 portal advertisement was confusing
     iscsid. Engine PR #400 made the v6 default best-effort, didn't
     help. CSI PR #2 pruned discovered-but-not-StorageClass portals,
     didn't help either — the v6 entry in node DB was real but
     wasn't causing the blkid hang.
  2. Actual cause: the existing NVMe-oF Tailscale-budget heuristic
     was matched only by /dev/nvme path prefix. iSCSI devices
     (/dev/sd*) over the same link got the LAN-optimized 5s budget
     and lost.

Fix
  * Extract `isNetworkBlockDevice(devicePath)` matching both /dev/nvme
    and /dev/sd. In this driver's NodeStageVolume path, /dev/sd is
    always iSCSI-attached (local SCSI disks don't reach this code)
    and /dev/nvme is always NVMe-oF, so the false-positive rate for
    treating either as high-latency is zero.
  * Per-blkid context.WithTimeout now goes from 5s to 30s for iSCSI
    too. The 30s is a ceiling, not a wait — blkid still exits in
    milliseconds on a healthy LAN link, so LAN deployments pay
    nothing for the change.
  * Retry budget for iSCSI goes from 3 to 6, matching NVMe-oF.
    Combined with the per-attempt timeout bump this gives up to
    ~3 minutes of blkid budget under sustained slowness — still
    well under the typical 2-minute gRPC deadline thanks to the
    exponential backoff and early-exit on first definitive result.

The non-network default branch (3 retries, 5s timeout) stays
unchanged for loop devices, mapper / LVM devices, and anything
else that isn't network-attached.

Tests
  * `TestIsNetworkBlockDevice` pins the classifier across the
    failure modes that drove this PR: iSCSI /dev/sd[a-z] and
    partitions get the budget, NVMe-oF /dev/nvme* with namespaces
    and partitions get it, /dev/loop* (block subvolumes) and
    /dev/mapper/* (LVM) stay on the fast path.

Predicted effect: the iSCSI concurrent E2E and Shared E2E suites
that were the only E2E failures after engine PR #400 should
finally go green.
@fenio fenio merged commit 275e449 into main Jun 4, 2026
1 check passed
@fenio fenio deleted the iscsi-tailscale-timeout branch June 4, 2026 16:52
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.

1 participant