Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix: firmware/check.sh script now fails when updating packages fails #8293

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jggc
Copy link

@jggc jggc commented Feb 6, 2025

When running /usr/local/opnsense/scripts/firmware/check.sh and updating the packages fail, the commant still completes and returns a successful 0 exit code. That seems incorrect to me.

Incorrect behavior, check.sh exits 0 when checking package list fails

root@OPNsense:/usr/local/opnsense/scripts/firmware # ./check.sh 
Fetching changelog information, please wait... fetch: https://pkg.opnsense.org/FreeBSD:14:amd64/24.7/sets/changelog.txz: Network is unreachable
Updating OPNsense repository catalogue...
pkg: https://pkg.opnsense.org/FreeBSD:14:amd64/24.7/latest/meta.txz: Network is unreachable
repository OPNsense has no meta file, using default settings
pkg: https://pkg.opnsense.org/FreeBSD:14:amd64/24.7/latest/packagesite.pkg: Network is unreachable
pkg: https://pkg.opnsense.org/FreeBSD:14:amd64/24.7/latest/packagesite.txz: Network is unreachable
Unable to update repository OPNsense
Error updating repositories!
Checking integrity... done (0 conflicting)
Your packages are up to date.
root@OPNsense:/usr/local/opnsense/scripts/firmware # echo $?
0

With this patch, the behavior makes more sense with exit code 1

Correct behavior

root@OPNsense:/usr/local/opnsense/scripts/firmware # vi check.sh     # <== add `set -e`
root@OPNsense:/usr/local/opnsense/scripts/firmware # ./check.sh 
Fetching changelog information, please wait... fetch: https://pkg.opnsense.org/FreeBSD:14:amd64/24.7/sets/changelog.txz: Network is unreachable
Updating OPNsense repository catalogue...
pkg: https://pkg.opnsense.org/FreeBSD:14:amd64/24.7/latest/meta.txz: Network is unreachable
repository OPNsense has no meta file, using default settings
pkg: https://pkg.opnsense.org/FreeBSD:14:amd64/24.7/latest/packagesite.pkg: Network is unreachable
pkg: https://pkg.opnsense.org/FreeBSD:14:amd64/24.7/latest/packagesite.txz: Network is unreachable
Unable to update repository OPNsense
Error updating repositories!
root@OPNsense:/usr/local/opnsense/scripts/firmware # echo $?
1

I guess changing this behavior can have significant impact in deployments without WAN access but I am still taking the time to suggest it, as it makes more sense to me.

I currently have to first check if I can reach pkg.opnsense.org before making the package update because the output of this script cannot be trusted.

But then if something else fails in the script such as a missing file or something like that, my patch of checking if pkg.opnsense.org is reachable won't be enough and my automation will be broken again.

@fichtner
Copy link
Member

fichtner commented Feb 7, 2025

I did check for use of "set -e" in the firmware script directory, but it confirmed my suspicion:

% git grep set.-e
changelog.sh:set -e

Thinking back it would have been nicer to have but the consequences of doing it now is very risky, also for the mere fact that the script's intention is to write a JSON file at the end to communicate what needs updating. It just blanks that part for the GUI and will likely cause unnecessary tickets and work.

Cheers,
Franco

@fichtner
Copy link
Member

fichtner commented Feb 7, 2025

output_done must also be called otherwise your browser keeps spinning. Did you notice? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants