Repository: netbox-community/netbox
Pull request: Do not delete all search indexes when reindexing specific models
* Clear all indexes only if neither `--lazy` nor a list of models are specified for "manage.py reindex"
* Otherwise, clear the index for a model immediately before rebuilding it

### Fixes: #16747


Problem statement:
`reindex` manage command behaves unexpectedly
### Deployment Type

Self-hosted

### NetBox Version

v4.0.6

### Python Version

3.11

### Steps to Reproduce

I was made aware of this by a question posted to the Slack channel.

A user asked whether the `reindex` management command deletes all search indexes except the ones for the application given as a parameter if `reindex` is used with an application name, for example
```bash
/opt/netbox/netbox/manage.py reindex netbox_dns
```

It turns out that this is in fact the case. The code for the management command always deletes all search indexes even if it was called with an application/plugin name as its parameters, but recreates only the ones for the given application or plugin.

First, I indexed the whole NetBox DNS inventory for all models:
```
/opt/netbox/netbox/manage.py reindex
Reindexing 82 models.
Clearing cached values... 467 entries deleted.
Indexing models
  netbox_dns.record... 365 entries cached.
  netbox_dns.view... 4 entries cached.
  netbox_dns.nameserver... 7 entries cached.
...
  wireless.wirelesslan... No objects found.
  wireless.wirelesslangroup... No objects found.
  wireless.wirelesslink... No objects found.
Completed. Total entries: 650
```

The followinig SQL query returns all application/model name pairs that are in the index:
```
(netbox) [root@dns netbox]# /opt/netbox/netbox/manage.py dbshell
psql (15.5)
Type "help" for help.

netbox=# SELECT DISTINCT (app_label, model) FROM django_content_type c INNER JOIN extras_cachedvalue v ON c.id=v.object_type_id;
           row           
-------------------------
 (core,datafile)
 (core,datasource)
 (dcim,device)
 (dcim,devicerole)
 (dcim,devicetype)
 (dcim,interface)
 (dcim,manufacturer)
 (dcim,site)
 (dcim,virtualchassis)
 (extras,customfield)
 (extras,webhook)
 (ipam,ipaddress)
 (ipam,prefix)
 (ipam,vlan)
 (ipam,vlangroup)
 (ipam,vrf)
 (netbox_dns,contact)
 (netbox_dns,nameserver)
 (netbox_dns,record)
 (netbox_dns,registrar)
 (netbox_dns,view)
 (netbox_dns,zone)
 (tenancy,tenant)
 (tenancy,tenantgroup)
 (vpn,ikepolicy)
 (vpn,ikeproposal)
 (vpn,ipsecpolicy)
 (vpn,ipsecprofile)
 (vpn,ipsecproposal)
(29 rows)
```

Now, reindex only the search indices for a given application:
```
(netbox) [root@dns netbox]# /opt/netbox/netbox/manage.py reindex netbox_dns
Reindexing 6 models.
Clearing cached values... 650 entries deleted.
Indexing models
  netbox_dns.record... 365 entries cached.
  netbox_dns.view... 4 entries cached.
  netbox_dns.nameserver... 7 entries cached.
  netbox_dns.zone... 78 entries cached.
  netbox_dns.contact... 10 entries cached.
  netbox_dns.registrar... 3 entries cached.
Completed. Total entries: 467
```

The same SQL query now shows that the indices for all other applications are missing:
```
netbox=# SELECT DISTINCT (app_label, model) FROM django_content_type c INNER JOIN extras_cachedvalue v ON c.id=v.object_type_id;
           row           
-------------------------
 (netbox_dns,contact)
 (netbox_dns,nameserver)
 (netbox_dns,record)
 (netbox_dns,registrar)
 (netbox_dns,view)
 (netbox_dns,zone)
(6 rows)
```

Deleting all other index can only be avoided by also specifying `--lazy`, but that option prevents the indices for a specific application or model to be recreated (i.e., deleted and then created again). There is currently no way to achieve this.

### Expected Behavior

Only the search indexes for the specified applications/models are deleted, not all of them, and certainly not silently.

### Observed Behavior

All indexes are deleted and only those for the specified application/model are (re-)created.

Hints from issue discussion:
This code looks like it clears all at start
```
        # Clear all cached values for the specified models (if not being lazy)
        if not kwargs['lazy']:
            self.stdout.write('Clearing cached values... ', ending='')
            self.stdout.flush()
            deleted_count = search_backend.clear()
            self.stdout.write(f'{deleted_count} entries deleted.')
```
> This code looks like it clears all at start

exactly. Which isn't a problem if you don't specify a list of applications/models, but if you do it does unexpected thinks.

You can assign it to me if that's OK for you.
@peteeckel assigning to you.

Commit message: Do not delete all search indexes when reindexing specific models

* Clear all indexes only if neither --lazy nor a list of models are
  specified for "manage.py reindex"

* Otherwise, clear the index for a model immediately before rebuilding
  it

Patch to review:
diff --git a/netbox/extras/management/commands/reindex.py b/netbox/extras/management/commands/reindex.py
index e20fad0ceb8..51241af9393 100644
--- a/netbox/extras/management/commands/reindex.py
+++ b/netbox/extras/management/commands/reindex.py
@@ -67,8 +67,8 @@ def handle(self, *model_labels, **kwargs):
         self.stdout.write(f'Reindexing {len(indexers)} models.')
 
         # Clear all cached values for the specified models (if not being lazy)
-        if not kwargs['lazy']:
-            self.stdout.write('Clearing cached values... ', ending='')
+        if not kwargs['lazy'] and not model_labels:
+            self.stdout.write('Clearing all cached values... ', ending='')
             self.stdout.flush()
             deleted_count = search_backend.clear()
             self.stdout.write(f'{deleted_count} entries deleted.')
@@ -81,12 +81,19 @@ def handle(self, *model_labels, **kwargs):
             self.stdout.write(f'  {app_label}.{model_name}... ', ending='')
             self.stdout.flush()
 
-            if kwargs['lazy']:
-                content_type = ContentType.objects.get_for_model(model)
-                if cached_count := search_backend.count(object_types=[content_type]):
+            content_type = ContentType.objects.get_for_model(model)
+            cached_count = search_backend.count(object_types=[content_type])
+
+            if cached_count:
+                if kwargs['lazy']:
                     self.stdout.write(f'Skipping (found {cached_count} existing).')
                     continue
 
+                if model_labels:
+                    self.stdout.write(f'clearing {cached_count} cached values... ', ending='')
+                    self.stdout.flush()
+                    search_backend.clear(object_types=[content_type])
+
             i = search_backend.cache(model.objects.iterator(), remove_existing=False)
             if i:
                 self.stdout.write(f'{i} entries cached.')


Code review comments:
diff_hunk: @@ -81,12 +81,19 @@ def handle(self, *model_labels, **kwargs):
             self.stdout.write(f'  {app_label}.{model_name}... ', ending='')
             self.stdout.flush()
 
-            if kwargs['lazy']:
-                content_type = ContentType.objects.get_for_model(model)
-                if cached_count := search_backend.count(object_types=[content_type]):
+            content_type = ContentType.objects.get_for_model(model)
+            cached_count = search_backend.count(object_types=[content_type])
+
+            if cached_count:
+                if kwargs['lazy']:
                     self.stdout.write(f'Skipping (found {cached_count} existing).')
                     continue
 
+                if model_labels:
+                    self.stdout.write(f'clearing {cached_count} cached values... ', ending='')
+                    self.stdout.flush()
+                    search_backend.clear(object_types=[content_type])
path: netbox/extras/management/commands/reindex.py
text: @user1:
We want to keep the cache clearing logic before and separate from the indexing logic. The call to `search_backend.clear()` above (originally line 73) should be wrapped in a conditional that checks for `model_labels` and passes either the appropriate content types or None. The indexing logic can then be retained in its original form.