Skip to content

Conversation

@o-nikolas
Copy link
Contributor

After this recent change: 18902, docker version parsing no longer works in all cases:

[onikolas@dev-dsk-onikolas airflow]$ ./breeze 
/home/onikolas/code/airflow/scripts/ci/libraries/_initialization.sh: line 932: printf: 190313ce: invalid number

This is specifically due to the whitespace being removed from this line:

-  printf "%04d%04d%04d%.0s" ${1//[.-]/ }
+  printf "%03d%03d%03d%.0s" ${1//[.-]/}

printf is expecting at least 3 %d arguments which it parses as integers to 3 decimal places. But the missing whitespace in the bash expansion causes the version to not be split into multiple inputs into printf as expected, but rather one string:

With whitespace in expansion:

[onikolas@dev-dsk-onikolas airflow]$ docker_version="19.03.13"
[onikolas@dev-dsk-onikolas airflow]$ echo ${docker_version//[.-]/ }
19 03 13
[onikolas@dev-dsk-onikolas airflow]$ printf "%03d%03d%03d%.0s" ${docker_version//[.-]/ }
019003013

Without whitespace in expansion:

[onikolas@dev-dsk-onikolas airflow]$ docker_version="19.03.13"
[onikolas@dev-dsk-onikolas airflow]$ echo ${docker_version//[.-]/}
190313
[onikolas@dev-dsk-onikolas airflow]$ printf "%03d%03d%03d%.0s" ${docker_version_2//[.-]/}
190313000000

As you can see in the latter case the entire version is used as the first argument to printf and then printf defaults the second and third %d with zeros since input is not provided, which is why 6 trailing zeros are seen in the output.
This defaulting allowed the code to pass undetected when it was tested originally, since the <= version comparison still passes with this input. It is not until a community edition docker version is used which includes '-ce' in the version that this breaks:
docker community edition version output:

[onikolas@dev-dsk-onikolas airflow]$ docker --version
Docker version 19.03.13-ce, build 4484c46

When run through the breeze version extraction logic yields:

[onikolas@dev-dsk-onikolas airflow]$ docker_version=$(docker version --format '{{.Client.Version}}' | sed 's/\+.*$//' || true)
[onikolas@dev-dsk-onikolas airflow]$ echo $docker_version
19.03.13-ce

When this input is run through the printf logic, with whitespace, you can see the input version is properly split into separate inputs for printf, which uses the first three arguments and the last argument (the ce portion) is unused. Yielding a useable version:

[onikolas@dev-dsk-onikolas airflow]$ echo ${docker_version//[.-]/ }
19 03 13 ce
[onikolas@dev-dsk-onikolas airflow]$ printf "%03d%03d%03d%.0s" ${docker_version//[.-]/ }
019003013

However when the version of the code without the whitespace, printf is given one large input which it uses as the first arg. Which includes ce which is not parseable as a number. Which yields the original error message:

[onikolas@dev-dsk-onikolas airflow]$ echo ${docker_version//[.-]/}
190313ce
[onikolas@dev-dsk-onikolas airflow]$ printf "%03d%03d%03d%.0s" ${docker_version_2//[.-]/}
190313000000
[onikolas@dev-dsk-onikolas airflow]$ printf "%03d%03d%03d%.0s" ${docker_version//[.-]/}
-logbash: printf: 190313ce: invalid number

This change re-adds the whitespace to the expansion which correctly handles both usecases.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

@potiuk potiuk merged commit d709094 into apache:main Oct 23, 2021
@potiuk
Copy link
Member

potiuk commented Oct 23, 2021

Thanks! yeah. The space removal was accidental.

@o-nikolas o-nikolas deleted the docker_version_parsing branch October 25, 2021 18:10
sharon2719 pushed a commit to sharon2719/airflow that referenced this pull request Oct 27, 2021
potiuk pushed a commit that referenced this pull request Jan 22, 2022
@potiuk potiuk added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Jan 22, 2022
@potiuk potiuk added this to the Airflow 2.2.4 milestone Jan 22, 2022
@o-nikolas o-nikolas restored the docker_version_parsing branch May 5, 2022 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:dev-tools changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants