Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

virtiofs: Update feature gate usage, privileged containers and live migration #866

Merged
merged 1 commit into from
Feb 5, 2025
Merged

virtiofs: Update feature gate usage, privileged containers and live migration #866

merged 1 commit into from
Feb 5, 2025

Conversation

jcanocan
Copy link
Contributor

@jcanocan jcanocan commented Jan 24, 2025 •
edited
Loading

What this PR does / why we need it:
Change #11997 introduces new feature gates for virtiofs and dropped the support of privileged sharing mode. It syncs the documentation to the mentioned change:

  • Users need to enable the feature gate EnableVirtioFsConfigVolumes for sharing configMaps, secrets, and serviceAccounts.
  • Users need to enable the feature gate EnableVirtioFsStorageVolumes for sharing PVCs.
  • Dropped support of privileged sharing mode.

Moreover, change #13756 enables to migrate VMs while sharing devices with virtiofs. Therefore, all mentions to live migration limitation has been dropped.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes # CNV-35472

Special notes for your reviewer:

Checklist

This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

Release note:

Updates `virtiofs` feature gate usage and drops support for privileged sharing mode

@kubevirt-bot kubevirt-bot added the dco-signoff: yes Indicates the PR's author has DCO signed all their commits. label Jan 24, 2025
@jcanocan
Copy link
Contributor Author

/cc @germag

@kubevirt-bot kubevirt-bot requested a review from germag January 24, 2025 09:24
germag
germag reviewed Jan 27, 2025
View reviewed changes
docs/storage/disks_and_volumes.md Outdated
Comment on lines 1909 to 1898
The **non-privileged mode** is enabled by default. This mode has the advantage of not requiring any
administrative privileges for creating the VM. However, it has some limitations:
KubeVirt only supports to share PVCs in non-privileged sharing mode. The **non-privileged mode** sharing mode has some
limitations:
Copy link

Choose a reason for hiding this comment

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

Since the "privileged mode" doesn't exist anymore, I suggest avoiding mention "non-privileged sharing mode" because it implies that exist another mode. So, maybe something like:
"KubeVirt only supports to share PVCs using a non-privileged virtiofs daemon. Because the lack of privileges, it has some limitations:"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You right, nice catch. Done.

@jcanocan jcanocan force-pushed the virtiofs-feature-gates branch from 12d9d22 to 44b063f Compare January 28, 2025 10:53
@germag
Copy link

germag commented Jan 28, 2025

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 28, 2025
@jcanocan
Copy link
Contributor Author

/cc @aburdenthehand
Could you please take a look? Thanks! :)

@jcanocan jcanocan force-pushed the virtiofs-feature-gates branch from 44b063f to cae28fb Compare January 30, 2025 14:23
@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 30, 2025
@jcanocan jcanocan changed the title virtiofs: Update feature gate usage and privileged containers virtiofs: Update feature gate usage, privileged containers and live migration Jan 30, 2025
@jcanocan
Copy link
Contributor Author

v2:

  • Dropped all references to live migration limitations.

@jcanocan
Copy link
Contributor Author

/hold
Waiting for kubevirt/kubevirt#13756 to be merged.

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 30, 2025
@jcanocan
Copy link
Contributor Author

/hold cancel

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 31, 2025
@jcanocan
Copy link
Contributor Author

@alicefr Could you please take a look? Thanks in advance.

@germag
Copy link

germag commented Jan 31, 2025

I still see this phrase

`filesystem` does not support live migration.

in few places, configmap, secrets and serviceaccount

please check that all liv.mig references are correct

@jcanocan jcanocan force-pushed the virtiofs-feature-gates branch from cae28fb to 202db56 Compare January 31, 2025 14:33
@jcanocan
Copy link
Contributor Author

I still see this phrase

`filesystem` does not support live migration.

in few places, configmap, secrets and serviceaccount

please check that all liv.mig references are correct

Addressed.

@jcanocan
virtiofs: Update feature gate usage, privileged containers and live m…
325c57d 
…igration

Change [#11997](kubevirt/kubevirt#11997)
introduces new feature gates for virtiofs and dropped the support of
privileged sharing mode. It syncs the documentation to the mentioned
change:

* Users need to enable the feature gate
  `EnableVirtioFsConfigVolumes` for sharing `configMaps`, `secrets`,
and `serviceAccounts`.
* Users need to enable the feature gate `EnableVirtioFsStorageVolumes`
  for sharing `PVCs`.
* Dropped support of privileged sharing mode.

Moreover, change
[#13756](kubevirt/kubevirt#13756) enables to
migrate VMs while sharing devices with virtiofs. Therefore, all mentions
to live migration has been dropped.

Signed-off-by: Javier Cano Cano <[email protected]>
@jcanocan jcanocan force-pushed the virtiofs-feature-gates branch from 202db56 to 325c57d Compare January 31, 2025 14:39
@germag
Copy link

germag commented Jan 31, 2025

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 31, 2025
@alicefr
Copy link
Member

alicefr commented Feb 5, 2025

/approve
Don't remember if I have approver rights here

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alicefr

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

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 5, 2025
@kubevirt-bot kubevirt-bot merged commit 946faff into kubevirt:main Feb 5, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@germag germag germag left review comments

@akalenyu akalenyu Awaiting requested review from akalenyu

@awels awels Awaiting requested review from awels

@aburdenthehand aburdenthehand Awaiting requested review from aburdenthehand

Assignees

@germag germag

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jcanocan @germag @alicefr @kubevirt-bot