Netbox randomly doesn't commit changes unless "Commit changes" box is reticked

[Cennepal]

Hello, I'm having a very annoying issue with one of my scripts currently. Source code here.

The title already says it and I'm kinda lost on what to do. The script works absolutely fine, the code throws no errors at me. However, it just randomly throws me an "Database changes have been reverted automatically" and doesn't commit the data. Or, with larger scans (~30 subnets), there is no revert message at all, the log looks like the script did its function, but the data still isn't commited.

The only reliable 'fix' for this that I found, is manually reticking the "Commit changes" box before execution, but that's just not optimal for my usecase. All of my other scripts work fine, this one is the only one with this issue.

Any thoughts?

[candlerb]

Netbox version?

I wonder if your script is raising an uncaught exception sometimes. But then (a) I'd have thought that would be logged, and (b) unticking and ticking the box wouldn't make any difference.

I can't see any mechanism by which (b) would make any difference, but if you use your browser's console to capture the POST data in both cases (i.e. once without touching the box, and unticking/ticking the box), and you can show a difference between the two, then that could be investigated further.

One thing that looks potentially non-determinstic in your script is reverse_dns_lookup(). If you can change this to return a static value, or a value derived directly from the ip address without calling gethostbyaddr, does that make a difference? I'm thinking that maybe some DNS queries are failing, e.g. when TTL expires.

[Cennepal]

Ah, sorry. Just checked and yes it's different. Also it seems like my earlier fail with the "dummyname" was due to using "Run Again", which seems to fail 100% of the time, even with commit_default set to True. So it probably is something with the DNS reverse lookup.

So here is the POST body for a successful scan:

csrfmiddlewaretoken: LpSSzghTYYCOPT43t7sVbnCSSsOGNvuIWD41PCndeIDrHDWt5Juh6ORlrKPtl1FS
_schedule_at: 
_interval: 
_commit: 
_commit: on
_run: 

POST body of a randomly failed scan:

csrfmiddlewaretoken: VIFlXjSVdsUilKKqRvKqBYp74IFIN5BRGCw7ae2JWCQfkyS0HBT6hGvKVTaCXnhC
_schedule_at: 
_interval: 
_commit: 
_run: 

POST body using "Run Again" button:

csrfmiddlewaretoken: pvuIhhlFPaoM7mHhfFZaDLcYEmmmLaFsapluucvtykkJ6aPR5L8QjtiBvxRgVsld
_run: 

[candlerb]

_commit: 
_commit: on

Interesting. HTML shows (on my system):

      <div class="form-check mb-0">
        <input type="hidden" name="_commit" value="">
<input type="checkbox" name="_commit"
 aria-describedby="id__commit_helptext" id="id__commit" checked
 class="form-check-input">
        <label for="id__commit" class="form-check-label">
          Commit changes
        </label>

          <span class="form-text">Commit changes to the database (uncheck for a dry-run)</span>

      </div>

That is: there's a hidden field and a checkbox, both with name _commit. That's pretty ugly. Having said that, when the box has been pre-selected as 'checked' I'd expect it to return 'on', and toggling it twice in the UI shouldn't change this; but I don't know enough about the semantics of HTML forms to say for sure. All this doesn't explain why you have this problem with one script but not others.

FWIW, the code in Netbox uses the last value of _commit supplied by the client:

netbox/extras/views.py:                commit=form.cleaned_data.pop('_commit')

Therefore, having both the hidden field and the form field is arguably not an outright bug, but it depends on the browser sending the fields in a deterministic order, and this seems unnecessary IMO.

I haven't traced to see what it does when the value is blank, but I think it should be logged:

    logger.info(f"Running script (commit={commit})")

If the raw string value is passed around, then empty string would be treated as if it were False:

                    if not commit:
                        raise AbortTransaction()

[jeremystretch]

That is: there's a hidden field and a checkbox, both with name _commit. That's pretty ugly.

Ugly, but necessary in certain situations to force the browser to convey false values for a checkbox. Without the hidden field, the value is omitted from the request (rather than being included as "false"). This breaks e.g. dynamic form rendering; see bug #13064.

[Cennepal]

Yeah, it's weird. Not to mention that the "Run Again" button doesn't return _commit at all. I'm going to rewrite the lookup to use dnspython (just in case) and put in an if statement that catches if commit isn't true and aborts. Other than that I haven't a clue.

[Cennepal]

I have switched to using dnspython for resolution and put in a condition to catch the commit not being set. Everything works fine for now, but I'm still a bit paranoid. The "Run Again" button is not so important, but it's still somewhat troubling that it won't pass commit at all.

@candlerb Thank you very much for your time and help regarding this. You saved me a lot of time :D