8000 ssh: Avoid unneeded sleep when waiting for SSH server by nirs · Pull Request #23001 · kubernetes/minikube · GitHub
[go: up one dir, main page]

Skip to content

ssh: Avoid unneeded sleep when waiting for SSH server - #23001

Merged
medyagh merged 1 commit into
kubernetes:masterfrom
nirs:minimize-ssh-delay
May 21, 2026
Merged

medyagh merged 1 commit into
kubernetes:masterfrom
nirs:minimize-ssh-delay

Conversation

@nirs
@nirs nirs commented May 19, 2026 •
Copy link
Copy Markdown
Member

The previous code slept unconditionally after each failed attempt.
Sleep only if the check returned in under 500ms (retryThreshold); if
the dial already blocked in the kernel, sleeping again just adds
unnecessary delay.

On local machines with vmnet, the dial typically blocks 1-5 seconds
in the kernel before returning "connection refused". The sleep is
skipped, saving 1 second per attempt.

In GitHub runners, where the VM takes longer to boot, we observe three
phases during the SSH wait:

  • "operation timed out": dial blocks 10-16 seconds in the kernel (host
    not up, no ARP response). The sleep is skipped, saving 1 second per
    attempt.

  • "host is down": kernel returns instantly after cached ARP failure.
    The sleep prevents a busy loop, retrying every 1 second.

  • "connection refused": guest network is up but sshd not yet listening.
    Returns in under 100ms. Same as above, we sleep 1 second.

Examples from GitHub Actions

Sleep eliminated — dial blocked in kernel

vfkit second boot — dial blocked 75s, retried immediately (no 1s sleep):

I0519 01:16:03.137589  libmachine: Dialing to SSH server "192.168.65.2:22"
I0519 01:17:18.137821  libmachine: Failed to dial: dial tcp 192.168.65.2:22: connect: operation timed out
I0519 01:17:18.137897  libmachine: Dialing to SSH server "192.168.65.2:22"

QEMU (socket_vmnet) second boot — dial blocked 16s, retried immediately:

I0519 01:18:53.298024  libmachine: Dialing to SSH server "192.168.105.2:22"
I0519 01:19:09.156964  libmachine: Failed to dial: dial tcp 192.168.105.2:22: connect: operation timed out
I0519 01:19:09.157063  libmachine: Dialing to SSH server "192.168.105.2:22"

Sleep kept — dial returned instantly

"host is down" returned in <1ms, slept 1s before retry:

I0519 01:19:29.562156  libmachine: Dialing to SSH server "192.168.105.2:22"
I0519 01:19:29.562318  libmachine: Failed to dial: dial tcp 192.168.105.2:22: connect: host is down
I0519 01:19:30.563060  libmachine: Dialing to SSH server "192.168.105.2:22"

"connection refused" returned in 50ms, slept 1s before retry:

I0519 01:22:17.421092  libmachine: Dialing to SSH server "192.168.105.2:22"
I0519 01:22:17.471344  libmachine: Failed to dial: dial tcp 192.168.105.2:22: connect: connection refused
I0519 01:22:18.472487  libmachine: Dialing to SSH server "192.168.105.2:22"

Assisted by Cursor/Claude Opus 4.6

@nirs
nirs requested a review from medyagh May 19, 2026 01:09
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 19, 2026
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 19, 2026
@k8s-ci-robot
k8s-ci-robot requested a review from prezha May 19, 2026 01:09
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 19, 2026
@nirs
nirs requested a review from afbjorklund May 19, 2026 01:09
@nirs
nirs commented May 19, 2026
Copy link
Copy Markdown
Member Author

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label May 19, 2026
@nirs
nirs force-pushed the minimize-ssh-delay branch from 99a6f35 to 5df6d29 Compare May 19, 2026 01:37
@minikube-pr-bot

This comment has been minimized.

@minikube-pr-bot

This comment has been minimized.

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 optimizes WaitForSSHAccess’s retry loop to avoid adding an extra fixed delay when the SSH dial attempt already blocks for a long time (common on GitHub macOS runners during early boot/network bring-up), while still preventing a busy loop when failures return immediately.

Changes:

  • Measure per-attempt duration in WaitForSSHAccess.
  • Skip the retry sleep when the attempt itself took at least retryDelay.
  • Retain a delay for fast-failing cases (e.g. “host is down”, “connection refused”) to avoid tight retry loops.

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

Comment thread pkg/drivers/common/ssh.go Resolved
Comment thread pkg/drivers/common/ssh.go Resolved
@nirs

This comment was marked as outdated.

The previous code slept unconditionally after each failed attempt.
Sleep only if the check returned in under 500ms (retryThreshold); if
the dial already blocked in the kernel, sleeping again just adds
unnecessary delay.

On local machines with vmnet, the dial typically blocks 1-5 seconds
in the kernel before returning "connection refused". The sleep is
skipped, saving 1 second per attempt.

In GitHub runners, where the VM takes longer to boot, we observe three
phases during the SSH wait:

- "operation timed out": dial blocks 10-16 seconds in the kernel (host
  not up, no ARP response). The sleep is skipped, saving 1 second per
  attempt.

- "host is down": kernel returns instantly after cached ARP failure.
  The sleep prevents a busy loop, retrying every 1 second.

- "connection refused": guest network is up but sshd not yet listening.
  Returns in under 100ms. Same as above, we sleep 1 second.
@nirs
nirs force-pushed the minimize-ssh-delay branch from 5df6d29 to 8f11d96 Compare May 20, 2026 21:59
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 20, 2026
@minikube-pr-bot
Copy link
Copy Markdown

kvm2 driver with docker runtime

┌────────────────┬──────────┬────────────────────────┐
8000

│    COMMAND     │ MINIKUBE │ MINIKUBE  ( PR 23001 ) │
├────────────────┼──────────┼────────────────────────┤
│ minikube start │ 37.2s    │ 36.3s                  │
│ enable ingress │ 16.0s    │ 16.8s                  │
└────────────────┴──────────┴────────────────────────┘
Details

Times for minikube start: 39.6s 35.7s 35.6s 36.7s 38.7s
Times for minikube (PR 23001) start: 36.1s 35.1s 37.0s 36.7s 36.4s

Times for minikube ingress: 15.3s 15.2s 15.7s 18.7s 15.2s
Times for minikube (PR 23001) ingress: 18.8s 15.8s 15.3s 15.2s 18.8s

docker driver with docker runtime

┌────────────────┬──────────┬────────────────────────┐
│    COMMAND     │ MINIKUBE │ MINIKUBE  ( PR 23001 ) │
├────────────────┼──────────┼────────────────────────┤
│ minikube start │ 19.5s    │ 20.9s                  │
│ enable ingress │ 12.0s    │ 12.1s                  │
└────────────────┴──────────┴────────────────────────┘
Details

Times for minikube start: 21.0s 18.3s 21.2s 18.6s 18.4s
Times for minikube (PR 23001) start: 21.5s 21.0s 19.0s 21.6s 21.3s

Times for minikube ingress: 10.1s 12.6s 12.6s 12.6s 12.1s
Times for minikube (PR 23001) ingress: 12.6s 12.6s 12.1s 10.6s 12.7s

docker driver with containerd runtime

┌────────────────┬──────────┬────────────────────────┐
│    COMMAND     │ MINIKUBE │ MINIKUBE  ( PR 23001 ) │
├────────────────┼──────────┼────────────────────────┤
│ minikube start │ 16.9s    │ 16.8s                  │
│ enable ingress │ 24.8s    │ 24.8s                  │
└────────────────┴──────────┴────────────────────────┘
Details

Times for minikube start: 16.4s 19.2s 16.3s 16.4s 16.0s
Times for minikube (PR 23001) start: 16.3s 16.9s 16.8s 16.8s 17.1s

Times for minikube ingress: 24.6s 25.1s 25.1s 24.6s 24.6s
Times for minikube (PR 23001) ingress: 24.6s 25.6s 24.7s 24.6s 24.6s

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

@nirs: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-minikube-docker-containerd-linux-x86 8f11d96 link true /test pull-minikube-docker-containerd-linux-x86
pull-minikube-docker-containerd-linux-arm 8f11d96 link false /test pull-minikube-docker-containerd-linux-arm
pull-minikube-docker-docker-linux-arm 8f11d96 link true /test pull-minikube-docker-docker-linux-arm
pull-minikube-kvm-crio-linux-x86 8f11d96 link false /test pull-minikube-kvm-crio-linux-x86
pull-minikube-docker-crio-linux-x86 8f11d96 link false /test pull-minikube-docker-crio-linux-x86

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

@medyagh
medyagh merged commit 39b3c69 into kubernetes:master May 21, 2026
42 of 58 checks passed
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: medyagh, nirs

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:

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

@nirs
nirs deleted the minimize-ssh-delay branch May 21, 2026 16:52
@nirs
nirs removed the request for review from Copilot May 21, 2026 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

0