Skip to content

Commit e46e816

Browse files
author
mn-ram
committed
[fix] Add group-scoped view cache invalidation for device group context change openwisp#1070
When device group context variables are updated, bulk_invalidate_config_get_cached_checksum was already called to mark affected configs as modified, but invalidate_controller_views_cache was never called, leaving the DeviceChecksumView cache stale for devices in the group. Replace the org-wide invalidate_controller_views_cache with a new group-scoped task, invalidate_controller_views_for_group, that only clears DeviceChecksumView entries for devices in the changed group, avoiding unnecessary invalidation of unrelated devices and VPNs.
1 parent a3abade commit e46e816

File tree

4 files changed

+42
-11
lines changed

4 files changed

+42
-11
lines changed

.github/workflows/bot-ci-failure.yml

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ on:
77
- completed
88

99
permissions:
10-
pull-requests: write
10+
pull-requests: read
1111
actions: read
1212
contents: read
1313

@@ -18,7 +18,7 @@ concurrency:
1818
jobs:
1919
find-pr:
2020
runs-on: ubuntu-latest
21-
if: ${{ github.event.workflow_run.conclusion == 'failure' }}
21+
if: ${{ github.event.workflow_run.conclusion == 'failure' && github.event.workflow_run.event == 'pull_request' }}
2222
outputs:
2323
pr_number: ${{ steps.pr.outputs.number }}
2424
pr_author: ${{ steps.pr.outputs.author }}
@@ -35,9 +35,8 @@ jobs:
3535
local pr_number="$1"
3636
local pr_author
3737
pr_author=$(gh pr view "$pr_number" --repo "$REPO" --json author --jq '.author.login // empty' 2>/dev/null || echo "")
38-
if [ -z "$pr_author" ]; then
39-
pr_author="${{ github.event.workflow_run.actor.login }}"
40-
echo "::warning::Could not fetch PR author for PR #$pr_number; falling back to @$pr_author"
38+
if [ -z "$pr_author" ] || [ "$pr_author" = "null" ]; then
39+
echo "::warning::Could not fetch PR author for PR #$pr_number"
4140
fi
4241
echo "number=$pr_number" >> "$GITHUB_OUTPUT"
4342
echo "author=$pr_author" >> "$GITHUB_OUTPUT"
@@ -69,6 +68,10 @@ jobs:
6968
call-ci-failure-bot:
7069
needs: find-pr
7170
if: ${{ needs.find-pr.outputs.pr_number != '' }}
71+
permissions:
72+
pull-requests: write
73+
actions: write
74+
contents: read
7275
uses: openwisp/openwisp-utils/.github/workflows/reusable-bot-ci-failure.yml@master
7376
with:
7477
pr_number: ${{ needs.find-pr.outputs.pr_number }}

openwisp_controller/config/base/device_group.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@
1515
from .. import settings as app_settings
1616
from ..signals import group_templates_changed
1717
from ..sortedm2m.fields import SortedManyToManyField
18-
from ..tasks import bulk_invalidate_config_get_cached_checksum
18+
from ..tasks import (
19+
bulk_invalidate_config_get_cached_checksum,
20+
invalidate_controller_views_for_group,
21+
)
1922
from .config import TemplatesThrough
2023

2124

@@ -88,6 +91,7 @@ def save(
8891
bulk_invalidate_config_get_cached_checksum.delay(
8992
{"device__group_id": str(self.id)}
9093
)
94+
invalidate_controller_views_for_group.delay(str(self.id))
9195

9296
def get_context(self):
9397
return deepcopy(self.context)

openwisp_controller/config/tasks.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,3 +217,21 @@ def invalidate_controller_views_cache(organization_id):
217217
Vpn.objects.filter(organization_id=organization_id).only("id").iterator()
218218
):
219219
GetVpnView.invalidate_get_vpn_cache(vpn)
220+
221+
222+
@shared_task(base=OpenwispCeleryTask)
223+
def invalidate_controller_views_for_group(group_id):
224+
"""
225+
Invalidates the DeviceChecksumView cache only for devices in the given group.
226+
227+
Unlike invalidate_controller_views_cache, this is scoped to a single device
228+
group and does not invalidate VPN caches.
229+
"""
230+
from .controller.views import DeviceChecksumView
231+
232+
Device = load_model("config", "Device")
233+
234+
for device in (
235+
Device.objects.filter(group_id=group_id).only("id").iterator()
236+
):
237+
DeviceChecksumView.invalidate_get_device_cache(device)

openwisp_controller/config/tests/test_device.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -440,8 +440,8 @@ def test_status_update_on_org_variable_change(self):
440440
c1.templates.add(
441441
self._create_template(
442442
name="t-with-var",
443-
config={"interfaces": [{"name": "eth0", "type": "{{ ssid }}"}]},
444-
default_values={"ssid": "default"},
443+
config={"interfaces": [{"name": "{{ ssid }}", "type": "ethernet"}]},
444+
default_values={"ssid": "eth0"},
445445
)
446446
)
447447
c1.set_status_applied()
@@ -468,8 +468,8 @@ def test_status_update_on_group_variable_change(self):
468468
c1.templates.add(
469469
self._create_template(
470470
name="t-with-var",
471-
config={"interfaces": [{"name": "eth0", "type": "{{ ssid }}"}]},
472-
default_values={"ssid": "default"},
471+
config={"interfaces": [{"name": "{{ ssid }}", "type": "ethernet"}]},
472+
default_values={"ssid": "eth0"},
473473
)
474474
)
475475
c1.set_status_applied()
@@ -480,7 +480,13 @@ def test_status_update_on_group_variable_change(self):
480480
c2.set_status_applied()
481481
dg.context = {"ssid": "OrgA"}
482482
dg.full_clean()
483-
dg.save()
483+
patch_path = (
484+
"openwisp_controller.config.base.device_group"
485+
".invalidate_controller_views_for_group"
486+
)
487+
with mock.patch(patch_path) as mocked_task:
488+
dg.save()
489+
mocked_task.delay.assert_called_once_with(str(dg.id))
484490
c1.refresh_from_db()
485491
c2.refresh_from_db()
486492
with self.subTest("affected config changes to modified"):

0 commit comments

Comments
 (0)