REQ: add --check flag to setup_coupling command

[wranders]

Is your feature request related to a problem? Please describe.
This is not a problem per se, more of an enhancement that would make automated deployment a bit easier.

Describe the solution you'd like
Include a manage.py setup_coupling --check flag that operates similarly to manage.py migrate --check, where the existence and validity of the IPAM coupling custom fields is verified and returns a non-zero status code if a problem was found.

Describe alternatives you've considered

• Use a separate psql process to query extras_customfield to verify that the fields exist, as well as the relationships in extras_customfield_object_types to django_content_type. This adds a bit of overhead and not being coupled seems hack-y. Wouldn't handle migrations very well if the fields change in the future.
• Just run manage.py setup_coupling every time Netbox starts. Seems like unnecessary brute force.
• Pipe in a Python script to manage.py shell that iterates the fields the way the manage.py setup_coupling --remove does and checks if the field exists. Could cause issues if these fields change in the future or the Netbox API changes.

Additional context
This would help with containerized deployments where coupling could be setup without additional admin intervention, al la ENTRYPOINT. If coupling is enabled, then the script could check and verify that coupling is set up before Netbox starts.

The current netbox-docker image calls docker-entrypoint.sh which performs migrations before anything else happens then runs launch-netbox.sh to start Netbox. For my purposes, I'm considering inserting a script between these two that performs these checks. With this command progression, migrations are performed before manage.py setup_coupling --check would be run, so changes to the fields shouldn't cause any issues.

[peteeckel]

Hi @wranders, thanks for raising this topic and for the effort you made to elaborate on it.

There is, however, an issue that probably makes the --check feature pretty inefficient and totally voids the positive effect it would have.

(netbox) [root@dns netbox]# time /opt/netbox/netbox/manage.py setup_coupling

real	0m10.343s
user	0m8.412s
sys	0m0.853s

In other words, it takes about 10 seconds (on my pretty slow dev system) to run the setup_coupling command, whether or not it makes any changes (in the above case, it didn't).

So, if you run the proposed --check command, then find that you need to run it without --check and run it again to fix the CFs, that would take 20 seconds, otherwise 10.

Most of the time manage.py uses is for setting up a complete NetBox runtime environment, then tearing it down again. The actual script takes only a few fractions of second. So running --check first to decide whether to run the actual setup job just wastes time - the setup job itself tests what needs to be done, and so you only add overhead by checking first.

You mentioned that the docker-entrypoint.sh runs migrate every time - same reason, running checkmigrations first would just waste time.

Do you still think the --check feature makes any sense in practice?

And, as every time someone suggests changes to the IPAM coupling feature: It is experimental, it will change, and don't rely on its current implementation ... :-)

[wranders]

You mentioned that the docker-entrypoint.sh runs migrate every time - same reason, running checkmigrations first would just waste time.

Not sure I'm following on this one. Since IPAM coupling is still experimental as you mentioned, are you referring to when it's no longer experimental and the fields would be handled directly by the standard migrations instead of a separate management command?

So, if you run the proposed --check command, then find that you need to run it without --check and run it again to fix the CFs, that would take 20 seconds, otherwise 10.

I didn't consider execution time, and TBH I've never really expected speed from Django. In the context of what I'm trying to accomplish, stability and correctness takes precedence over startup times.

And, as every time someone suggests changes to the IPAM coupling feature: It is experimental, it will change, and don't rely on its current implementation ... :-)

And this is precisely why I think there should be a way to verify that the fields are correctly implemented in the database. Returning a non-zero response from a --check flag would indicate that the database is not correctly configured for the current implementation and specific codes could indicate what needs to be done, potentially programmatically.

All of this will be unnecessary once the feature is no longer experimental, but since IPAM coupling modifies the database outside of the standard management and migration workflow, I do think there should be some sort of parity in how the fields are managed.

the setup job itself tests what needs to be done, and so you only add overhead by checking first.

I get that. So the intention is to have the command handler perform migrations itself should the fields change? For example, running the --remove logic on old fields before installing whatever the current implementation is?

I haven't touched Django or Python for any of my projects in probably about a decade, so it's really not my wheelhouse (hence Issues instead of PRs), so there's probably some bit of disconnect in my perception of how everything works.

I really appreciate the response. Talking through this is really helping me suss out everything. 😃

[peteeckel]

Not sure I'm following on this one. Since IPAM coupling is still experimental as you mentioned, are you referring to when it's no longer experimental and the fields would be handled directly by the standard migrations instead of a separate management command?

The gist of being 'experimental' is that it is entirely possible that the whole mechanism will be scrapped and replaced by something completely different. Whether there will be a migration path is unclear, but I will certainly do my best to obsolete as little data as possible - but no promises. The current mechanism is flawed in some ways, and it is likely that the new one will look very different - probably involving some custom fields as well, but very likely not the same ones.

So as I said - don't rely on it.

For the time being, running the setup_coupling command every time is the best bet. It is not likely that much will change with the feature, and if something changes, the setup command will handle it.

Be it as it may, the --check option does not deliver any useful functionality beyond what the setup command already does, so I won't put any effort into it.