[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rafaelmfried
Once this PR has been reviewed and has the lgtm label, please assign medyagh for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

The committers listed above are authorized under a signed CLA.

• ✅ login: rafaelmfried / name: Rafael Moura Friederick (5224bf7, 721fe7d)

Welcome @rafaelmfried!

It looks like this is your first PR to kubernetes/minikube 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/minikube has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

Hi @rafaelmfried. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Can one of the admins verify this patch?

/easycla

Local validation

Reporting test results from my local macOS Sequoia (Apple Silicon) box.

Targeted unit test

```
$ go test ./pkg/minikube/node/ -run TestShouldWarnVirtualBox -v

=== RUN TestShouldWarnVirtualBox
=== RUN TestShouldWarnVirtualBox/✅_user-specified_virtualbox_with_warning_enabled_→_warn
=== RUN TestShouldWarnVirtualBox/❌_auto-selected_virtualbox_→_suppress_(#15456)
=== RUN TestShouldWarnVirtualBox/❌_user-specified_virtualbox_with_warning_opted_out_→_suppress
=== RUN TestShouldWarnVirtualBox/❌_auto-selected_virtualbox_with_warning_opted_out_→_suppress
=== RUN TestShouldWarnVirtualBox/❌_non-virtualbox_driver,_user-specified_→_no_warning
=== RUN TestShouldWarnVirtualBox/❌_non-virtualbox_driver,_auto-selected_→_no_warning
--- PASS: TestShouldWarnVirtualBox (0.00s)
--- PASS: TestShouldWarnVirtualBox/[all 6 subtests]
PASS
ok k8s.io/minikube/pkg/minikube/node
```

The bug-fixing case is the second one (auto-selected virtualbox → suppress).

Full unit test suite

Ran `MINIKUBE_LDFLAGS="" TESTSUITE=unittest ./test.sh` covering `./cmd/...` and `./pkg/...` (~80 packages). All packages `ok`, no `FAIL`. `pkg/minikube/node` shown below for emphasis:

```
ok k8s.io/minikube/pkg/minikube/node 2.761s coverage: 0.1% of statements
```

(Coverage is low for this package overall because it has no other tests yet — this PR adds the first one.)

Note on local lint

Running `make test` (full suite including `make lint-ci`) on master without my changes already fails with a pre-existing govet warning in `pkg/drivers/parallels/parallels_darwin.go:425` (printf %d with a `*go-version.Version` arg). Not introduced by this PR; happy to file a separate cleanup PR if useful.

End-to-end validation against a real virtualbox cluster on Linux is planned as a follow-up; the function-level test above covers the regression boundary.

Fixes #15456

Problem

When minikube auto-selects the virtualbox driver (because no other driver was viable on the host), it still emits the steering-away-from-virtualbox warning:

* Automatically selected the virtualbox driver. Other choices: ssh, qemu2 (experimental)
...
╭───────────────────────────────────────────────────────────────────────╮
│ You have selected "virtualbox" driver, but there are better options ! │
│ For better performance and support consider using a different driver: │
│         - qemu2                                                       │
╰───────────────────────────────────────────────────────────────────────╯

This is contradictory: minikube has just told the user it picked virtualbox itself because the alternatives weren't viable, then immediately suggests using one of those alternatives.

The issue is the text, not the warning. The purpose of the warning is to recommend a better driver when a user selects virtaulbox, since the driver has a lower priority.

Lets start with an issue about this, and discuss what it the wanted behavior there.

To it looks like "You have selected "virtualbox" driver, but there are better options !" is the problem. The user did not select the driver. It can make sense to show this warning:

╭───────────────────────────────────────────────────────────────────────╮
│ For better performance and support consider using a different driver: │
│         - qemu2                                                       │
╰───────────────────────────────────────────────────────────────────────╯

Thanks for the careful review @nirs — the argument-validation framing makes sense, and you're right that pushing DriverSpecified through provisionWithDriver/Starter adds plumbing that wouldn't be needed if the warning lived in start_flags.go.

A few quick notes before I pivot, since the rework is non-trivial and I'd like to make sure we're aiming at the same target:

1. Equivalence question. Moving the warning to argument validation (gated on viper.GetString("driver") == "virtualbox" or "vm-driver") covers the same set of cases the current patch covers, with two small differences:
   • Existing profile reuse. With the current patch, a profile that was originally created with vbox still triggers the warning on every subsequent minikube start (because selectDriver returns specified=true for the existing-profile path). Moving to start_flags.go means the warning would only fire when the user passes --driver on this specific invocation — quieter, arguably better.
   • Output position. The warning would print before the * Automatically selected ... / * Using the X driver based on existing profile line instead of after node provisioning. I think that's actually an improvement.
2. Wording. Independent of where the check lives, your suggestion to drop You have selected \"virtualbox\" driver, but there are better options ! and just keep the For better performance and support consider using a different driver: … block is cleaner. Happy to fold that in too.
3. @medyagh — since you opened the issue, before I rewrite this could you confirm the desired behavior? Two options on the table:
   • A (this PR's current shape): suppress the warning when minikube auto-selects vbox; keep it when the user explicitly picks vbox or reuses an existing vbox profile.
   • B (@nirs's proposal): move the warning to argument validation in start_flags.go, fire only when --driver=virtualbox is on the current invocation; reword the message.

If B is the direction you want, I'll close this and open a fresh PR against start_flags.go. If A is closer to what you had in mind in the original issue, I'll address @nirs's concerns about the plumbing in a different way (e.g. plumbing the flag through a smaller surface).

Either way I'd rather not guess and end up with another PR rotting in lifecycle/rotten — so a quick steer from you would help a lot.

While we wait for direction, I went ahead and prototyped @nirs's suggestion as a separate draft PR for side-by-side comparison: #22960. Same end-user behavior on the bug case in this issue, but with the warning living in argument-validation and a reworded message (drops the contradictory "You have selected …" opener).

Either PR is mergeable; the other will be closed once we know which shape is preferred. No need to review #22960 in depth yet — it's there so the choice is concrete rather than hypothetical.

Thanks for the careful review @nirs — the argument-validation framing makes sense, and you're right that pushing DriverSpecified through provisionWithDriver/Starter adds plumbing that wouldn't be needed if the warning lived in start_flags.go.

A few quick notes before I pivot, since the rework is non-trivial and I'd like to make sure we're aiming at the same target:

1. Equivalence question. Moving the warning to argument validation (gated on viper.GetString("driver") == "virtualbox" or "vm-driver") covers the same set of cases the current patch covers, with two small differences:

   • Existing profile reuse. With the current patch, a profile that was originally created with vbox still triggers the warning on every subsequent minikube start (because selectDriver returns specified=true for the existing-profile path). Moving to start_flags.go means the warning would only fire when the user passes --driver on this specific invocation — quieter, arguably better.

The flag --driver virtualbox is ignored when starting existing cluster, and user has no reason to specify the flag in this case, but if we validate the flag very early we will warn again about the driver. So this check should be right after we checked if we have an existing cluster, and we should not warn when starting an existing cluster.

Unfortunately we don't have create command. We have start, stop, and delete. We should really have create, start, stop, and delete. And the warning about driver is relevant only to the create command.

• Output position. The warning would print before the * Automatically selected ... / * Using the X driver based on existing profile line instead of after node provisioning. I think that's actually an improvement.

I agree, this is the right place to warn about the user select argument. Also selecting a driver when we have an existing cluster does not make sense. If the cluster exists (stopped or started) there is nothing to select.

2. Wording. Independent of where the check lives, your suggestion to drop You have selected \"virtualbox\" driver, but there are better options ! and just keep the For better performance and support consider using a different driver: … block is cleaner. Happy to fold that in too.

3. @medyagh — since you opened the issue, before I rewrite this could you confirm the desired behavior? Two options on the table:

   • A (this PR's current shape): suppress the warning when minikube auto-selects vbox; keep it when the user explicitly picks vbox or reuses an existing vbox profile.
   • B (@nirs's proposal): move the warning to argument validation in start_flags.go, fire only when --driver=virtualbox is on the current invocation; reword the message.

If B is the direction you want, I'll close this and open a fresh PR against start_flags.go. If A is closer to what you had in mind in the original issue, I'll address @nirs's concerns about the plumbing in a different way (e.g. plumbing the flag through a smaller surface).

Either way I'd rather not guess and end up with another PR rotting in lifecycle/rotten — so a quick steer from you would help a lot.

I suggested to remove the text about select augment to warn about better driver when the user did not select the driver. But since we the warning we have now is only for user selection, we can consider this change later.