Cnv bz1875725 containerdisks

[aburdenthehand]

Adding a procedure to cover BZ#1875725 request for how to create a containerDisk. Since this doesn't belong anywhere it needed a new assembly, which also required a concept to cover containerDisk volumes.
Also moved a para re: ephemeral nature of containerDisks into an adomintion in the storage type table to increase visibility and added the new assembly to the 'Additional resources' section of 'creating vms'

Additional info taken from https://kubevirt.io/user-guide/#/creation/disks-and-volumes?id=containerdisk

New assembly: https://cnv-bz1875725-containerdisk--ocpdocs.netlify.app/openshift-enterprise/latest/virt/virtual_machines/advanced_vm_management/virt-using-containerdisk-volumes.html
Updated storage types table: https://cnv-bz1875725-containerdisk--ocpdocs.netlify.app/openshift-enterprise/latest/virt/virtual_machines/virt-create-vms.html#virt-vm-storage-volume-types_virt-create-vms

@fabiand and @dankenigsberg please review.
Note that the new assembly is under 'Advanced virtual machine management' rather than 'Virtual machine disks' due to the concerns Fabian had raised earlier.

[openshift-docs-preview-bot]

The preview will be available shortly at:

• https://cnv-bz1875725-containerdisk--ocpdocs.netlify.com/

[dankenigsberg]

This looks good to me, clearly better than my own #25289

[aburdenthehand]

@dankenigsberg If it's raising an eyebrow for you then it's not as clear as it should be. I'll update.
Just to clarify - the only negative in that scenario is that migration is slower? There's no additional risk of data loss, right?

[dankenigsberg]

Well, we already covered data loss in the former sentence. So yes the problem is tmp storage being wasted (a virtual filesystem of 30GiB can cost 30GiB of extra storage on the node) which then needs to be migrated with the VM to the destination node. Not fun. I just don't find the right words to express it.

[dankenigsberg]

Well, it's not the entire disk, but only the layer of diffs from the base images shipped in the container.

[fabiand]

We should also add that in the case of a powerouttage the data will be lost.

[aburdenthehand]

• Have s/entire disk/data/ for accuracy. I don't think we need to be more specific than that here, but I certainly appreciate the clarification.
• Added "Additionally, all data is lost if the node loses power or otherwise shuts down unexpectedly."

[fabiand]

A few remarks

[fabiand]

We should also add that in the case of a powerouttage the data will be lost.

[fabiand]

Some people could say this is to greedy.
Under certain circumstances the image will remain, but the statement above is safe, thus even if greedy, let's keep it.

[dankenigsberg]

@davidvossel when I tried this, I got myself an containerDisk; but running it produced

failed to get image info: failed to invoke qemu-img: exit status 1: 'qemu-img: Could not open '/var/run/kubevirt/container-disks/disk_0.img': Could not open '/var/run/kubevirt/container-disks/disk_0.img': Permission denied '

can you tell why? How can I chmod the ADDed file to 0444 in the Dockerfile?

[davidvossel]

This command ADD --chown=107:107 disk.qcow2 /disk/ worked without issue for me. I'm not sure what you're experiencing here.

[davidvossel]

i think you're correct that we should be setting 444 on disk now. This technically works without 444 if we get the ownership correct, but that's because the ownership allows us to modify the permissions to 444 after the fact.

The only way I'm aware of to set 444 permissions is to do it external of the scratch container's Dockerfile. ADD only has chown built in and the scratch container itself doesn't have the chmod tool internally.

So if i set 444 permissions on the file before running the docker build or podman build then it that works.

[davidvossel]

i think you're correct that we should be setting 444 on disk now. This technically works without 444 if we get the ownership correct, but that's because the ownership allows us to modify the permissions to 444 after the fact.

forget what I said there. 444 shouldn't be necessary. simply setting the owner to 107:107 and ensuring that the file has read permissions for the owner user is the minimum we need right now. The mutation of the permissions to 444 at runtime is something that we shouldn't be doing and will get addressed. that doesn't impact us here though.

[dankenigsberg]

@davidvossel I don't understand the bottom line. The the suggested Dockerfile did not produce a working container for me. What should be done to it to make it work?

[fabiand]

@davidvossel what is needed to make the CD work?

[davidvossel]

@davidvossel what is needed to make the CD work?

Our testsuite verifies 444 mode and 107:107 ownership for a virtualmachine image in the disk/ dir of a container. That's what we verify.

[davidvossel]

@dankenigsberg @fabiand

Would it make more sense for us to talk about ContainerDisks in the context of importing to a PVC with as a source for a DataVolume rather than using the ephemeral ContainerDisks directly attached to the VMI?

Maybe documenting both ways makes sense to a certain extent. I think the message is a bit confusing though. We talk about ContainerDisks being ephemeral, but then we are goign to use ContainerDisks as a source for Persistent VMs by importing the contents to a PVC in other use cases.

[davidvossel]

This command ADD --chown=107:107 disk.qcow2 /disk/ worked without issue for me. I'm not sure what you're experiencing here.

[dankenigsberg]

Would it make more sense for us to talk about ContainerDisks in the context of importing to a PVC with as a source for a DataVolume rather than using the ephemeral ContainerDisks directly attached to the VMI?

Maybe documenting both ways makes sense to a certain extent. I think the message is a bit confusing though. We talk about ContainerDisks being ephemeral, but then we are goign to use ContainerDisks as a source for Persistent VMs by importing the contents to a PVC in other use cases.

@davidvossel I think that for cdrom and other immutable filesystems, direct ContainerDisks make sense. We certainly allow them and document them for a very long time. In this PR we only make the documentation clearer, by actually explaining what a containerDisk is and what are the drawbacks of using it.

Could it be that you are suggesting to add ContainerDisks as a possible source for DataVolumes? I https://docs.openshift.com/container-platform/4.5/virt/virtual_machines/importing_vms/virt-importing-virtual-machine-images-datavolumes.html

I'd appreciate if you can suggest a draft PR, as I'm not sure I follow.

[davidvossel]

Could it be that you are suggesting to add ContainerDisks as a possible source for DataVolumes? I https://docs.openshift.com/container-platform/4.5/virt/virtual_machines/importing_vms/virt-importing-virtual-machine-images-datavolumes.html

yes, I'm referring to importing containerDisks via DataVolumes to a PVC.

Do we want users to make the connection that they can Import their containerDisks onto PVCs? From reading through what we have so far, I wouldn't draw that connection.

Without being able to draw the connection between containerDisks and DataVolumes, it's unclear how someone would know how to consume something like our RHEL 8.x containerDisk image using persistent storage. Am I missing something?

[dankenigsberg]

Am I missing something?

I think you do, I miss it too, @davidvossel. I think that we should add a section/example about importing data from a containerDisk on top of the suggested text of this PR. I would really appreciate if you can suggest a text (and location) for where it

[dankenigsberg]

@davidvossel I think that this PR, as it is, is instrumental to warning about the problems of containerDisks as well as how to create them. Would you please approve them and create a follow-up item about further improvements?

[aburdenthehand]

@davidvossel @dankenigsberg - just to be clear for my own sake, when you say 'importing a containerDisk into a DataVolume', that is actually importing a virtual machine image into a DataVolume, right?
There's not a use case for creating a containerDisk, and then importing that containerDisk into a DataVolume.

If so, maybe the solution to this is to link out to the 'Importing a VM image to a DataVolume' (that Dan linked to earlier) after we've warned folks about the limitations of using containerDisks.

[fabiand]

There's not a use case for creating a containerDisk, and then importing that containerDisk into a DataVolume.

There is: We call it default OS image flow ("golden image flow"), and we are looking at introducing it.
Such a flow is leveraging the strengths of container based delivery, and smart storage.

IIUIC Then David is suggesting to not explain how to use containerDisks for VM consumption (speak attach containerDisks to a VM), instead focus on how containerDisks can be used as a source for DataVolumes.

Why? In order to get and keep users on track to leverage the primary (PVC based) storage story of CNV.

[fabiand]

@davidvossel what is needed to make the CD work?

[fabiand]

I'd appreciate if we can put a disclaimer here that the use-case for containerDisks is narrow, and that the general recommendation is to use CDI et al for VM storage.

[aburdenthehand]

I think the updated concept (modules/virt-about-container-disks.adoc) satisfies this request now

[aburdenthehand]

@fabiand Thanks for the info!

There's not a use case for creating a containerDisk, and then importing that containerDisk into a DataVolume.

Sorry, this was intended as part of the question I was asking, but I see now that it reads like a declarative statement.

IIUIC Then David is suggesting to not explain how to use containerDisks for VM consumption (speak attach containerDisks to a VM), instead focus on how containerDisks can be used as a source for DataVolumes.

This came up in email yesterday and we're scoping the related stories (in KNIP-258) in our docs planning meeting today.
Likely I will amend the procedure in this PR so that it is not specific to preparing the image/pushing to registry for a containerDisk volume. I can also add a disclaimer as suggested and link to the content that will be developed in KNIP-258 - creating a DataVolume from a container image using 'source: registry:'

[davidvossel]

sorry for the delay in getting back to this.

@davidvossel I think that this PR, as it is, is instrumental to warning about the problems of containerDisks as well as how to create them. Would you please approve them and create a follow-up item about further improvements?

@dankenigsberg I'm in the middle of a big discussion about the future of persistent container disks. we're getting close to a conclusion. I want to make sure we finalize that discussion before I commit to how we word our documentation as it could impact our recommendation here.

[aburdenthehand]

@davidvossel @dankenigsberg
I either have great or terrible timing then. #26591 has been created to satisfy the case of Importing container disks into DVs.
Build link here: https://cnv-1403-containerdisk-source--ocpdocs.netlify.app/openshift-enterprise/latest/virt/virtual_machines/importing_vms/virt-importing-container-disk-with-datavolumes.html

I'll wait to hear back from y'all before moving these further.

[davidvossel]

I'll wait to hear back from y'all before moving these further.

sorry if that previous comment caused any confusion. We're on target for documenting containerDisks and the DataVolume import flow. There's no technical change to that direction at the moment. The conversation I was referencing is mostly concluded. It won't impact this documentation.

[ousleyp]

Mostly just nits and small fixes. Most of my comments have to do with backticks and the passive voice. There are a couple of things rendering incorrectly that should be fixed. Overall, it's looking good.

[aburdenthehand]

/cherry-pick enterprise-4.5

[aburdenthehand]

/cherry-pick enterprise-4.6

[aburdenthehand]

/cherry-pick enterprise-4.7

[openshift-cherrypick-robot]

@aburdenthehand: #25359 failed to apply on top of branch "enterprise-4.5":

Applying: Adding a procedure to cover BZ#1875725 request for how to create a containerDisk. During development and review this overlapped with [CNV-1403](https://issues.redhat.com/browse/CNV-1403) and mutated some. This now covers the concept of container disks, with adequate warnings about using containerDisk volumes, and the procedure for preparing container disks.
.git/rebase-apply/patch:212: trailing whitespace.
* xref:../../virt/virtual_machines/virtual_disks/virt-using-container-disks-with-vms.adoc#virt-using-container-disks-with-vms[Prepare a container disk] before adding it to a virtual machine as a `containerDisk` volume. 
.git/rebase-apply/patch:58: new blank line at EOF.
+
warning: 2 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	_topic_map.yml
M	virt/virtual_machines/virt-create-vms.adoc
Falling back to patching base and 3-way merge...
Auto-merging virt/virtual_machines/virt-create-vms.adoc
CONFLICT (content): Merge conflict in virt/virtual_machines/virt-create-vms.adoc
Auto-merging _topic_map.yml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Adding a procedure to cover BZ#1875725 request for how to create a containerDisk. During development and review this overlapped with [CNV-1403](https://issues.redhat.com/browse/CNV-1403) and mutated some. This now covers the concept of container disks, with adequate warnings about using containerDisk volumes, and the procedure for preparing container disks.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick enterprise-4.5

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/test-infra repository.

[openshift-cherrypick-robot]

@aburdenthehand: new pull request created: #27015

In response to this:

/cherry-pick enterprise-4.6

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/test-infra repository.

[openshift-cherrypick-robot]

@aburdenthehand: new pull request created: #27016

In response to this:

/cherry-pick enterprise-4.7

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/test-infra repository.