From 4e0ea2021d75b2af9e4e72d822c561282137bbcb Mon Sep 17 00:00:00 2001 From: mn-ram <152869502+mn-ram@users.noreply.github.com> Date: Sat, 21 Mar 2026 20:50:58 +0530 Subject: [PATCH 1/4] [fix] Invalidate controller views cache when org context changes When organization context variables are updated, `bulk_invalidate_config_get_cached_checksum` was called to bust the Config checksum cache, but `invalidate_controller_views_cache` was never called. This left the DeviceChecksumView cache stale, so devices kept receiving the old checksum, never transitioned to `modified` status, and never pulled the updated configuration. Fix: also call `invalidate_controller_views_cache` whenever the org `context` field changes, mirroring the existing behaviour that already fires this task when an organisation is disabled/enabled. Fixes #1070 --- openwisp_controller/config/base/multitenancy.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openwisp_controller/config/base/multitenancy.py b/openwisp_controller/config/base/multitenancy.py index c2434e413..c21824855 100644 --- a/openwisp_controller/config/base/multitenancy.py +++ b/openwisp_controller/config/base/multitenancy.py @@ -12,7 +12,7 @@ from .. import settings as app_settings from ..exceptions import OrganizationDeviceLimitExceeded -from ..tasks import bulk_invalidate_config_get_cached_checksum +from ..tasks import bulk_invalidate_config_get_cached_checksum, invalidate_controller_views_cache class AbstractOrganizationConfigSettings(UUIDModel): @@ -100,6 +100,7 @@ def save( bulk_invalidate_config_get_cached_checksum.delay( {"device__organization_id": str(self.organization_id)} ) + invalidate_controller_views_cache.delay(str(self.organization_id)) class AbstractOrganizationLimits(models.Model): From 97cbf1408aa5cc4200b58dc8f790d06709091e12 Mon Sep 17 00:00:00 2001 From: mn-ram <152869502+mn-ram@users.noreply.github.com> Date: Sat, 21 Mar 2026 21:23:27 +0530 Subject: [PATCH 2/4] [fix] Fix import formatting to comply with isort/black Split the long import line into multi-line format to stay within the 88-character line length limit required by Black and isort. --- openwisp_controller/config/base/multitenancy.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/openwisp_controller/config/base/multitenancy.py b/openwisp_controller/config/base/multitenancy.py index c21824855..ef4f8a95f 100644 --- a/openwisp_controller/config/base/multitenancy.py +++ b/openwisp_controller/config/base/multitenancy.py @@ -12,7 +12,10 @@ from .. import settings as app_settings from ..exceptions import OrganizationDeviceLimitExceeded -from ..tasks import bulk_invalidate_config_get_cached_checksum, invalidate_controller_views_cache +from ..tasks import ( + bulk_invalidate_config_get_cached_checksum, + invalidate_controller_views_cache, +) class AbstractOrganizationConfigSettings(UUIDModel): From a3abade6a8dd3ae655591e2d2b9f35e9d1965791 Mon Sep 17 00:00:00 2001 From: mn-ram <152869502+mn-ram@users.noreply.github.com> Date: Thu, 19 Mar 2026 17:14:57 +0530 Subject: [PATCH 3/4] [tests] Add tests for config status update on org/group variable change Fixes #1070 Add tests to verify that when organization or device group configuration variables change, the config status updates to "modified" only for devices whose rendered configuration is actually affected by the change. --- .../config/tests/test_device.py | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/openwisp_controller/config/tests/test_device.py b/openwisp_controller/config/tests/test_device.py index 9603606d3..1ce6b5c29 100644 --- a/openwisp_controller/config/tests/test_device.py +++ b/openwisp_controller/config/tests/test_device.py @@ -433,6 +433,61 @@ def test_changing_group_variable_invalidates_cache(self): new_checksum = config.get_cached_checksum() self.assertNotEqual(old_checksum, new_checksum) + def test_status_update_on_org_variable_change(self): + org = self._get_org() + cs = OrganizationConfigSettings.objects.create(organization=org, context={}) + c1 = self._create_config(organization=org) + c1.templates.add( + self._create_template( + name="t-with-var", + config={"interfaces": [{"name": "eth0", "type": "{{ ssid }}"}]}, + default_values={"ssid": "default"}, + ) + ) + c1.set_status_applied() + d2 = self._create_device( + organization=org, name="d2", mac_address="00:11:22:33:44:56" + ) + c2 = self._create_config(device=d2) + c2.set_status_applied() + cs.context = {"ssid": "OrgA"} + cs.full_clean() + cs.save() + c1.refresh_from_db() + c2.refresh_from_db() + with self.subTest("affected config changes to modified"): + self.assertEqual(c1.status, "modified") + with self.subTest("unaffected config remains applied"): + self.assertEqual(c2.status, "applied") + + def test_status_update_on_group_variable_change(self): + org = self._get_org() + dg = self._create_device_group(organization=org, context={}) + d1 = self._create_device(organization=org, group=dg) + c1 = self._create_config(device=d1) + c1.templates.add( + self._create_template( + name="t-with-var", + config={"interfaces": [{"name": "eth0", "type": "{{ ssid }}"}]}, + default_values={"ssid": "default"}, + ) + ) + c1.set_status_applied() + d2 = self._create_device( + organization=org, group=dg, name="d2", mac_address="00:11:22:33:44:56" + ) + c2 = self._create_config(device=d2) + c2.set_status_applied() + dg.context = {"ssid": "OrgA"} + dg.full_clean() + dg.save() + c1.refresh_from_db() + c2.refresh_from_db() + with self.subTest("affected config changes to modified"): + self.assertEqual(c1.status, "modified") + with self.subTest("unaffected config remains applied"): + self.assertEqual(c2.status, "applied") + def test_management_ip_changed_not_emitted_on_creation(self): with catch_signal(management_ip_changed) as handler: self._create_device(organization=self._get_org()) From 4aff88cccbc0ed7f3017b3cad2a59692e62fa522 Mon Sep 17 00:00:00 2001 From: mn-ram <152869502+mn-ram@users.noreply.github.com> Date: Wed, 25 Mar 2026 18:07:00 +0530 Subject: [PATCH 4/4] [fix] Add group-scoped view cache invalidation for device group context change #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. --- .github/workflows/bot-ci-failure.yml | 13 ++++++++----- openwisp_controller/config/base/device_group.py | 6 +++++- openwisp_controller/config/tasks.py | 16 ++++++++++++++++ openwisp_controller/config/tests/test_device.py | 16 +++++++++++----- 4 files changed, 40 insertions(+), 11 deletions(-) diff --git a/.github/workflows/bot-ci-failure.yml b/.github/workflows/bot-ci-failure.yml index 17f32eb97..6582eb6db 100644 --- a/.github/workflows/bot-ci-failure.yml +++ b/.github/workflows/bot-ci-failure.yml @@ -7,7 +7,7 @@ on: - completed permissions: - pull-requests: write + pull-requests: read actions: read contents: read @@ -18,7 +18,7 @@ concurrency: jobs: find-pr: runs-on: ubuntu-latest - if: ${{ github.event.workflow_run.conclusion == 'failure' }} + if: ${{ github.event.workflow_run.conclusion == 'failure' && github.event.workflow_run.event == 'pull_request' }} outputs: pr_number: ${{ steps.pr.outputs.number }} pr_author: ${{ steps.pr.outputs.author }} @@ -35,9 +35,8 @@ jobs: local pr_number="$1" local pr_author pr_author=$(gh pr view "$pr_number" --repo "$REPO" --json author --jq '.author.login // empty' 2>/dev/null || echo "") - if [ -z "$pr_author" ]; then - pr_author="${{ github.event.workflow_run.actor.login }}" - echo "::warning::Could not fetch PR author for PR #$pr_number; falling back to @$pr_author" + if [ -z "$pr_author" ] || [ "$pr_author" = "null" ]; then + echo "::warning::Could not fetch PR author for PR #$pr_number" fi echo "number=$pr_number" >> "$GITHUB_OUTPUT" echo "author=$pr_author" >> "$GITHUB_OUTPUT" @@ -69,6 +68,10 @@ jobs: call-ci-failure-bot: needs: find-pr if: ${{ needs.find-pr.outputs.pr_number != '' }} + permissions: + pull-requests: write + actions: write + contents: read uses: openwisp/openwisp-utils/.github/workflows/reusable-bot-ci-failure.yml@master with: pr_number: ${{ needs.find-pr.outputs.pr_number }} diff --git a/openwisp_controller/config/base/device_group.py b/openwisp_controller/config/base/device_group.py index 366d61667..b331c79e3 100644 --- a/openwisp_controller/config/base/device_group.py +++ b/openwisp_controller/config/base/device_group.py @@ -15,7 +15,10 @@ from .. import settings as app_settings from ..signals import group_templates_changed from ..sortedm2m.fields import SortedManyToManyField -from ..tasks import bulk_invalidate_config_get_cached_checksum +from ..tasks import ( + bulk_invalidate_config_get_cached_checksum, + invalidate_controller_views_for_group, +) from .config import TemplatesThrough @@ -88,6 +91,7 @@ def save( bulk_invalidate_config_get_cached_checksum.delay( {"device__group_id": str(self.id)} ) + invalidate_controller_views_for_group.delay(str(self.id)) def get_context(self): return deepcopy(self.context) diff --git a/openwisp_controller/config/tasks.py b/openwisp_controller/config/tasks.py index 798e40f8c..b0c6e2b8c 100644 --- a/openwisp_controller/config/tasks.py +++ b/openwisp_controller/config/tasks.py @@ -217,3 +217,19 @@ def invalidate_controller_views_cache(organization_id): Vpn.objects.filter(organization_id=organization_id).only("id").iterator() ): GetVpnView.invalidate_get_vpn_cache(vpn) + + +@shared_task(base=OpenwispCeleryTask) +def invalidate_controller_views_for_group(group_id): + """ + Invalidates the DeviceChecksumView cache only for devices in the given group. + + Unlike invalidate_controller_views_cache, this is scoped to a single device + group and does not invalidate VPN caches. + """ + from .controller.views import DeviceChecksumView + + Device = load_model("config", "Device") + + for device in Device.objects.filter(group_id=group_id).only("id").iterator(): + DeviceChecksumView.invalidate_get_device_cache(device) diff --git a/openwisp_controller/config/tests/test_device.py b/openwisp_controller/config/tests/test_device.py index 1ce6b5c29..3e1fee0fd 100644 --- a/openwisp_controller/config/tests/test_device.py +++ b/openwisp_controller/config/tests/test_device.py @@ -440,8 +440,8 @@ def test_status_update_on_org_variable_change(self): c1.templates.add( self._create_template( name="t-with-var", - config={"interfaces": [{"name": "eth0", "type": "{{ ssid }}"}]}, - default_values={"ssid": "default"}, + config={"interfaces": [{"name": "{{ ssid }}", "type": "ethernet"}]}, + default_values={"ssid": "eth0"}, ) ) c1.set_status_applied() @@ -468,8 +468,8 @@ def test_status_update_on_group_variable_change(self): c1.templates.add( self._create_template( name="t-with-var", - config={"interfaces": [{"name": "eth0", "type": "{{ ssid }}"}]}, - default_values={"ssid": "default"}, + config={"interfaces": [{"name": "{{ ssid }}", "type": "ethernet"}]}, + default_values={"ssid": "eth0"}, ) ) c1.set_status_applied() @@ -480,7 +480,13 @@ def test_status_update_on_group_variable_change(self): c2.set_status_applied() dg.context = {"ssid": "OrgA"} dg.full_clean() - dg.save() + patch_path = ( + "openwisp_controller.config.base.device_group" + ".invalidate_controller_views_for_group" + ) + with mock.patch(patch_path) as mocked_task: + dg.save() + mocked_task.delay.assert_called_once_with(str(dg.id)) c1.refresh_from_db() c2.refresh_from_db() with self.subTest("affected config changes to modified"):