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

Unable to revert a branch that removed a device. #136

Open
ewscott9 opened this issue Sep 17, 2024 · 9 comments
Open

Unable to revert a branch that removed a device. #136

ewscott9 opened this issue Sep 17, 2024 · 9 comments
Assignees
Labels
app: branching type: bug A confirmed report of unexpected behavior in the application

Comments

@ewscott9
Copy link

ewscott9 commented Sep 17, 2024

Plugin Version

0.5.0

NetBox Version

NetBox Community v4.1.0 (2024-09-03) (In Netbox-Docker-3.0.1)

Python Version

3.12.3

Steps to Reproduce

On main branch:

Create Device Role "test":
name,slug,color
"test","test",888888

Create Manufacturer "test":
name,slug
"test","test"

Create Device Type "test":
manufacturer,model,slug,u_height
"test","test","test",1.0

Create Site "test":
name,slug,status
"test","test",active

Create Device "test" and "test2":
role,manufacturer,device_type,status,site,name
"test","test","test",active,"test","test"
"test","test","test",active,"test","test2"

Create Interfaces on the 2 test devices:
device,name,type
test,1,1000base-tx
test2,1,1000base-tx

Create Cable:
side_a_device,side_a_type,side_a_name,side_b_device,side_b_type,side_b_name,label
test,dcim.interface,1,test2,dcim.interface,1,"test:1,test2:1"

Create branch "test":
name: test

On Branch "test":

Activate branch "test"
Delete "test" device
Revert "test" branch

Expected Behavior

The branch should revert.

Observed Behavior

It fails when I try to revert the branch.
I get the following log for the revert branch job:

{
    "log": [
        "Reverting branch test (branch_zwiumn32)",
        "Found 3 changes to revert",
        "Setting branch status to reverting",
        "Undoing change DCIM | device test deleted by ewscott using default",
        "Restoring device <DeserializedObject: dcim.Device(pk=16)>",
        "Undoing change DCIM | cable termination Cable test:1,test2:1 to 1 deleted by ewscott using default",
        "Restoring cable termination <DeserializedObject: dcim.CableTermination(pk=23)>",
        "'NoneType' object has no attribute 'type'"
    ]
}

Seems like it's failing to recreate a cable connection

@ewscott9 ewscott9 added the type: bug A confirmed report of unexpected behavior in the application label Sep 17, 2024
@ewscott9
Copy link
Author

I tested, and I don't seem to have a problem removing a single cable termination, and reverting it. I'll have to run more testing, but seems to be an issue with reverting after merging a branch that deleted a device.

@jeremystretch
Copy link
Contributor

@ewscott9 please modify your post above to include exact reproduction steps, beginning with the creation of any objects to be deleted.

@jeremystretch jeremystretch added the status: revisions needed This issue requires additional information to be actionable label Sep 18, 2024
@ewscott9
Copy link
Author

ewscott9 commented Sep 18, 2024

Thanks @jeremystretch I updated the reproduction steps.

I did some additional testing, and it will revert a device delete unless there is a cable connection between 2 devices.

@ewscott9
Copy link
Author

I'm also getting an error when trying to merge a branch after I create a device that has an interface. I get a DeserializationError("['Invalid WWN format: ']: (dcim.interface:pk=180) field_value was ''") error. Might be related.

@jeremystretch jeremystretch self-assigned this Sep 23, 2024
@jeremystretch jeremystretch removed the status: revisions needed This issue requires additional information to be actionable label Sep 23, 2024
@jeremystretch
Copy link
Contributor

The root issue here appears to be the generic foreign key termination on NetBox's CableTermination model. Although the termination_type and termination_id fields are populated for the object when reverting the branch, the GFK itself is not:

Traceback (most recent call last):
  File "/home/jstretch/projects/nbl-netbox-branching/netbox_branching/models/branches.py", line 412, in revert
    change.undo(logger=logger)
  File "/home/jstretch/projects/nbl-netbox-branching/netbox_branching/models/changes.py", line 91, in undo
    instance.object.full_clean()
  File "/home/jstretch/projects/netbox/venv/lib/python3.10/site-packages/django/db/models/base.py", line 1536, in full_clean
    self.clean()
  File "/home/jstretch/projects/netbox/netbox/dcim/models/cables.py", line 341, in clean
    if self.termination_type.model == 'interface' and self.termination.type in NONCONNECTABLE_IFACE_TYPES:
AttributeError: 'NoneType' object has no attribute 'type'

The best approach here is probably to ensure any GFKs are populated on each object during deserialization, much as a form or API serializer would.

@ewscott9
Copy link
Author

The best approach here is probably to ensure any GFKs are populated on each object during deserialization, much as a form or API serializer would.

How would I go about doing that?

@jeremystretch
Copy link
Contributor

@ewscott9 sorry, that was a note on a potential solution for the bug, not something that you can do as a user.

@ewscott9
Copy link
Author

No problem. Thanks for looking into the issue.

@arthanson
Copy link
Contributor

Another issue here is that the objects are serialized in the wrong order:

<class 'dcim.models.devices.Device'>
<class 'dcim.models.cables.CableTermination'>
<class 'dcim.models.device_components.Interface'>

The interface needs to be deserialized after the CableTermination

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app: branching type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Development

No branches or pull requests

4 participants