On a failing custom script, database revert doesn't work

[wvandervaart]

Plugin Version

0.5.4.

NetBox Version

4.2.9

Python Version

3.10

Steps to Reproduce

We have a custom script which creates vlans, and links them to VNI's, When the script fails on creating the VNI, because it's not unique. Normally there is a rollback which reverts all database changes from the script.
However, when we run it in a branch, the revert doesn't work, and the VLAN which is created first in the script is still in the database.

Expected Behavior

Database revert should still remove all created objects on script failure.

Observed Behavior

Not all database changes are reverted on script failure.

[arthanson]

@wvandervaart can you please provide some example script code that shows the issue?

[wvandervaart]

The log shows:

The script code is:

class AddVLAN(Script):
    class Meta:
        name = "Add VLAN"
        description = "Add a new VLAN to a site"
        commit_default = False

    site = ObjectVar(
        model=Site,   
        required=False
    )

    vlan_description = StringVar(
        label="VLAN description",
        description="VLAN description, example: MGMT VLAN TC2"
    )

    vid = IntegerVar(
        label="VLAN tag",
        description="VLAN tag, example: 20"
    )
    tenant = ObjectVar(
        model = Tenant,
        required=True
    )

    L3 = BooleanVar(
        label="L3 y/n",
        default=True

    )
    
    primaryprefix = IPNetworkVar(
        label="Primary Prefix",
        required=False,
        description="Prefix for this VLAN, example: 10.10.10.0/24"
    ) 

    primarygateway = IPAddressWithMaskVar(
        label="Primary Anycast gateway",
        required=False,
        description="Anycast gateway for this VLAN, example: 10.10.10.1/24"
    )

    secondaryprefix = IPNetworkVar(
        label="Secondary Prefix",
        required=False,
        description="Prefix for this VLAN, example: 10.10.10.0/24"
    )

    secondarygateway = IPAddressWithMaskVar(
        label="Secondary Anycast gateway",
        required=False,
        description="Anycast gateway for this VLAN, example: 10.10.10.1/24"
    )

    vrf = ObjectVar(
        model=VRF,
        required=False
    )

    acl = ObjectVar(
        model=ConfigContext,
        required=False,
        query_params = {
            'tag': 'acl'
            }
    )

    def run(self, data, commit):
        # if site is chosen, get VNIrange, otherwise set vnirange to 0
        if data['site'] == None:
            vnirange = 0
        else:
            vnirange = data['site'].custom_field_data["VNIRANGE"]
        tenant = data['tenant']
        # create vlan object
        vlan = VLAN(
            site=data['site'],
            vid=data['vid'],
            name="VLAN"+str(data['vid']),
            description=data['vlan_description'],
            custom_field_data = {"LAYER3": data['L3']},
            tenant=tenant
        )
        vlan.full_clean()
        vlan.save()
        self.log_success(f"VLAN: {vlan} aangemaakt")
        # if the L3 is checked, set pfx and ip
        if data['L3'] == True:
            if data['primaryprefix'] != None and data['primarygateway'] != None:
                # create pfx object
                pfx = Prefix(
                    prefix=data['primaryprefix'],
                    description=data['vlan_description'],
                    scope=vlan.site,
                    vlan=vlan,
                    vrf=data['vrf'],
                    tenant=tenant
                )
                pfx.full_clean()
                pfx.save()
                # create AC gateway object
                ip = IPAddress(
                    address=data['primarygateway'],
                    status=IPAddressStatusChoices.STATUS_RESERVED,
                    role=IPAddressRoleChoices.ROLE_ANYCAST,
                    vrf=data['vrf'],
                    tenant=tenant
                )
                ip.full_clean()
                ip.save()
                self.log_success(f"Prefix {pfx} created, and attached to VLAN {vlan}")
                self.log_success(f"Anycast GW added:{ip}")
            if data['secondaryprefix'] != None and data['secondarygateway'] != None:
                pfx = Prefix(
                    prefix=data['secondaryprefix'],
                    description=data['vlan_description'],
                    scope=vlan.site,
                    vlan=vlan,
                    vrf=data['vrf'],
                    tenant=tenant
                )
                pfx.full_clean()
                pfx.save()
                ip = IPAddress(
                    address=data['secondarygateway'],
                    status=IPAddressStatusChoices.STATUS_RESERVED,
                    role=IPAddressRoleChoices.ROLE_ANYCAST,
                    vrf=data['vrf'],
                    tenant=tenant
                )
                ip.full_clean()
                ip.save()
                self.log_success(f"Prefix {pfx} created, and attached to VLAN {vlan}")
                self.log_success(f"Anycast GW added:{ip}")
        if data['acl'] != None:
            vlan.custom_field_data['ACL'] = data['acl'].id
            vlan.full_clean()
            vlan.save()
        # add vlan-id to VNI range.
        vnitag = "VNI"+str((vnirange+data['vid']))
        # create VNI l2vpn
        vni = L2VPN(
            name=vnitag,
            slug=vnitag.lower(),
            type=L2VPNTypeChoices.TYPE_VXLAN,
            identifier=vnirange+data['vid'],
            tenant=tenant
        )
        vni.full_clean()
        vni.save()
        self.log_success(f"L2VPN created: {vni}")
        # Add VLAN to L2VPN
        term = L2VPNTermination(
            l2vpn=vni,
            assigned_object=vlan
        )
        term.full_clean()
        term.save()
        self.log_success(f"VLAN {vlan} added to L2VPN: {vni}")

[wvandervaart]

The branch diff shows that the VLAN was created and not reverted

[arthanson]

@wvandervaart The script isn't usable as is, it is missing a lot of import statements and I'm not sure if other issues.

[wouterdebruijn]

Running into the same issue on different version, for which I took the time to create a small test script which still shows the issue for me.

NetBox version: 4.4.1
Netbox Branching version: 0.7.1

Reproduction steps:
Script ran on fresh install of Netbox. Requires one device to be created.

Add the script in Netbox:

from typing import Any

from extras.scripts import Script, ObjectVar
from dcim.models import Device


class BranchDummy(Script):
    class Meta:
        name = "Branch Dummy"
        description = "Test for branching"
        commit_default = True
        scheduling_enabled = False

    device = ObjectVar(
        model=Device,
        label="Device",
    )

    def run(self, data: Any, commit: bool) -> str:  # type: ignore
        device = data["device"]

        device.name = f"Branch Dummy {device.name}"

        device.full_clean()
        device.save()

        self.log_info(f"Device [{device.name}]({device.get_absolute_url()}) updated")

Create a new branch and activate the branch.
Run the script with the without the "Commit Changes" box checked on execution.

Expected behavior:
Device name change should be revered when viewing the device object after script execution. (This is the case on the main branch)

Observed behavior:
Device name is changed and now contains the prefix Branch Dummy