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

Introduce a separate model for virtual machine interfaces #4721

Closed
jeremystretch opened this issue Jun 4, 2020 · 5 comments
Closed

Introduce a separate model for virtual machine interfaces #4721

jeremystretch opened this issue Jun 4, 2020 · 5 comments
Assignees
Labels
status: accepted This issue has been accepted for implementation type: housekeeping Changes to the application which do not directly impact the end user
Milestone

Comments

@jeremystretch
Copy link
Member

Proposed Changes

  • Create a new abstract Interface model to house fields which are common to both device and VM interfaces
  • Update the DCIM Interface model to inherit from the abstract model
  • Create a new Interface model under the virtualization app
  • Update the ForeignKey field on the IPAddress model accordingly

Justification

There are some fields on the current Interface model which don't really apply to virtual machine interfaces, namely mgmt_only and cable. #4615 would introduce a physical label field, which likewise does not apply. Similarly, VM interfaces are restricted to only virtual types. Using a separate model would mitigate the need for special validation when handling VM interfaces. It also greatly simplifies the definition and use of forms, serializers, etc.

There are some downsides to the proposed change that should be pointed out:

  • It will no longer be possible to view all interfaces via a single UI page or REST API endpoint: There will be separate views for device and VM interfaces.
  • The ForeignKey field to the current Interface model on IPAddress will need to be changed. This will presumably be undertaken using separate (ForeignKey fields for the device and VM interface classes, unless we want to revisit multi-table inheritance.)
@jeremystretch jeremystretch added status: under review Further discussion is needed to determine this issue's scope and/or implementation type: housekeeping Changes to the application which do not directly impact the end user labels Jun 4, 2020
@a31amit
Copy link

a31amit commented Jun 5, 2020

VM interfaces are restricted to only virtual types. Using a separate model would mitigate the need for special validation when handling VM interfaces.

A VM could have physical interface or devices if it been done through PCI passthrough which makes physical PCI devices not directly seen or used in base hypervisor os but directly in a virtual machine as physical attached and when using scripts or automated discovery inside OS will see those interfaces showed as the physical interface but updates on netbox service might failed if they don't allow physical types of interfaces.

Additionally, if Virtual Machine virtual interfaces can be connected to physical hypervisor device interfaces. In a general production environment, virtual machine interfaces usually tied to the specific physical interfaces of hypervisor so that hypervisor network traffic remains separate from virtual machine network traffic. For a user to know which virtual to the physical path has been taken could help when someone doing troubleshooting and wanted to end-to-end network connection path.

@jeremystretch
Copy link
Member Author

@a31amit what you're describing is outside the scope of this issue. This proposal does not change any of the existing interface types or validation; it is merely for internal purposes.

@a31amit
Copy link

a31amit commented Jun 7, 2020

@jeremystretch thanks for the confirmation

@jeremystretch
Copy link
Member Author

Locking as this issue is for some reason generating a lot of off-topic comments.

@netbox-community netbox-community locked as off-topic and limited conversation to collaborators Jun 15, 2020
@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Jun 15, 2020
@jeremystretch jeremystretch added this to the v2.9 milestone Jun 23, 2020
@jeremystretch
Copy link
Member Author

Implemented by PR #4781

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: housekeeping Changes to the application which do not directly impact the end user
Projects
None yet
Development

No branches or pull requests

2 participants