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

Closes #8356: Add virtual disk to Virtual Machines #14087

Merged
merged 46 commits into from
Nov 17, 2023

Conversation

arthanson
Copy link
Collaborator

@arthanson arthanson commented Oct 19, 2023

Fixes: #8356

introduces a new VirtualDisk model with name and size fields, instances of which must be assigned to a specific virtual machine. Retains the existing disk field on VirtualMachine.

Switched Virtual Machine list view to show Disk Size (from Virtual Disks) instead of Disk - made it default, but user can add disks back if needed. I think this makes it very clear that they should be moved.

Also put in deprecation warnings for Disk in detail / edit view:

Monosnap Editing virtual machine vm1 | NetBox 2023-10-23 11-22-17
Monosnap vm1 | NetBox 2023-10-23 11-24-08

Added Virtual Disks to end of Virtual Machine detail view:
Monosnap vm1 | NetBox 2023-10-24 10-13-32

@jeremystretch jeremystretch added the beta Concerns a bug/feature in a beta release label Oct 20, 2023
@arthanson arthanson changed the title DRAFT: 8356 vm virtual disk 8356 Add virtual disk to Virtual Machines Oct 20, 2023
@arthanson arthanson marked this pull request as ready for review October 20, 2023 21:18
@arthanson arthanson added this to the v3.7 milestone Oct 20, 2023
@ITJamie
Copy link
Contributor

ITJamie commented Oct 22, 2023

Can i suggest that we add config context to the new VirtualDisk model.
this would be useful for documenting disk type information (disk io settings, adaptor type, disk position ids, storage location info etc).

@jeremystretch jeremystretch removed the beta Concerns a bug/feature in a beta release label Oct 23, 2023
Copy link
Member

@jeremystretch jeremystretch left a comment

Choose a reason for hiding this comment

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

Great work, @arthanson! Just have some minor cleanup notes.

Also, we should consider how best to report disk space for a VM given the presence of both VirtualDisks and the disk field on VirtualMachine. E.g. it might make sense to introduce a total_disk_size property on VirtualMachine.

@jeremystretch jeremystretch changed the title 8356 Add virtual disk to Virtual Machines Closes #8356: Add virtual disk to Virtual Machines Nov 1, 2023
@amhn
Copy link
Contributor

amhn commented Nov 11, 2023

Just a minor note:
Using the GUI the URL component is named "disks" (e.g. http://localhost:8000/virtualization/disks/1/) while in the API it is named "virtual-disks" (e.g. http://localhost:8000/api/virtualization/virtual-disks/1/). Probably should be the same in both.

Copy link
Member

@jeremystretch jeremystretch left a comment

Choose a reason for hiding this comment

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

I'm going to take a stab at annotating the disk size on VM querysets.

@jeremystretch jeremystretch merged commit 549b0ea into feature Nov 17, 2023
@jeremystretch jeremystretch deleted the 8356-vm-virtual-disk branch November 17, 2023 20:03
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants