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

Added docker buildx support in cico scripts. #109

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cico_build_ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ export SCRIPT_DIR

load_jenkins_vars
install_deps
check_buildx_support
build_and_push
1 change: 1 addition & 0 deletions cico_build_nightly.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ export SCRIPT_DIR

load_jenkins_vars
install_deps
check_buildx_support
set_nightly_tag
build_and_push
1 change: 1 addition & 0 deletions cico_build_release.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ export SCRIPT_DIR

load_jenkins_vars
install_deps
check_buildx_support
set_release_tag
build_and_push
50 changes: 39 additions & 11 deletions cico_functions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,24 @@ function load_jenkins_vars() {
fi
}

function check_buildx_support() {
prabhav-thali marked this conversation as resolved.
Show resolved Hide resolved
docker_version="$(docker --version | cut -d' ' -f3 | tr -cd '0-9.')"
if [[ "$(version "$docker_version")" < "$(version '19.03')" ]]; then
amisevsk marked this conversation as resolved.
Show resolved Hide resolved
echo "CICO: Docker $docker_version is too old. Greater than or equal to 19.03 is required."
exit 1
fi

# Kernel
kernel_version="$(uname -r)"
if [[ "$(version "$kernel_version")" < "$(version '4.8')" ]]; then
echo "Kernel $kernel_version too old - need >= 4.8." \
" Install a newer kernel."
prabhav-thali marked this conversation as resolved.
Show resolved Hide resolved
else
echo "kernel $kernel_version has binfmt_misc fix-binary (F) support."
fi

}

function install_deps() {
# We need to disable selinux for now, XXX
/usr/sbin/setenforce 0 || true
Expand All @@ -44,9 +62,22 @@ function install_deps() {
git

service docker start

#set buildx env
export DOCKER_BUILD_KIT=1
export DOCKER_CLI_EXPERIMENTAL=enabled
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


#Enable qemu and binfmt support
docker run --rm --privileged docker/binfmt:66f9012c56a8316f9244ffd7622d7c21c1f6f28d
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this tag expected to be used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

docker run --rm --privileged multiarch/qemu-user-static --reset -p yes

echo 'CICO: Dependencies installed'
}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, it's make sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@nickboldt nickboldt Jun 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@amisevsk amisevsk Jun 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

function set_release_tag() {
# Let's obtain the tag based on the
# version defined in the 'VERSION' file
Expand All @@ -66,12 +97,6 @@ function set_git_commit_tag() {
export GIT_COMMIT_TAG
}

function tag_push() {
local TARGET=$1
docker tag "${IMAGE}" "$TARGET"
docker push "$TARGET" | cat
}

function build_and_push() {
REGISTRY="quay.io"
DOCKERFILE="Dockerfile"
Expand All @@ -86,13 +111,16 @@ function build_and_push() {

# Let's build and push image to 'quay.io' using git commit hash as tag first
set_git_commit_tag
docker build -t ${IMAGE} -f ./build/dockerfiles/${DOCKERFILE} . | cat
tag_push "${REGISTRY}/${ORGANIZATION}/${IMAGE}:${GIT_COMMIT_TAG}"
echo "CICO: '${GIT_COMMIT_TAG}' version of images pushed to '${REGISTRY}/${ORGANIZATION}' organization"

# If additional tag is set (e.g. "nightly"), let's tag the image accordingly and also push to 'quay.io'
# Create a new builder instance using buildx
docker buildx create --use --name builder
docker buildx inspect --bootstrap
docker buildx build --platform linux/amd64,linux/s390x -t ${REGISTRY}/${ORGANIZATION}/${IMAGE}:${GIT_COMMIT_TAG} -f ./build/dockerfiles/${DOCKERFILE} --push --progress plain --no-cache .
echo "CICO: '${GIT_COMMIT_TAG}' version of images pushed to '${REGISTRY}/${ORGANIZATION}' organization"

prabhav-thali marked this conversation as resolved.
Show resolved Hide resolved
# If additional tag is set (e.g. "nightly"), let's build the image accordingly and also push to 'quay.io'
if [ -n "${TAG}" ]; then
tag_push "${REGISTRY}/${ORGANIZATION}/${IMAGE}:${TAG}"
docker buildx build --platform linux/amd64,linux/s390x -t ${REGISTRY}/${ORGANIZATION}/${IMAGE}:${TAG} -f ./build/dockerfiles/${DOCKERFILE} --push --progress plain --no-cache .
echo "CICO: '${TAG}' version of images pushed to '${REGISTRY}/${ORGANIZATION}' organization"
fi
}