Add livepatch to manager instead of monitor, refactored (#260)

fix: Added livepatch and pro reboot info manager, Refactored

landscape/client/manager/config.py

@@ -15,6 +15,8 @@
     "SnapManager",
     "SnapServicesManager",
     "UbuntuProInfo",
+    "LivePatch",
+    "UbuntuProRebootRequired",
 ]
 
 

landscape/client/monitor/livepatch.py → landscape/client/manager/livepatch.py

@@ -1,11 +1,14 @@
 import json
+import logging
 import subprocess
+import traceback
 import yaml
 
-from landscape.client.monitor.plugin import DataWatcher
 
+from landscape.client.manager.plugin import DataWatcherManager
 
-class LivePatch(DataWatcher):
+
+class LivePatch(DataWatcherManager):
     """
     Plugin that captures and reports Livepatch status information
     information.
@@ -46,6 +49,7 @@ def get_livepatch_status(format_type):
         data['return_code'] = -2
         data['error'] = str(exc)
         data['output'] = ""
+        logging.error(traceback.format_exc())
     else:
         output = completed_process.stdout.strip()
         try:

landscape/client/manager/plugin.py

@@ -1,8 +1,12 @@
+import logging
+from pathlib import Path
+
 from twisted.internet.defer import maybeDeferred
 
 from landscape.client.broker.client import BrokerClientPlugin
 from landscape.lib.format import format_object
 from landscape.lib.log import log_failure
+from landscape.lib.persist import Persist
 
 # Protocol messages! Same constants are defined in the server.
 FAILED = 5
@@ -66,3 +70,71 @@ def send(args):
         deferred.addCallback(send)
 
         return deferred
+
+
+class DataWatcherManager(ManagerPlugin):
+    """
+    A utility for plugins which send data to the Landscape server
+    which does not constantly change. New messages will only be sent
+    when the result of get_data() has changed since the last time it
+    was called. Note this is the same as the DataWatcher plugin but
+    for Manager plugins instead of Monitor.Subclasses should provide
+    a get_data method
+    """
+
+    message_type = None
+
+    def __init__(self):
+        super().__init__()
+        self._persist = None
+
+    def register(self, registry):
+        super().register(registry)
+        self._persist_filename = Path(
+            self.registry.config.data_path,
+            self.message_type + '.manager.bpkl',
+        )
+        self._persist = Persist(filename=self._persist_filename)
+        self.call_on_accepted(self.message_type, self.send_message)
+
+    def run(self):
+        return self.registry.broker.call_if_accepted(
+            self.message_type,
+            self.send_message,
+        )
+
+    def send_message(self):
+        """Send a message to the broker if the data has changed since the last
+        call"""
+        result = self.get_new_data()
+        if not result:
+            logging.debug("{} unchanged so not sending".format(
+                          self.message_type))
+            return
+        logging.debug("Sending new {} data!".format(self.message_type))
+        message = {"type": self.message_type, self.message_type: result}
+        return self.registry.broker.send_message(message, self._session_id)
+
+    def get_new_data(self):
+        """Returns the data only if it has changed"""
+        data = self.get_data()
+        if self._persist is None:  # Persist not initialized yet
+            return data
+        elif self._persist.get("data") != data:
+            self._persist.set("data", data)
+            return data
+        else:  # Data not changed
+            return None
+
+    def get_data(self):
+        """
+        The result of this will be cached and subclasses must implement this
+        and return the correct return type defined in the server bound message
+        schema
+        """
+        raise NotImplementedError("Subclasses must implement get_data()")
+
+    def _reset(self):
+        """Reset the persist."""
+        if self._persist:
+            self._persist.remove("data")

landscape/client/manager/tests/test_config.py

@@ -23,6 +23,8 @@ def test_plugin_factories(self):
                 "SnapManager",
                 "SnapServicesManager",
                 "UbuntuProInfo",
+                "LivePatch",
+                "UbuntuProRebootRequired"
             ],
             ALL_PLUGINS,
         )

landscape/client/monitor/tests/test_livepatch.py → landscape/client/manager/tests/test_livepatch.py

@@ -2,8 +2,8 @@
 import yaml
 from unittest import mock
 
-from landscape.client.monitor.livepatch import LivePatch
-from landscape.client.tests.helpers import LandscapeTest, MonitorHelper
+from landscape.client.manager.livepatch import LivePatch
+from landscape.client.tests.helpers import LandscapeTest, ManagerHelper
 
 
 def subprocess_livepatch_mock(*args, **kwargs):
@@ -19,20 +19,21 @@ def subprocess_livepatch_mock(*args, **kwargs):
 class LivePatchTest(LandscapeTest):
     """Livepatch status plugin tests."""
 
-    helpers = [MonitorHelper]
+    helpers = [ManagerHelper]
 
     def setUp(self):
         super(LivePatchTest, self).setUp()
+        self.mstore = self.broker_service.message_store
         self.mstore.set_accepted_types(["livepatch"])
 
     def test_livepatch(self):
         """Tests calling livepatch status."""
         plugin = LivePatch()
-        self.monitor.add(plugin)
 
         with mock.patch("subprocess.run") as run_mock:
             run_mock.side_effect = subprocess_livepatch_mock
-            plugin.exchange()
+            self.manager.add(plugin)
+            plugin.run()
 
         messages = self.mstore.get_pending_messages()
         self.assertTrue(len(messages) > 0)
@@ -47,11 +48,11 @@ def test_livepatch(self):
     def test_livepatch_when_not_installed(self):
         """Tests calling livepatch when it is not installed."""
         plugin = LivePatch()
-        self.monitor.add(plugin)
 
         with mock.patch("subprocess.run") as run_mock:
             run_mock.side_effect = FileNotFoundError("Not found!")
-            plugin.exchange()
+            self.manager.add(plugin)
+            plugin.run()
 
         messages = self.mstore.get_pending_messages()
         message = json.loads(messages[0]["livepatch"])
@@ -61,14 +62,17 @@ def test_livepatch_when_not_installed(self):
         self.assertEqual(message["json"]["return_code"], -1)
         self.assertEqual(message["humane"]["return_code"], -1)
 
-    def test_undefined_exception(self):
+    @mock.patch('landscape.client.manager.livepatch.logging.error')
+    def test_undefined_exception(self, logger_mock):
         """Tests calling livepatch when random exception occurs"""
         plugin = LivePatch()
-        self.monitor.add(plugin)
 
         with mock.patch("subprocess.run") as run_mock:
             run_mock.side_effect = ValueError("Not found!")
-            plugin.exchange()
+            self.manager.add(plugin)
+            plugin.run()
+
+        logger_mock.assert_called()
 
         messages = self.mstore.get_pending_messages()
         message = json.loads(messages[0]["livepatch"])
@@ -83,13 +87,13 @@ def test_yaml_json_parse_error(self):
         If json or yaml parsing error than show exception and unparsed data
         """
         plugin = LivePatch()
-        self.monitor.add(plugin)
 
         invalid_data = "'"
         with mock.patch("subprocess.run") as run_mock:
             run_mock.return_value = mock.Mock(stdout=invalid_data)
             run_mock.return_value.returncode = 0
-            plugin.exchange()
+            self.manager.add(plugin)
+            plugin.run()
 
         messages = self.mstore.get_pending_messages()
         message = json.loads(messages[0]["livepatch"])
@@ -104,14 +108,14 @@ def test_empty_string(self):
         If livepatch is disabled, stdout is empty string
         """
         plugin = LivePatch()
-        self.monitor.add(plugin)
 
         invalid_data = ""
         with mock.patch("subprocess.run") as run_mock:
             run_mock.return_value = mock.Mock(stdout=invalid_data,
                                               stderr='Error')
             run_mock.return_value.returncode = 1
-            plugin.exchange()
+            self.manager.add(plugin)
+            plugin.run()
 
         messages = self.mstore.get_pending_messages()
         message = json.loads(messages[0]["livepatch"])
@@ -126,11 +130,11 @@ def test_timestamped_fields_deleted(self):
         """This is so data doesn't keep getting sent if not changed"""
 
         plugin = LivePatch()
-        self.monitor.add(plugin)
 
         with mock.patch("subprocess.run") as run_mock:
             run_mock.side_effect = subprocess_livepatch_mock
-            plugin.exchange()
+            self.manager.add(plugin)
+            plugin.run()
 
         messages = self.mstore.get_pending_messages()
         self.assertTrue(len(messages) > 0)

landscape/client/manager/tests/test_plugin.py

@@ -1,7 +1,7 @@
 from twisted.internet.defer import Deferred
 
 from landscape.client.manager.plugin import FAILED
-from landscape.client.manager.plugin import ManagerPlugin
+from landscape.client.manager.plugin import ManagerPlugin, DataWatcherManager
 from landscape.client.manager.plugin import SUCCEEDED
 from landscape.client.tests.helpers import LandscapeTest
 from landscape.client.tests.helpers import ManagerHelper
@@ -126,3 +126,37 @@ def assert_messages(ignored):
         result.addCallback(assert_messages)
         deferred.callback("blah")
         return result
+
+
+class StubDataWatchingPlugin(DataWatcherManager):
+
+    message_type = "wubble"
+
+    def __init__(self, data=None):
+        self.data = data
+
+    def get_data(self):
+        return self.data
+
+
+class DataWatcherManagerTest(LandscapeTest):
+
+    helpers = [ManagerHelper]
+
+    def setUp(self):
+        LandscapeTest.setUp(self)
+        self.plugin = StubDataWatchingPlugin("hello world")
+        self.plugin.register(self.manager)
+
+    def test_get_message(self):
+        self.assertEqual(
+            self.plugin.get_new_data(),
+            "hello world",
+        )
+
+    def test_get_message_unchanging(self):
+        self.assertEqual(
+            self.plugin.get_new_data(),
+            "hello world",
+        )
+        self.assertEqual(self.plugin.get_new_data(), None)

landscape/client/manager/tests/test_ubuntuproinfo.py

@@ -7,16 +7,17 @@
 from landscape.client.manager.ubuntuproinfo import get_ubuntu_pro_info
 from landscape.client.manager.ubuntuproinfo import UbuntuProInfo
 from landscape.client.tests.helpers import LandscapeTest
-from landscape.client.tests.helpers import MonitorHelper
+from landscape.client.tests.helpers import ManagerHelper
 
 
 class UbuntuProInfoTest(LandscapeTest):
     """Ubuntu Pro info plugin tests."""
 
-    helpers = [MonitorHelper]
+    helpers = [ManagerHelper]
 
     def setUp(self):
         super().setUp()
+        self.mstore = self.broker_service.message_store
         self.mstore.set_accepted_types(["ubuntu-pro-info"])
 
     def test_ubuntu_pro_info(self):
@@ -27,7 +28,7 @@ def test_ubuntu_pro_info(self):
             run_mock.return_value = mock.Mock(
                 stdout='"This is a test"',
             )
-            self.monitor.add(plugin)
+            self.manager.add(plugin)
             plugin.run()
 
         run_mock.assert_called()
@@ -39,7 +40,7 @@ def test_ubuntu_pro_info(self):
     def test_ubuntu_pro_info_no_pro(self):
         """Tests calling `pro status` when it is not installed."""
         plugin = UbuntuProInfo()
-        self.monitor.add(plugin)
+        self.manager.add(plugin)
 
         with mock.patch("subprocess.run") as run_mock:
             run_mock.side_effect = FileNotFoundError()
@@ -78,7 +79,7 @@ def datetime_is_aware(d):
     def test_persistence_unchanged_data(self):
         """If data hasn't changed, a new message is not sent"""
         plugin = UbuntuProInfo()
-        self.monitor.add(plugin)
+        self.manager.add(plugin)
         data = '"Initial data!"'
 
         with mock.patch("subprocess.run") as run_mock:
@@ -106,7 +107,7 @@ def test_persistence_unchanged_data(self):
     def test_persistence_changed_data(self):
         """New data will be sent in a new message in the queue"""
         plugin = UbuntuProInfo()
-        self.monitor.add(plugin)
+        self.manager.add(plugin)
 
         with mock.patch("subprocess.run") as run_mock:
             run_mock.return_value = mock.Mock(
@@ -135,7 +136,7 @@ def test_persistence_reset(self):
         """Resetting the plugin will allow a message with identical data to
         be sent"""
         plugin = UbuntuProInfo()
-        self.monitor.add(plugin)
+        self.manager.add(plugin)
         data = '"Initial data!"'
 
         with mock.patch("subprocess.run") as run_mock: