From d232232b3cad65c4bd94e236dc5b687a0462345a Mon Sep 17 00:00:00 2001 From: Adrian Celebanski Date: Wed, 6 Mar 2024 16:18:33 +0100 Subject: [PATCH 1/5] Modify and rename check_non_finished_jobs method --- panos_upgrade_assurance/check_firewall.py | 35 ++++++++++++++++------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/panos_upgrade_assurance/check_firewall.py b/panos_upgrade_assurance/check_firewall.py index 43edc1c..8a8475b 100644 --- a/panos_upgrade_assurance/check_firewall.py +++ b/panos_upgrade_assurance/check_firewall.py @@ -103,7 +103,7 @@ def __init__(self, node: FirewallProxy, skip_force_locale: Optional[bool] = Fals CheckType.MP_DP_CLOCK_SYNC: self.check_mp_dp_sync, CheckType.CERTS: self.check_ssl_cert_requirements, CheckType.UPDATES: self.check_scheduled_updates, - CheckType.JOBS: self.check_non_finished_jobs, + CheckType.JOBS: self.check_jobs, } self._health_check_method_mapping = { @@ -1048,33 +1048,48 @@ def check_scheduled_updates(self, test_window: int = 60) -> CheckResult: return result - def check_non_finished_jobs(self) -> CheckResult: - """Check for any job with status different than FIN. + def check_jobs(self, job_type: str = None, job_status: str = "FIN", job_result: str = None) -> CheckResult: + """Check for any job that does not match with the type, status and result set in the parameters (by default looks for + jobs with status different than FIN). # Returns CheckResult: Object of [`CheckResult`](/panos/docs/panos-upgrade-assurance/api/utils#class-checkresult) class taking \ value of: - * [`CheckStatus.SUCCESS`](/panos/docs/panos-upgrade-assurance/api/utils#class-checkstatus) when all jobs are in FIN state. + * [`CheckStatus.SUCCESS`](/panos/docs/panos-upgrade-assurance/api/utils#class-checkstatus) when all jobs match the type, + status and result set in the parameters (by default, when all jobs are in FIN state). * [`CheckStatus.FAIL`](/panos/docs/panos-upgrade-assurance/api/utils#class-checkstatus) otherwise, `CheckResult.reason` - field contains information about the 1st job found with status different than FIN (job ID and the actual - status). + field contains information about the 1st job found with type, status or result different than desired (job + ID and the actual value). * [`CheckStatus.SKIPPED`](/panos/docs/panos-upgrade-assurance/api/utils#class-checkstatus) when there are no jobs on a device. """ + + if job_type is None and job_status is None and job_result is None: + result.status = CheckStatus.SKIPPED + result.reason = "Neither 'job_type', 'job_status' nor 'job_result' parameters were set." + return result + result = CheckResult() all_jobs = self._node.get_jobs() if all_jobs: for jid, job in all_jobs.items(): - if job["status"] != "FIN": - result.reason = f"At least one job (ID={jid}) is not in finished state (state={job['status']})." + if job_type and (job["type"] != job_type): + result.reason = f"At least one job (ID={jid}) does not have a desired type of {job_type} (status={job['type']})." + return result + elif job_status and (job["status"] != job_status): + result.reason = f"At least one job (ID={jid}) does not have a desired status of {job_status} (status={job['status']})." + return result + elif job_result and (job["result"] != job_result): + result.reason = f"At least one job (ID={jid}) does not have a desired result of {job_result} (result={job['result']})." + return result + else: + result.status = CheckStatus.SUCCESS return result - result.status = CheckStatus.SUCCESS - return result else: result.status = CheckStatus.SKIPPED result.reason = "No jobs found on device. This is unusual, please investigate." From 39ed6cc9293bcddb10ce0f3ea1c9756b72e9f81a Mon Sep 17 00:00:00 2001 From: Adrian Celebanski Date: Wed, 6 Mar 2024 16:42:26 +0100 Subject: [PATCH 2/5] Fix unit test for checking jobs --- tests/test_check_firewall.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_check_firewall.py b/tests/test_check_firewall.py index 39e22d2..0a74aa9 100644 --- a/tests/test_check_firewall.py +++ b/tests/test_check_firewall.py @@ -1061,7 +1061,7 @@ def test_check_jobs_success(self, check_firewall_mock): check_firewall_mock._node.get_jobs = lambda: jobs - assert check_firewall_mock.check_non_finished_jobs() == CheckResult(status=CheckStatus.SUCCESS) + assert check_firewall_mock.check_jobs() == CheckResult(status=CheckStatus.SUCCESS) def test_check_jobs_failure(self, check_firewall_mock): jobs = { @@ -1099,13 +1099,13 @@ def test_check_jobs_failure(self, check_firewall_mock): }, } check_firewall_mock._node.get_jobs = lambda: jobs - result = CheckResult(status=CheckStatus.FAIL, reason="At least one job (ID=4) is not in finished state (state=ACC).") - assert check_firewall_mock.check_non_finished_jobs() == result + result = CheckResult(status=CheckStatus.FAIL, reason="At least one job (ID=4) does not have a desired status of FIN (status=ACC).") + assert check_firewall_mock.check_jobs() == result def test_check_jobs_no_jobs(self, check_firewall_mock): check_firewall_mock._node.get_jobs = lambda: {} result = CheckResult(status=CheckStatus.SKIPPED, reason="No jobs found on device. This is unusual, please investigate.") - assert check_firewall_mock.check_non_finished_jobs() == result + assert check_firewall_mock.check_jobs() == result def test_run_readiness_checks(self, check_firewall_mock): check_firewall_mock._check_method_mapping = { From 81446d8477ba95e5ee54231d57354ce1c154b5f8 Mon Sep 17 00:00:00 2001 From: Adrian Celebanski Date: Wed, 6 Mar 2024 16:48:15 +0100 Subject: [PATCH 3/5] Fix linting and update api docs --- .../api/check_firewall.md | 16 ++++++++++------ panos_upgrade_assurance/check_firewall.py | 17 +++++++++++------ tests/test_check_firewall.py | 4 +++- 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/docs/panos-upgrade-assurance/api/check_firewall.md b/docs/panos-upgrade-assurance/api/check_firewall.md index 72dc44c..42f3bae 100644 --- a/docs/panos-upgrade-assurance/api/check_firewall.md +++ b/docs/panos-upgrade-assurance/api/check_firewall.md @@ -529,23 +529,27 @@ __Returns__ * [`CheckStatus.ERROR`](/panos/docs/panos-upgrade-assurance/api/utils#class-checkstatus) when the `test_window` parameter does not meet criteria. -### `CheckFirewall.check_non_finished_jobs` +### `CheckFirewall.check_jobs` ```python -def check_non_finished_jobs() -> CheckResult +def check_jobs(job_type: str = None, + job_status: str = "FIN", + job_result: str = None) -> CheckResult ``` -Check for any job with status different than FIN. +Check for any job that does not match with the type, status and result set in the parameters (by default looks for +jobs with status different than FIN). __Returns__ `CheckResult`: Object of [`CheckResult`](/panos/docs/panos-upgrade-assurance/api/utils#class-checkresult) class taking value of: -* [`CheckStatus.SUCCESS`](/panos/docs/panos-upgrade-assurance/api/utils#class-checkstatus) when all jobs are in FIN state. +* [`CheckStatus.SUCCESS`](/panos/docs/panos-upgrade-assurance/api/utils#class-checkstatus) when all jobs match the type, + status and result set in the parameters (by default, when all jobs are in FIN state). * [`CheckStatus.FAIL`](/panos/docs/panos-upgrade-assurance/api/utils#class-checkstatus) otherwise, `CheckResult.reason` - field contains information about the 1st job found with status different than FIN (job ID and the actual - status). + field contains information about the 1st job found with type, status or result different than desired (job + ID and the actual value). * [`CheckStatus.SKIPPED`](/panos/docs/panos-upgrade-assurance/api/utils#class-checkstatus) when there are no jobs on a device. diff --git a/panos_upgrade_assurance/check_firewall.py b/panos_upgrade_assurance/check_firewall.py index 8a8475b..f0d50c8 100644 --- a/panos_upgrade_assurance/check_firewall.py +++ b/panos_upgrade_assurance/check_firewall.py @@ -1066,26 +1066,31 @@ def check_jobs(self, job_type: str = None, job_status: str = "FIN", job_result: device. """ - + result = CheckResult() + if job_type is None and job_status is None and job_result is None: result.status = CheckStatus.SKIPPED result.reason = "Neither 'job_type', 'job_status' nor 'job_result' parameters were set." return result - - result = CheckResult() all_jobs = self._node.get_jobs() if all_jobs: for jid, job in all_jobs.items(): if job_type and (job["type"] != job_type): - result.reason = f"At least one job (ID={jid}) does not have a desired type of {job_type} (status={job['type']})." + result.reason = ( + f"At least one job (ID={jid}) does not have a desired type of {job_type} (status={job['type']})." + ) return result elif job_status and (job["status"] != job_status): - result.reason = f"At least one job (ID={jid}) does not have a desired status of {job_status} (status={job['status']})." + result.reason = ( + f"At least one job (ID={jid}) does not have a desired status of {job_status} (status={job['status']})." + ) return result elif job_result and (job["result"] != job_result): - result.reason = f"At least one job (ID={jid}) does not have a desired result of {job_result} (result={job['result']})." + result.reason = ( + f"At least one job (ID={jid}) does not have a desired result of {job_result} (result={job['result']})." + ) return result else: result.status = CheckStatus.SUCCESS diff --git a/tests/test_check_firewall.py b/tests/test_check_firewall.py index 0a74aa9..da94eff 100644 --- a/tests/test_check_firewall.py +++ b/tests/test_check_firewall.py @@ -1099,7 +1099,9 @@ def test_check_jobs_failure(self, check_firewall_mock): }, } check_firewall_mock._node.get_jobs = lambda: jobs - result = CheckResult(status=CheckStatus.FAIL, reason="At least one job (ID=4) does not have a desired status of FIN (status=ACC).") + result = CheckResult( + status=CheckStatus.FAIL, reason="At least one job (ID=4) does not have a desired status of FIN (status=ACC)." + ) assert check_firewall_mock.check_jobs() == result def test_check_jobs_no_jobs(self, check_firewall_mock): From aa0a71c92c3793d5a9676a76d9235554dc750fdd Mon Sep 17 00:00:00 2001 From: Adrian Celebanski Date: Wed, 6 Mar 2024 16:54:40 +0100 Subject: [PATCH 4/5] Update documentation --- docs/panos-upgrade-assurance/configuration_details.mdx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/panos-upgrade-assurance/configuration_details.mdx b/docs/panos-upgrade-assurance/configuration_details.mdx index ee3f9f6..0c7e7ef 100644 --- a/docs/panos-upgrade-assurance/configuration_details.mdx +++ b/docs/panos-upgrade-assurance/configuration_details.mdx @@ -762,12 +762,12 @@ checks_configuration: ### `jobs` -Verify if there are any running/pending jobs in the job queue. Any job with a status different than `FIN` will cause -the check to fail. +Verify if there are any running/pending jobs in the job queue. Any job with a type, status or result different than set in the +parameters (by default checks only whether the job status equals to `FIN`), will cause the check to fail. Does not require configuration. -**Method**: [`CheckFirewall.check_non_finished_jobs()`](/panos/docs/panos-upgrade-assurance/api/check_firewall#checkfirewallcheck_non_finished_jobs) +**Method**: [`CheckFirewall.check_jobs()`](/panos/docs/panos-upgrade-assurance/api/check_firewall#checkfirewallcheck_jobs) ### `ntp_sync` From bbe3f4137451108d2a75c52d13002de25222c744 Mon Sep 17 00:00:00 2001 From: Adrian Celebanski Date: Tue, 12 Mar 2024 12:37:06 +0100 Subject: [PATCH 5/5] Rename and modify the method, add custom unit test --- .../api/check_firewall.md | 8 ++-- .../configuration_details.mdx | 2 +- panos_upgrade_assurance/check_firewall.py | 11 ++--- tests/test_check_firewall.py | 47 +++++++++++++++++-- 4 files changed, 54 insertions(+), 14 deletions(-) diff --git a/docs/panos-upgrade-assurance/api/check_firewall.md b/docs/panos-upgrade-assurance/api/check_firewall.md index 42f3bae..e1e2658 100644 --- a/docs/panos-upgrade-assurance/api/check_firewall.md +++ b/docs/panos-upgrade-assurance/api/check_firewall.md @@ -529,12 +529,12 @@ __Returns__ * [`CheckStatus.ERROR`](/panos/docs/panos-upgrade-assurance/api/utils#class-checkstatus) when the `test_window` parameter does not meet criteria. -### `CheckFirewall.check_jobs` +### `CheckFirewall.check_non_matching_jobs` ```python -def check_jobs(job_type: str = None, - job_status: str = "FIN", - job_result: str = None) -> CheckResult +def check_non_matching_jobs(job_type: str = None, + job_status: str = "FIN", + job_result: str = None) -> CheckResult ``` Check for any job that does not match with the type, status and result set in the parameters (by default looks for diff --git a/docs/panos-upgrade-assurance/configuration_details.mdx b/docs/panos-upgrade-assurance/configuration_details.mdx index 0c7e7ef..bb6b817 100644 --- a/docs/panos-upgrade-assurance/configuration_details.mdx +++ b/docs/panos-upgrade-assurance/configuration_details.mdx @@ -767,7 +767,7 @@ parameters (by default checks only whether the job status equals to `FIN`), will Does not require configuration. -**Method**: [`CheckFirewall.check_jobs()`](/panos/docs/panos-upgrade-assurance/api/check_firewall#checkfirewallcheck_jobs) +**Method**: [`CheckFirewall.check_non_matching_jobs()`](/panos/docs/panos-upgrade-assurance/api/check_firewall#checkfirewallcheck_non_matching_jobs) ### `ntp_sync` diff --git a/panos_upgrade_assurance/check_firewall.py b/panos_upgrade_assurance/check_firewall.py index f0d50c8..f5fc8df 100644 --- a/panos_upgrade_assurance/check_firewall.py +++ b/panos_upgrade_assurance/check_firewall.py @@ -103,7 +103,7 @@ def __init__(self, node: FirewallProxy, skip_force_locale: Optional[bool] = Fals CheckType.MP_DP_CLOCK_SYNC: self.check_mp_dp_sync, CheckType.CERTS: self.check_ssl_cert_requirements, CheckType.UPDATES: self.check_scheduled_updates, - CheckType.JOBS: self.check_jobs, + CheckType.JOBS: self.check_non_matching_jobs, } self._health_check_method_mapping = { @@ -1048,7 +1048,7 @@ def check_scheduled_updates(self, test_window: int = 60) -> CheckResult: return result - def check_jobs(self, job_type: str = None, job_status: str = "FIN", job_result: str = None) -> CheckResult: + def check_non_matching_jobs(self, job_type: str = None, job_status: str = "FIN", job_result: str = None) -> CheckResult: """Check for any job that does not match with the type, status and result set in the parameters (by default looks for jobs with status different than FIN). @@ -1079,7 +1079,7 @@ def check_jobs(self, job_type: str = None, job_status: str = "FIN", job_result: for jid, job in all_jobs.items(): if job_type and (job["type"] != job_type): result.reason = ( - f"At least one job (ID={jid}) does not have a desired type of {job_type} (status={job['type']})." + f"At least one job (ID={jid}) does not have a desired type of {job_type} (type={job['type']})." ) return result elif job_status and (job["status"] != job_status): @@ -1092,9 +1092,8 @@ def check_jobs(self, job_type: str = None, job_status: str = "FIN", job_result: f"At least one job (ID={jid}) does not have a desired result of {job_result} (result={job['result']})." ) return result - else: - result.status = CheckStatus.SUCCESS - return result + result.status = CheckStatus.SUCCESS + return result else: result.status = CheckStatus.SKIPPED result.reason = "No jobs found on device. This is unusual, please investigate." diff --git a/tests/test_check_firewall.py b/tests/test_check_firewall.py index da94eff..00aeafd 100644 --- a/tests/test_check_firewall.py +++ b/tests/test_check_firewall.py @@ -1061,7 +1061,7 @@ def test_check_jobs_success(self, check_firewall_mock): check_firewall_mock._node.get_jobs = lambda: jobs - assert check_firewall_mock.check_jobs() == CheckResult(status=CheckStatus.SUCCESS) + assert check_firewall_mock.check_non_matching_jobs() == CheckResult(status=CheckStatus.SUCCESS) def test_check_jobs_failure(self, check_firewall_mock): jobs = { @@ -1102,12 +1102,53 @@ def test_check_jobs_failure(self, check_firewall_mock): result = CheckResult( status=CheckStatus.FAIL, reason="At least one job (ID=4) does not have a desired status of FIN (status=ACC)." ) - assert check_firewall_mock.check_jobs() == result + assert check_firewall_mock.check_non_matching_jobs() == result + + def test_check_jobs_custom(self, check_firewall_mock): + jobs = { + "4": { + "tenq": "2023/08/07 04:00:40", + "tdeq": "04:00:40", + "user": "Auto update agent", + "type": "WildFire", + "status": "ACC", + "queued": "NO", + "stoppable": "no", + "result": "OK", + "tfin": "2023/08/07 04:00:45", + "description": None, + "positionInQ": "0", + "progress": "2023/08/07 04:00:45", + "details": {"line": ["Configuration committed successfully", "Successfully committed last configuration"]}, + "warnings": None, + }, + "1": { + "tenq": "2023/08/07 03:59:57", + "tdeq": "03:59:57", + "user": None, + "type": "AutoCom", + "status": "FIN", + "queued": "NO", + "stoppable": "no", + "result": "OK", + "tfin": "2023/08/07 04:00:28", + "description": None, + "positionInQ": "0", + "progress": "100", + "details": {"line": ["Configuration committed successfully", "Successfully committed last configuration"]}, + "warnings": None, + }, + } + check_firewall_mock._node.get_jobs = lambda: jobs + result = CheckResult( + status=CheckStatus.FAIL, reason="At least one job (ID=4) does not have a desired type of AutoCom (type=WildFire)." + ) + assert check_firewall_mock.check_non_matching_jobs(job_type="AutoCom", job_status="ACC") == result def test_check_jobs_no_jobs(self, check_firewall_mock): check_firewall_mock._node.get_jobs = lambda: {} result = CheckResult(status=CheckStatus.SKIPPED, reason="No jobs found on device. This is unusual, please investigate.") - assert check_firewall_mock.check_jobs() == result + assert check_firewall_mock.check_non_matching_jobs() == result def test_run_readiness_checks(self, check_firewall_mock): check_firewall_mock._check_method_mapping = {