Merge ~alexsander-souza/maas:lp2028284_smart_sync_hints into maas:master

[Alexsander de Souza]

don't save hints if nothing changed

fixes LP#2028284

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp2028284_smart_sync_hints lp:~alexsander-souza/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 3a354cf69001fb651fba7b268e6b5af0e16e05bd

review: Approve

[Björn Tillenius (bjornt)]

+1

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b lp2028284_smart_sync_hints lp:~alexsander-souza/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci.internal:8080/job/maas-tester/3405/console

diff --git a/src/maasserver/models/bmc.py b/src/maasserver/models/bmc.py
index b317f34..54506e3 100644
--- a/src/maasserver/models/bmc.py
+++ b/src/maasserver/models/bmc.py
@@ -69,7 +69,10 @@ from maasserver.utils.orm import transactional
 from maasserver.utils.threads import deferToDatabase
 from metadataserver.enum import RESULT_TYPE
 from provisioningserver.drivers import SETTING_SCOPE
-from provisioningserver.drivers.pod import InterfaceAttachType
+from provisioningserver.drivers.pod import (
+    DiscoveredPodHints,
+    InterfaceAttachType,
+)
 from provisioningserver.drivers.power.registry import (
     PowerDriverRegistry,
     sanitise_power_parameters,
@@ -800,21 +803,30 @@ class Pod(BMC):
     def sync_hints(self, discovered_hints, cluster=None):
         """Sync the hints with `discovered_hints`."""
 
+        def update_hint(
+            hints: PodHints, discovered: DiscoveredPodHints, attr: str
+        ) -> bool:
+            new_val = getattr(discovered, attr)
+            if new_val != DiscoveredPodHints.UNDEFINED and new_val != getattr(
+                hints, attr
+            ):
+                setattr(hints, attr, new_val)
+                return True
+            return False
+
+        changed = False
         try:
             hints = self.hints
         except PodHints.DoesNotExist:
             hints = self.hints = PodHints()
-        if discovered_hints.cores != -1:
-            hints.cores = discovered_hints.cores
-        if discovered_hints.cpu_speed != -1:
-            hints.cpu_speed = discovered_hints.cpu_speed
-        if discovered_hints.memory != -1:
-            hints.memory = discovered_hints.memory
-        if discovered_hints.local_storage != -1:
-            hints.local_storage = discovered_hints.local_storage
-        if cluster is not None:
+            changed = True
+        for a in ["cores", "cpu_speed", "memory", "local_storage"]:
+            changed |= update_hint(hints, discovered_hints, a)
+        if cluster is not None and hints.cluster != cluster:
             hints.cluster = cluster
-        hints.save()
+            changed = True
+        if changed:
+            hints.save()
 
     def add_tag(self, tag):
         """Add tag to Pod."""
diff --git a/src/maasserver/models/tests/test_bmc.py b/src/maasserver/models/tests/test_bmc.py
index b766810..0e56f59 100644
--- a/src/maasserver/models/tests/test_bmc.py
+++ b/src/maasserver/models/tests/test_bmc.py
@@ -44,6 +44,7 @@ from maasserver.models.fabric import Fabric
 from maasserver.models.filesystem import Filesystem
 from maasserver.models.node import Controller, Machine, Node
 from maasserver.models.physicalblockdevice import PhysicalBlockDevice
+from maasserver.models.podhints import PodHints
 from maasserver.models.podstoragepool import PodStoragePool
 from maasserver.models.resourcepool import ResourcePool
 from maasserver.models.staticipaddress import StaticIPAddress
@@ -3035,6 +3036,39 @@ class TestPod(MAASServerTestCase, PodTestMixin):
         self.assertEqual(vm.bmc_id, intended_bmc.id)
 
 
+class TestPodHints(MAASServerTestCase, PodTestMixin):
+    def test_sync_hints_doesnt_save_empty_hints(self):
+        cluster = factory.make_VMCluster(pods=0)
+        discovered = self.make_discovered_pod()
+        pod1 = Pod(
+            power_type="lxd",
+            power_parameters={"project": factory.make_name("project")},
+        )
+        pod1.save()
+        mock_hint_save = self.patch(PodHints, "save")
+        pod1.sync_hints(discovered.hints, cluster=cluster)
+        mock_hint_save.assert_called()
+        mock_hint_save.reset_mock()
+        blank = DiscoveredPodHints()
+        pod1.sync_hints(blank)
+        mock_hint_save.assert_not_called()
+
+    def test_sync_hints_doesnt_save_same_hints(self):
+        cluster = factory.make_VMCluster(pods=0)
+        discovered = self.make_discovered_pod()
+        pod1 = Pod(
+            power_type="lxd",
+            power_parameters={"project": factory.make_name("project")},
+        )
+        pod1.save()
+        mock_hint_save = self.patch(PodHints, "save")
+        pod1.sync_hints(discovered.hints, cluster=cluster)
+        mock_hint_save.assert_called()
+        mock_hint_save.reset_mock()
+        pod1.sync_hints(discovered.hints)
+        mock_hint_save.assert_not_called()
+
+
 class TestPodDelete(MAASTransactionServerTestCase, PodTestMixin):
     def test_delete_is_not_allowed(self):
         pod = factory.make_Pod()
diff --git a/src/provisioningserver/drivers/pod/__init__.py b/src/provisioningserver/drivers/pod/__init__.py
index 815e076..197909c 100644
--- a/src/provisioningserver/drivers/pod/__init__.py
+++ b/src/provisioningserver/drivers/pod/__init__.py
@@ -259,10 +259,12 @@ class DiscoveredPodHints:
     Limiting the maximum cores allow request on a per machine basis.
     """
 
-    cores = attr.ib(converter=int, default=-1)
-    cpu_speed = attr.ib(converter=int, default=-1)
-    memory = attr.ib(converter=int, default=-1)
-    local_storage = attr.ib(converter=int, default=-1)
+    UNDEFINED = -1
+
+    cores = attr.ib(converter=int, default=UNDEFINED)
+    cpu_speed = attr.ib(converter=int, default=UNDEFINED)
+    memory = attr.ib(converter=int, default=UNDEFINED)
+    local_storage = attr.ib(converter=int, default=UNDEFINED)
 
 
 @attr.s