SW-1014 fix missing netconnectd logrotate (#1579)

* fix missing netconnectd logrotate * added additional unittests to check if all commands did run * fixed restart command for legacy image
mrbeam · Sep 15, 2022 · a8bb7b1 · a8bb7b1
1 parent 7fe0387
commit a8bb7b1
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 4 deletions.
diff --git a/octoprint_mrbeam/migration/Mig003.py b/octoprint_mrbeam/migration/Mig003.py
@@ -31,6 +31,7 @@ def _run(self):
             "mount_manager",
             "mrb_check",
             "mrbeam_ledstrips",
+            "netconnectd",
         ]
         for logrotate in logrotates:
             self._logger.debug("enable logrotate of " + logrotate)
@@ -43,7 +44,9 @@ def _run(self):
         self._logger.debug(
             "restarting logrotate in order for the changed config to take effect"
         )
-        self.exec_cmd("sudo logrotate /etc/logrotate.conf")
+
+        # needs to be optional for legacy image, as this is returning 1 instead of 0
+        self.exec_cmd("sudo logrotate /etc/logrotate.conf", optional=True)
         super(Mig003EnableLogrotateBuster, self)._run()
 
     def _rollback(self):

diff --git a/octoprint_mrbeam/migration/migration_base.py b/octoprint_mrbeam/migration/migration_base.py
@@ -218,7 +218,11 @@ def exec_cmd(self, command, optional=False):
         if optional and not command_success:
             self._logger.warn("optional command failed - cmd: {}".format(command))
         else:
-            raise MigrationException("error during migration for cmd:", command)
+            raise MigrationException(
+                "error during migration for cmd: {} - return: {}".format(
+                    command, command_success
+                )
+            )
 
     @staticmethod
     def execute_restart(restart):

diff --git a/octoprint_mrbeam/util/cmd_exec.py b/octoprint_mrbeam/util/cmd_exec.py
@@ -19,15 +19,15 @@ def exec_cmd(cmd, log=True, shell=True, loglvl=DEBUG):
     try:
         code = subprocess.call(cmd, shell=shell)
     except Exception as e:
-        _logger.debug(
+        _logger.error(
             "Failed to execute command '%s', return code: %s, Exception: %s",
             cmd,
             code,
             e,
         )
         return None
     if code != 0 and log:
-        _logger.info("cmd= '%s', return code: '%s'", code)
+        _logger.error("cmd= '{}', return code: {}".format(cmd, code))
     return code == 0
 
 

diff --git a/tests/migrations/test-migration-Mig003.py b/tests/migrations/test-migration-Mig003.py
@@ -1,3 +1,7 @@
+import os
+
+from mock import patch
+
 from octoprint_mrbeam.migration import Mig003EnableLogrotateBuster
 import unittest
 
@@ -27,3 +31,33 @@ def test_beamos_versions(self):
 
     def test_migration_id(self):
         self.assertEqual(self.m003.id, "003")
+
+    @patch.object(
+        Mig003EnableLogrotateBuster,
+        "exec_cmd",
+    )
+    def test_commands_executed(self, exec_cmd_mock):
+        self.m003.run()
+        exec_cmd_mock.assert_any_call(
+            "sudo rm /etc/logrotate.d/iobeam.logrotate", optional=True
+        )
+        logrotates = [
+            "analytics",
+            "iobeam",
+            "mount_manager",
+            "mrb_check",
+            "mrbeam_ledstrips",
+            "netconnectd",
+        ]
+        for logrotate in logrotates:
+            dst = os.path.join("/etc/logrotate.d/" + logrotate)
+            src = os.path.join(
+                __package_path__,
+                "files/migrate_logrotate",
+                logrotate,
+            )
+            exec_cmd_mock.assert_any_call(
+                "sudo cp {src} {dst}".format(src=src, dst=dst)
+            )
+
+        exec_cmd_mock.ssert_any_call("sudo logrotate /etc/logrotate.conf")