From 27ff24d1888eff47b55c4d64b71acf8a78bd8e69 Mon Sep 17 00:00:00 2001 From: FroggyFlox Date: Sat, 11 Mar 2023 17:22:40 -0500 Subject: [PATCH 1/3] Account for empty meta of scheduled tasks of type reboot #2514 We currently assume a taskdefinition meta is always filled with information when running the reboot_shutdown.py script. While this is the case for tasks of type shutdown and suspend, tasks of type reboot only have an empty Dict as meta. This commit thus first checks for an empty meta before trying to extract information from it. Add unit tests coverage for reboot_shutdown.py. --- .../scheduled_tasks/reboot_shutdown.py | 8 +- src/rockstor/scripts/tests/__init__.py | 0 .../scripts/tests/test_reboot_shutdown.py | 162 ++++++++++++++++++ 3 files changed, 166 insertions(+), 4 deletions(-) create mode 100644 src/rockstor/scripts/tests/__init__.py create mode 100644 src/rockstor/scripts/tests/test_reboot_shutdown.py diff --git a/src/rockstor/scripts/scheduled_tasks/reboot_shutdown.py b/src/rockstor/scripts/scheduled_tasks/reboot_shutdown.py index 1fd723f57..b735ff71f 100644 --- a/src/rockstor/scripts/scheduled_tasks/reboot_shutdown.py +++ b/src/rockstor/scripts/scheduled_tasks/reboot_shutdown.py @@ -1,5 +1,5 @@ """ -Copyright (c) 2012-2020 RockStor, Inc. +Copyright (c) 2012-2023 RockStor, Inc. This file is part of RockStor. RockStor is free software; you can redistribute it and/or modify @@ -32,7 +32,7 @@ logger = logging.getLogger(__name__) -def validate_shutdown_meta(meta): +def validate_reboot_shutdown_meta(meta): if type(meta) != dict: raise Exception("meta must be a dictionary, not %s" % type(meta)) return meta @@ -47,7 +47,7 @@ def all_devices_offline(addresses): def run_conditions_met(meta): - if meta["ping_scan"]: + if meta and meta["ping_scan"]: address_parser = csv_reader([meta["ping_scan_addresses"]], delimiter=",") addresses = list(address_parser) @@ -104,7 +104,7 @@ def main(): ) return meta = json.loads(tdo.json_meta) - validate_shutdown_meta(meta) + validate_reboot_shutdown_meta(meta) if not run_conditions_met(meta): logger.debug( diff --git a/src/rockstor/scripts/tests/__init__.py b/src/rockstor/scripts/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/rockstor/scripts/tests/test_reboot_shutdown.py b/src/rockstor/scripts/tests/test_reboot_shutdown.py new file mode 100644 index 000000000..7fc37cee5 --- /dev/null +++ b/src/rockstor/scripts/tests/test_reboot_shutdown.py @@ -0,0 +1,162 @@ +""" +Copyright (c) 2012-2023 RockStor, Inc. +This file is part of RockStor. +RockStor is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published +by the Free Software Foundation; either version 2 of the License, +or (at your option) any later version. +RockStor is distributed in the hope that it will be useful, but +WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +General Public License for more details. +You should have received a copy of the GNU General Public License +along with this program. If not, see . +""" +import unittest +from mock import patch + +from scripts.scheduled_tasks.reboot_shutdown import ( + validate_reboot_shutdown_meta, + all_devices_offline, + run_conditions_met, +) + + +class ScriptTests(unittest.TestCase): + """ + The tests in this suite can be run via the following command: + cd + ./bin/test --settings=test-settings -v 3 -p test_system_network* + """ + + def setUp(self): + self.patch_is_network_device_responding = patch( + "scripts.scheduled_tasks.reboot_shutdown.is_network_device_responding" + ) + self.mock_is_network_device_responding = ( + self.patch_is_network_device_responding.start() + ) + + def tearDown(self): + pass + + def test_validate_reboot_shutdown_meta(self): + """ + Test the following scenarios: + 1. Reboot task valid + 2. Suspend/Shutdown valid + 3. Reboot/Suspend/Shutdown task invalid + """ + # 1. Valid meta is a Dict (empty if reboot task) + meta = {} + expected = meta + returned = validate_reboot_shutdown_meta(meta) + self.assertEqual( + returned, + expected, + msg="Un-expected validate_reboot_shutdown_meta result:\n " + "returned = ({}).\n " + "expected = ({}).".format(returned, expected), + ) + + # 2. Valid Shutdown meta + meta = { + "ping_scan_addresses": "1.1.1.1,8.8.8.8", + "ping_scan_interval": "5", + "rtc_minute": 0, + "ping_scan": True, + "wakeup": False, + "rtc_hour": 0, + "ping_scan_iterations": "3", + } + expected = meta + returned = validate_reboot_shutdown_meta(meta) + self.assertEqual( + returned, + expected, + msg="Un-expected validate_reboot_shutdown_meta result:\n " + "returned = ({}).\n " + "expected = ({}).".format(returned, expected), + ) + + # 3. Invalid meta (not a Dict) + meta = [] + with self.assertRaises(Exception): + validate_reboot_shutdown_meta(meta) + + def test_all_devices_offline(self): + """ + Test the following scenarios: + 1. Target devices are OFFLINE + 2. Target devices are ONLINE + """ + addresses = ["1.1.1.1", "8.8.8.8"] + # 1. Mock target devices as OFFLINE: should return True + self.mock_is_network_device_responding.return_value = False + returned = all_devices_offline(addresses) + self.assertTrue( + returned, + msg="Un-expected all_devices_offline result:\n " + "returned = ({}).\n " + "expected = True.".format(returned), + ) + + # 2. Mock target devices as ONLINE: should return False + self.mock_is_network_device_responding.return_value = True + returned = all_devices_offline(addresses) + self.assertFalse( + returned, + msg="Un-expected all_devices_offline result:\n " + "returned = ({}).\n " + "expected = False.".format(returned), + ) + + def test_run_conditions_met(self): + """ + Test the following scenarios: + 1. Reboot task: empty meta + 2. Shutdown/Suspend task, valid meta, ping targets OFFLINE + 3. Shutdown/Suspend task, valid meta, ping targets ONLINE + + Note that for Shutdown/Suspend tasks, an invalid meta will be caught + by validate_reboot_shutdown_meta() before run_conditions_met() + so no need to test for an invalid meta here. + """ + # 1. Reboot task, empty meta: should return True + meta = {} + returned = run_conditions_met(meta) + self.assertTrue( + returned, + msg="Un-expected run_conditions_met result:\n " + "returned = ({}).\n " + "expected = True.".format(returned), + ) + + # Shutdown/Suspend task + meta = { + "ping_scan_addresses": "1.1.1.1,8.8.8.8", + "ping_scan_interval": "5", + "rtc_minute": 0, + "ping_scan": True, + "wakeup": False, + "rtc_hour": 0, + "ping_scan_iterations": "1", + } + # 2. ping targets OFFLINE: should return True + self.mock_is_network_device_responding.return_value = False + returned = run_conditions_met(meta) + self.assertTrue( + returned, + msg="Un-expected run_conditions_met result:\n " + "returned = ({}).\n " + "expected = True.".format(returned), + ) + # 3. ping targets ONLINE: should return False + self.mock_is_network_device_responding.return_value = True + returned = run_conditions_met(meta) + self.assertFalse( + returned, + msg="Un-expected run_conditions_met result:\n " + "returned = ({}).\n " + "expected = False.".format(returned), + ) From 20060dba624f16f32053d9ad4aa9af3dc054cd31 Mon Sep 17 00:00:00 2001 From: FroggyFlox Date: Sat, 11 Mar 2023 17:31:52 -0500 Subject: [PATCH 2/3] Convert to str.format() #2514 --- .../scripts/scheduled_tasks/reboot_shutdown.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/rockstor/scripts/scheduled_tasks/reboot_shutdown.py b/src/rockstor/scripts/scheduled_tasks/reboot_shutdown.py index b735ff71f..791a10cf3 100644 --- a/src/rockstor/scripts/scheduled_tasks/reboot_shutdown.py +++ b/src/rockstor/scripts/scheduled_tasks/reboot_shutdown.py @@ -34,7 +34,7 @@ def validate_reboot_shutdown_meta(meta): if type(meta) != dict: - raise Exception("meta must be a dictionary, not %s" % type(meta)) + raise Exception("meta must be a dictionary, not {}".format(type(meta))) return meta @@ -99,8 +99,8 @@ def main(): aw = APIWrapper() if tdo.task_type not in ["reboot", "shutdown", "suspend"]: logger.error( - "task_type(%s) is not a system reboot, " - "shutdown or suspend." % tdo.task_type + "task_type({}) is not a system reboot, " + "shutdown or suspend.".format(tdo.task_type) ) return meta = json.loads(tdo.json_meta) @@ -119,7 +119,7 @@ def main(): try: # set default command url before checking if it's a shutdown # and if we have an rtc wake up - url = "commands/%s" % tdo.task_type + url = "commands/{}".format(tdo.task_type) # if task_type is shutdown and rtc wake up true # parse crontab hour & minute vs rtc hour & minute to state @@ -144,15 +144,15 @@ def main(): epoch += timedelta(days=1) epoch = epoch.strftime("%s") - url = "%s/%s" % (url, epoch) + url = "{}/{}".format(url, epoch) aw.api_call(url, data=None, calltype="post", save_error=False) - logger.debug("System %s scheduled" % tdo.task_type) + logger.debug("System {} scheduled".format(tdo.task_type)) t.state = "finished" except Exception as e: t.state = "failed" - logger.error("Failed to schedule system %s" % tdo.task_type) + logger.error("Failed to schedule system {}".format(tdo.task_type)) logger.exception(e) finally: From 82acd081be68e2e61d21b79c877becdaa9a413ed Mon Sep 17 00:00:00 2001 From: FroggyFlox Date: Sat, 11 Mar 2023 18:16:34 -0500 Subject: [PATCH 3/3] Fix Class name and docstring for reboot_shutdown.py tests #2514 --- src/rockstor/scripts/tests/test_reboot_shutdown.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/rockstor/scripts/tests/test_reboot_shutdown.py b/src/rockstor/scripts/tests/test_reboot_shutdown.py index 7fc37cee5..67d805d49 100644 --- a/src/rockstor/scripts/tests/test_reboot_shutdown.py +++ b/src/rockstor/scripts/tests/test_reboot_shutdown.py @@ -22,11 +22,11 @@ ) -class ScriptTests(unittest.TestCase): +class RebootShutdownScriptTests(unittest.TestCase): """ - The tests in this suite can be run via the following command: - cd - ./bin/test --settings=test-settings -v 3 -p test_system_network* + To run the tests: + export DJANGO_SETTINGS_MODULE="settings" + cd src/rockstor && poetry run django-admin test -v 2 -p test_reboot_shutdown.py """ def setUp(self):