feat!: check for scheduled update jobs #106

FoSix · 2023-08-04T09:41:41Z

Description

Check if there are any upcoming dynamic update jobs within a time window.
BREAKING CHANGE: management and data plane FirewallProxy method have their return values format changed from a dict to a datetime object.

Motivation and Context

An upgrade might fail if an update job kicks in. This check will verify if there are any jobs pending to run in a given time frame (default: 60 minutes).

The time window is calculated based on device's local time (management plane clock).

How Has This Been Tested?

Tested on a device connected to Panorama.

Screenshots (if appropriate)

Types of changes

New feature (non-breaking change which adds functionality)

Checklist

I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes if appropriate.
All new and existing tests passed.

…eduled-update-jobs

examples/readiness_checks/run_readiness_checks.py

alperenkose · 2023-08-09T09:25:46Z

panos_upgrade_assurance/check_firewall.py

+            if schedule_type.split("-")[1] == "min":
+                time_distance = 1
+                details = "every minute"
+            elif schedule_type.split("-")[1] == "hour":


There is also this "at" element for "every-hour", "every-15-mins" and "every-30-mins" schedules, do we ignore that on purpose?

wildfire { recurring { every-30-mins { at 2; } } }

see comment below

alperenkose · 2023-08-09T10:08:31Z

panos_upgrade_assurance/check_firewall.py

+            result.reason = "No scheduled job present on the device."
+            return result
+
+        if test_window < 60:


Can we make the min test window lower like 10 or even 5 mins? For example we can use this to check if there are any scheduled jobs just before starting a software upgrade and checking the immediate time window would suffice since it's ok to start the software download/upgrade if a scheduled job exists 15 min later let's say.

I did 60 minutes the minimum value on purpose, for several reasons:

you cannot predict how long will an update take

if it fails and you retry, or downgrade you might hit an update job if the time window is to small

if you do multiple upgrades at the same time and you do not use free strategy you might hit an update if the window is to small

if you do an HA pair, you would do one check for the whole pair, if you split the checks per each node you might fail the 2nd node and have the pair in an inconsistent state

if you do multiple upgrades in one run (9.1 -> 10.0 -> 10.1, etc) you would typically assume that you check once, upgrade multiple times, if you use a short window and check before each upgrade you might brake the run somewhere in between

FWs are usually crucial, if you do maintenance windows for an upgrade it lasts several hours, it does not make sense to check next 10 minutes, you would like to have the whole maintenance window clear

etc, etc

Hence I've figured it does not make sens to write logic to calculate the 5, 15, 30 minutes intervals as normally you wouldn't and shouldn't use them

I see your points, though I'm thinking the library should be flexible as possible. In an ideal environment, I agree that the maintenance window should be clear but I think that won't be easy to achieve. Let's keep it though, we can get back to this later if needed once we have more usage areas.

I think that we should enforce something like best practices, we already do it in couple of places (like minor+1 upgrade strategy). I know you can think about it as a way of forcing a user to some values that he might not be OK with. But I think these are sane defaults. @adambaumeister, @horiagunica - what do you think?

panos_upgrade_assurance/check_firewall.py

alperenkose · 2023-08-09T11:40:12Z

panos_upgrade_assurance/check_firewall.py

+            details = f"at {next_occurrence.time()}"
+
+        elif schedule_type == "hourly":
+            time_distance = 60


why don't we calculate the time difference when it's hourly?

check this comment

I see but I think we can still calculate it since this is a time difference function, and check_scheduled_updates would still work the same. This is also the current case with the "daily" schedule when there might be less than 60 minutes for the next occurrence we return the exact difference.

if we would calculate the exact difference, the only thing that would change would be the way we present the next occurrence in the comments.

I understand but it looks inconsistent with the "daily" schedule where we provide the exact minutes of difference if it's less than 60 minutes, isn't it?

panos_upgrade_assurance/firewall_proxy.py

github-actions · 2023-08-10T09:04:33Z

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines	Covered	Coverage	Threshold	Status
909	895	98%	95%	🟢

New Files

No new covered files...

Modified Files

File	Coverage	Status
panos_upgrade_assurance/check_firewall.py	97%	🟢
panos_upgrade_assurance/exceptions.py	100%	🟢
panos_upgrade_assurance/firewall_proxy.py	100%	🟢
panos_upgrade_assurance/utils.py	98%	🟢
TOTAL	99%	🟢

updated for commit: a29352c by action🐍

github-actions · 2023-08-10T09:06:11Z

A Preview PR in PanDev repo has been created. You can view it here.

Co-authored-by: Alp Eren Kose <alperenkose@gmail.com>

examples/low_level_methods/run_low_level_methods.py

panos_upgrade_assurance/firewall_proxy.py

alperenkose

Some additional comments :)

alperenkose · 2023-08-28T12:12:30Z

examples/low_level_methods/run_low_level_methods.py

+    print(f"\n  dynamic schedules: {firewall.get_update_schedules()}")
+    pprint(firewall.get_update_schedules())


Why do we have the firewall.get_update_schedules() call twice?

it's by mistake

alperenkose · 2023-08-28T12:19:14Z

panos_upgrade_assurance/check_firewall.py

+            details = f"at {next_occurrence.time()}"
+
+        elif schedule_type == "hourly":
+            time_distance = 60


I understand but it looks inconsistent with the "daily" schedule where we provide the exact minutes of difference if it's less than 60 minutes, isn't it?

panos_upgrade_assurance/exceptions.py

alperenkose · 2023-08-28T12:55:28Z

panos_upgrade_assurance/firewall_proxy.py

+        dict: All dynamic updates schedules, key is the entity type to update, like: threats, wildfire, etc.
+
+        ```python showLineNumbers title="Sample output, showing values coming from Panorama"
+        {'@ptpl': 'lab',


I'm wondering if it would be easier to parse with XML response, do you have these '@' and '#text' fields with it?

that's a sample output in XML:

<response status="success" code="19"> <result total-count="1" count="1"> <update-schedule ptpl="interfaces" src="tpl"> <threats ptpl="interfaces" src="tpl"> <recurring ptpl="interfaces" src="tpl"> <sync-to-peer ptpl="interfaces" src="tpl">yes</sync-to-peer> <daily ptpl="interfaces" src="tpl"> <at ptpl="interfaces" src="tpl">15:30</at> <action ptpl="interfaces" src="tpl">download-and-install</action> </daily> </recurring> </threats> <anti-virus ptpl="interfaces" src="tpl"> <recurring ptpl="interfaces" src="tpl"> <daily ptpl="interfaces" src="tpl"> <at ptpl="interfaces" src="tpl">07:45</at> <action ptpl="interfaces" src="tpl">download-and-install</action> </daily> </recurring> </anti-virus> <global-protect-clientless-vpn ptpl="interfaces" src="tpl"> <recurring ptpl="interfaces" src="tpl"> <hourly ptpl="interfaces" src="tpl"> <at ptpl="interfaces" src="tpl">0</at> <action ptpl="interfaces" src="tpl">download-and-install</action> </hourly> </recurring> </global-protect-clientless-vpn> <global-protect-datafile ptpl="interfaces" src="tpl"> <recurring ptpl="interfaces" src="tpl"> <weekly ptpl="interfaces" src="tpl"> <at ptpl="interfaces" src="tpl">00:45</at> <day-of-week ptpl="interfaces" src="tpl">sunday</day-of-week> <action ptpl="interfaces" src="tpl">download-and-install</action> </weekly> </recurring> </global-protect-datafile> <wf-private ptpl="interfaces" src="tpl"> <recurring ptpl="interfaces" src="tpl"> <every-5-mins ptpl="interfaces" src="tpl"> <at ptpl="interfaces" src="tpl">1</at> <action ptpl="interfaces" src="tpl">download-and-install</action> </every-5-mins> <sync-to-peer ptpl="interfaces" src="tpl">yes</sync-to-peer> </recurring> </wf-private> <wildfire ptpl="interfaces" src="tpl"> <recurring src="tpl"> <every-min src="tpl"> <action src="tpl">download-and-install</action> </every-min> </recurring> </wildfire> </update-schedule> </result> </response>

I still do not feel cormortable in doind XML. I do prefer dicts. But if you feel it would be more elegant, you can try to refactor that code.

## [0.2.0](v0.1.4...v0.2.0) (2023-09-13) ### ⚠ BREAKING CHANGES * check for scheduled update jobs (#106) ### Features * check for scheduled update jobs ([#106](#106)) ([4b303f9](4b303f9))

FoSix · 2023-09-13T01:28:07Z

🎉 This PR is included in version 0.2.0 🎉

The release is available on PyPI and GitHub release

Posted by semantic-release bot

FoSix added 4 commits August 2, 2023 11:49

low level method getting schedules

d63da04

wrong branch code

5f64be2

working code

a2dd1ad

working code with failing or missing tests

9c5cbc9

FoSix linked an issue Aug 4, 2023 that may be closed by this pull request

feat: check for scheduled update jobs #88

Closed

FoSix requested review from alperenkose, adambaumeister and horiagunica as code owners August 4, 2023 09:41

FoSix changed the title ~~feat: check for scheduled update jobs~~ feat!: check for scheduled update jobs Aug 4, 2023

FoSix added 3 commits August 4, 2023 12:12

Merge remote-tracking branch 'origin/main' into 88-feat-check-for-sch…

83391db

…eduled-update-jobs

cleanup ignored files

092a345

Merge branch 'main' into 88-feat-check-for-scheduled-update-jobs

6c61024

alperenkose reviewed Aug 9, 2023

View reviewed changes

panos_upgrade_assurance/firewall_proxy.py Outdated Show resolved Hide resolved

panos_upgrade_assurance/firewall_proxy.py Outdated Show resolved Hide resolved

panos_upgrade_assurance/firewall_proxy.py Outdated Show resolved Hide resolved

updating tests

e6eb1a9

FoSix and others added 2 commits August 10, 2023 11:33

pr suggestions

2eb4992

Update panos_upgrade_assurance/firewall_proxy.py

bab1ccf

Co-authored-by: Alp Eren Kose <alperenkose@gmail.com>

FoSix requested a review from alperenkose August 10, 2023 09:47

FoSix added 3 commits August 10, 2023 11:48

update documentation

93086e6

Merge branch 'main' into 88-feat-check-for-scheduled-update-jobs

12ccd6b

update configuration details document

2b7a1d9

alperenkose reviewed Aug 14, 2023

View reviewed changes

examples/low_level_methods/run_low_level_methods.py Outdated Show resolved Hide resolved

alperenkose reviewed Aug 14, 2023

View reviewed changes

panos_upgrade_assurance/firewall_proxy.py Outdated Show resolved Hide resolved

alperenkose reviewed Aug 14, 2023

View reviewed changes

FoSix added 2 commits August 22, 2023 13:56

refactor methods to use config/get XMLAPI instead of OP commands

1329284

add and update tests

49aac4c

FoSix force-pushed the 88-feat-check-for-scheduled-update-jobs branch from 3fbf3c7 to 49aac4c Compare August 22, 2023 14:35

FoSix requested a review from alperenkose August 22, 2023 14:50

alperenkose reviewed Aug 28, 2023

View reviewed changes

panos_upgrade_assurance/exceptions.py Show resolved Hide resolved

alperenkose reviewed Aug 28, 2023

View reviewed changes

pushing changes from comments

a29352c

alperenkose approved these changes Sep 12, 2023

View reviewed changes

FoSix merged commit 4b303f9 into main Sep 12, 2023
7 checks passed

FoSix deleted the 88-feat-check-for-scheduled-update-jobs branch September 12, 2023 13:16

FoSix added the released label Sep 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: check for scheduled update jobs #106

feat!: check for scheduled update jobs #106

FoSix commented Aug 4, 2023 •

edited

Loading

alperenkose Aug 9, 2023

FoSix Aug 10, 2023

alperenkose Aug 9, 2023

FoSix Aug 10, 2023

alperenkose Aug 14, 2023

FoSix Aug 22, 2023

alperenkose Aug 9, 2023

FoSix Aug 10, 2023

alperenkose Aug 14, 2023

FoSix Aug 21, 2023

alperenkose Aug 28, 2023

github-actions bot commented Aug 10, 2023 •

edited

Loading

github-actions bot commented Aug 10, 2023

alperenkose left a comment

alperenkose Aug 28, 2023

FoSix Aug 29, 2023

alperenkose Aug 28, 2023

alperenkose Aug 28, 2023

FoSix Aug 29, 2023

FoSix commented Sep 13, 2023

		print(f"\n dynamic schedules: {firewall.get_update_schedules()}")
		pprint(firewall.get_update_schedules())

feat!: check for scheduled update jobs #106

feat!: check for scheduled update jobs #106

Conversation

FoSix commented Aug 4, 2023 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate)

Types of changes

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 10, 2023 • edited Loading

☂️ Python Coverage

Overall Coverage

New Files

Modified Files

github-actions bot commented Aug 10, 2023

alperenkose left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FoSix commented Sep 13, 2023

FoSix commented Aug 4, 2023 •

edited

Loading

github-actions bot commented Aug 10, 2023 •

edited

Loading