Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

add pkg_upgrade #3

Merged
merged 2 commits into from
Aug 16, 2024
Merged

add pkg_upgrade #3

merged 2 commits into from
Aug 16, 2024

Conversation

crpb
Copy link
Contributor

@crpb crpb commented Aug 15, 2024

get data generated by /usr/local/opnsense/scripts/firmware/check.sh for Rosa-Luxemburgstiftung-Berlin/ansible-opnsense-update/issues/6

Bumbed versionnumber

The 5 minutes to not always trigger the script might be a bit too long for minor upgrades now that i think about it ¯\_(ツ)_/¯.

Or maybe the upgrade-job from ansible-opnsense-update should just delete it for such cases?

@zerwes
Copy link
Contributor

zerwes commented Aug 16, 2024

Hey @crpb
Thank you for the PR!

Bumbed versionnumber

yes, that is what the version number is intended for
but would you mind leaving the version number as a last line?
(see CR)

The 5 minutes ...

Well I think the 5 minutes are safe ...
But I am not sure if it is safe to run a possibly time consuming script from the facts script.
If the /usr/local/opnsense/scripts/firmware/check.sh script will have problems fetching the changelogs, updating the repository catalogue etc. this will slow down the facts gathering or even let it will run into a timeout.
Wouldn't it be safer to run /usr/local/opnsense/scripts/firmware/check.sh scheduled via cron and use the output from /tmp/pkg_upgrade.json in the facts if available?

Or maybe the upgrade-job from ansible-opnsense-update should just delete it for such cases?

this would mean mixing scopes of roles, I think this is not desirable

Copy link
Contributor

@zerwes zerwes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 thank you

files/opnsense.fact Outdated Show resolved Hide resolved
@zerwes
Copy link
Contributor

zerwes commented Aug 16, 2024

Hello @crpb
Thank you for the enhancement!
Despite of the concerns that I have expressed and that continue to exist regarding the desirable relocation of the execution of the /usr/local/opnsense/scripts/firmware/check.sh script via cron I will merge this for now. Should my concerns prove to be null and void, so much the better. Otherwise the cron option can be implemented later.

@zerwes zerwes merged commit bc95986 into Rosa-Luxemburgstiftung-Berlin:main Aug 16, 2024
@crpb
Copy link
Contributor Author

crpb commented Aug 16, 2024

@zerwes ahh great. nevertheless i will try out some things in a vm with no uplink and such and maybe i can take the script apart and only use the needed features which might make it a bit faster 🙊

@crpb
Copy link
Contributor Author

crpb commented Aug 16, 2024

Uplink down - Cache cleared

# opnsense-update -e ; rm -rf /tmp/pkg_up* /var/cache/opnsense-update/*; time /usr/local/opnsense/scripts/firmware/check.sh > /dev/null ; time jq '{product_version, needs_reboot, upgrade_needs_reboot, upgrade_major_version}, [.upgrade_sets[]|{name, current_version, new_version}], [.upgrade_packages[]|select(.name=="opnsense")|{current_version, new_version}] | del(select(length == 0))//empty?' /tmp/pkg_upgrade.json
        0.32 real         0.12 user         0.11 sys
{
  "product_version": "24.7_9",
  "needs_reboot": "0",
  "upgrade_needs_reboot": "0",
  "upgrade_major_version": ""
}
        0.00 real         0.00 user         0.00 sys
#

Uplink up

# opnsense-update -e ; rm -rf /tmp/pkg_up* /var/cache/opnsense-update/*; time /usr/local/opnsense/scripts/firmware/check.sh > /dev/null ; time jq '{product_version, needs_reboot, upgrade_needs_reboot, upgrade_major_version}, [.upgrade_sets[]|{name, current_version, new_version}], [.upgrade_packages[]|select(.name=="opnsense")|{current_version, new_version}] | del(select(length == 0))//empty?' /tmp/pkg_upgrade.json
        3.80 real         1.46 user         0.77 sys
{
  "product_version": "24.7_9",
  "needs_reboot": "1",
  "upgrade_needs_reboot": "0",
  "upgrade_major_version": ""
}
[
  {
    "current_version": "24.7_9",
    "new_version": "24.7.1"
  }
]
        0.00 real         0.00 user         0.00 sys
#

Uplink up - same again

# opnsense-update -e ; rm -rf /tmp/pkg_up* /var/cache/opnsense-update/*; time /usr/local/opnsense/scripts/firmware/check.sh > /dev/null ; time jq '{product_version, needs_reboot, upgrade_needs_reboot, upgrade_major_version}, [.upgrade_sets[]|{name, current_version, new_version}], [.upgrade_packages[]|select(.name=="opnsense")|{current_version, new_version}] | del(select(length == 0))//empty?' /tmp/pkg_upgrade.json
        5.74 real         1.53 user         0.67 sys
{
  "product_version": "24.7_9",
  "needs_reboot": "1",
  "upgrade_needs_reboot": "0",
  "upgrade_major_version": ""
}
[
  {
    "current_version": "24.7_9",
    "new_version": "24.7.1"
  }
]
        0.00 real         0.00 user         0.00 sys
#

Uplink up - started and immediately disconnected the uplink

# time /usr/local/opnsense/scripts/firmware/check.sh > /dev/null ; time jq '{product_version, needs_reboot, upgrade_needs_reboot, upgrade_major_version}, [.upgrade_sets[]|{name, current_version, new_version}], [.upgrade_packages[]|select(.name=="opnsense")|{current_version, new_version}] | del(select(length == 0))//empty?' /tmp/pkg_upgrade.json
       30.29 real         0.15 user         0.08 sys
{
  "product_version": "24.7_9",
  "needs_reboot": "0",
  "upgrade_needs_reboot": "0",
  "upgrade_major_version": ""
}
        0.00 real         0.00 user         0.00 sys
#

These 30 seconds are basically this if fetch -qT 30 -o ${LICENSEFILE} "$(opnsense-update -M)/subscription" >> ${LOCKFILE} 2>&1; then afaics.

So for now i would assume it should behave. 🤞🏼

@zerwes
Copy link
Contributor

zerwes commented Aug 16, 2024

Thank you for the tests.
30 seconds are somehow justifiable and bearable esp. as they reflect a abnormal state.
So we can assume all is fine!
Thx

@crpb
Copy link
Contributor Author

crpb commented Aug 16, 2024

And we probably are disconnected anyway in such cases *jbol*

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants