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

Unify Device and VirtualMachine in a single table. Differentiate between the 2 using is_virtual flag. #1178

Open
jedelman8 opened this issue Dec 17, 2021 Discussed in #1015 · 6 comments
Labels
impact: breaking change This change or feature will remove or replace existing functionality; needs to be an X.0.0 release type: feature Introduction of new or enhanced functionality to the application

Comments

@jedelman8
Copy link
Contributor

Discussed in #1015

Originally posted by slappey-ibkr October 19, 2021

Environment

  • Python version: 3.6
  • Nautobot version: 1.1.4

Proposed Functionality

Add is_virtual flag to Device model and deprecate VirtualMachine or make it a proxy model back to Device with is_virtual hardcoded to True. I know there's more to it than that but I'd love to hear everyone's thoughts on this.

Use Case

The problem with having 2 tables is that we must know if a host is physical or virtual when running remote jobs. Otherwise, we have to query both tables until we find the host we’re looking for. We must also know if a host is a physical or virtual when creating relations to objects such as IPAddress. The proposed functionality would unify all devices in a single table.

Database Changes

  1. Add is_virtual flag to Device
  2. (not required) Update VirtualMachine model to be a proxy to Device with is_virtual hardcoded to True.
  3. (not required) Create new model PhysicalMachine as a proxy to Device with is_virtual hardcoded to False.

if device.is_virtual == False:

  • device is a physical machine
  • validation of device.rack is enabled and rack is visible in UI
  • validation of device.position is enabled and rack position is visible in UI
  • device.cluster becomes the cluster this physical machine is a member of
  • device.interfaces.type choices are enabled and are visible in UI

if device.is_virtual == True:

  • device is a virtual machine
  • validation of device.rack is disabled and is hidden in UI
  • validation of device.position is disabled and is hidden in UI
  • device.cluster becomes the cluster this virtual machine is running on
  • device.interfaces.type choices are disabled, value defaults to virtual and is hidden in UI

Example Queries
device_count = Device.objects.filter(is_virtual=False).count()
vm_count = Device.objects.filter(is_virtual=True).count()

cluster = Cluster.objects.first()
cluster_devices = cluster.devices.filter(is_virtual=False)
cluster_vms = cluster.devices.filter(is_virtual=True)

External Dependencies

None


Again, I know there's more to it than this but I'd love to kick off the conversation and hear everyone's thoughts.

@jedelman8 jedelman8 added this to the v1.3.0 milestone Dec 17, 2021
@jedelman8 jedelman8 added type: feature Introduction of new or enhanced functionality to the application group: dcim labels Dec 17, 2021
@sirtux
Copy link
Contributor

sirtux commented Dec 17, 2021

Just 2 cents: Interfaces and VMInterfaces needs to be consolidated as well, then

@glennmatthews
Copy link
Contributor

@sirtux Oh absolutely, that would definitely need to be a part of this as well.

A few random thoughts:

  • I'm not sure about hardcoding interface.type to "Virtual" for virtual devices. It might actually be good to extend this to a selection of virtual interface types (e.g. virtio, vmxnet3, e1000, etc.) that could be chosen from. This also relates to Support dynamic definition of powerport and interface types #838.
  • Would be good to revisit the Cluster design as part of this. Not all virtual devices are hosted on-prem, does a Cluster association even make sense for an AWS VM? If we do keep the model as-is, we should have two different DB fields on the consolidated Device model (cluster_host and cluster_member or something like that) to allow for logical distinction between the physical and virtual meanings of a Cluster association.
  • This ties into Add ability to connect "virtual" interfaces #259 as well, allowing virtual devices to be logically cabled together.
  • Do we need to allow virtual Devices to be created with no associated Site? Or is associating them to a "virtual site" such as "us-east-1" a reasonable compromise for the design? (Tagging Add abstract location model  #774 here for sake of completness)

@jakubkrysl
Copy link

I like this! It's in line with our previous workflows, ones we could not easily transfer to Nautobot.
Since all the relations between VMs, Clusters and Devices need to be redrawn, can you please take into consideration hypervisors? That is the ability to skip Cluster and assign VM directly to Device. We have quite a few of those and it would be nice to have this supported in core instead of Custom Relationship per Cluster workaround. Thanks

@chadell
Copy link
Contributor

chadell commented Dec 21, 2021

@jedelman8 @glennmatthews related to this, and before opening a new issue, I would like to ask if you think that following this path, Device could work to model cloud networking black boxes. For example, using AWS, we could model an NAT Gateway, Internet Gateway, Transit Gateway, etc.
If this kind of modeling makes sense, I think that it could make sense to use the is_virtual flag, and also agreeing on reviewing the Cluster association.
Thinking on modeling the membership of these entities to cloud network segments (such as AWS VPC or Azure VNet), and assuming these could be mapped with the new ProviderNetwork model, it would be interesting to define this relationship as optional for the model (instead of adding a custom relationship)

Do we need to allow virtual Devices to be created with no associated Site? Or is associating them to a "virtual site" such as "us-east-1" a reasonable compromise for the design? (Tagging Add abstract location model #774 here for sake of completness)

Even the Site modeling could do the job, it seems to be a bit forced to map to a region/location from a Cloud Service Provider. I tend to see regions or availability zones (using AWS naming) as Regions (using the parent relationship between them), so for the case I propose, I believe that it would look more natural to leave it optional and take this relationship from the network segment they are linked to (using ProviderNetwork)

@cmacnevin
Copy link

Has there been any movement on this? We're quite keen to see it implemented

@bryanculver
Copy link
Member

@cmacnevin: Has there been any movement on this? We're quite keen to see it implemented

It was not committed for 2.0. Highly likely for 3.0.

Until then, what is preventing you from using Device to track VMs today?

@bryanculver bryanculver added this to the v3.0.0 milestone Apr 18, 2023
@bryanculver bryanculver added the impact: breaking change This change or feature will remove or replace existing functionality; needs to be an X.0.0 release label Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact: breaking change This change or feature will remove or replace existing functionality; needs to be an X.0.0 release type: feature Introduction of new or enhanced functionality to the application
Projects
No open projects
Status: To Groom
Development

No branches or pull requests

8 participants