From 5fadc5fcb4bda353c7b38d8caf9ae2a91e941697 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Fri, 27 Nov 2020 15:11:27 +0100 Subject: [PATCH 01/31] trace-forwarder: Add void "install" target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise `make install` run from the top directory would just fail as the target is not defined. Fixes: #1149 Signed-off-by: Fabiano Fidêncio --- src/trace-forwarder/Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/trace-forwarder/Makefile b/src/trace-forwarder/Makefile index ae73325922..cfb46560ef 100644 --- a/src/trace-forwarder/Makefile +++ b/src/trace-forwarder/Makefile @@ -13,10 +13,13 @@ clean: test: +install: + check: .PHONY: \ build \ test \ check \ + install \ clean From c9d4e2c4b052f5d3e25c30c9b05f7702f862e508 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Fri, 27 Nov 2020 15:12:57 +0100 Subject: [PATCH 02/31] agent-ctl: Add void "install" target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise `make install` run from the top directory would just fail as the target is not defined. Fixes: #1149 Signed-off-by: Fabiano Fidêncio --- tools/agent-ctl/Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/agent-ctl/Makefile b/tools/agent-ctl/Makefile index ae73325922..54c948a817 100644 --- a/tools/agent-ctl/Makefile +++ b/tools/agent-ctl/Makefile @@ -13,10 +13,13 @@ clean: test: +install: + check: .PHONY: \ build \ test \ check \ + install \ clean From 3c36ce8139340d50176ee6770118918ae3755ef5 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Tue, 8 Dec 2020 09:55:56 -0600 Subject: [PATCH 03/31] rootfs-builder: add functions to run before and after the container Define `before_starting_container` and `after_stopping_container` functions, these functions run before and after the container that builds the rootfs respectively. Signed-off-by: Julio Montes --- tools/osbuilder/rootfs-builder/rootfs.sh | 3 +++ .../template/rootfs_lib_template.sh | 25 ++++++++++++++++--- tools/osbuilder/scripts/lib.sh | 8 ++++++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/tools/osbuilder/rootfs-builder/rootfs.sh b/tools/osbuilder/rootfs-builder/rootfs.sh index 92c2e55829..7a71ba902d 100755 --- a/tools/osbuilder/rootfs-builder/rootfs.sh +++ b/tools/osbuilder/rootfs-builder/rootfs.sh @@ -400,6 +400,9 @@ build_rootfs_distro() done fi + before_starting_container + trap after_stopping_container EXIT + #Make sure we use a compatible runtime to build rootfs # In case Clear Containers Runtime is installed we dont want to hit issue: #https://github.com/clearcontainers/runtime/issues/828 diff --git a/tools/osbuilder/rootfs-builder/template/rootfs_lib_template.sh b/tools/osbuilder/rootfs-builder/template/rootfs_lib_template.sh index 238b6f702d..978a89bc87 100644 --- a/tools/osbuilder/rootfs-builder/template/rootfs_lib_template.sh +++ b/tools/osbuilder/rootfs-builder/template/rootfs_lib_template.sh @@ -12,18 +12,19 @@ # # BIN_AGENT: Name of the Kata-Agent binary # -# REPO_URL: URL to distribution repository ( should be configured in +# REPO_URL: URL to distribution repository ( should be configured in # config.sh file) # -# Any other configuration variable for a specific distro must be added +# Any other configuration variable for a specific distro must be added # and documented on its own config.sh -# +# # - Expected result # # rootfs_dir populated with rootfs pkgs # It must provide a binary in /sbin/init # -# Note: For some distros, the build_rootfs() function provided in scripts/lib.sh +# Note: For some distros, the build_rootfs(), before_starting_container() +# and after_starting_container() functions provided in scripts/lib.sh # will suffice. If a new distro is introduced with a special requirement, # then, a rootfs_builder//rootfs_lib.sh file should be created # using this template. @@ -52,3 +53,19 @@ build_rootfs() { # Populate ROOTFS_DIR # Must provide /sbin/init and /bin/${BIN_AGENT} } + +before_starting_container() { + # Run the following tasks before starting the container that builds the rootfs. + # For example: + # * Create a container + # * Create a volume + return 0 +} + +after_stopping_container() { + # Run the following tasks after stoping the container that builds the rootfs. + # For example: + # * Delete a container + # * Delete a volume + return 0 +} diff --git a/tools/osbuilder/scripts/lib.sh b/tools/osbuilder/scripts/lib.sh index e7a39d889a..e43f78143e 100644 --- a/tools/osbuilder/scripts/lib.sh +++ b/tools/osbuilder/scripts/lib.sh @@ -421,3 +421,11 @@ detect_musl_version() [ "$?" == "0" ] && [ "$MUSL_VERSION" != "null" ] } + +before_starting_container() { + return 0 +} + +after_stopping_container() { + return 0 +} From d4c1b768a64d34b4e17db5665bc8edd63740377f Mon Sep 17 00:00:00 2001 From: Wainer dos Santos Moschetta Date: Tue, 1 Dec 2020 12:01:09 -0500 Subject: [PATCH 04/31] packaging/qemu: Add QEMU_DESTDIR argument to dockerfiles The dockerfiles used to build qemu and qemu-virtiofs have the QEMU destination path hardcoded, which in turn is also on the build scripts. This refactor the dockerfiles to add the QEMU_DESTDIR argument, which value is passed by the scripts. Fixes #1168 Signed-off-by: Wainer dos Santos Moschetta --- tools/packaging/static-build/qemu-virtiofs/Dockerfile | 10 ++++++---- .../qemu-virtiofs/build-static-qemu-virtiofs.sh | 4 +++- tools/packaging/static-build/qemu/Dockerfile | 5 +++-- tools/packaging/static-build/qemu/build-static-qemu.sh | 4 +++- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/tools/packaging/static-build/qemu-virtiofs/Dockerfile b/tools/packaging/static-build/qemu-virtiofs/Dockerfile index c16049d3e9..15d94dcfe9 100644 --- a/tools/packaging/static-build/qemu-virtiofs/Dockerfile +++ b/tools/packaging/static-build/qemu-virtiofs/Dockerfile @@ -4,6 +4,7 @@ # SPDX-License-Identifier: Apache-2.0 from ubuntu:20.04 +ARG QEMU_DESTDIR ARG QEMU_VIRTIOFS_REPO # commit/tag/branch ARG QEMU_VIRTIOFS_TAG @@ -70,7 +71,8 @@ RUN PREFIX="${PREFIX}" /root/configure-hypervisor.sh -s kata-qemu | sed -e 's|-- RUN make -j$(nproc) RUN make -j$(nproc) virtiofsd -RUN make install DESTDIR=/tmp/qemu-virtiofs-static -RUN mv /tmp/qemu-virtiofs-static/"${PREFIX}"/bin/qemu-system-x86_64 /tmp/qemu-virtiofs-static/"${PREFIX}"/bin/qemu-virtiofs-system-x86_64 -RUN mv /tmp/qemu-virtiofs-static/"${PREFIX}"/libexec/kata-qemu/virtiofsd /tmp/qemu-virtiofs-static/opt/kata/bin/virtiofsd-dax -RUN cd /tmp/qemu-virtiofs-static && tar -czvf "${QEMU_TARBALL}" * +RUN make install DESTDIR="${QEMU_DESTDIR}" +RUN cd "${QEMU_DESTDIR}/${PREFIX}" && \ + mv bin/qemu-system-x86_64 bin/qemu-virtiofs-system-x86_64 && \ + mv libexec/kata-qemu/virtiofsd bin/virtiofsd-dax +RUN cd "${QEMU_DESTDIR}" && tar -czvf "${QEMU_TARBALL}" * diff --git a/tools/packaging/static-build/qemu-virtiofs/build-static-qemu-virtiofs.sh b/tools/packaging/static-build/qemu-virtiofs/build-static-qemu-virtiofs.sh index 4823393175..9e5b808610 100755 --- a/tools/packaging/static-build/qemu-virtiofs/build-static-qemu-virtiofs.sh +++ b/tools/packaging/static-build/qemu-virtiofs/build-static-qemu-virtiofs.sh @@ -26,6 +26,7 @@ qemu_virtiofs_repo=$(get_from_kata_deps "assets.hypervisor.qemu-experimental.url qemu_virtiofs_tag=$(get_from_kata_deps "assets.hypervisor.qemu-experimental.tag" "${kata_version}") qemu_virtiofs_tar="kata-static-qemu-virtiofsd.tar.gz" qemu_tmp_tar="kata-static-qemu-virtiofsd-tmp.tar.gz" +qemu_destdir="/tmp/qemu-virtiofs-static" info "Build ${qemu_virtiofs_repo} tag: ${qemu_virtiofs_tag}" @@ -37,6 +38,7 @@ sudo "${DOCKER_CLI}" build \ --no-cache \ --build-arg http_proxy="${http_proxy}" \ --build-arg https_proxy="${https_proxy}" \ + --build-arg QEMU_DESTDIR="${qemu_destdir}" \ --build-arg QEMU_VIRTIOFS_REPO="${qemu_virtiofs_repo}" \ --build-arg QEMU_VIRTIOFS_TAG="${qemu_virtiofs_tag}" \ --build-arg QEMU_TARBALL="${qemu_virtiofs_tar}" \ @@ -48,7 +50,7 @@ sudo "${DOCKER_CLI}" build \ sudo "${DOCKER_CLI}" run \ -i \ -v "${PWD}":/share qemu-virtiofs-static \ - mv "/tmp/qemu-virtiofs-static/${qemu_virtiofs_tar}" /share/ + mv "${qemu_destdir}/${qemu_virtiofs_tar}" /share/ sudo chown ${USER}:${USER} "${PWD}/${qemu_virtiofs_tar}" diff --git a/tools/packaging/static-build/qemu/Dockerfile b/tools/packaging/static-build/qemu/Dockerfile index 74d479c9fc..1043eee96a 100644 --- a/tools/packaging/static-build/qemu/Dockerfile +++ b/tools/packaging/static-build/qemu/Dockerfile @@ -4,6 +4,7 @@ # SPDX-License-Identifier: Apache-2.0 from ubuntu:20.04 +ARG QEMU_DESTDIR ARG QEMU_REPO # commit/tag/branch ARG QEMU_VERSION @@ -62,5 +63,5 @@ RUN PREFIX="${PREFIX}" /root/configure-hypervisor.sh -s kata-qemu | xargs ./conf RUN make -j$(nproc) RUN make -j$(nproc) virtiofsd -RUN make install DESTDIR=/tmp/qemu-static -RUN cd /tmp/qemu-static && tar -czvf "${QEMU_TARBALL}" * +RUN make install DESTDIR="${QEMU_DESTDIR}" +RUN cd "${QEMU_DESTDIR}" && tar -czvf "${QEMU_TARBALL}" * diff --git a/tools/packaging/static-build/qemu/build-static-qemu.sh b/tools/packaging/static-build/qemu/build-static-qemu.sh index fd6f52439f..4872f33a33 100755 --- a/tools/packaging/static-build/qemu/build-static-qemu.sh +++ b/tools/packaging/static-build/qemu/build-static-qemu.sh @@ -16,6 +16,7 @@ source "${script_dir}/../qemu.blacklist" packaging_dir="${script_dir}/../.." qemu_tar="kata-static-qemu.tar.gz" qemu_tmp_tar="kata-static-qemu-tmp.tar.gz" +qemu_destdir="/tmp/qemu-static/" qemu_repo="${qemu_repo:-}" qemu_version="${qemu_version:-}" @@ -45,6 +46,7 @@ sudo docker build \ --no-cache \ --build-arg http_proxy="${http_proxy}" \ --build-arg https_proxy="${https_proxy}" \ + --build-arg QEMU_DESTDIR="${qemu_destdir}" \ --build-arg QEMU_REPO="${qemu_repo}" \ --build-arg QEMU_VERSION="${qemu_version}" \ --build-arg QEMU_TARBALL="${qemu_tar}" \ @@ -56,7 +58,7 @@ sudo docker build \ sudo docker run \ -i \ -v "${PWD}":/share qemu-static \ - mv "/tmp/qemu-static/${qemu_tar}" /share/ + mv "${qemu_destdir}/${qemu_tar}" /share/ sudo chown ${USER}:${USER} "${PWD}/${qemu_tar}" From 1dde0de1d721212a6147b82ff5f2bb1fad560510 Mon Sep 17 00:00:00 2001 From: Wainer dos Santos Moschetta Date: Tue, 1 Dec 2020 13:47:06 -0500 Subject: [PATCH 05/31] packaging/qemu: Build and package completely in the container Currently QEMU is built inside the container, its tarball pulled to the host, files removed then packaged again. Instead, let's run all those steps inside the container and the resulting tarball will be the final version. For that end, it is introduced the qemu-build-post.sh script which will remove the uneeded files and create the tarball. The patterns for directories on qemu.blacklist had to be changed to work properly with `find -path`. Fixes #1168 Signed-off-by: Wainer dos Santos Moschetta Signed-off-by: Peng Tao --- .../static-build/qemu-virtiofs/Dockerfile | 4 ++- .../build-static-qemu-virtiofs.sh | 4 --- tools/packaging/static-build/qemu.blacklist | 6 ++-- tools/packaging/static-build/qemu/Dockerfile | 4 ++- .../static-build/qemu/build-static-qemu.sh | 4 --- .../static-build/scripts/qemu-build-post.sh | 28 +++++++++++++++++++ 6 files changed, 37 insertions(+), 13 deletions(-) create mode 100755 tools/packaging/static-build/scripts/qemu-build-post.sh diff --git a/tools/packaging/static-build/qemu-virtiofs/Dockerfile b/tools/packaging/static-build/qemu-virtiofs/Dockerfile index 15d94dcfe9..380e9eb855 100644 --- a/tools/packaging/static-build/qemu-virtiofs/Dockerfile +++ b/tools/packaging/static-build/qemu-virtiofs/Dockerfile @@ -55,6 +55,8 @@ RUN git checkout "${QEMU_VIRTIOFS_TAG}" ADD scripts/configure-hypervisor.sh /root/configure-hypervisor.sh ADD qemu /root/kata_qemu +ADD scripts/apply_patches.sh /root/apply_patches.sh +ADD static-build /root/static-build # Apply experimental specific patches # Patches to quick fix virtiofs fork @@ -75,4 +77,4 @@ RUN make install DESTDIR="${QEMU_DESTDIR}" RUN cd "${QEMU_DESTDIR}/${PREFIX}" && \ mv bin/qemu-system-x86_64 bin/qemu-virtiofs-system-x86_64 && \ mv libexec/kata-qemu/virtiofsd bin/virtiofsd-dax -RUN cd "${QEMU_DESTDIR}" && tar -czvf "${QEMU_TARBALL}" * +RUN /root/static-build/scripts/qemu-build-post.sh diff --git a/tools/packaging/static-build/qemu-virtiofs/build-static-qemu-virtiofs.sh b/tools/packaging/static-build/qemu-virtiofs/build-static-qemu-virtiofs.sh index 9e5b808610..419719151f 100755 --- a/tools/packaging/static-build/qemu-virtiofs/build-static-qemu-virtiofs.sh +++ b/tools/packaging/static-build/qemu-virtiofs/build-static-qemu-virtiofs.sh @@ -53,7 +53,3 @@ sudo "${DOCKER_CLI}" run \ mv "${qemu_destdir}/${qemu_virtiofs_tar}" /share/ sudo chown ${USER}:${USER} "${PWD}/${qemu_virtiofs_tar}" - -# Remove blacklisted binaries -gzip -d < "${qemu_virtiofs_tar}" | tar --delete --wildcards -f - ${qemu_black_list[*]} | gzip > "${qemu_tmp_tar}" -mv -f "${qemu_tmp_tar}" "${qemu_virtiofs_tar}" diff --git a/tools/packaging/static-build/qemu.blacklist b/tools/packaging/static-build/qemu.blacklist index e52c54dc9f..77ce3a28a3 100644 --- a/tools/packaging/static-build/qemu.blacklist +++ b/tools/packaging/static-build/qemu.blacklist @@ -6,7 +6,7 @@ qemu_black_list=( */bin/qemu-pr-helper */bin/virtfs-proxy-helper */libexec/kata-qemu/qemu* -*/share/*/applications/ +*/share/*/applications */share/*/*.dtb */share/*/efi-e1000e.rom */share/*/efi-e1000.rom @@ -15,9 +15,9 @@ qemu_black_list=( */share/*/efi-pcnet.rom */share/*/efi-rtl8139.rom */share/*/efi-vmxnet3.rom -*/share/*/icons/ +*/share/*/icons */share/*/*.img -*/share/*/keymaps/ +*/share/*/keymaps */share/*/multiboot.bin */share/*/openbios-ppc */share/*/openbios-sparc32 diff --git a/tools/packaging/static-build/qemu/Dockerfile b/tools/packaging/static-build/qemu/Dockerfile index 1043eee96a..26fc6c811c 100644 --- a/tools/packaging/static-build/qemu/Dockerfile +++ b/tools/packaging/static-build/qemu/Dockerfile @@ -55,6 +55,8 @@ RUN git clone https://github.com/qemu/keycodemapdb.git ui/keycodemapdb ADD scripts/configure-hypervisor.sh /root/configure-hypervisor.sh ADD qemu /root/kata_qemu +ADD scripts/apply_patches.sh /root/apply_patches.sh +ADD static-build /root/static-build RUN /root/kata_qemu/apply_patches.sh @@ -64,4 +66,4 @@ RUN PREFIX="${PREFIX}" /root/configure-hypervisor.sh -s kata-qemu | xargs ./conf RUN make -j$(nproc) RUN make -j$(nproc) virtiofsd RUN make install DESTDIR="${QEMU_DESTDIR}" -RUN cd "${QEMU_DESTDIR}" && tar -czvf "${QEMU_TARBALL}" * +RUN /root/static-build/scripts/qemu-build-post.sh diff --git a/tools/packaging/static-build/qemu/build-static-qemu.sh b/tools/packaging/static-build/qemu/build-static-qemu.sh index 4872f33a33..d91d722c9b 100755 --- a/tools/packaging/static-build/qemu/build-static-qemu.sh +++ b/tools/packaging/static-build/qemu/build-static-qemu.sh @@ -61,7 +61,3 @@ sudo docker run \ mv "${qemu_destdir}/${qemu_tar}" /share/ sudo chown ${USER}:${USER} "${PWD}/${qemu_tar}" - -# Remove blacklisted binaries -gzip -d < "${qemu_tar}" | tar --delete --wildcards -f - ${qemu_black_list[*]} | gzip > "${qemu_tmp_tar}" -mv -f "${qemu_tmp_tar}" "${qemu_tar}" diff --git a/tools/packaging/static-build/scripts/qemu-build-post.sh b/tools/packaging/static-build/scripts/qemu-build-post.sh new file mode 100755 index 0000000000..fbb8f931c4 --- /dev/null +++ b/tools/packaging/static-build/scripts/qemu-build-post.sh @@ -0,0 +1,28 @@ +#!/bin/bash +# +# Copyright (c) 2020 Red Hat, Inc. +# +# SPDX-License-Identifier: Apache-2.0 +# +# This script process QEMU post-build. +# +set -e + +script_dir="$(realpath $(dirname $0))" +source "${script_dir}/../qemu.blacklist" + +if [[ -z "${QEMU_TARBALL}" || -z "${QEMU_DESTDIR}" ]]; then + echo "$0: needs QEMU_TARBALL and QEMU_DESTDIR exported" + exit 1 +fi + +pushd "${QEMU_DESTDIR}" +# Remove files to reduce the surface. +echo "INFO: remove uneeded files" +for pattern in ${qemu_black_list[@]}; do + find . -path "$pattern" | xargs rm -rfv +done + +echo "INFO: create the tarball" +tar -czvf "${QEMU_TARBALL}" * +popd From 14b18b55be18e4cf4db713bbcd524b385dd26c2c Mon Sep 17 00:00:00 2001 From: Wainer dos Santos Moschetta Date: Wed, 2 Dec 2020 13:43:11 -0500 Subject: [PATCH 06/31] packaging/qemu: Delete the temporary container It is used a temporary container to pull the QEMU tarball out of the build image, but this container is never deleted. This will ensure it gets deleted after its execution. Fixes #1168 Signed-off-by: Wainer dos Santos Moschetta --- .../static-build/qemu-virtiofs/build-static-qemu-virtiofs.sh | 1 + tools/packaging/static-build/qemu/build-static-qemu.sh | 1 + 2 files changed, 2 insertions(+) diff --git a/tools/packaging/static-build/qemu-virtiofs/build-static-qemu-virtiofs.sh b/tools/packaging/static-build/qemu-virtiofs/build-static-qemu-virtiofs.sh index 419719151f..1178dafd59 100755 --- a/tools/packaging/static-build/qemu-virtiofs/build-static-qemu-virtiofs.sh +++ b/tools/packaging/static-build/qemu-virtiofs/build-static-qemu-virtiofs.sh @@ -48,6 +48,7 @@ sudo "${DOCKER_CLI}" build \ -t qemu-virtiofs-static sudo "${DOCKER_CLI}" run \ + --rm \ -i \ -v "${PWD}":/share qemu-virtiofs-static \ mv "${qemu_destdir}/${qemu_virtiofs_tar}" /share/ diff --git a/tools/packaging/static-build/qemu/build-static-qemu.sh b/tools/packaging/static-build/qemu/build-static-qemu.sh index d91d722c9b..22cb8d0c06 100755 --- a/tools/packaging/static-build/qemu/build-static-qemu.sh +++ b/tools/packaging/static-build/qemu/build-static-qemu.sh @@ -56,6 +56,7 @@ sudo docker build \ -t qemu-static sudo docker run \ + --rm \ -i \ -v "${PWD}":/share qemu-static \ mv "${qemu_destdir}/${qemu_tar}" /share/ From 7e92833bd4b317ec535a61cf8370f751af2a809b Mon Sep 17 00:00:00 2001 From: Wainer dos Santos Moschetta Date: Wed, 21 Oct 2020 17:36:19 -0400 Subject: [PATCH 07/31] packaging: Make qemu/apply_patches.sh common Moved the qemu/apply_patches.sh to the common scripts directory and refactor it so that it can be used as a generic and consistent way to apply patches. Fixes #1014 Signed-off-by: Wainer dos Santos Moschetta Signed-off-by: Peng Tao --- tools/packaging/scripts/apply_patches.sh | 48 +++++++++++++++++++ .../static-build/qemu-virtiofs/Dockerfile | 10 ++-- tools/packaging/static-build/qemu/Dockerfile | 3 +- 3 files changed, 54 insertions(+), 7 deletions(-) create mode 100755 tools/packaging/scripts/apply_patches.sh diff --git a/tools/packaging/scripts/apply_patches.sh b/tools/packaging/scripts/apply_patches.sh new file mode 100755 index 0000000000..aa6b19b609 --- /dev/null +++ b/tools/packaging/scripts/apply_patches.sh @@ -0,0 +1,48 @@ +#!/bin/bash +# +# Copyright (c) 2020 Red Hat, Inc. +# +# SPDX-License-Identifier: Apache-2.0 +# +# This script apply patches. +# +set -e + +script_dir="$(realpath $(dirname $0))" +patches_dir="$1" + +if [ -z "$patches_dir" ]; then + cat <<-EOT + Apply patches to the sources at the current directory. + + Patches are expected to be named in the standard git-format-patch(1) format where + the first part of the filename represents the patch ordering (lowest numbers + apply first): + 'NUMBER-DASHED_DESCRIPTION.patch' + + For example, + + 0001-fix-the-bad-thing.patch + 0002-improve-the-fix-the-bad-thing-fix.patch + 0003-correct-compiler-warnings.patch + + Usage: + $0 PATCHES_DIR + Where: + PATCHES_DIR is the directory containing the patches + EOT + exit 1 +fi + +echo "INFO: Apply patches from $patches_dir" +if [ -d "$patches_dir" ]; then + patches=($(find "$patches_dir" -name '*.patch'|sort -t- -k1,1n)) + echo "INFO: Found ${#patches[@]} patches" + for patch in ${patches[@]}; do + echo "INFO: Apply $patch" + git apply "$patch" || \ + { echo >&2 "ERROR: Not applied. Exiting..."; exit 1; } + done +else + echo "INFO: Patches directory does not exist" +fi diff --git a/tools/packaging/static-build/qemu-virtiofs/Dockerfile b/tools/packaging/static-build/qemu-virtiofs/Dockerfile index 380e9eb855..0caa064514 100644 --- a/tools/packaging/static-build/qemu-virtiofs/Dockerfile +++ b/tools/packaging/static-build/qemu-virtiofs/Dockerfile @@ -61,12 +61,10 @@ ADD static-build /root/static-build # Apply experimental specific patches # Patches to quick fix virtiofs fork ENV VIRTIOFS_PATCHES_DIR=/root/kata_qemu/patches/${QEMU_VIRTIOFS_TAG}/ -RUN if [ -d ${VIRTIOFS_PATCHES_DIR} ]; then \ - echo "Patches to apply for virtiofs fixes:"; \ - for patch in $(find "${VIRTIOFS_PATCHES_DIR}" -name '*.patch' -type f |sort -t- -k1,1n); do \ - git apply $patch; \ - done;fi -RUN /root/kata_qemu/apply_patches.sh +RUN /root/apply_patches.sh ${VIRTIOFS_PATCHES_DIR} +# Apply the stable branch patches +RUN stable_branch=$(cat VERSION | awk 'BEGIN{FS=OFS="."}{print $1 "." $2 ".x"}') && \ + /root/apply_patches.sh "/root/kata_qemu/patches/${stable_branch}" RUN PREFIX="${PREFIX}" /root/configure-hypervisor.sh -s kata-qemu | sed -e 's|--disable-seccomp||g' | xargs ./configure \ --with-pkgversion=kata-static diff --git a/tools/packaging/static-build/qemu/Dockerfile b/tools/packaging/static-build/qemu/Dockerfile index 26fc6c811c..502b9018ad 100644 --- a/tools/packaging/static-build/qemu/Dockerfile +++ b/tools/packaging/static-build/qemu/Dockerfile @@ -58,7 +58,8 @@ ADD qemu /root/kata_qemu ADD scripts/apply_patches.sh /root/apply_patches.sh ADD static-build /root/static-build -RUN /root/kata_qemu/apply_patches.sh +RUN stable_branch=$(cat VERSION | awk 'BEGIN{FS=OFS="."}{print $1 "." $2 ".x"}') && \ + /root/apply_patches.sh "/root/kata_qemu/patches/${stable_branch}" RUN PREFIX="${PREFIX}" /root/configure-hypervisor.sh -s kata-qemu | xargs ./configure \ --with-pkgversion=kata-static From 7ab8f62d43ffbff722b65a994751435252a5fbdf Mon Sep 17 00:00:00 2001 From: Wainer dos Santos Moschetta Date: Mon, 7 Dec 2020 20:37:32 +0000 Subject: [PATCH 08/31] runtime: Allow to overwrite DESTDIR On runtime/Makefile the value of DESTDIR is set to "/", unless one pass that variable as an argument to `make`. This change will allow its overwrite if DESTDIR is exported in the environment as well. Fixes #1182 Signed-off-by: Wainer dos Santos Moschetta --- src/runtime/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/Makefile b/src/runtime/Makefile index 690755f2ae..0c03b7065f 100644 --- a/src/runtime/Makefile +++ b/src/runtime/Makefile @@ -68,7 +68,7 @@ NETMON_TARGET = $(PROJECT_TYPE)-netmon NETMON_TARGET_OUTPUT = $(CURDIR)/$(NETMON_TARGET) BINLIBEXECLIST += $(NETMON_TARGET) -DESTDIR := / +DESTDIR ?= / ifeq ($(PREFIX),) PREFIX := /usr From 4727a9c3e45ed2759b63e5a9bb816651328fd599 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Thu, 10 Dec 2020 09:20:56 -0600 Subject: [PATCH 09/31] rootfs: reduce size of debian image Improve Kata Containers memory footprint by reducing debian image size. Without this change: Debian image -> 256MB With this change: Debian image -> 128MB Note: this change *will not* impact ubuntu image. fixes #1188 Signed-off-by: Julio Montes --- tools/osbuilder/rootfs-builder/ubuntu/rootfs_lib.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/osbuilder/rootfs-builder/ubuntu/rootfs_lib.sh b/tools/osbuilder/rootfs-builder/ubuntu/rootfs_lib.sh index a012a5cc4c..e773c62a8f 100644 --- a/tools/osbuilder/rootfs-builder/ubuntu/rootfs_lib.sh +++ b/tools/osbuilder/rootfs-builder/ubuntu/rootfs_lib.sh @@ -80,5 +80,8 @@ build_rootfs() { ${ROOTFS_DIR} chroot $ROOTFS_DIR ln -s /lib/systemd/systemd /usr/lib/systemd/systemd -} + # Reduce image size and memory footprint + # removing not needed files and directories. + chroot $ROOTFS_DIR rm -rf /usr/share/{bash-completion,bug,doc,info,lintian,locale,man,menu,misc,pixmaps,terminfo,zoneinfo,zsh} +} From 0fd70f7ec373136238f6f397c82bdd3446f20d5b Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Tue, 8 Dec 2020 10:12:06 -0600 Subject: [PATCH 10/31] rootfs-builder: add support for gentoo Generate images based on gentoo fixes #1178 Signed-off-by: Julio Montes --- .../rootfs-builder/gentoo/Dockerfile.in | 13 ++ .../osbuilder/rootfs-builder/gentoo/config.sh | 22 ++ .../rootfs-builder/gentoo/rootfs_lib.sh | 210 ++++++++++++++++++ tools/osbuilder/rootfs-builder/rootfs.sh | 21 +- 4 files changed, 265 insertions(+), 1 deletion(-) create mode 100644 tools/osbuilder/rootfs-builder/gentoo/Dockerfile.in create mode 100644 tools/osbuilder/rootfs-builder/gentoo/config.sh create mode 100644 tools/osbuilder/rootfs-builder/gentoo/rootfs_lib.sh diff --git a/tools/osbuilder/rootfs-builder/gentoo/Dockerfile.in b/tools/osbuilder/rootfs-builder/gentoo/Dockerfile.in new file mode 100644 index 0000000000..5c6e2fdcea --- /dev/null +++ b/tools/osbuilder/rootfs-builder/gentoo/Dockerfile.in @@ -0,0 +1,13 @@ +# +# Copyright (c) 2020 Intel Corporation +# +# SPDX-License-Identifier: Apache-2.0 + +from docker.io/gentoo/stage3-amd64:latest + +# This dockerfile needs to provide all the componets need to build a rootfs +# Install any package need to create a rootfs (package manager, extra tools) + +# This will install the proper golang to build Kata components +@INSTALL_GO@ +@INSTALL_RUST@ diff --git a/tools/osbuilder/rootfs-builder/gentoo/config.sh b/tools/osbuilder/rootfs-builder/gentoo/config.sh new file mode 100644 index 0000000000..ec83a9d9d3 --- /dev/null +++ b/tools/osbuilder/rootfs-builder/gentoo/config.sh @@ -0,0 +1,22 @@ +# This is a configuration file add extra variables to +# +# Copyright (c) 2020 Intel Corporation +# +# SPDX-License-Identifier: Apache-2.0 +# be used by build_rootfs() from rootfs_lib.sh the variables will be +# loaded just before call the function. For more information see the +# rootfs-builder/README.md file. + +OS_VERSION=${OS_VERSION:-latest} +OS_NAME=${OS_NAME:-"gentoo"} + +# packages to be installed by default +PACKAGES="sys-apps/systemd net-firewall/iptables net-misc/chrony" + +# Init process must be one of {systemd,kata-agent} +INIT_PROCESS=systemd +# List of zero or more architectures to exclude from build, +# as reported by `uname -m` +ARCH_EXCLUDE_LIST=( aarch64 ppc64le s390x ) + +[ "$SECCOMP" = "yes" ] && PACKAGES+=" sys-libs/libseccomp" || true diff --git a/tools/osbuilder/rootfs-builder/gentoo/rootfs_lib.sh b/tools/osbuilder/rootfs-builder/gentoo/rootfs_lib.sh new file mode 100644 index 0000000000..0226bb2508 --- /dev/null +++ b/tools/osbuilder/rootfs-builder/gentoo/rootfs_lib.sh @@ -0,0 +1,210 @@ +# Copyright (c) 2020 Intel Corporation +# +# SPDX-License-Identifier: Apache-2.0 +# +# - Arguments +# rootfs_dir=$1 +# +# - Optional environment variables +# +# EXTRA_PKGS: Variable to add extra PKGS provided by the user +# +# BIN_AGENT: Name of the Kata-Agent binary +# +# REPO_URL: URL to distribution repository ( should be configured in +# config.sh file) +# +# Any other configuration variable for a specific distro must be added +# and documented on its own config.sh +# +# - Expected result +# +# rootfs_dir populated with rootfs pkgs +# It must provide a binary in /sbin/init +# +gentoo_portage_container=gentoo_portage +gentoo_local_portage_dir="${HOME}/gentoo-$(date +%s)" + +build_rootfs() { + # Mandatory + local ROOTFS_DIR=$1 + + # In case of support EXTRA packages, use it to allow + # users to add more packages to the base rootfs + local EXTRA_PKGS=${EXTRA_PKGS:-} + + # Populate ROOTFS_DIR + # Must provide /sbin/init and /bin/${BIN_AGENT} + check_root + mkdir -p "${ROOTFS_DIR}" + + # trim whitespace + PACKAGES=$(echo $PACKAGES |xargs ) + EXTRA_PKGS=$(echo $EXTRA_PKGS |xargs) + + # extra packages are added to packages and finally passed to debootstrap + if [ "${EXTRA_PKGS}" = "" ]; then + echo "no extra packages" + else + PACKAGES="${PACKAGES} ${EXTRA_PKGS}" + fi + + local packageuseconf="/etc/portage/package.use/user" + local makeconf="/etc/portage/make.conf" + local systemd_optimizations=( + acl + -apparmor + -audit + cgroup-hybrid + -cryptsetup + -curl + -dns-over-tls + -gcrypt + -gnuefi + -homed + -http + -hwdb + -idn + -importd + kmod + -lz4 + -lzma + -nat + -pkcs11 + -policykit + -pwquality + -qrcode + -repart + -resolvconf + sysv-utils + -test + -xkb + -zstd + ) + + local packages_optimizations=( + -abi_x86_32 + -abi_x86_x32 + -debug + -doc + -examples + multicall + -ncurses + -nls + -selinux + systemd + -udev + -unicode + -X + ) + + local compiler_optimizations=( + -O3 + -fassociative-math + -fasynchronous-unwind-tables + -feliminate-unused-debug-types + -fexceptions + -ffat-lto-objects + -fno-semantic-interposition + -fno-signed-zeros + -fno-trapping-math + -fstack-protector + -ftree-loop-distribute-patterns + -m64 + -mtune=skylake + --param=ssp-buffer-size=32 + -pipe + -Wl,--copy-dt-needed-entries + -Wp,-D_REENTRANT + -Wl,--enable-new-dtags + -Wl,-sort-common + -Wl,-z -Wl,now + -Wl,-z -Wl,relro + ) + + local build_dependencies=( + dev-vcs/git + ) + + local conflicting_packages=( + net-misc/netifrc sys-apps/sysvinit + sys-fs/eudev sys-apps/openrc + virtual/service-manager + ) + + # systemd optimizations + echo "sys-apps/systemd ${systemd_optimizations[*]}" >> ${packageuseconf} + echo "MAKEOPTS=\"-j$(nproc)\"" >> ${makeconf} + + # Packages optimizations + echo "USE=\"${packages_optimizations[*]}\"" >> ${makeconf} + + # compiler optimizations + echo "CFLAGS=\"${compiler_optimizations[*]}\"" >> ${makeconf} + echo 'CXXFLAGS="${CFLAGS}"' >> ${makeconf} + + # remove conflicting packages + emerge -Cv $(echo "${conflicting_packages[*]}") + + # Get the latest systemd portage profile and set it + systemd_profile=$(profile-config list | grep stable | grep -E "[[:digit:]]/systemd" | xargs | cut -d' ' -f2) + profile-config set "${systemd_profile}" + + # Install build dependencies + emerge --newuse $(echo "${build_dependencies[*]}") + + quickpkg --include-unmodified-config=y "*/*" + + # Install needed packages excluding conflicting packages + ROOT=${ROOTFS_DIR} emerge --exclude "$(echo "${conflicting_packages[*]}")" --newuse -k ${PACKAGES} + + pushd ${ROOTFS_DIR} + + # systemd will need this library + cp /usr/lib/gcc/x86_64-pc-linux-gnu/*/libgcc_s.so* lib64/ + + # Clean up the rootfs. there are things that we don't need + rm -rf etc/{udev,X11,kernel,runlevels,terminfo,init.d} + rm -rf var/lib/{gentoo,portage} + rm -rf var/{db,cache} + rm -rf usr/share/* + rm -rf usr/lib/{udev,gconv,kernel} + rm -rf usr/{include,local} + rm -rf usr/lib64/gconv + rm -rf lib/{udev,gentoo} + + # Make sure important directories exist in the rootfs + ln -s ../run var/run + mkdir -p proc opt sys dev home root + + popd +} + +before_starting_container() { + gentoo_portage_image="gentoo/portage" + + if [ "${OS_VERSION}" = "latest" ];then + ${container_engine} pull "${gentoo_portage_image}:latest" + OS_VERSION=$(docker image inspect -f {{.Created}} ${gentoo_portage_image} | cut -dT -f1 | sed 's|-||g') + else + ${container_engine} pull "${gentoo_portage_image}:${OS_VERSION}" + fi + + # create portage volume and container + ${container_engine} create -v /usr/portage --name "${gentoo_portage_container}" "${gentoo_portage_image}" /bin/true +} + +after_stopping_container() { + # Get the list of volumes + volumes="" + for i in $(seq $(${container_engine} inspect -f "{{len .Mounts}}" "${gentoo_portage_container}")); do + volumes+="$(${container_engine} inspect -f "{{(index .Mounts $((i-1))).Name}}" "${gentoo_portage_container}") " + done + + # remove portage container + ${container_engine} rm -f "${gentoo_portage_container}" + sudo rm -rf "${gentoo_local_portage_dir}" + + # remove portage volumes + ${container_engine} volume rm -f ${volumes} +} diff --git a/tools/osbuilder/rootfs-builder/rootfs.sh b/tools/osbuilder/rootfs-builder/rootfs.sh index 7a71ba902d..dc902f57b0 100755 --- a/tools/osbuilder/rootfs-builder/rootfs.sh +++ b/tools/osbuilder/rootfs-builder/rootfs.sh @@ -182,6 +182,19 @@ docker_extra_args() local args="" case "$1" in + gentoo) + # Requred to chroot + args+=" --cap-add SYS_CHROOT" + # debootstrap needs to create device nodes to properly function + args+=" --cap-add MKNOD" + # Required to mount inside a container + args+=" --cap-add SYS_ADMIN" + # Required to build glibc + args+=" --cap-add SYS_PTRACE" + # mount portage volume + args+=" -v ${gentoo_local_portage_dir}:/usr/portage/packages" + args+=" --volumes-from ${gentoo_portage_container}" + ;; ubuntu | debian) # Requred to chroot args+=" --cap-add SYS_CHROOT" @@ -506,6 +519,10 @@ EOT mkdir -p "${ROOTFS_DIR}/etc" case "${distro}" in + "gentoo") + chrony_conf_file="${ROOTFS_DIR}/etc/chrony/chrony.conf" + chrony_systemd_service="${ROOTFS_DIR}/lib/systemd/system/chronyd.service" + ;; "ubuntu" | "debian") echo "I am ubuntu or debian" chrony_conf_file="${ROOTFS_DIR}/etc/chrony/chrony.conf" @@ -530,7 +547,9 @@ EOT sed -i 's/^\(server \|pool \|peer \)/# &/g' ${chrony_conf_file} if [ -f "$chrony_systemd_service" ]; then - sed -i '/^\[Unit\]/a ConditionPathExists=\/dev\/ptp0' ${chrony_systemd_service} + # Remove user option, user could not exist in the rootfs + sed -i -e 's/^\(ExecStart=.*\)-u [[:alnum:]]*/\1/g' \ + -e '/^\[Unit\]/a ConditionPathExists=\/dev\/ptp0' ${chrony_systemd_service} fi # The CC on s390x for fedora needs to be manually set to gcc when the golang is downloaded from the main page. From fce14f3697666573cc46c993092db556fe412f56 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 29 Oct 2020 14:34:52 +1100 Subject: [PATCH 11/31] runtime/network: Correct error reporting in listInterfaces() If the upcast from resultingInterfaces to *grpc.Interfaces fails, we return (nil, err), but previous code ensures that err is nil at that point, so we return no error. Forward port of https://github.com/kata-containers/runtime/pull/3033/commits/b86e904c2d13dfd148b3bcf2ad93748e2b45a886 fixes #1206 Signed-off-by: David Gibson --- src/runtime/virtcontainers/kata_agent.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/runtime/virtcontainers/kata_agent.go b/src/runtime/virtcontainers/kata_agent.go index b88bec4488..a46f39a4c2 100644 --- a/src/runtime/virtcontainers/kata_agent.go +++ b/src/runtime/virtcontainers/kata_agent.go @@ -635,10 +635,10 @@ func (k *kataAgent) listInterfaces() ([]*pbTypes.Interface, error) { return nil, err } resultInterfaces, ok := resultingInterfaces.(*grpc.Interfaces) - if ok { - return resultInterfaces.Interfaces, err + if !ok { + return nil, fmt.Errorf("Unexpected type %T for interfaces", resultingInterfaces) } - return nil, err + return resultInterfaces.Interfaces, nil } func (k *kataAgent) listRoutes() ([]*pbTypes.Route, error) { From 9117dd409eb40570b05efe36e019263e2d742502 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 29 Oct 2020 14:38:50 +1100 Subject: [PATCH 12/31] runtime/network: Fix error reporting in listRoutes() If the upcast from resultingRoutes to *grpc.IRoutes fails, we return (nil, err), but previous code ensures that err is nil at that point, so we return no error. fixes #1206 Forward port of https://github.com/kata-containers/runtime/pull/3033/commits/0ffaeeb5d84054cf2726e5fbdfbfde54c75ec6d7 Signed-off-by: David Gibson --- src/runtime/virtcontainers/kata_agent.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/runtime/virtcontainers/kata_agent.go b/src/runtime/virtcontainers/kata_agent.go index a46f39a4c2..58ff1a4e89 100644 --- a/src/runtime/virtcontainers/kata_agent.go +++ b/src/runtime/virtcontainers/kata_agent.go @@ -648,10 +648,10 @@ func (k *kataAgent) listRoutes() ([]*pbTypes.Route, error) { return nil, err } resultRoutes, ok := resultingRoutes.(*grpc.Routes) - if ok { - return resultRoutes.Routes, err + if !ok { + return nil, fmt.Errorf("Unexpected type %T for routes", resultingRoutes) } - return nil, err + return resultRoutes.Routes, nil } func (k *kataAgent) getAgentURL() (string, error) { From 662e8db5dd188ba0ca78b142716df904e254d157 Mon Sep 17 00:00:00 2001 From: Maruth Goyal Date: Wed, 9 Dec 2020 15:13:25 +0530 Subject: [PATCH 13/31] agent/sandbox: Don't update cpuset when ncpus = 0 When receiving an OnlineCpuMemory RPC, if the number of CPUs to be made available is 0, then updating the cpusets is a redundant operation. Fixes: #1172 Signed-off-by: Maruth Goyal Signed-off-by: Peng Tao --- src/agent/src/sandbox.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/agent/src/sandbox.rs b/src/agent/src/sandbox.rs index ee5ab08f98..b831119289 100644 --- a/src/agent/src/sandbox.rs +++ b/src/agent/src/sandbox.rs @@ -233,6 +233,10 @@ impl Sandbox { online_memory(&self.logger)?; } + if req.nb_cpus == 0 { + return Ok(()); + } + let cpuset = rustjail_cgroups::fs::get_guest_cpuset()?; for (_, ctr) in self.containers.iter() { From 35b619ff583e7cb4ae9fc090119407c8dc55df41 Mon Sep 17 00:00:00 2001 From: Liu Jiang Date: Tue, 22 Dec 2020 00:03:27 +0800 Subject: [PATCH 14/31] oci: fix a typo in "addtionalGids" There's a typo in "addtionalGids", which should be "additionalGids". Fixes: #1211 Signed-off-by: Liu Jiang --- src/agent/oci/src/lib.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/agent/oci/src/lib.rs b/src/agent/oci/src/lib.rs index 0c4794f6e7..94e1c798c9 100644 --- a/src/agent/oci/src/lib.rs +++ b/src/agent/oci/src/lib.rs @@ -142,7 +142,7 @@ pub struct User { pub gid: u32, #[serde( default, - rename = "addtionalGids", + rename = "additionalGids", skip_serializing_if = "Vec::is_empty" )] pub additional_gids: Vec, @@ -1223,8 +1223,7 @@ mod tests { uid: 1, gid: 1, // incompatible with oci - // additional_gids: vec![5, 6], - additional_gids: vec![], + additional_gids: vec![5, 6], username: "".to_string(), }, args: vec!["sh".to_string()], From 40316f688aa74bed5e4f7168b82d35f8f83974f0 Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Tue, 8 Dec 2020 11:43:54 -0800 Subject: [PATCH 15/31] qemu: no state to save if QEMU isn't running On pod delete, we were looking to read files that we had just deleted. In particular, stopSandbox for QEMU was called (we cleanup up vmpath), and then QEMU's save function was called, which immediately checks for the PID file. Let's only update the persist store for QEMU if QEMU is actually running. This'll avoid Error messages being displayed when we are stopping and deleting a sandbox: ``` level=error msg="Could not read qemu pid file" ``` I reviewed CLH, and it looks like it is already taking appropriate action, so no changes needed. Ideally we won't spend much time saving state to persist.json unless there's an actual error during stop/delete/shutdown path, as the persist will also be removed after the pod is removed. We may want to optimize this, as currently we are doing a persist store when deleting each container (after the sandbox is stopped, VM is killed), and when we stop the sandbox. This'll require more rework... tracked in: https://github.com/kata-containers/kata-containers/issues/1181 Fixes: #1179 Signed-off-by: Eric Ernst --- src/runtime/virtcontainers/qemu.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index 5c64f1ad4c..88e5ab71e7 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -2203,6 +2203,12 @@ func (q *qemu) toGrpc() ([]byte, error) { } func (q *qemu) save() (s persistapi.HypervisorState) { + + // If QEMU isn't even running, there isn't any state to save + if q.stopped { + return + } + pids := q.getPids() if len(pids) != 0 { s.Pid = pids[0] From b745e5ff02182dc0c25c610dcdb13739ec17268d Mon Sep 17 00:00:00 2001 From: Liu Jiang Date: Sat, 30 May 2020 23:09:53 +0800 Subject: [PATCH 16/31] agent: consume ttrpc crate from crates.io The ttrpc v0.3.0 has been published to crates.io, so consume from crates.io. Fixes: #1213 Signed-off-by: Liu Jiang --- src/agent/Cargo.lock | 3 ++- src/agent/Cargo.toml | 2 +- src/agent/protocols/Cargo.toml | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/agent/Cargo.lock b/src/agent/Cargo.lock index c2c16f3885..d8ad331f36 100644 --- a/src/agent/Cargo.lock +++ b/src/agent/Cargo.lock @@ -1174,7 +1174,8 @@ dependencies = [ [[package]] name = "ttrpc" version = "0.3.0" -source = "git+https://github.com/containerd/ttrpc-rust.git?branch=0.3.0#ba1efe3bbb8f8af4895b7623ed1d11561e70e566" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bfa9da24c351f0feef5e66c0b28c18373a7ef3e1bfdfd5852170de494f9bf870" dependencies = [ "byteorder", "libc", diff --git a/src/agent/Cargo.toml b/src/agent/Cargo.toml index 3013b2d107..505313c856 100644 --- a/src/agent/Cargo.toml +++ b/src/agent/Cargo.toml @@ -11,7 +11,7 @@ rustjail = { path = "rustjail" } protocols = { path = "protocols" } netlink = { path = "netlink", features = ["with-log", "with-agent-handler"] } lazy_static = "1.3.0" -ttrpc = { git = "https://github.com/containerd/ttrpc-rust.git", branch="0.3.0" } +ttrpc = "0.3.0" protobuf = "=2.14.0" libc = "0.2.58" nix = "0.17.0" diff --git a/src/agent/protocols/Cargo.toml b/src/agent/protocols/Cargo.toml index 7b383a2170..59ab72427a 100644 --- a/src/agent/protocols/Cargo.toml +++ b/src/agent/protocols/Cargo.toml @@ -5,7 +5,7 @@ authors = ["The Kata Containers community "] edition = "2018" [dependencies] -ttrpc = { git = "https://github.com/containerd/ttrpc-rust.git", branch="0.3.0" } +ttrpc = "0.3.0" protobuf = "=2.14.0" futures = "0.1.27" From 21fad464e890999f157f3ea5eaed30bb667e9cf8 Mon Sep 17 00:00:00 2001 From: Liu Jiang Date: Tue, 22 Dec 2020 11:16:15 +0800 Subject: [PATCH 17/31] oci: fix two incompatible issues with OCI spec The first incompatible issue is caused by a typo, "swapiness" should be "swappiness". The second incompatible issue is caused by a serde format. The struct LinuxBlockIODevice is introduced for convenience, but it also changes serialized data, so "#[serde(flatten)]" should be used for compatibility with OCI spec. Fixes: #1211 Signed-off-by: Liu Jiang --- src/agent/oci/src/lib.rs | 52 ++++++------------------ src/agent/rustjail/src/cgroups/fs/mod.rs | 8 ++-- src/agent/rustjail/src/lib.rs | 2 +- 3 files changed, 18 insertions(+), 44 deletions(-) diff --git a/src/agent/oci/src/lib.rs b/src/agent/oci/src/lib.rs index 94e1c798c9..b51d78436b 100644 --- a/src/agent/oci/src/lib.rs +++ b/src/agent/oci/src/lib.rs @@ -302,6 +302,7 @@ pub struct LinuxBlockIODevice { #[derive(Serialize, Deserialize, Debug, Default, Clone, PartialEq)] pub struct LinuxWeightDevice { + #[serde(flatten)] pub blk: LinuxBlockIODevice, #[serde(default, skip_serializing_if = "Option::is_none")] pub weight: Option, @@ -315,6 +316,7 @@ pub struct LinuxWeightDevice { #[derive(Serialize, Deserialize, Debug, Default, Clone, PartialEq)] pub struct LinuxThrottleDevice { + #[serde(flatten)] pub blk: LinuxBlockIODevice, #[serde(default)] pub rate: u64, @@ -375,7 +377,7 @@ pub struct LinuxMemory { #[serde(default, skip_serializing_if = "Option::is_none", rename = "kernelTCP")] pub kernel_tcp: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - pub swapiness: Option, + pub swappiness: Option, #[serde( default, skip_serializing_if = "Option::is_none", @@ -833,7 +835,7 @@ mod tests { } #[test] - fn test_deserialize_sepc() { + fn test_deserialize_spec() { let data = r#"{ "ociVersion": "1.0.1", "process": { @@ -1118,36 +1120,28 @@ mod tests { "leafWeight": 10, "weightDevice": [ { - "blk": { - "major": 8, - "minor": 0 - }, + "major": 8, + "minor": 0, "weight": 500, "leafWeight": 300 }, { - "blk":{ - "major": 8, - "minor": 16 - }, + "major": 8, + "minor": 16, "weight": 500 } ], "throttleReadBpsDevice": [ { - "blk":{ - "major": 8, - "minor": 0 - }, + "major": 8, + "minor": 0, "rate": 600 } ], "throttleWriteIOPSDevice": [ { - "blk":{ - "major": 8, - "minor": 16 - }, + "major": 8, + "minor": 16, "rate": 300 } ] @@ -1436,8 +1430,7 @@ mod tests { swap: Some(536870912), kernel: Some(-1), kernel_tcp: Some(-1), - // incompatible with oci - swapiness: None, + swappiness: Some(0), disable_oom_killer: Some(false), }), cpu: Some(crate::LinuxCPU { @@ -1590,25 +1583,6 @@ mod tests { vm: None, }; - // warning : incompatible with oci : https://github.com/opencontainers/runtime-spec/blob/master/config.md - // 1. User use addtionalGids while oci use additionalGids - // 2. LinuxMemory use swapiness while oci use swappiness - // 3. LinuxWeightDevice with blk - // { - // "blk": { - // "major": 8, - // "minor": 0 - // }, - // "weight": 500, - // "leafWeight": 300 - // } - // oci without blk - // { - // "major": 8, - // "minor": 0, - // "weight": 500, - // "leafWeight": 300 - // } let current: crate::Spec = serde_json::from_str(data).unwrap(); assert_eq!(expected, current); } diff --git a/src/agent/rustjail/src/cgroups/fs/mod.rs b/src/agent/rustjail/src/cgroups/fs/mod.rs index c7fcee8c89..80d18fefe7 100644 --- a/src/agent/rustjail/src/cgroups/fs/mod.rs +++ b/src/agent/rustjail/src/cgroups/fs/mod.rs @@ -421,13 +421,13 @@ fn set_memory_resources(cg: &cgroups::Cgroup, memory: &LinuxMemory, update: bool } } - if let Some(swapiness) = memory.swapiness { - if swapiness >= 0 && swapiness <= 100 { - mem_controller.set_swappiness(swapiness as u64)?; + if let Some(swappiness) = memory.swappiness { + if swappiness >= 0 && swappiness <= 100 { + mem_controller.set_swappiness(swappiness as u64)?; } else { return Err(anyhow!( "invalid value:{}. valid memory swappiness range is 0-100", - swapiness + swappiness )); } } diff --git a/src/agent/rustjail/src/lib.rs b/src/agent/rustjail/src/lib.rs index 63dc77046f..af9847b28d 100644 --- a/src/agent/rustjail/src/lib.rs +++ b/src/agent/rustjail/src/lib.rs @@ -309,7 +309,7 @@ pub fn resources_grpc_to_oci(res: &grpcLinuxResources) -> ociLinuxResources { swap: Some(mem.Swap), kernel: Some(mem.Kernel), kernel_tcp: Some(mem.KernelTCP), - swapiness: Some(mem.Swappiness as i64), + swappiness: Some(mem.Swappiness as i64), disable_oom_killer: Some(mem.DisableOOMKiller), }) } else { From 49516ef6f21b83058e792d6d3ca0efd8a9349f87 Mon Sep 17 00:00:00 2001 From: Liu Jiang Date: Wed, 23 Dec 2020 16:24:51 +0800 Subject: [PATCH 18/31] rustjail: add more context info for errors To help debug. Fixes: #1214 Signed-off-by: Liu Jiang Signed-off-by: Peng Tao --- src/agent/rustjail/src/validator.rs | 45 +++++++++++++++++------------ 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/src/agent/rustjail/src/validator.rs b/src/agent/rustjail/src/validator.rs index 4e3ce43182..aa1d3b46d5 100644 --- a/src/agent/rustjail/src/validator.rs +++ b/src/agent/rustjail/src/validator.rs @@ -4,7 +4,7 @@ // use crate::container::Config; -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, Context, Result}; use lazy_static; use nix::errno::Errno; use oci::{LinuxIDMapping, LinuxNamespace, Spec}; @@ -60,7 +60,7 @@ fn rootfs(root: &str) -> Result<()> { cleaned.push(e); } - let canon = path.canonicalize()?; + let canon = path.canonicalize().context("canonicalize")?; if cleaned != canon { // There is symbolic in path return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); @@ -123,8 +123,8 @@ fn usernamespace(oci: &Spec) -> Result<()> { } // check if idmappings is correct, at least I saw idmaps // with zero size was passed to agent - idmapping(&linux.uid_mappings)?; - idmapping(&linux.gid_mappings)?; + idmapping(&linux.uid_mappings).context("idmapping uid")?; + idmapping(&linux.gid_mappings).context("idmapping gid")?; } else { // no user namespace but idmap if linux.uid_mappings.len() != 0 || linux.gid_mappings.len() != 0 { @@ -165,14 +165,20 @@ fn check_host_ns(path: &str) -> Result<()> { let cpath = PathBuf::from(path); let hpath = PathBuf::from("/proc/self/ns/net"); - let real_hpath = hpath.read_link()?; - let meta = cpath.symlink_metadata()?; + let real_hpath = hpath + .read_link() + .context(format!("read link {:?}", hpath))?; + let meta = cpath + .symlink_metadata() + .context(format!("symlink metadata {:?}", cpath))?; let file_type = meta.file_type(); if !file_type.is_symlink() { return Ok(()); } - let real_cpath = cpath.read_link()?; + let real_cpath = cpath + .read_link() + .context(format!("read link {:?}", cpath))?; if real_cpath == real_hpath { return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } @@ -254,7 +260,10 @@ fn rootless_euid_mount(oci: &Spec) -> Result<()> { return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } - let id = fields[1].trim().parse::()?; + let id = fields[1] + .trim() + .parse::() + .context(format!("parse field {}", &fields[1]))?; if opt.starts_with("uid=") { if !has_idmapping(&linux.uid_mappings, id) { @@ -274,8 +283,8 @@ fn rootless_euid_mount(oci: &Spec) -> Result<()> { } fn rootless_euid(oci: &Spec) -> Result<()> { - rootless_euid_mapping(oci)?; - rootless_euid_mount(oci)?; + rootless_euid_mapping(oci).context("rootless euid mapping")?; + rootless_euid_mount(oci).context("rotless euid mount")?; Ok(()) } @@ -292,16 +301,16 @@ pub fn validate(conf: &Config) -> Result<()> { } let root = oci.root.as_ref().unwrap().path.as_str(); - rootfs(root)?; - network(oci)?; - hostname(oci)?; - security(oci)?; - usernamespace(oci)?; - cgroupnamespace(oci)?; - sysctl(&oci)?; + rootfs(root).context("rootfs")?; + network(oci).context("network")?; + hostname(oci).context("hostname")?; + security(oci).context("security")?; + usernamespace(oci).context("usernamespace")?; + cgroupnamespace(oci).context("cgroupnamespace")?; + sysctl(&oci).context("sysctl")?; if conf.rootless_euid { - rootless_euid(oci)?; + rootless_euid(oci).context("rootless euid")?; } Ok(()) From 8fdb85e062d7ad360dfb441b74aeebb561b0cfbf Mon Sep 17 00:00:00 2001 From: Liu Jiang Date: Thu, 24 Dec 2020 09:47:10 +0800 Subject: [PATCH 19/31] jail/validator: avoid unwrap() for safety Explicitly return error codes instead of unwrap(). Fixes: #1214 Signed-off-by: Liu Jiang Signed-off-by: Peng Tao --- src/agent/rustjail/src/validator.rs | 59 ++++++++++++++++++++--------- 1 file changed, 42 insertions(+), 17 deletions(-) diff --git a/src/agent/rustjail/src/validator.rs b/src/agent/rustjail/src/validator.rs index aa1d3b46d5..0a88fbcec2 100644 --- a/src/agent/rustjail/src/validator.rs +++ b/src/agent/rustjail/src/validator.rs @@ -52,7 +52,11 @@ fn rootfs(root: &str) -> Result<()> { continue; } - stack.push(c.as_os_str().to_str().unwrap().to_string()); + if let Some(v) = c.as_os_str().to_str() { + stack.push(v.to_string()); + } else { + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); + } } let mut cleaned = PathBuf::from("/"); @@ -78,10 +82,10 @@ fn hostname(oci: &Spec) -> Result<()> { return Ok(()); } - if oci.linux.is_none() { - return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); - } - let linux = oci.linux.as_ref().unwrap(); + let linux = oci + .linux + .as_ref() + .ok_or(anyhow!(nix::Error::from_errno(Errno::EINVAL)))?; if !contain_namespace(&linux.namespaces, "uts") { return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } @@ -90,8 +94,11 @@ fn hostname(oci: &Spec) -> Result<()> { } fn security(oci: &Spec) -> Result<()> { - let linux = oci.linux.as_ref().unwrap(); - if linux.masked_paths.len() == 0 && linux.readonly_paths.len() == 0 { + let linux = oci + .linux + .as_ref() + .ok_or(anyhow!(nix::Error::from_errno(Errno::EINVAL)))?; + if linux.masked_paths.is_empty() && linux.readonly_paths.is_empty() { return Ok(()); } @@ -115,7 +122,10 @@ fn idmapping(maps: &Vec) -> Result<()> { } fn usernamespace(oci: &Spec) -> Result<()> { - let linux = oci.linux.as_ref().unwrap(); + let linux = oci + .linux + .as_ref() + .ok_or(anyhow!(nix::Error::from_errno(Errno::EINVAL)))?; if contain_namespace(&linux.namespaces, "user") { let user_ns = PathBuf::from("/proc/self/ns/user"); if !user_ns.exists() { @@ -136,7 +146,10 @@ fn usernamespace(oci: &Spec) -> Result<()> { } fn cgroupnamespace(oci: &Spec) -> Result<()> { - let linux = oci.linux.as_ref().unwrap(); + let linux = oci + .linux + .as_ref() + .ok_or(anyhow!(nix::Error::from_errno(Errno::EINVAL)))?; if contain_namespace(&linux.namespaces, "cgroup") { let path = PathBuf::from("/proc/self/ns/cgroup"); if !path.exists() { @@ -187,7 +200,10 @@ fn check_host_ns(path: &str) -> Result<()> { } fn sysctl(oci: &Spec) -> Result<()> { - let linux = oci.linux.as_ref().unwrap(); + let linux = oci + .linux + .as_ref() + .ok_or(anyhow!(nix::Error::from_errno(Errno::EINVAL)))?; for (key, _) in linux.sysctl.iter() { if SYSCTLS.contains_key(key.as_str()) || key.starts_with("fs.mqueue.") { if contain_namespace(&linux.namespaces, "ipc") { @@ -226,7 +242,10 @@ fn sysctl(oci: &Spec) -> Result<()> { } fn rootless_euid_mapping(oci: &Spec) -> Result<()> { - let linux = oci.linux.as_ref().unwrap(); + let linux = oci + .linux + .as_ref() + .ok_or(anyhow!(nix::Error::from_errno(Errno::EINVAL)))?; if !contain_namespace(&linux.namespaces, "user") { return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } @@ -249,7 +268,10 @@ fn has_idmapping(maps: &Vec, id: u32) -> bool { } fn rootless_euid_mount(oci: &Spec) -> Result<()> { - let linux = oci.linux.as_ref().unwrap(); + let linux = oci + .linux + .as_ref() + .ok_or(anyhow!(nix::Error::from_errno(Errno::EINVAL)))?; for mnt in oci.mounts.iter() { for opt in mnt.options.iter() { @@ -290,16 +312,19 @@ fn rootless_euid(oci: &Spec) -> Result<()> { pub fn validate(conf: &Config) -> Result<()> { lazy_static::initialize(&SYSCTLS); - let oci = conf.spec.as_ref().unwrap(); + let oci = conf + .spec + .as_ref() + .ok_or(anyhow!(nix::Error::from_errno(Errno::EINVAL)))?; if oci.linux.is_none() { return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } - if oci.root.is_none() { - return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); - } - let root = oci.root.as_ref().unwrap().path.as_str(); + let root = match oci.root.as_ref() { + Some(v) => v.path.as_str(), + None => return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))), + }; rootfs(root).context("rootfs")?; network(oci).context("network")?; From 9a41d09f39c83355253836ba8044cf026e7f126c Mon Sep 17 00:00:00 2001 From: Snir Sheriber Date: Sun, 27 Dec 2020 17:42:44 +0200 Subject: [PATCH 20/31] shimv2: Avoid double removing of container from sandbox RemoveContainerRequest results in calling to deleteContainer, according to spec calling to RemoveContainer is idempotent and "must not return an error if the container has already been removed", hence, don't return error if the error reports that the container is not found. Fixes: #836 Signed-off-by: Snir Sheriber --- src/runtime/containerd-shim-v2/delete.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/runtime/containerd-shim-v2/delete.go b/src/runtime/containerd-shim-v2/delete.go index 70631c4fa2..f1d7ebbc5e 100644 --- a/src/runtime/containerd-shim-v2/delete.go +++ b/src/runtime/containerd-shim-v2/delete.go @@ -17,13 +17,12 @@ import ( func deleteContainer(ctx context.Context, s *service, c *container) error { if !c.cType.IsSandbox() { if c.status != task.StatusStopped { - _, err := s.sandbox.StopContainer(c.id, false) - if err != nil { + if _, err := s.sandbox.StopContainer(c.id, false); err != nil && !isNotFound(err) { return err } } - if _, err := s.sandbox.DeleteContainer(c.id); err != nil { + if _, err := s.sandbox.DeleteContainer(c.id); err != nil && !isNotFound(err) { return err } } From 1f943bd6bfa32002be993a234695a282cedf215d Mon Sep 17 00:00:00 2001 From: Wainer dos Santos Moschetta Date: Mon, 28 Dec 2020 15:32:35 -0500 Subject: [PATCH 21/31] agent: Return error on trying to persist a pid namespace An pid namespace cannot be persisted, so add a check-and-error on Namespace::setup() for handling that case. Fixes #1220 Signed-off-by: Wainer dos Santos Moschetta Signed-off-by: Peng Tao --- src/agent/src/namespace.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/agent/src/namespace.rs b/src/agent/src/namespace.rs index f5c6fa3b0a..6019ffff87 100644 --- a/src/agent/src/namespace.rs +++ b/src/agent/src/namespace.rs @@ -81,7 +81,10 @@ impl Namespace { fs::create_dir_all(&self.persistent_ns_dir)?; let ns_path = PathBuf::from(&self.persistent_ns_dir); - let ns_type = self.ns_type.clone(); + let ns_type = self.ns_type; + if ns_type == NamespaceType::PID { + return Err(anyhow!("Cannot persist namespace of PID type")); + } let logger = self.logger.clone(); let new_ns_path = ns_path.join(&ns_type.get()); @@ -211,6 +214,17 @@ mod tests { assert!(ns_uts.is_ok()); assert!(remove_mounts(&vec![ns_uts.unwrap().path]).is_ok()); + + // Check it cannot persist pid namespaces. + let logger = slog::Logger::root(slog::Discard, o!()); + let tmpdir = Builder::new().prefix("pid").tempdir().unwrap(); + + let ns_pid = Namespace::new(&logger) + .as_pid() + .set_root_dir(tmpdir.path().to_str().unwrap()) + .setup(); + + assert!(ns_pid.is_err()); } #[test] From e6d68349fa2ffce7e5a49a6ca1933bd46910184f Mon Sep 17 00:00:00 2001 From: Wainer dos Santos Moschetta Date: Mon, 28 Dec 2020 15:38:06 -0500 Subject: [PATCH 22/31] agent: Fix temp prefix on Namespace::test_setup_persistent_ns Wrong prefix on the created temp directory on the test_setup_persistent_ns for uts namesmpace type test. Signed-off-by: Wainer dos Santos Moschetta --- src/agent/src/namespace.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/agent/src/namespace.rs b/src/agent/src/namespace.rs index 6019ffff87..6c33c5664d 100644 --- a/src/agent/src/namespace.rs +++ b/src/agent/src/namespace.rs @@ -205,7 +205,7 @@ mod tests { assert!(remove_mounts(&vec![ns_ipc.unwrap().path]).is_ok()); let logger = slog::Logger::root(slog::Discard, o!()); - let tmpdir = Builder::new().prefix("ipc").tempdir().unwrap(); + let tmpdir = Builder::new().prefix("uts").tempdir().unwrap(); let ns_uts = Namespace::new(&logger) .as_uts("test_hostname") From a7568b520c3d8e72fbfbed2b69dffd6bb80912d9 Mon Sep 17 00:00:00 2001 From: Wainer dos Santos Moschetta Date: Mon, 28 Dec 2020 15:47:33 -0500 Subject: [PATCH 23/31] agent: Clean up commented use declarations There are some commented use declarations, removed them all. Signed-off-by: Wainer dos Santos Moschetta --- src/agent/src/namespace.rs | 1 - src/agent/src/sandbox.rs | 2 -- 2 files changed, 3 deletions(-) diff --git a/src/agent/src/namespace.rs b/src/agent/src/namespace.rs index 6c33c5664d..db0b2ffe33 100644 --- a/src/agent/src/namespace.rs +++ b/src/agent/src/namespace.rs @@ -16,7 +16,6 @@ use std::thread::{self}; use crate::mount::{BareMount, FLAGS}; use slog::Logger; -//use container::Process; const PERSISTENT_NS_DIR: &str = "/var/run/sandbox-ns"; pub const NSTYPEIPC: &str = "ipc"; pub const NSTYPEUTS: &str = "uts"; diff --git a/src/agent/src/sandbox.rs b/src/agent/src/sandbox.rs index b831119289..5ba25218bc 100644 --- a/src/agent/src/sandbox.rs +++ b/src/agent/src/sandbox.rs @@ -3,7 +3,6 @@ // SPDX-License-Identifier: Apache-2.0 // -//use crate::container::Container; use crate::linux_abi::*; use crate::mount::{get_mount_fs_type, remove_mounts, TYPEROOTFS}; use crate::namespace::Namespace; @@ -397,7 +396,6 @@ fn online_memory(logger: &Logger) -> Result<()> { #[cfg(test)] mod tests { - //use rustjail::Error; use super::Sandbox; use crate::{mount::BareMount, skip_if_not_root}; use anyhow::Error; From 3306195f6600a35fcaeb758b1573f1d39958c814 Mon Sep 17 00:00:00 2001 From: Tim Zhang Date: Mon, 28 Dec 2020 17:57:37 +0800 Subject: [PATCH 24/31] agent: Avoid container stats panic caused by cgroup controller non-exist Return SingularPtrField::none() instead of panic when getting stats from cgroup failed caused by cgroup controller missing. Signed-off-by: Tim Zhang Signed-off-by: Peng Tao --- src/agent/rustjail/src/cgroups/fs/mod.rs | 81 +++++++++++++----------- 1 file changed, 43 insertions(+), 38 deletions(-) diff --git a/src/agent/rustjail/src/cgroups/fs/mod.rs b/src/agent/rustjail/src/cgroups/fs/mod.rs index 80d18fefe7..03c95efd8d 100644 --- a/src/agent/rustjail/src/cgroups/fs/mod.rs +++ b/src/agent/rustjail/src/cgroups/fs/mod.rs @@ -67,6 +67,15 @@ pub fn load<'a>(h: Box<&'a dyn cgroups::Hierarchy>, path: &str) -> Option { + match $cg.controller_of() { + Some(c) => c, + None => return SingularPtrField::none(), + } + }; +} + #[derive(Serialize, Deserialize, Debug, Clone)] pub struct Manager { pub paths: HashMap, @@ -605,10 +614,8 @@ lazy_static! { } fn get_cpu_stats(cg: &cgroups::Cgroup) -> SingularPtrField { - let cpu_controller: &CpuController = cg.controller_of().unwrap(); - + let cpu_controller: &CpuController = get_controller_or_return_singular_none!(cg); let stat = cpu_controller.cpu().stat; - let h = lines_to_map(&stat); SingularPtrField::some(ThrottlingData { @@ -621,27 +628,18 @@ fn get_cpu_stats(cg: &cgroups::Cgroup) -> SingularPtrField { } fn get_cpuacct_stats(cg: &cgroups::Cgroup) -> SingularPtrField { - let cpuacct_controller: Option<&CpuAcctController> = cg.controller_of(); - if cpuacct_controller.is_none() { - if cg.v2() { - return SingularPtrField::some(CpuUsage { - total_usage: 0, - percpu_usage: vec![], - usage_in_kernelmode: 0, - usage_in_usermode: 0, - unknown_fields: UnknownFields::default(), - cached_size: CachedSize::default(), - }); - } + if let Some(cpuacct_controller) = cg.controller_of::() { + let cpuacct = cpuacct_controller.cpuacct(); - // try to get from cpu controller - let cpu_controller: &CpuController = cg.controller_of().unwrap(); - let stat = cpu_controller.cpu().stat; - let h = lines_to_map(&stat); - let usage_in_usermode = *h.get("user_usec").unwrap(); - let usage_in_kernelmode = *h.get("system_usec").unwrap(); - let total_usage = *h.get("usage_usec").unwrap(); - let percpu_usage = vec![]; + let h = lines_to_map(&cpuacct.stat); + let usage_in_usermode = + (((*h.get("user").unwrap() * NANO_PER_SECOND) as f64) / *CLOCK_TICKS) as u64; + let usage_in_kernelmode = + (((*h.get("system").unwrap() * NANO_PER_SECOND) as f64) / *CLOCK_TICKS) as u64; + + let total_usage = cpuacct.usage; + + let percpu_usage = line_to_vec(&cpuacct.usage_percpu); return SingularPtrField::some(CpuUsage { total_usage, @@ -653,18 +651,25 @@ fn get_cpuacct_stats(cg: &cgroups::Cgroup) -> SingularPtrField { }); } - let cpuacct_controller = cpuacct_controller.unwrap(); - let cpuacct = cpuacct_controller.cpuacct(); + if cg.v2() { + return SingularPtrField::some(CpuUsage { + total_usage: 0, + percpu_usage: vec![], + usage_in_kernelmode: 0, + usage_in_usermode: 0, + unknown_fields: UnknownFields::default(), + cached_size: CachedSize::default(), + }); + } - let h = lines_to_map(&cpuacct.stat); - let usage_in_usermode = - (((*h.get("user").unwrap() * NANO_PER_SECOND) as f64) / *CLOCK_TICKS) as u64; - let usage_in_kernelmode = - (((*h.get("system").unwrap() * NANO_PER_SECOND) as f64) / *CLOCK_TICKS) as u64; - - let total_usage = cpuacct.usage; - - let percpu_usage = line_to_vec(&cpuacct.usage_percpu); + // try to get from cpu controller + let cpu_controller: &CpuController = get_controller_or_return_singular_none!(cg); + let stat = cpu_controller.cpu().stat; + let h = lines_to_map(&stat); + let usage_in_usermode = *h.get("user_usec").unwrap(); + let usage_in_kernelmode = *h.get("system_usec").unwrap(); + let total_usage = *h.get("usage_usec").unwrap(); + let percpu_usage = vec![]; SingularPtrField::some(CpuUsage { total_usage, @@ -677,7 +682,7 @@ fn get_cpuacct_stats(cg: &cgroups::Cgroup) -> SingularPtrField { } fn get_memory_stats(cg: &cgroups::Cgroup) -> SingularPtrField { - let memory_controller: &MemController = cg.controller_of().unwrap(); + let memory_controller: &MemController = get_controller_or_return_singular_none!(cg); // cache from memory stat let memory = memory_controller.memory_stat(); @@ -734,7 +739,7 @@ fn get_memory_stats(cg: &cgroups::Cgroup) -> SingularPtrField { } fn get_pids_stats(cg: &cgroups::Cgroup) -> SingularPtrField { - let pid_controller: &PidController = cg.controller_of().unwrap(); + let pid_controller: &PidController = get_controller_or_return_singular_none!(cg); let current = pid_controller.get_pid_current().unwrap_or(0); let max = pid_controller.get_pid_max(); @@ -841,7 +846,7 @@ fn build_blkio_stats_entry(major: i16, minor: i16, op: &str, value: u64) -> Blki } fn get_blkio_stats_v2(cg: &cgroups::Cgroup) -> SingularPtrField { - let blkio_controller: &BlkIoController = cg.controller_of().unwrap(); + let blkio_controller: &BlkIoController = get_controller_or_return_singular_none!(cg); let blkio = blkio_controller.blkio(); let mut resp = BlkioStats::new(); @@ -869,7 +874,7 @@ fn get_blkio_stats(cg: &cgroups::Cgroup) -> SingularPtrField { return get_blkio_stats_v2(&cg); } - let blkio_controller: &BlkIoController = cg.controller_of().unwrap(); + let blkio_controller: &BlkIoController = get_controller_or_return_singular_none!(cg); let blkio = blkio_controller.blkio(); let mut m = BlkioStats::new(); From 26f176e2d9a90f5ee13c5aa5d014399aa49537fc Mon Sep 17 00:00:00 2001 From: Snir Sheriber Date: Tue, 5 Jan 2021 16:36:22 +0200 Subject: [PATCH 25/31] rustjail: allow network sysctls The network ns is shared with the guest skip looking for it in the spec Fixes: #1228 Signed-off-by: Snir Sheriber Signed-off-by: Peng Tao --- src/agent/rustjail/src/validator.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/agent/rustjail/src/validator.rs b/src/agent/rustjail/src/validator.rs index 0a88fbcec2..554ec40e7d 100644 --- a/src/agent/rustjail/src/validator.rs +++ b/src/agent/rustjail/src/validator.rs @@ -214,16 +214,8 @@ fn sysctl(oci: &Spec) -> Result<()> { } if key.starts_with("net.") { - if !contain_namespace(&linux.namespaces, "network") { - return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); - } - - let net = get_namespace_path(&linux.namespaces, "network")?; - if net.is_empty() || net == "".to_string() { - continue; - } - - check_host_ns(net.as_str())?; + // the network ns is shared with the guest, don't expect to find it in spec + continue; } if contain_namespace(&linux.namespaces, "uts") { From 607a892f2e3e6e19109bf51969f0e459483aaa7c Mon Sep 17 00:00:00 2001 From: "fupan.lfp" Date: Wed, 6 Jan 2021 16:05:49 +0800 Subject: [PATCH 26/31] rustjail: fix the issue of bind mount /dev In case the container rootfs's /dev was overrided by binding mount from another directory, then there's no need to create the default devices nodes and symlinks in /dev. Fixes: #692 Signed-off-by: fupan.lfp --- src/agent/rustjail/src/mount.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/agent/rustjail/src/mount.rs b/src/agent/rustjail/src/mount.rs index 1942fcc5b8..5f4c9e26d3 100644 --- a/src/agent/rustjail/src/mount.rs +++ b/src/agent/rustjail/src/mount.rs @@ -185,6 +185,7 @@ pub fn init_rootfs( None::<&str>, )?; + let mut bind_mount_dev = false; for m in &spec.mounts { let (mut flags, data) = parse_mount(&m); if !m.destination.starts_with("/") || m.destination.contains("..") { @@ -198,6 +199,9 @@ pub fn init_rootfs( mount_cgroups(cfd_log, &m, rootfs, flags, &data, cpath, mounts)?; } else { if m.destination == "/dev" { + if m.r#type == "bind" { + bind_mount_dev = true; + } flags &= !MsFlags::MS_RDONLY; } @@ -239,9 +243,14 @@ pub fn init_rootfs( let olddir = unistd::getcwd()?; unistd::chdir(rootfs)?; - default_symlinks()?; - create_devices(&linux.devices, bind_device)?; - ensure_ptmx()?; + // in case the /dev directory was binded mount from guest, + // then there's no need to create devices nodes and symlinks + // in /dev. + if !bind_mount_dev { + default_symlinks()?; + create_devices(&linux.devices, bind_device)?; + ensure_ptmx()?; + } unistd::chdir(&olddir)?; From 1edb7fe7da2e18c2d18d0512a9e10bc8fd10974e Mon Sep 17 00:00:00 2001 From: "fupan.lfp" Date: Thu, 7 Jan 2021 17:21:22 +0800 Subject: [PATCH 27/31] rustjail: fix the issue of sync read It should check the read count and return an error if read count didn't match the expected number. Fixes: #1233 Signed-off-by: fupan.lfp --- src/agent/rustjail/src/sync.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/agent/rustjail/src/sync.rs b/src/agent/rustjail/src/sync.rs index 9e98b0ad76..422827b941 100644 --- a/src/agent/rustjail/src/sync.rs +++ b/src/agent/rustjail/src/sync.rs @@ -72,7 +72,15 @@ fn read_count(fd: RawFd, count: usize) -> Result> { } } - Ok(v[0..len].to_vec()) + if len != count { + Err(anyhow::anyhow!( + "invalid read count expect {} get {}", + count, + len + )) + } else { + Ok(v[0..len].to_vec()) + } } pub fn read_sync(fd: RawFd) -> Result> { From 499aa24d38b04e952ec6b7d218480964a34bf1cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Mon, 14 Dec 2020 12:53:57 +0100 Subject: [PATCH 28/31] rootfs: Don't fallthrough in the docker_extra_args() switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Falling through the switch cases in docker_extra_args() looks like a typo and causes issues when building with podman, as `--security-opt apparmor=unconfinded" shouldn't be passed if Apparmor is no enable on the system. Fixes: #1241 Signed-off-by: Fabiano Fidêncio --- tools/osbuilder/rootfs-builder/rootfs.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/osbuilder/rootfs-builder/rootfs.sh b/tools/osbuilder/rootfs-builder/rootfs.sh index dc902f57b0..a10907bc08 100755 --- a/tools/osbuilder/rootfs-builder/rootfs.sh +++ b/tools/osbuilder/rootfs-builder/rootfs.sh @@ -200,7 +200,7 @@ docker_extra_args() args+=" --cap-add SYS_CHROOT" # debootstrap needs to create device nodes to properly function args+=" --cap-add MKNOD" - ;& + ;; suse) # Required to mount inside a container args+=" --cap-add SYS_ADMIN" From 2478b8f4006a93bbb1ec9ab57d833d477e2a2a40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Fri, 8 Jan 2021 20:07:24 +0100 Subject: [PATCH 29/31] rootfs: Always add SYS_ADMIN, CHROOT, and MKNOD caps to docker cmdline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We use those, independently of the distro. Signed-off-by: Fabiano Fidêncio --- tools/osbuilder/rootfs-builder/rootfs.sh | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/tools/osbuilder/rootfs-builder/rootfs.sh b/tools/osbuilder/rootfs-builder/rootfs.sh index a10907bc08..6f627d14ec 100755 --- a/tools/osbuilder/rootfs-builder/rootfs.sh +++ b/tools/osbuilder/rootfs-builder/rootfs.sh @@ -181,29 +181,22 @@ docker_extra_args() { local args="" + # Required to mount inside a container + args+=" --cap-add SYS_ADMIN" + # Requred to chroot + args+=" --cap-add SYS_CHROOT" + # debootstrap needs to create device nodes to properly function + args+=" --cap-add MKNOD" + case "$1" in gentoo) - # Requred to chroot - args+=" --cap-add SYS_CHROOT" - # debootstrap needs to create device nodes to properly function - args+=" --cap-add MKNOD" - # Required to mount inside a container - args+=" --cap-add SYS_ADMIN" # Required to build glibc args+=" --cap-add SYS_PTRACE" # mount portage volume args+=" -v ${gentoo_local_portage_dir}:/usr/portage/packages" args+=" --volumes-from ${gentoo_portage_container}" ;; - ubuntu | debian) - # Requred to chroot - args+=" --cap-add SYS_CHROOT" - # debootstrap needs to create device nodes to properly function - args+=" --cap-add MKNOD" - ;; suse) - # Required to mount inside a container - args+=" --cap-add SYS_ADMIN" # When AppArmor is enabled, mounting inside a container is blocked with docker-default profile. # See https://github.com/moby/moby/issues/16429 args+=" --security-opt apparmor=unconfined" From 91b43a996491d74f3ae8d1c2178fb30e565cd072 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Fri, 8 Jan 2021 21:24:24 +0100 Subject: [PATCH 30/31] rootfs: apparmor=unconfined is needed for non Red Hat host OSes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is not needed for Fedora, RHEL, and CentOS, but it is required when using any other host OS. Having --security-opt apparmor=unconfined used unconditionally is a no go as it'd break podman. The reason this was only added when building for SUSE (as target distro) was because debian and ubuntu condition would fall-through the switch to the suse case (which makes me think that the fall-through was not accidental). Signed-off-by: Fabiano Fidêncio --- tools/osbuilder/rootfs-builder/rootfs.sh | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/tools/osbuilder/rootfs-builder/rootfs.sh b/tools/osbuilder/rootfs-builder/rootfs.sh index 6f627d14ec..dc53653212 100755 --- a/tools/osbuilder/rootfs-builder/rootfs.sh +++ b/tools/osbuilder/rootfs-builder/rootfs.sh @@ -196,10 +196,23 @@ docker_extra_args() args+=" -v ${gentoo_local_portage_dir}:/usr/portage/packages" args+=" --volumes-from ${gentoo_portage_container}" ;; - suse) - # When AppArmor is enabled, mounting inside a container is blocked with docker-default profile. - # See https://github.com/moby/moby/issues/16429 - args+=" --security-opt apparmor=unconfined" + debian | ubuntu | suse) + source /etc/os-release + + case "$ID" in + fedora | centos | rhel) + # Depending on the podman version, we'll face issues when passing + # `--security-opt apparmor=unconfined` on a system where not apparmor is not installed. + # Because of this, let's just avoid adding this option when the host OS comes from Red Hat. + + # A explict check for podman, at least for now, can be avoided. + ;; + *) + # When AppArmor is enabled, mounting inside a container is blocked with docker-default profile. + # See https://github.com/moby/moby/issues/16429 + args+=" --security-opt apparmor=unconfined" + ;; + esac ;; *) ;; From 6005026416da3a140a7a09360109bf394933d5d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Fri, 8 Jan 2021 21:29:51 +0100 Subject: [PATCH 31/31] rootfs: Fix indentation inside a switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While touching this part of the code, let's help my OCD. Signed-off-by: Fabiano Fidêncio --- tools/osbuilder/rootfs-builder/rootfs.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/osbuilder/rootfs-builder/rootfs.sh b/tools/osbuilder/rootfs-builder/rootfs.sh index dc53653212..4d69e89d94 100755 --- a/tools/osbuilder/rootfs-builder/rootfs.sh +++ b/tools/osbuilder/rootfs-builder/rootfs.sh @@ -189,7 +189,7 @@ docker_extra_args() args+=" --cap-add MKNOD" case "$1" in - gentoo) + gentoo) # Required to build glibc args+=" --cap-add SYS_PTRACE" # mount portage volume