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

Allow the enabled field to be set on InterfaceTemplates #11440

Closed
kkthxbye-code opened this issue Jan 9, 2023 · 10 comments · Fixed by #11458
Closed

Allow the enabled field to be set on InterfaceTemplates #11440

kkthxbye-code opened this issue Jan 9, 2023 · 10 comments · Fixed by #11458
Assignees
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Milestone

Comments

@kkthxbye-code
Copy link
Contributor

NetBox version

v3.4.2

Feature type

Change to existing functionality

Proposed functionality

Allow the enabled field to be set on InterfaceTemplates added to DeviceTypes or ModuleTypes. The implementation would mirror the mgmt_only field which is currently supported.

Use case

DeviceTypes might be configured with disabled interfaces by default. In some cases ports might be configurable at different speeds depending on the installed SFP which changes the name of the interface on the device. Currently we have both combinations on the DeviceType, e.g.:

GigabitEthernet0/0/0/20
TenGigE0/0/0/20

Having the option to disabled one of the interfaces by default would help manage this.

Database changes

Add enabled BooleanField to InterfaceTemplate.

External dependencies

None

@kkthxbye-code kkthxbye-code added the type: feature Introduction of new functionality to the application label Jan 9, 2023
@kkthxbye-code
Copy link
Contributor Author

I implemented it here to see if there were any hard stuff needing to be done, there wasn't.

develop...kkthxbye-code:netbox:11440-add-enabled-interfacetemplate

If accepted, it might need to be targeted v3.5 as there's db and API changes, however existing DeviceTypes in the devicetype-library should be compatible as the enabled field defaults to True.

@PieterL75
Copy link
Contributor

Why create two interfaces, and not rename the one interfaces ?
The slot/mod/port/forgotwhattheyare should be the identifiers...
I even tend to just put Eth0/0/0/20 in netbox, and forget about all the rest,,,

(also to tackle the interface naturalization sorting that messes some use cases up)

@kkthxbye-code
Copy link
Contributor Author

@PieterL75 - Our users prefer to have both interfaces and disable the one not being used. It also helps with custom scripts where the user has to choose an interface, as it allows both configurations to be shown and we can do the enabling of the chosen interface.

The slot/mod/port/forgotwhattheyare should be the identifiers...
I even tend to just put Eth0/0/0/20 in netbox, and forget about all the rest,,,

I'm not sure I understand here. So while the device uses the following names:

GigabitEthernet0/0/0/20
TenGigE0/0/0/20

You would just use Eth0/0/0/20 for both configurations? If that's the case, that would not work for us. We rely on netbox representing the real world to allow automation.

Total disclosure, I'm not really a networking guy, so the request is as explained by our users at my place of work. There might very well be a better/more correct way of doing it. I still think the feature makes sense though, at least as much as mgmt_only being avalible on interface templates. I'll ask for feedback form our users.

@sleepinggenius2
Copy link
Contributor

We don't create duplicate interfaces like in @kkthxbye-code's example, but still face the same problem. In our use case, devices are often deployed with the majority of their interfaces disabled. When we just had DeviceTypes, this was annoying, but manageable, to go into a device after it was instantiated, do a select all, deselect the handful of interfaces that were being used for uplinks, then do a bulk edit to set them all as disabled. The addition of modules make this significantly more painful, as we have a number of high-density cards that have maybe 48 interfaces on them and so now when you slot in that module in NetBox, you have to find just those interfaces that it added in order to select and disable them. This had led to a number of problems, and ultimately workarounds, to be able to support automated provisioning without leaving a bunch of interfaces enabled that generate alarms in our monitoring application for being Up/Down, when the engineers inevitably forget to manually disable the interfaces they aren't using.

@jeremystretch
Copy link
Member

This is largely going to be dependent on caveats across different platforms, but I want to clarify that the enabled field should only be used to document an interface's administrative state, and not as a hint for any other function. With regard to interfaces that get renamed based on their configured speed (as in the example above), only one or the other name should be present on the device in NetBox. The exception would be if the device normally represents both variations in its native configuration; again, this is all platform-dependent.

That said, I'm in favor of adding an enabled field to the InterfaceTemplate model for the purpose of disabling interfaces upon instantiation on a new device or module where needed.

@jeremystretch jeremystretch added the status: under review Further discussion is needed to determine this issue's scope and/or implementation label Jan 10, 2023
@PieterL75
Copy link
Contributor

This feels like a customfield solution. Add a Field that holds the info you need, so that it works the way you want..

@kkthxbye-code
Copy link
Contributor Author

@PieterL75 - Custom fields are not replicated.

@PieterL75
Copy link
Contributor

I must have missed the part that he requested this to be added to the template.. you're right, CF will not cut it then

@kkthxbye-code
Copy link
Contributor Author

I realize this is still under review, but I wanted to just create the PR as the code was done as a quick PoC already. If the FR is ultimately rejected it can easily be closed.

@jeremystretch
Copy link
Member

This will need to wait for v3.5 as it introduces a new database field. But I'm fine with tagging it as a milestone.

@kkthxbye-code kkthxbye-code 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 Jan 12, 2023
@kkthxbye-code kkthxbye-code added this to the v3.5 milestone Jan 12, 2023
jeremystretch added a commit that referenced this issue Jan 24, 2023
jeremystretch added a commit that referenced this issue Feb 20, 2023
jeremystretch added a commit that referenced this issue Feb 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 19, 2023
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: feature Introduction of new functionality to the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants