netbox icon indicating copy to clipboard operation
netbox copied to clipboard

Also adopt child interfaces when adding modules

Open sleepinggenius2 opened this issue 2 years ago • 11 comments

NetBox version

v3.2.3

Feature type

Change to existing functionality

Proposed functionality

The feature that was added in #9280 is great, but it only seems to adopt the interfaces that would have been created by a template, leaving their children unassociated. It would be nice to have the children associated as well, to avoid a second pass to clean them up. I don't see a need for a separate option to enable this functionality, as I can't think of a use case for disabling it, but I would also be fine if there was an additional checkbox. If I am misunderstanding the intention of how the module association is supposed to work (i.e. only parent interfaces, because I can't templatize child interfaces today), then please let me know.

Use case

Since the addition of the module functionality, we've been working to add the appropriate module bays and modules to existing devices, then to associate the existing interfaces to those new modules, which is an activity that I expect a lot of organizations are doing. We end up with a lot of sub-interfaces on our routers, so the work that was done in #9280 was a great start, but we're still finding that we're having to spend a lot of time getting all the sub-interfaces associated as part of this process.

Database changes

None

External dependencies

None

sleepinggenius2 avatar May 13 '22 16:05 sleepinggenius2

Would you mind splitting this up into two seperate feature requests, with this issue being for adopting child interfaces?

Regarding adopting child interfaces, the logic seems sound to me. As child interfaces can be added to a module after creation, I see no reason that the adoption feature shouldn't also adopt child interfaces.

If I am misunderstanding the intention of how the module association is supposed to work (i.e. only parent interfaces, because I can't templatize child interfaces today), then please let me know.

@jeremystretch can you comment on the intention when modules were implemented?

I'll implement this if Jeremy agrees with the soundness of the idea.

kkthxbye-code avatar May 13 '22 17:05 kkthxbye-code

I have reverted my changes back to the original FR and opened #9361 to address the second part.

sleepinggenius2 avatar May 13 '22 21:05 sleepinggenius2

I don't see any reason to associate child interfaces with the module of their parent, as the child interfaces must be virtual. They are dependent upon their parents, which are in turn dependent upon the module.

jeremystretch avatar May 16 '22 13:05 jeremystretch

A reason I can see, is that child interfaces do not get removed when removing the module if the child interface is not associated with the module.

kkthxbye-code avatar May 16 '22 13:05 kkthxbye-code

IMO it seems like they should be. I'm not sure why that's not the case; after skimming #1519 I didn't see any mention of the intended behavior upon deletion of the parent interface.

jeremystretch avatar May 16 '22 14:05 jeremystretch

I however still think it makes sense to associate virtual child interfaces with the installed module directly. I guess it depends on what you expect when going to the installed module and pressing the interface count:

image

Is there any reason to only expect physical interfaces when filtering by module?

They are dependent upon their parents, which are in turn dependent upon the module.

If this is always the case, shouldn't the module field on child interfaces either be disabled or inherited from the parent and immutable?

kkthxbye-code avatar May 16 '22 14:05 kkthxbye-code

It probably makes sense to force nullification of the module field for child interfaces. Otherwise we would have to provide additional validation to ensure it's always kept in sync with the parent interface.

jeremystretch avatar May 16 '22 15:05 jeremystretch

@sleepinggenius2 - Do you have any input here? Do you have a usecase for explictily assigning virtual child interfaces to the module.

Removing child interfaces when removing modules make sense imo, but I'm not sure if it gets confusing as we are not currently removing child interfaces in general when deleting the parent.

Not sure what the best solution here is.

kkthxbye-code avatar May 18 '22 05:05 kkthxbye-code

I agree that I think it makes sense to remove child interfaces when the parent is removed. I honestly assumed it was already set up that way. It would definitely be really confusing to remove a module and just end up with all these orphaned child interfaces that don't really mean anything on their own. I have similar issues with just disabling a parent interface and all the child interfaces still show enabled. In the SNMP world they could still have an admin state of up, but at least the operational state would be lower layer down to key off of.

From a data normalization standpoint, I also agree with Jeremy, but I think it would be nice to have the module/module bay column in the interfaces table still show the module that the interface is associated with, even if it's through a parent interface.

One of the biggest struggles that I have is that child interfaces don't always sort under their parent interface. If there was a way to make sure that child interfaces always sort under their parent, maybe even indented like child prefixes, that would be amazing and likely eliminate a number of different problems. I see #9368 has recently been opened related to this, so that may be sufficient, if implemented.

sleepinggenius2 avatar May 18 '22 21:05 sleepinggenius2

Not sure where to go from here.

Deleting child interfaces when deleting the parent should be as easy as setting the on_delete to cascade.

I don't see how we can force child interfaces to have a null value for module in a non-messy way. I guess we could just do it in validation, but it would potentially cause confusion when creating/editing an interface if we don't dynamically disable the module field in the form when a parent is selected.

Both of these are not really what you request though. I guess this FR should just be closed, and then I can create a FR for deleting child interfaces when removing parent. I'll let Jeremy decide as I kinda like the original FR.

kkthxbye-code avatar May 25 '22 09:05 kkthxbye-code

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Do not attempt to circumvent this process by "bumping" the issue; doing so will result in its immediate closure and you may be barred from participating in any future discussions. Please see our contributing guide.

github-actions[bot] avatar Aug 06 '22 04:08 github-actions[bot]

This issue has been automatically closed due to lack of activity. In an effort to reduce noise, please do not comment any further. Note that the core maintainers may elect to reopen this issue at a later date if deemed necessary.

github-actions[bot] avatar Sep 05 '22 04:09 github-actions[bot]