Skip to content

Conversation

nirs
Copy link
Contributor

@nirs nirs commented Aug 9, 2025

This change prepares TestMountStart for adding virtiofs support in the drivers.

Add findmnt package for more precise mount validation and skip options validation for virtiofs mounts. Fix 9p mounts options names hidden by imprecise strings matching.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 9, 2025
@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot requested review from medyagh and prezha August 9, 2025 15:45
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 9, 2025
@nirs
Copy link
Contributor Author

nirs commented Aug 9, 2025

/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 Aug 9, 2025
@nirs
Copy link
Contributor Author

nirs commented Aug 9, 2025

/cc @ComradeProgrammer

@nirs
Copy link
Contributor Author

nirs commented Aug 9, 2025

/cc @afbjorklund

@nirs
Copy link
Contributor Author

nirs commented Aug 9, 2025

/kind improvement

@k8s-ci-robot k8s-ci-robot added the kind/improvement Categorizes issue or PR as related to improving upon a current feature. label Aug 9, 2025
@nirs
Copy link
Contributor Author

nirs commented Aug 9, 2025

/area mount

@nirs
Copy link
Contributor Author

nirs commented Aug 9, 2025

/area testing

@minikube-pr-bot

This comment has been minimized.

@nirs nirs force-pushed the test-virtiofs branch 2 times, most recently from ba2f126 to fc50051 Compare August 10, 2025 01:31
@nirs
Copy link
Contributor Author

nirs commented Aug 10, 2025

Rebased to consume the docker service fix

@minikube-pr-bot

This comment has been minimized.

@minikube-pr-bot

This comment has been minimized.

Copy link
Member

@medyagh medyagh left a comment

Choose a reason for hiding this comment

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

plz rebase and look at the comments

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 12, 2025
@nirs
Copy link
Contributor Author

nirs commented Aug 12, 2025

Rebased to consume #21250, comments not addressed yet.

@minikube-pr-bot

This comment has been minimized.

nirs added 2 commits August 15, 2025 21:43
When testing mounts we can use findmnt --json output to parse the output
cleanly. The package provides only ParseOutput() now, but it can be
extended later to run the findmnt command.
Use findmnt command to get the mounted filesystem details cleanly.

We use the actual mount fstype instead of driver name check so we can
switch drivers to virtiofs without changing the test.

For virtiofs mount we skip options validation since we don't support
setting virtiofs options yet, and the options are not the same as 9p
options.

For 9p mounts the uid= and gid= flags were fixed to match the real flags
(dfltuid=,dfltgid=). The issue was hidden by imprecise string matching.
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 15, 2025
@nirs nirs requested a review from medyagh August 15, 2025 19:18
@nirs nirs marked this pull request as ready for review August 15, 2025 19:18
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 15, 2025
@nirs
Copy link
Contributor Author

nirs commented Aug 15, 2025

@medyagh changes in this version:

  • Rebase on master (unfortunately I forgot to push the rebase before the changes so it is hard to see the diff)
  • Support nested output in findmnt
  • Support comparing findmnt results
  • Add tests for findmnt package
  • Add constants for 9p and virtiofs
  • Improve error handling in the mount_start_test

Tested locally with vfkit and krunkit, should be ready for merge.

@minikube-pr-bot
Copy link

kvm2 driver with docker runtime

┌────────────────┬──────────┬────────────────────────┐
│    COMMAND     │ MINIKUBE │ MINIKUBE  ( PR 21272 ) │
├────────────────┼──────────┼────────────────────────┤
│ minikube start │ 49.3s    │ 47.7s                  │
│ enable ingress │ 15.4s    │ 15.0s                  │
└────────────────┴──────────┴────────────────────────┘

Times for minikube start: 49.3s 46.1s 49.8s 50.3s 50.8s
Times for minikube (PR 21272) start: 47.7s 50.3s 48.9s 44.9s 46.8s

Times for minikube (PR 21272) ingress: 15.0s 15.0s 15.0s 15.0s 14.9s
Times for minikube ingress: 15.4s 16.0s 15.0s 14.9s 15.4s

docker driver with docker runtime

┌────────────────┬──────────┬────────────────────────┐
│    COMMAND     │ MINIKUBE │ MINIKUBE  ( PR 21272 ) │
├────────────────┼──────────┼────────────────────────┤
│ minikube start │ 23.3s    │ 23.8s                  │
│ enable ingress │ 13.1s    │ 12.9s                  │
└────────────────┴──────────┴────────────────────────┘

Times for minikube ingress: 13.2s 12.8s 12.8s 12.8s 13.7s
Times for minikube (PR 21272) ingress: 13.8s 12.2s 12.3s 13.7s 12.7s

Times for minikube start: 23.1s 22.2s 25.3s 23.4s 22.6s
Times for minikube (PR 21272) start: 24.1s 24.8s 22.7s 24.1s 23.2s

docker driver with containerd runtime

┌────────────────┬──────────┬────────────────────────┐
│    COMMAND     │ MINIKUBE │ MINIKUBE  ( PR 21272 ) │
├────────────────┼──────────┼────────────────────────┤
│ minikube start │ 22.7s    │ 23.4s                  │
│ enable ingress │ 26.1s    │ 26.3s                  │
└────────────────┴──────────┴────────────────────────┘

Times for minikube start: 24.6s 21.6s 22.4s 22.0s 23.2s
Times for minikube (PR 21272) start: 25.1s 23.4s 24.9s 22.3s 21.5s

Times for minikube ingress: 23.2s 22.8s 22.7s 22.8s 39.3s
Times for minikube (PR 21272) ingress: 22.8s 23.7s 22.7s 22.8s 39.3s

@nirs
Copy link
Contributor Author

nirs commented Aug 17, 2025

/retest

@medyagh
Copy link
Member

medyagh commented Aug 18, 2025

/lgtm

@medyagh medyagh merged commit 6fbe84c into kubernetes:master Aug 18, 2025
31 of 44 checks passed
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 18, 2025
@k8s-ci-robot
Copy link
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

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 18, 2025
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. area/mount area/testing cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/improvement Categorizes issue or PR as related to improving upon a current feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants