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.

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

Update persistent TPM & UEFI page to clarify storageclass requirements #830

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ianb-mp
Copy link
Contributor

@ianb-mp ianb-mp commented Aug 29, 2024

What this PR does / why we need it:

Update Persistent TPM and UEFI state to reflect the changes made in kubevirt/kubevirt#11773

Also, small spelling fix: prerequesites -> prerequisites

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 #

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:


@kubevirt-bot kubevirt-bot added the dco-signoff: yes Indicates the PR's author has DCO signed all their commits. label Aug 29, 2024
@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

The full list of commands accepted by this bot can be found 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
Copy link
Contributor

Hi @ianb-mp. Thanks for your PR.

I'm waiting for a kubevirt 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.

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.

@kubevirt-bot kubevirt-bot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S labels Aug 29, 2024
@ianb-mp
Copy link
Contributor Author

ianb-mp commented Aug 29, 2024

I was unsure if the following statement is still true - can anyone confirm?

Backend storage is currently incompatible with VM snapshot. It is planned to add snapshot support in the future.

@akalenyu
Copy link
Contributor

I was unsure if the following statement is still true - can anyone confirm?

Backend storage is currently incompatible with VM snapshot. It is planned to add snapshot support in the future.

I think so - https://github.com/kubevirt/kubevirt/blob/effcb5b8e6fc951ec5c85ba47ad1daedc0173d79/pkg/virt-api/webhooks/validating-webhook/admitters/vmsnapshot-admitter.go#L166-L174
/cc @jean-edouard

@kubevirt-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@kubevirt-bot kubevirt-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 27, 2024
@ianb-mp
Copy link
Contributor Author

ianb-mp commented Nov 27, 2024

@akalenyu @victortoso @jean-edouard could someone please review?

The storage class must support read-write-many (RWX) in filesystem mode (FS).
KubeVirt automatically creates backend storage PVCs for VMs that need it. However, to persist TPM and UEFI state, the admin must first enable the `VMPersistentState` feature gate.

If [live migration](live_migration.md) is required, then the `vmStateStorageClass` configuration parameter should be set, and must reference a storage class that supports read-write-many (RWX) in filesystem mode (FS).
Copy link
Contributor

Choose a reason for hiding this comment

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

Not anymore. In 1.4, RWO FS backend-storage is migratable. vmStateStorageClass can still be used to manually specify a storage class though, instead of relying on defaults.

- As mentioned above, the backend storage PVC can only be created using a storage class that supports RWX FS. There is ongoing work to support block storage in future versions of KubeVirt.
### Notes:
- If no storage class is specified, the default storage class will be used
- If the storage class has a storage profile that indicates it only supports read-write-once (RWO) then a RWO PVC will be created and the VMI will be marked as non-migratable.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

@jean-edouard
Copy link
Contributor

Thank you @ianb-mp for this update. It looks good but needs to be reworked a bit to reflect most recent changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants