Added docker buildx support in cico scripts. #109

prabhav-thali · 2020-06-03T12:18:31Z

Signed-off-by: Prabhav Thali [email protected]

What does this PR do?
Adds docker buildx support in cico scripts to build multi-arch images.

What issues does this PR fix or reference?
This refers to #17124

prabhav-thali · 2020-06-05T05:11:07Z

@AndrienkoAleksandr Could you please trigger build for this PR on ci.centos?

Signed-off-by: Prabhav Thali <[email protected]>

AndrienkoAleksandr · 2020-06-05T10:54:48Z

cico_functions.sh

  echo 'CICO: Dependencies installed'
 }

+function version() {
+  printf '%02d' $(echo "$1" | tr . ' ' | sed -e 's/ 0*/ /g') 2>/dev/null


Does this code always work like expected?

For me:
$ uname -a
5.5.17-200.fc31.x86_64

result of function:

$ printf '%02d' $(echo "$(uname -r)" | tr . ' ' | sed -e 's/ 0*/ /g') 2>/dev/null
0505170000

Maybe we could simple split by '-':

$ kernel_version=$(uname -r); echo ${kernel_version%%-*}
5.5.17

What do you think?

We wanted comparison between 2 whole numbers, checking whichever is greater, for this reason we added this check. This code is working as per our expectation. Do you sense any possible issue in this?

Code :-

# Kernel kernel_version="$(uname -r)" if [[ "${kernel_version%%-*}" < "4.8" ]]; then echo >&2 "Kernel $kernel_version too old - need >= 4.8." \ " Install a newer kernel." else echo >&2 "kernel $kernel_version has binfmt_misc fix-binary (F) support." fi

If kernel Version = 3.10.0-1062.el7 (Works as expected)
Condition : 3.10.0 < 4.8
O/P : Kernel 3.10.0-1062.el7 too old - need >= 4.8. Install a newer kernel.

If Kernel Version = 4.15.0-66-generic (Doesn't work as expected)
Condition : 4.15.0 < 4.8
O/P : Kernel 4.15.0-66-generic too old - need >= 4.8. Install a newer kernel.

If kernel Version = 5.5.17-200.fc31 (Works as expected)
Condition : 5.5.17 < 4.8
O/P : kernel 5.5.17-200.fc31.x86_64 has binfmt_misc fix-binary (F) support.

Thanks, it's make sense.

@AndrienkoAleksandr Could you please tell me whether you are testing these changes on env similar to ci.centos?

On F31, I get:

$➔ cat /etc/os-release | grep NAME NAME=Fedora VERSION_CODENAME="" PRETTY_NAME="Fedora 31 (Thirty One)" CPE_NAME="cpe:/o:fedoraproject:fedora:31" $➔ kernel_version="$(uname -r)" $➔ if [[ "${kernel_version%%-*}" < "4.8" ]]; then > echo >&2 "Kernel $kernel_version too old - need >= 4.8." \ > " Install a newer kernel." > else > echo >&2 "kernel $kernel_version has binfmt_misc fix-binary (F) support." > fi kernel 5.6.15-200.fc31.x86_64 has binfmt_misc fix-binary (F) support.

On RHEL7.7:

+ cat /etc/os-release + grep NAME NAME="Red Hat Enterprise Linux Server" PRETTY_NAME="Red Hat Enterprise Linux Server 7.7 (Maipo)" CPE_NAME="cpe:/o:redhat:enterprise_linux:7.7:GA:server" ++ uname -r + kernel_version=3.10.0-957.el7.x86_64 + [[ 3.10.0 < 4.8 ]] + echo 'Kernel 3.10.0-957.el7.x86_64 too old - need >= 4.8.' ' Install a newer kernel.' Kernel 3.10.0-957.el7.x86_64 too old - need >= 4.8. Install a newer kernel.

I'm also on the side of this being too fragile here. Apparently sort has a flag (-V) for sorting version numbers, so something like

function check_version() { local query=$1 local target=$2 echo "$target" "$query" | tr ' ' '\n' | sort -V -c 2> /dev/null }

could work

@amisevsk Thanks for the suggestion. Implemented version check using sort with -V. Please review.

build/dockerfiles/Dockerfile

cico_functions.sh

AndrienkoAleksandr · 2020-06-11T07:25:15Z

I asked QA to help us test this changes on the Centos ci. It is important to take a look how it works on the changed job. I afraid that this pr will damage job, because I tested this changes locally on the fedora 31. And I saw that build hangs for me on the arch linux/s390x . Centos based on fedora...

prabhav-thali · 2020-06-11T09:48:53Z

@AndrienkoAleksandr Thanks for getting these changes tested on Centos ci.
We had tested these change on RHEL 8.2 as the buildx requirements were satisfied. And build was successful on both the archs i.e amd64, s390x.

Could you please provide your env details? Such that we can try out at our end.

Signed-off-by: Prabhav Thali <[email protected]>

amisevsk · 2020-06-22T17:57:47Z

cico_functions.sh

  echo 'CICO: Dependencies installed'
 }

+function version() {
+  printf '%02d' $(echo "$1" | tr . ' ' | sed -e 's/ 0*/ /g') 2>/dev/null


I'm also on the side of this being too fragile here. Apparently sort has a flag (-V) for sorting version numbers, so something like

function check_version() { local query=$1 local target=$2 echo "$target" "$query" | tr ' ' '\n' | sort -V -c 2> /dev/null }

could work

cico_functions.sh

Signed-off-by: Prabhav Thali <[email protected]>

prabhav-thali · 2020-06-25T10:34:52Z

And I saw that build hangs for me on the arch linux/s390x .

@AndrienkoAleksandr As we used latest multiarch/qemu-user-static image, build used to hang for s390x on go build. After switching to multiarch/qemu-user-static:4.2.0-7 image, build is successful on both the architectures. Could you please test the updated changes?

prabhav-thali · 2020-07-14T10:48:40Z

@AndrienkoAleksandr any update?

AndrienkoAleksandr · 2020-07-14T11:26:57Z

Sorry, Some time ago I asked QA about set up test job and check this pr. But they were busy. I will ask them again. Without test job we don't know how this change will affect CI.

dmytro-ndp · 2020-07-14T12:36:08Z

@AndrienkoAleksandr: I am sorry, but QE team doesn't support che-machine-exec build scripts at the moment, and we don't have enough expertise to verify corresponding changes.

prabhav-thali · 2020-07-14T13:19:30Z

@dmytro-ndp
We are working on adding docker buildx support in other Che code repositores as well. If you could point us to any other Che code repository which is maintained by QE team, we can work/discuss that repository first. We have draft PRs for docker buildx support ready for following Che images:
che-operator
che-jwtproxy
che-plugin-broker
che-plugin-registry
che-devfile-registry

ibuziuk · 2020-07-14T15:01:07Z

cico_functions.sh

@@ -33,6 +33,30 @@ function load_jenkins_vars() {
  fi
 }

+function check_version() {


@prabhav-thali could you please make the function name more descriptive. For me it is not clear what exactly 'check_version' does

Updated the function name and added docs for the same.

cico_functions.sh

ibuziuk · 2020-07-14T15:09:19Z

cico_functions.sh

+  export DOCKER_CLI_EXPERIMENTAL=enabled
+
+  #Enable qemu and binfmt support
+  docker run --rm --privileged docker/binfmt:66f9012c56a8316f9244ffd7622d7c21c1f6f28d


is this tag expected to be used?

Yes @ibuziuk The tag is expected to be used. Available tags for the image docker/binfmt can be found [here].(https://hub.docker.com/r/docker/binfmt/tags)
Also, updated the tag to the latest one.

ibuziuk

Left a couple of comments but in general LGTM

nickboldt · 2020-07-14T15:13:30Z

cico_functions.sh

+
+  #set buildx env
+  export DOCKER_BUILD_KIT=1
+  export DOCKER_CLI_EXPERIMENTAL=enabled


I'm a bit iffy about relying on experimental features from Docker but if this is the One True Way to do multiarch builds, then so be it.

The buildah discussion appears to be here: containers/buildah#1590

Renamed method check_version Extracted docker buildx in separate method Signed-off-by: Prabhav Thali <[email protected]>

ibuziuk · 2020-07-15T14:00:29Z

cico_functions.sh

+function check_buildx_support() {
+  docker_version="$(docker --version | cut -d' ' -f3 | tr -cd '0-9.')"
+  if [[ $(check_supported_version "$docker_version" "19.03") != 19.03 ]]; then
+    echo "CICO: Docker $docker_version greater than or equal to 19.03 is required."


I just suspect that centos CI has the version lower than 19.03 - we might need to install new version of docker manually

ibuziuk

Approving this PR conceptually, but CI will very likely to fail due to the fact that it is unlikely that 19.03 version of docker will be installed by default on centos CI. So, the solution would be installing the right version of docker in the scripts. please wait for @nickboldt approval before merging

prabhav-thali · 2020-07-29T13:22:51Z

@ibuziuk @nickboldt @AndrienkoAleksandr Any update?

nickboldt · 2020-07-29T16:55:46Z

I haven't had time to look at this, as it doesn't impact downstream builds at all.

So, the solution would be installing the right version of docker in the scripts.

Can you make that change?

@mkuznyetsov do you have any thoughts on whether this will work?

AndrienkoAleksandr · 2020-07-29T18:06:55Z

@ibuziuk @nickboldt @AndrienkoAleksandr Any update?

I don't have access to the CI to check this pr...

nickboldt · 2020-12-11T13:03:32Z

As the cico scripts are no longer being maintained (centos CI is EOL) this PR will never be merged.

Instead, please submit a new PR with code that calls a PLATFORMS file and runs buildx like this:

https://github.com/eclipse/che-plugin-registry/blob/master/.ci/sidecar-build-publish.sh#L31-L35
or like this:

https://github.com/che-incubator/che-dashboard-next/blob/master/.github/workflows/dockerimage.yml#L35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added docker buildx support in cico scripts. #109

Added docker buildx support in cico scripts. #109

prabhav-thali commented Jun 3, 2020 •

edited

Loading

prabhav-thali commented Jun 5, 2020

AndrienkoAleksandr Jun 5, 2020

prabhav-thali Jun 5, 2020

AndrienkoAleksandr Jun 5, 2020

prabhav-thali Jun 8, 2020

nickboldt Jun 10, 2020 •

edited

Loading

amisevsk Jun 22, 2020 •

edited

Loading

prabhav-thali Jun 25, 2020

AndrienkoAleksandr commented Jun 11, 2020

prabhav-thali commented Jun 11, 2020

amisevsk Jun 22, 2020 •

edited

Loading

prabhav-thali commented Jun 25, 2020

prabhav-thali commented Jul 14, 2020

AndrienkoAleksandr commented Jul 14, 2020

dmytro-ndp commented Jul 14, 2020 •

edited

Loading

prabhav-thali commented Jul 14, 2020

ibuziuk Jul 14, 2020

prabhav-thali Jul 15, 2020

ibuziuk Jul 14, 2020

prabhav-thali Jul 15, 2020

ibuziuk left a comment

nickboldt Jul 14, 2020

amisevsk Jul 14, 2020

ibuziuk Jul 15, 2020

ibuziuk left a comment

prabhav-thali commented Jul 29, 2020

nickboldt commented Jul 29, 2020

AndrienkoAleksandr commented Jul 29, 2020

nickboldt commented Dec 11, 2020

Added docker buildx support in cico scripts. #109

Added docker buildx support in cico scripts. #109

Conversation

prabhav-thali commented Jun 3, 2020 • edited Loading

prabhav-thali commented Jun 5, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickboldt Jun 10, 2020 • edited Loading

Choose a reason for hiding this comment

amisevsk Jun 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndrienkoAleksandr commented Jun 11, 2020

prabhav-thali commented Jun 11, 2020

amisevsk Jun 22, 2020 • edited Loading

Choose a reason for hiding this comment

prabhav-thali commented Jun 25, 2020

prabhav-thali commented Jul 14, 2020

AndrienkoAleksandr commented Jul 14, 2020

dmytro-ndp commented Jul 14, 2020 • edited Loading

prabhav-thali commented Jul 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibuziuk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibuziuk left a comment

Choose a reason for hiding this comment

prabhav-thali commented Jul 29, 2020

nickboldt commented Jul 29, 2020

AndrienkoAleksandr commented Jul 29, 2020

nickboldt commented Dec 11, 2020

prabhav-thali commented Jun 3, 2020 •

edited

Loading

nickboldt Jun 10, 2020 •

edited

Loading

amisevsk Jun 22, 2020 •

edited

Loading

amisevsk Jun 22, 2020 •

edited

Loading

dmytro-ndp commented Jul 14, 2020 •

edited

Loading