CNV-60438: Correct misleading information about vTPM devices #92553

jhradilek · 2025-04-23T16:23:21Z

Version(s): 4.18+

Issue: CNV-60438

Link to docs preview:

QE review:

QE has approved this change.

Additional information: N/A

openshift-ci-robot · 2025-04-23T16:23:26Z

@jhradilek: This pull request references CNV-60438 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.19.0" version, but no target version was set.

Details

In response to this:

Version(s): 4.18+

Issue: CNV-60438

Link to docs preview:

QE review:

QE has approved this change.

Additional information: N/A

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 openshift-eng/jira-lifecycle-plugin repository.

jhradilek · 2025-04-23T16:23:56Z

/label cnv

ocpdocs-previewbot · 2025-04-23T16:54:45Z

🤖 Mon May 12 18:10:01 - Prow CI generated the docs preview:

https://92553--ocpdocs-pr.netlify.app/openshift-enterprise/latest/virt/managing_vms/virt-using-vtpm-devices.html
https://92553--ocpdocs-pr.netlify.app/openshift-rosa-hcp/latest/virt/managing_vms/virt-using-vtpm-devices.html
https://92553--ocpdocs-pr.netlify.app/openshift-rosa/latest/virt/managing_vms/virt-using-vtpm-devices.html

ocpdocs-vale-bot · 2025-04-23T16:55:25Z

modules/virt-about-vtpm-devices.adoc

+from a Windows 11 image to function without a physical TPM chip. A vTPM device also protects virtual machines by storing secrets without physical hardware.

-A vTPM device also protects virtual machines by storing secrets without physical hardware. {VirtProductName} supports persisting vTPM device state by using Persistent Volume Claims (PVCs) for VMs. You must specify the storage class to be used by the PVC by setting the `vmStateStorageClass` attribute in the `HyperConverged` custom resource (CR):
+{VirtProductName} supports persisting vTPM device state by using Persistent Volume Claims (PVCs) for VMs. If you do not specify the storage class for this PVC, {VirtProductName} uses the default storage class for virtualization workloads. If the default storage class for virtualization workloads is not set, {VirtProductName} uses the default storage class for the cluster. For information about how to review or change the default storage classes, see xref:../../virt/storage/virt-automatic-bootsource-updates.adoc#virt-configuring-default-and-virt-default-storage-class_virt-automatic-bootsource-updates[Configuring the default and virt-default storage classes].


🤖 [error] OpenShiftAsciiDoc.NoXrefInModules: Do not include xrefs in modules, only assemblies.

openshift-ci-robot · 2025-04-23T18:05:42Z

@jhradilek: This pull request references CNV-60438 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.19.0" version, but no target version was set.

Details

In response to this:

Version(s): 4.18+

Issue: CNV-60438

Link to docs preview:

https://92553--ocpdocs-pr.netlify.app/openshift-enterprise/latest/virt/managing_vms/virt-using-vtpm-devices.html#virt-about-vtpm-devices_virt-using-vtpm-devices

https://92553--ocpdocs-pr.netlify.app/openshift-rosa-hcp/latest/virt/managing_vms/virt-using-vtpm-devices.html#virt-about-vtpm-devices_virt-using-vtpm-devices

https://92553--ocpdocs-pr.netlify.app/openshift-rosa/latest/virt/managing_vms/virt-using-vtpm-devices.html#virt-about-vtpm-devices_virt-using-vtpm-devices

QE review:

QE has approved this change.

Additional information: N/A

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 openshift-eng/jira-lifecycle-plugin repository.

awels · 2025-04-24T14:36:38Z

modules/virt-about-vtpm-devices.adoc

 You can use a vTPM device with any operating system, but Windows 11 requires
 the presence of a TPM chip to install or boot. A vTPM device allows VMs created
-from a Windows 11 image to function without a physical TPM chip.
+from a Windows 11 image to function without a physical TPM chip. A vTPM device also protects virtual machines by storing secrets without physical hardware.


I don't think we should say this. I am not sure what it means, or if it is accurate. When I see secrets I immediately think kubernetes secrets which is not the case.

Thank you, you are right that this is misleading and I wonder if we are entering the territory of trying to explain what a physical TPM device is. If this information is required, I think it would make more sense earlier in the paragraph right after the first sentence and we should be more specific about what certificates and private keys it is used for. @alromeros What do you think?

I think it would make more sense earlier in the paragraph right after the first sentence and we should be more specific about what certificates and private keys it is used for

I agree. If we want to include this sentence it makes more sense to have it earlier in the paragraph. I don't consider it a must though.

When I see secrets I immediately think kubernetes secrets which is not the case.

This was my first thought too. In case we want to keep this sentence I'd try to say something more generic, such as "sensitive data" instead of secrets.

Thank you. I removed it as it does not feel like we are doing justice to explaining what TPM devices are and I don't think it is the purpose of this section.

alromeros · 2025-04-25T09:35:47Z

modules/virt-about-vtpm-devices.adoc

+To ensure consistent behavior, configure only one storage class as the default for virtualization workloads and for the cluster respectively.
+====
+
+Because this behavior might change in the future, it is recommended that you specify the storage class explicitly by setting the `vmStateStorageClass` attribute in the `HyperConverged` custom resource (CR):


This is a personal concern but I don't know if I would voice it in downstream documentation. I'd say it's still recommended to set vmStateStorageClass as it's the intended way to define the storage class for backend PVCs, but don't know if it's appropriate to say that the fallback behavior could change in the future.

Thank you, that makes sense. I removed the first part of the sentence.

jhradilek · 2025-04-25T15:01:22Z

I have updated the wording as suggested. @alromeros do you have any other suggestions or can I proceed to seek QE review?

alromeros · 2025-04-28T08:16:27Z

I have updated the wording as suggested. @alromeros do you have any other suggestions or can I proceed to seek QE review?

Looks good to me!

ghost · 2025-05-07T08:43:04Z

@jhradilek From QE review, I have reviewed it and the content looks good

jhradilek · 2025-05-07T10:47:09Z

Thank you very much, @stesrn!

/label peer-review-needed

mburke5678 · 2025-05-07T13:38:46Z

modules/virt-about-vtpm-devices.adoc

+
+[source,terminal]
+----
+$ oc get sc -o json | jq '.items[].metadata|select(.annotations."storageclass.kubevirt.io/is-default-virt-class"=="true")|.name'


Per the doc guidelines, Do not use jq in commands (unless it is truly required).

mburke5678 · 2025-05-07T13:39:10Z

modules/virt-about-vtpm-devices.adoc

+$ oc get sc -o json | jq '.items[].metadata|select(.annotations."storageclass.kubevirt.io/is-default-virt-class"=="true")|.name'
+----
+
+Similarly, the default storage class for the cluster has the annotation `storageclass.kubernetes.io/is-default-class` set to "true". To find this storage class, run:


Suggested change

Similarly, the default storage class for the cluster has the annotation `storageclass.kubernetes.io/is-default-class` set to "true". To find this storage class, run:

Similarly, the default storage class for the cluster has the annotation `storageclass.kubernetes.io/is-default-class` set to "true". To find this storage class, run the following command:

Thank you, I will fix this.

mburke5678 · 2025-05-07T13:39:27Z

modules/virt-about-vtpm-devices.adoc

+
+[source,terminal]
+----
+$ oc get sc -o json | jq '.items[].metadata|select(.annotations."storageclass.kubernetes.io/is-default-class"=="true")|.name'


Per the doc guidelines, Do not use jq in commands (unless it is truly required).

Thank you. I was hoping this would be justified as it is sadly the most straightforward way to get this information that I could find. The same jq command is actually also used later on in the book, but I cannot link to it as cross-references from modules are forbidden too. The alternative is to oc get sc and look for lines with (default) in the first column, but then it is not clear which default it is.

mburke5678 · 2025-05-07T13:40:04Z

@jhradilek A few comments. Otherwise LGTM

jhradilek · 2025-05-12T17:35:42Z

Hi @stesrn, during a peer review Michael correctly pointed out that according to our guidelines, jq should be avoided unless completely necessary. Turns out it was not necessary and I figured out how to achieve the same result with the -o jsonpath option, but that means I had to change both of the commands. Would you mind checking the updated version when the preview build is available?

openshift-ci · 2025-05-12T18:11:44Z

@jhradilek: all tests passed!

Full PR test history. Your PR dashboard.

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.

ghost · 2025-05-13T11:27:34Z

@jhradilek No problems. I have verified with the new commands that uses jsonpath. That looks good. Overall content is also good.

openshift-ci-robot · 2025-05-13T11:30:41Z

@jhradilek: This pull request references CNV-60438 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.20.0" version, but no target version was set.

Details

In response to this:

Version(s): 4.18+

Issue: CNV-60438

Link to docs preview:

https://92553--ocpdocs-pr.netlify.app/openshift-enterprise/latest/virt/managing_vms/virt-using-vtpm-devices.html#virt-about-vtpm-devices_virt-using-vtpm-devices

https://92553--ocpdocs-pr.netlify.app/openshift-rosa-hcp/latest/virt/managing_vms/virt-using-vtpm-devices.html#virt-about-vtpm-devices_virt-using-vtpm-devices

https://92553--ocpdocs-pr.netlify.app/openshift-rosa/latest/virt/managing_vms/virt-using-vtpm-devices.html#virt-about-vtpm-devices_virt-using-vtpm-devices

QE review:

QE has approved this change.

Additional information: N/A

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 openshift-eng/jira-lifecycle-plugin repository.

jhradilek · 2025-05-13T11:31:05Z

Thank you!

/label merge-review-needed

sheriff-rh

LGTM; merging.

sheriff-rh · 2025-05-13T13:43:34Z

/cherrypick enterprise-4.19

sheriff-rh · 2025-05-13T13:43:37Z

/cherrypick enterprise-4.18

openshift-cherrypick-robot · 2025-05-13T13:44:23Z

@sheriff-rh: new pull request created: #93285

Details

In response to this:

/cherrypick enterprise-4.19

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.

openshift-cherrypick-robot · 2025-05-13T13:44:29Z

@sheriff-rh: new pull request created: #93286

Details

In response to this:

/cherrypick enterprise-4.18

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.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Apr 23, 2025

openshift-ci bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 23, 2025

openshift-ci bot added the CNV Label for all CNV PRs label Apr 23, 2025

ocpdocs-vale-bot reviewed Apr 23, 2025

View reviewed changes

jhradilek force-pushed the CNV-60438-vmstatestorageslass-for-vtpm branch from b72d5d5 to ec12150 Compare April 23, 2025 19:59

openshift-ci bot 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 Apr 23, 2025

awels reviewed Apr 24, 2025

View reviewed changes

alromeros reviewed Apr 25, 2025

View reviewed changes

jhradilek force-pushed the CNV-60438-vmstatestorageslass-for-vtpm branch from ec12150 to 4839c20 Compare April 25, 2025 13:42

openshift-ci bot added the peer-review-needed Signifies that the peer review team needs to review this PR label May 7, 2025

mburke5678 added the peer-review-in-progress Signifies that the peer review team is reviewing this PR label May 7, 2025

mburke5678 reviewed May 7, 2025

View reviewed changes

mburke5678 added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR peer-review-needed Signifies that the peer review team needs to review this PR labels May 7, 2025

CNV-60438: Corrected misleading information about vTPM devices

cefaaf1

jhradilek force-pushed the CNV-60438-vmstatestorageslass-for-vtpm branch from 4839c20 to cefaaf1 Compare May 12, 2025 17:33

openshift-ci bot added the merge-review-needed Signifies that the merge review team needs to review this PR label May 13, 2025

sheriff-rh added merge-review-in-progress Signifies that the merge review team is reviewing this PR branch/enterprise-4.18 branch/enterprise-4.19 and removed merge-review-needed Signifies that the merge review team needs to review this PR labels May 13, 2025

sheriff-rh added this to the Continuous Release milestone May 13, 2025

sheriff-rh approved these changes May 13, 2025

View reviewed changes

sheriff-rh merged commit 0913928 into openshift:main May 13, 2025
2 checks passed

This was referenced May 13, 2025

[enterprise-4.19] CNV-60438: Correct misleading information about vTPM devices #93285

Merged

[enterprise-4.18] CNV-60438: Correct misleading information about vTPM devices #93286

Merged

jhradilek deleted the CNV-60438-vmstatestorageslass-for-vtpm branch May 13, 2025 13:46

	Similarly, the default storage class for the cluster has the annotation `storageclass.kubernetes.io/is-default-class` set to "true". To find this storage class, run:
	Similarly, the default storage class for the cluster has the annotation `storageclass.kubernetes.io/is-default-class` set to "true". To find this storage class, run the following command:

CNV-60438: Correct misleading information about vTPM devices #92553

CNV-60438: Correct misleading information about vTPM devices #92553

Uh oh!

Conversation

jhradilek commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Apr 23, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jhradilek commented Apr 23, 2025

Uh oh!

ocpdocs-previewbot commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Apr 23, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alromeros Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhradilek Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhradilek commented Apr 25, 2025

Uh oh!

alromeros commented Apr 28, 2025

Uh oh!

ghost commented May 7, 2025

Uh oh!

jhradilek commented May 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhradilek May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mburke5678 commented May 7, 2025

Uh oh!

jhradilek commented May 12, 2025

Uh oh!

openshift-ci bot commented May 12, 2025

Uh oh!

ghost commented May 13, 2025

Uh oh!

openshift-ci-robot commented May 13, 2025 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jhradilek commented May 13, 2025

Uh oh!

sheriff-rh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sheriff-rh commented May 13, 2025

Uh oh!

sheriff-rh commented May 13, 2025

Uh oh!

openshift-cherrypick-robot commented May 13, 2025

Uh oh!

openshift-cherrypick-robot commented May 13, 2025

jhradilek commented Apr 23, 2025 •

edited

Loading

openshift-ci-robot commented Apr 23, 2025 •

edited by openshift-ci bot

Loading

ocpdocs-previewbot commented Apr 23, 2025 •

edited

Loading

openshift-ci-robot commented Apr 23, 2025 •

edited by openshift-ci bot

Loading

alromeros Apr 25, 2025 •

edited

Loading

jhradilek Apr 25, 2025 •

edited

Loading

jhradilek May 7, 2025 •

edited

Loading

openshift-ci-robot commented May 13, 2025 •

edited by openshift-ci bot

Loading