search

fedora-iot / greenboot

Generic Health Checking Framework for systemd
GNU Lesser General Public License v2.1
101 stars 29 forks source link

Failing 01_update_platforms_check.sh on Fedora IoT #93

Closed alcir closed 1 year ago

alcir commented 1 year ago

I don't know if it is a duplicate of #90 and #71, so excuse me in advance:

On a fresh Fedora IoT 37 I always get 

There are problems connecting with the following URLs:
https://ostree.fedoraproject.org/iot
https://ostree.fedoraproject.org/iot/mirrorlist

Also running /usr/lib/greenboot/check/wanted.d/01_update_platforms_check.sh by hand after the boot, it returns the same result.

I'm not a great bash expert, but I suspect that the double quotes around the ${UPDATE_PLATFORM_URLS[@]} will result in a single line containing all the UPDATE_PLATFORM_URLS making ineffective the for loop https://github.com/fedora-iot/greenboot/blob/7ad3ea53fd4154005bb26d2d48d41d4dc42853e1/usr/lib/greenboot/check/wanted.d/01_update_platforms_check.sh#L16

At the end of the day, the curl command is curl -o /dev/null -Isw '%{http_code}\n' "https://ostree.fedoraproject.org/iot https://ostree.fedoraproject.org/iot/mirrorlist" leading to a 000 HTTP_STATUS variable (that is curl: (3) URL using bad/illegal format or missing URL)

In short: if in such line I remove the double quotes for UPDATE_PLATFORM_URL in ${UPDATE_PLATFORM_URLS[@]}; do the script works as expected.

pcdubs commented 1 year ago

This regression happened recently with https://github.com/fedora-iot/greenboot/commit/84bbd674a7da12ea7c07657129d489942c69e449

pcdubs commented 1 year ago

@alcir did you want to open a RHBZ for this issue so we can track it for F38? (if not I can)

Thanks for finding it!

alcir commented 1 year ago

@alcir did you want to open a RHBZ for this issue so we can track it for F38? (if not I can)

I will

alcir commented 1 year ago

It is already tracked https://bugzilla.redhat.com/show_bug.cgi?id=2062455

alcir commented 1 year ago

Again, I'm not a bash expert, but instead of disable shellcheck, here https://github.com/fedora-iot/greenboot/blob/7ad3ea53fd4154005bb26d2d48d41d4dc42853e1/usr/lib/greenboot/check/wanted.d/01_update_platforms_check.sh#L8 we could actually populate the UPDATE_PLATFORM_URLS array simply in this way UPDATE_PLATFORM_URLS=($(grep -P -ho 'http[s]?.*' $REPOS_DIRECTORY/*)) or readarray -t UPDATE_PLATFORM_URLS < <(grep -P -ho 'http[s]?.*' $REPOS_DIRECTORY/*)

miabbott commented 1 year ago

Again, I'm not a bash expert, but instead of disable shellcheck, here

https://github.com/fedora-iot/greenboot/blob/7ad3ea53fd4154005bb26d2d48d41d4dc42853e1/usr/lib/greenboot/check/wanted.d/01_update_platforms_check.sh#L8

we could actually populate the UPDATE_PLATFORM_URLS array simply in this way UPDATE_PLATFORM_URLS=($(grep -P -ho 'http[s]?.*' $REPOS_DIRECTORY/*)) or readarray -t UPDATE_PLATFORM_URLS < <(grep -P -ho 'http[s]?.*' $REPOS_DIRECTORY/*)

Could you make this same suggestion on the PR itself (#94)? Maintaining the context of the suggestion would make it easier to discuss.

joeelliott76 commented 1 year ago

Yes that works for me, I also I made this that works as well using the tr command.

UPDATE_PLATFORM_URLS=$(grep -P -ho 'http[s]?.' $REPOS_DIRECTORY/ | tr '\n' ' ')