[Build] fix md5sum calculation of web packages if transient error #13013

kv-y · 2022-12-10T02:48:51Z

Why I did it

Curl can fail when we calculate md5sum of web package.
E.g. if server responsed with 503 error.
But we don't validate this and pass any output from curl directly to md5sum.
After that we save incorrect md5 hash to versions-web file.

How I did it

use option --retry 5 for transient errors (default value is 0)
use option -f for curl and set -o pipefail for shell to detect errors
stop build if curl failed

Signed-off-by: Konstantin Vasin [email protected]

lguohan · 2022-12-14T07:34:42Z

@xumia , can you review this one?

src/sonic-build-hooks/scripts/buildinfo_base.sh

xumia · 2022-12-15T00:56:39Z

src/sonic-build-hooks/scripts/buildinfo_base.sh

-    /usr/bin/curl -Lks $package_url | md5sum | cut -d' ' -f1
+    local file_hash="$(/usr/bin/curl -Lfks --retry 5 $package_url | md5sum | cut -d' ' -f1)"
+    # compare with md5 hash of empty string (no output from curl)
+    if [ "$file_hash" = "d41d8cd98f00b204e9800998ecf8427e" ]; then


The hash value may be only one of the cases, see below. We only save the version if the build succeeded, d41d8cd98f00b204e9800998ecf8427e should not be an issue, right?

xumia@xumia-vm1:~$ curl http://deb.debian.org/debian/pool/main/n/ntp/xxxxxxxxxx.xz | md5sum % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 260 100 260 0 0 795 0 --:--:-- --:--:-- --:--:-- 797 e47a50feb3ec93a1fb70309f586c1aac - xumia@xumia-vm1:~$ curl http://deb.debian.org/debian/pool/main/n/ntp/xxxxxxxxxx.xz <!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN"> <html><head> <title>404 Not Found</title> </head><body> <h1>Not Found</h1> <p>The requested URL was not found on this server.</p> <hr> <address>Apache Server at ftp.debian.org Port 80</address> </body></html> xumia@xumia-vm1:~$ echo $? 0

We only save the version if the build succeeded

This statement is not correct.
1st time I detected this bug the build was succeeded but the hash of downloaded file in versions-web was incorrect.
Because It's not atomic operation. We have two separate actions:

We calculate md5 hash of the file (curl | md5sum) and save it

We download the file using wget or curl

So you can get something like Connection Refused from server when calculate md5 hash.
As a result there is no output from curl and we get a hash of the empty string.

curl: (7) Failed to connect to localhost port 80: Connection refused d41d8cd98f00b204e9800998ecf8427e -

But after that file itself in $REAL_COMMAND "${parameters[@]}" will be downloaded and build will be finished.

Also about this:

xumia@xumia-vm1:~$ echo $? 0

Curl's fails are not detected because:

it's without -f flag here

it's part of a pipeline here.

Something like this:

hash=$(false|md5sum) && echo $hash d41d8cd98f00b204e9800998ecf8427e -

Probably another solution here is starting get_url_version with set -o pipefail or something like this to detect curl's fails. Probably it will be more correct solution.

@kv-y , thanks for the info about '-f'.
Yes, set -o pipefail is better than using the md5 hash value to evaluate whether the curl failed.

test with local nginx server

original file: ntp.tar.xz
md5sum: c1c557036197188a22ec285fa53149d8
Also assume main shell doesn't have flags like set -e

A) current version:

#!/bin/bash get_url_version() { local package_url=$1 /usr/bin/curl -Lks $package_url | md5sum | cut -d' ' -f1 } real_version=$(get_url_version $1) echo "hash: $real_version exit code: $?"

no errors:
./test.sh http://localhost/ntp.tar.xz
hash: c1c557036197188a22ec285fa53149d8 exit code: 0

404 error (incorrect URL):
./test.sh http://localhost/ntp.tar.x
hash: 1b7c22a214949975556626d7217e9a39 exit code: 0

503 error (Service Temporarily Unavailable)
./test.sh http://localhost/ntp.tar.xz
hash: 47e655a40d6414e2e20fb7200ebbb7d5 exit code: 0

Connection Refused
./test.sh http://localhost/ntp.tar.xz
hash: d41d8cd98f00b204e9800998ecf8427e exit code: 0

B) same bash script, but run curl with -f flag

no errors:
./test.sh http://localhost/ntp.tar.xz
hash: c1c557036197188a22ec285fa53149d8 exit code: 0

404 error (incorrect URL):
./test.sh http://localhost/ntp.tar.x
hash: d41d8cd98f00b204e9800998ecf8427e exit code: 0

503 error (Service Temporarily Unavailable)
./test.sh http://localhost/ntp.tar.xz
hash: d41d8cd98f00b204e9800998ecf8427e exit code: 0

Connection Refused
./test.sh http://localhost/ntp.tar.xz
hash: d41d8cd98f00b204e9800998ecf8427e exit code: 0

C) use set -o pipefail for subshell

#!/bin/bash get_url_version() { local package_url=$1 /usr/bin/curl -Lfks $package_url | md5sum | cut -d' ' -f1 } real_version=$(set -o pipefail; get_url_version $1) echo "hash: $real_version exit code: $?"

no errors:
./test.sh http://localhost/ntp.tar.xz
hash: c1c557036197188a22ec285fa53149d8 exit code: 0

404 error (incorrect URL):
./test.sh http://localhost/ntp.tar.x
hash: d41d8cd98f00b204e9800998ecf8427e exit code: 22

503 error (Service Temporarily Unavailable)
./test.sh http://localhost/ntp.tar.xz
hash: d41d8cd98f00b204e9800998ecf8427e exit code: 22

Connection Refused
./test.sh http://localhost/ntp.tar.xz
hash: d41d8cd98f00b204e9800998ecf8427e exit code: 7

D) curl with retry 5 (default is 0)

no errors:
time ./test.sh http://localhost/ntp.tar.xz
hash: c1c557036197188a22ec285fa53149d8 exit code: 0
./test.sh http://localhost/ntp.tar.xz 0,03s user 0,01s system 112% cpu 0,036 total

404 error
time ./test.sh http://localhost/ntp.tar.x
hash: d41d8cd98f00b204e9800998ecf8427e exit code: 22
./test.sh http://localhost/ntp.tar.x 0,02s user 0,01s system 110% cpu 0,023 total

503 error (Service Temporarily Unavailable)
time ./test.sh http://localhost/ntp.tar.xz
hash: d41d8cd98f00b204e9800998ecf8427e exit code: 22
./test.sh http://localhost/ntp.tar.xz 0,02s user 0,01s system 0% cpu 31,057 total

503 error (Service Temporarily Unavailable) at first, but enable server after
time ./test.sh http://localhost/ntp.tar.xz
hash: c1c557036197188a22ec285fa53149d8 exit code: 0
./test.sh http://localhost/ntp.tar.xz 0,03s user 0,01s system 0% cpu 15,048 total

Connection Refused
time ./test.sh http://localhost/ntp.tar.xz
hash: d41d8cd98f00b204e9800998ecf8427e exit code: 7
./test.sh http://localhost/ntp.tar.xz 0,01s user 0,00s system 127% cpu 0,013 total

kv-y · 2022-12-15T11:38:02Z

BTW, we use real_version=$(get_url_version $url) in 2 places:

for normal build
for repr build

For repr build this error will be detected anyway because there is a condition:
if [ "$real_version" != "$version" ]; then

But probably need to fix it too.

Signed-off-by: Konstantin Vasin <[email protected]>

lguohan requested a review from xumia December 14, 2022 07:34

xumia reviewed Dec 15, 2022

View reviewed changes

src/sonic-build-hooks/scripts/buildinfo_base.sh Outdated Show resolved Hide resolved

xumia reviewed Dec 15, 2022

View reviewed changes

fix md5sum calculation of web packages if transient error

d7edcf8

Signed-off-by: Konstantin Vasin <[email protected]>

xumia approved these changes Dec 16, 2022

View reviewed changes

lguohan merged commit 67ced07 into sonic-net:master Dec 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Build] fix md5sum calculation of web packages if transient error #13013

[Build] fix md5sum calculation of web packages if transient error #13013

kv-y commented Dec 10, 2022 •

edited

Loading

lguohan commented Dec 14, 2022

xumia Dec 15, 2022

kv-y Dec 15, 2022 •

edited

Loading

xumia Dec 15, 2022 •

edited

Loading

kv-y Dec 15, 2022

kv-y commented Dec 15, 2022 •

edited

Loading

[Build] fix md5sum calculation of web packages if transient error #13013

[Build] fix md5sum calculation of web packages if transient error #13013

Conversation

kv-y commented Dec 10, 2022 • edited Loading

Why I did it

How I did it

lguohan commented Dec 14, 2022

xumia Dec 15, 2022

Choose a reason for hiding this comment

kv-y Dec 15, 2022 • edited Loading

Choose a reason for hiding this comment

xumia Dec 15, 2022 • edited Loading

Choose a reason for hiding this comment

kv-y Dec 15, 2022

Choose a reason for hiding this comment

kv-y commented Dec 15, 2022 • edited Loading

kv-y commented Dec 10, 2022 •

edited

Loading

kv-y Dec 15, 2022 •

edited

Loading

xumia Dec 15, 2022 •

edited

Loading

kv-y commented Dec 15, 2022 •

edited

Loading