Proposal for inject/eject CD-ROM

[mhenriks]

What this PR does / why we need it:

Currently, a VM has to be rebooted to change the CD-ROM media. Let's leverage the hotplug volume API to fix that gaping hole in functionality

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.

• Design: A design document was considered and is present (link) or not required
• PR: The PR description is expressive enough and will help future contributors
• Code: Write code that humans can understand and Keep it simple
• Refactor: You have left the code cleaner than you found it (Boy Scout Rule)
• Upgrade: Impact of this change on upgrade flows was considered and addressed if required
• Testing: New code requires new unit tests. New features and bug fixes require at least on e2e test
• Documentation: A user-guide update was considered and is present (link) or not required. You want a user-guide update if it's a user facing feature / API change.
• Community: Announcement to kubevirt-dev was considered

Release note:

Proposal for inject/eject CD-ROM

[kubevirt-bot]

[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 davidvossel 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:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[dankenigsberg]

Thanks for caring for this much-requested functionality.

[dankenigsberg]

Please consider alternative designs and their pros/cons. In particular, can you consider following https://github.com/kubevirt/community/blob/e4645eb/design-proposals/vm-rollout-strategy/vm-rollout-strategy.md to enable a simpler, declarative, API. According to it, the user expresses a VM spec that includes a CDROM with a specific PVC and the VM controller makes that happen.

[mhenriks]

I can address that in the design, but just to answer you inline, I think the functionality you describe can be generalized and included in an all encompassing "Declarative Volume Hotplug" design

[dankenigsberg]

I can address that in the design,

Thanks.

I think the functionality you describe can be generalized and included in an all encompassing "Declarative Volume Hotplug" design

Right, I'd love to see that. But I'd be satisfied in seeing CDROM-only as a first step.

[mhenriks]

I think just CD-ROM being declarative would be confusing to users.

To me, what is proposed here is a good first step in making this long awaited functionality available to users soon. Going full declarative is a lot more complicated. But is something I do agree that we should tackle eventually

[dankenigsberg]

Please consider pros and cons each of these options in the text of the design.

[alicefr]

Adding my 2 cents here, declarative is a nice to have, however, this type of installation operation is imperative and kind of "manual". Do we really expect this to be used with git ops?

[mw-0]

this would be great. this is a real user missing feature moving from alternatives.

[jean-edouard]

I would add an important non-goal about "config volumes", typically for service accounts.
Config volumes are ISOs but they're exposed as disks, not cdroms for some reason, so they can't be ejected/inserted.

[mhenriks]

I would add an important non-goal about "config volumes", typically for service accounts. Config volumes are ISOs but they're exposed as disks, not cdroms for some reason, so they can't be ejected/inserted.

The fact that they they are isos is more of an implementation detail to me. Do end users really know/care when it is exposed as a disk?

I state as a non-goal Support for any volume type other than DataVolume/PersistentVolumeClaim. To me that is pretty clear, but maybe I can link to all the volume types we support

[jean-edouard]

I would add an important non-goal about "config volumes", typically for service accounts. Config volumes are ISOs but they're exposed as disks, not cdroms for some reason, so they can't be ejected/inserted.

The fact that they they are isos is more of an implementation detail to me. Do end users really know/care when it is exposed as a disk?

They do know, since they are directly consuming them. And they probably do care, since that limitation prevents service account data from getting updated.

I state as a non-goal Support for any volume type other than DataVolume/PersistentVolumeClaim. To me that is pretty clear, but maybe I can link to all the volume types we support

You're right that's probably enough, thanks!

[stu-gott]

Is there a way to do this using only kubectl. In general we've been trying to avoid adding things to virtctl that can already be done entirely in kubectl.

[mhenriks]

That would be declarative hotplug, which again, I think we should do eventually.

Users are currently used to adding/removing disks in a non-declarative way via virtctl. Also, there explicit requirements from customers to have a non declarative way to inject/eject CD-ROM.

See https://issues.redhat.com/browse/CNV-7383

[stu-gott]

/hold

A request was made to compare pros/cons of doing just CD-ROM vs a more generalized API. This hold can be removed once that's addressed.

[alicefr]

@mhenriks aren't we missing the setting of the boot order for the other disks? I mean, if we eject the cdrom, how does the VM know from which disk it has to boot the next time it is restarted?