From ff929fc0810278a70853b70fb0572b42fdb246e7 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Mon, 6 Dec 2021 08:42:07 -0600 Subject: [PATCH 01/10] snap: read initrd and image distros from version.yaml Build initrd or image rootfs using the distro name specified in the versions.yaml fixes #3208 Signed-off-by: Julio Montes --- snap/snapcraft.yaml | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index 14d0401a72..bd75f92056 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -118,18 +118,19 @@ parts: export AGENT_INIT=yes export USE_DOCKER=1 export DEBUG=1 - case "$(uname -m)" in - aarch64) - sudo -E PATH=$PATH make initrd DISTRO=alpine - ;; - ppc64le|s390x) - # Cannot use alpine on ppc64le/s390x because it would require a musl agent - sudo -E PATH=$PATH make initrd DISTRO=ubuntu - ;; + arch="$(uname -m)" + initrd_distro=$(${yq} r -X ${kata_dir}/versions.yaml assets.initrd.architecture.${arch}.name) + image_distro=$(${yq} r -X ${kata_dir}/versions.yaml assets.image.architecture.${arch}.name) + case "$arch" in x86_64) # In some build systems it's impossible to build a rootfs image, try with the initrd image - sudo -E PATH=$PATH make image DISTRO=clearlinux || sudo -E PATH=$PATH make initrd DISTRO=alpine + sudo -E PATH=$PATH make image DISTRO=${image_distro} || sudo -E PATH=$PATH make initrd DISTRO=${initrd_distro} ;; + + aarch64|ppc64le|s390x) + sudo -E PATH=$PATH make initrd DISTRO=${initrd_distro} + ;; + *) echo "unsupported architecture: $(uname -m)"; exit 1;; esac From d7cc952cb1da924af6acd23bfab111b0ab628d5f Mon Sep 17 00:00:00 2001 From: Jakob Naucke Date: Mon, 6 Dec 2021 17:13:38 +0100 Subject: [PATCH 02/10] versions: Use Ubuntu initrd for non-musl archs ppc64le & s390x have no (well supported) musl target for Rust, therefore, the agent must use glibc and cannot use Alpine. Specify Ubuntu as the distribution to be used for initrd. Fixes: #3212 Signed-off-by: Jakob Naucke --- versions.yaml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/versions.yaml b/versions.yaml index 3dbc262862..fa403c7e79 100644 --- a/versions.yaml +++ b/versions.yaml @@ -140,12 +140,14 @@ assets: aarch64: name: &default-initrd-name "alpine" version: &default-initrd-version "3.13.5" + # Do not use Alpine on ppc64le & s390x, the agent cannot use musl because + # there is no such Rust target ppc64le: - name: *default-initrd-name - version: *default-initrd-version + name: &glibc-initrd-name "ubuntu" + version: &glibc-initrd-version "20.04" s390x: - name: *default-initrd-name - version: *default-initrd-version + name: *glibc-initrd-name + version: *glibc-initrd-version x86_64: name: *default-initrd-name version: *default-initrd-version From 33f343ee08b868549596a23821b0f29950bd54df Mon Sep 17 00:00:00 2001 From: bin Date: Tue, 7 Dec 2021 15:59:18 +0800 Subject: [PATCH 03/10] runtime: correct span name for stopSandbox function Normally the span name should be the same as function name, so chagne `StopVM` to `stopSandbox`. Fixes: #3217 Signed-off-by: bin --- src/runtime/virtcontainers/kata_agent.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/virtcontainers/kata_agent.go b/src/runtime/virtcontainers/kata_agent.go index 6cecbd4523..dce79ddc7d 100644 --- a/src/runtime/virtcontainers/kata_agent.go +++ b/src/runtime/virtcontainers/kata_agent.go @@ -908,7 +908,7 @@ func setupStorages(ctx context.Context, sandbox *Sandbox) []*grpc.Storage { } func (k *kataAgent) stopSandbox(ctx context.Context, sandbox *Sandbox) error { - span, ctx := katatrace.Trace(ctx, k.Logger(), "StopVM", kataAgentTracingTags) + span, ctx := katatrace.Trace(ctx, k.Logger(), "stopSandbox", kataAgentTracingTags) defer span.End() req := &grpc.DestroySandboxRequest{} From 40bd34caaf82a3c17204ce15240a0558763555db Mon Sep 17 00:00:00 2001 From: bin Date: Tue, 7 Dec 2021 16:06:26 +0800 Subject: [PATCH 04/10] runtime: only call stopVirtiofsd when shared_fs is virtio-fs If shared_fs is set to virtio-9p, the virtiofsd is not started, so there is no need to stop it. Fixes: #3219 Signed-off-by: bin --- src/runtime/virtcontainers/qemu.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index 9b78fb24ee..b5d6d4904f 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -984,8 +984,10 @@ func (q *qemu) StopVM(ctx context.Context, waitOnly bool) error { } } - if err := q.stopVirtiofsd(ctx); err != nil { - return err + if q.config.SharedFS == config.VirtioFS { + if err := q.stopVirtiofsd(ctx); err != nil { + return err + } } return nil From b92babf91b995faeb20353c87c635e104444e75a Mon Sep 17 00:00:00 2001 From: bin Date: Tue, 7 Dec 2021 19:31:03 +0800 Subject: [PATCH 05/10] runtime/template: Handling new attributes for hypervisor config Some new attributes are added to hypervisor config: - VMStorePath - RunStorePath - SharedPath These attributes should be handled in two places: - reset when check the new hypervisor's config is suitable to the base config. - copy from new hypervisor's config when create new VM Fixes: #3193 Signed-off-by: bin --- src/runtime/virtcontainers/factory/factory.go | 17 ++++++++++------- .../virtcontainers/factory/template/template.go | 4 ++++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/runtime/virtcontainers/factory/factory.go b/src/runtime/virtcontainers/factory/factory.go index 4c7e1e8603..7461a5158e 100644 --- a/src/runtime/virtcontainers/factory/factory.go +++ b/src/runtime/virtcontainers/factory/factory.go @@ -113,20 +113,23 @@ func resetHypervisorConfig(config *vc.VMConfig) { config.HypervisorConfig.BootFromTemplate = false config.HypervisorConfig.MemoryPath = "" config.HypervisorConfig.DevicesStatePath = "" + config.HypervisorConfig.SharedPath = "" + config.HypervisorConfig.VMStorePath = "" + config.HypervisorConfig.RunStorePath = "" } // It's important that baseConfig and newConfig are passed by value! -func checkVMConfig(config1, config2 vc.VMConfig) error { - if config1.HypervisorType != config2.HypervisorType { - return fmt.Errorf("hypervisor type does not match: %s vs. %s", config1.HypervisorType, config2.HypervisorType) +func checkVMConfig(baseConfig, newConfig vc.VMConfig) error { + if baseConfig.HypervisorType != newConfig.HypervisorType { + return fmt.Errorf("hypervisor type does not match: %s vs. %s", baseConfig.HypervisorType, newConfig.HypervisorType) } // check hypervisor config details - resetHypervisorConfig(&config1) - resetHypervisorConfig(&config2) + resetHypervisorConfig(&baseConfig) + resetHypervisorConfig(&newConfig) - if !utils.DeepCompare(config1, config2) { - return fmt.Errorf("hypervisor config does not match, base: %+v. new: %+v", config1, config2) + if !utils.DeepCompare(baseConfig, newConfig) { + return fmt.Errorf("hypervisor config does not match, base: %+v. new: %+v", baseConfig, newConfig) } return nil diff --git a/src/runtime/virtcontainers/factory/template/template.go b/src/runtime/virtcontainers/factory/template/template.go index 22f848cd74..0b3e96dae8 100644 --- a/src/runtime/virtcontainers/factory/template/template.go +++ b/src/runtime/virtcontainers/factory/template/template.go @@ -163,6 +163,10 @@ func (t *template) createFromTemplateVM(ctx context.Context, c vc.VMConfig) (*vc config.HypervisorConfig.BootFromTemplate = true config.HypervisorConfig.MemoryPath = t.statePath + "/memory" config.HypervisorConfig.DevicesStatePath = t.statePath + "/state" + config.HypervisorConfig.SharedPath = c.HypervisorConfig.SharedPath + config.HypervisorConfig.VMStorePath = c.HypervisorConfig.VMStorePath + config.HypervisorConfig.RunStorePath = c.HypervisorConfig.RunStorePath + return vc.NewVM(ctx, config) } From 6b3e4c212c7391ddc95cddfe8ef4638a900ed498 Mon Sep 17 00:00:00 2001 From: zhanghj Date: Wed, 8 Dec 2021 17:25:08 +0800 Subject: [PATCH 06/10] image_build: add help info for '-f' option and 'BLOCK_SIZE' env. The help information of '-f' option is missing, and same issue with 'BLOCK_SIZE' env variables, fix it in usage() function. Fixes: #3231 Signed-off-by: zhanghj --- tools/osbuilder/image-builder/image_builder.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/osbuilder/image-builder/image_builder.sh b/tools/osbuilder/image-builder/image_builder.sh index a1cf97eae4..bfea8f659d 100755 --- a/tools/osbuilder/image-builder/image_builder.sh +++ b/tools/osbuilder/image-builder/image_builder.sh @@ -79,14 +79,15 @@ Usage: ${script_name} [options] Options: -h Show this help - -o path to generate image file ENV: IMAGE - -r Free space of the root partition in MB ENV: ROOT_FREE_SPACE + -o Path to generate image file. ENV: IMAGE + -r Free space of the root partition in MB. ENV: ROOT_FREE_SPACE + -f Filesystem type to use, only xfs and ext4 are supported. ENV: FS_TYPE Extra environment variables: AGENT_BIN: Use it to change the expected agent binary name AGENT_INIT: Use kata agent as init process + BLOCK_SIZE: Use to specify the size of blocks in bytes. DEFAULT: 4096 IMAGE_REGISTRY: Hostname for the image registry used to pull down the rootfs build image. - FS_TYPE: Filesystem type to use. Only xfs and ext4 are supported. NSDAX_BIN: Use to specify path to pre-compiled 'nsdax' tool. USE_DOCKER: If set will build image in a Docker Container (requries docker) DEFAULT: not set From dfd0732ff99e2cc75acfcef86eb83c05e406d903 Mon Sep 17 00:00:00 2001 From: Jakob Naucke Date: Tue, 7 Dec 2021 12:00:54 +0100 Subject: [PATCH 07/10] osbuilder: Revert to using apk.static for Alpine #2399 partially reverted #418, missing on returning to bootstrapping a rootfs with `apk.static` instead of copying the entire root, which can result in drastically larger (more than 10x) images. Revert this as well (requires some updates to URL building). Fixes: #3216 Signed-off-by: Jakob Naucke --- tools/osbuilder/rootfs-builder/alpine/config.sh | 2 +- .../rootfs-builder/alpine/rootfs_lib.sh | 17 +++++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/tools/osbuilder/rootfs-builder/alpine/config.sh b/tools/osbuilder/rootfs-builder/alpine/config.sh index d07f70b03a..4d6ed2265a 100644 --- a/tools/osbuilder/rootfs-builder/alpine/config.sh +++ b/tools/osbuilder/rootfs-builder/alpine/config.sh @@ -11,7 +11,7 @@ BASE_PACKAGES="alpine-base" # Alpine mirror to use # See a list of mirrors at http://nl.alpinelinux.org/alpine/MIRRORS.txt -MIRROR=http://dl-5.alpinelinux.org/alpine +MIRROR=https://dl-5.alpinelinux.org/alpine PACKAGES="" diff --git a/tools/osbuilder/rootfs-builder/alpine/rootfs_lib.sh b/tools/osbuilder/rootfs-builder/alpine/rootfs_lib.sh index d41cbd5766..f2456e7c94 100644 --- a/tools/osbuilder/rootfs-builder/alpine/rootfs_lib.sh +++ b/tools/osbuilder/rootfs-builder/alpine/rootfs_lib.sh @@ -9,6 +9,8 @@ # # - Optional environment variables # +# EXTRA_PKGS: Variable to add extra PKGS provided by the user +# # BIN_AGENT: Name of the Kata-Agent binary # # Any other configuration variable for a specific distro must be added @@ -22,13 +24,20 @@ build_rootfs() { # Mandatory local ROOTFS_DIR=$1 + # Add extra packages to the rootfs when specified + local EXTRA_PKGS=${EXTRA_PKGS:-} + # Populate ROOTFS_DIR check_root mkdir -p "${ROOTFS_DIR}" - rm -rf ${ROOTFS_DIR}/var/tmp - cp -a -r -f /bin /etc /lib /sbin /usr /var ${ROOTFS_DIR} - mkdir -p ${ROOTFS_DIR}{/root,/proc,/dev,/home,/media,/mnt,/opt,/run,/srv,/sys,/tmp} + /sbin/apk.static \ + -X ${MIRROR}/v${OS_VERSION}/main \ + -U \ + --allow-untrusted \ + --root ${ROOTFS_DIR} \ + --initdb add ${BASE_PACKAGES} ${EXTRA_PKGS} ${PACKAGES} - echo "${MIRROR}/${OS_VERSION}/main" > ${ROOTFS_DIR}/etc/apk/repositories + mkdir -p ${ROOTFS_DIR}{/root,/etc/apk,/proc} + echo "${MIRROR}/v${OS_VERSION}/main" > ${ROOTFS_DIR}/etc/apk/repositories } From 2204ecac39bb049ccae0e24589a93b9d717bf5da Mon Sep 17 00:00:00 2001 From: Jakob Naucke Date: Wed, 8 Dec 2021 13:40:48 +0100 Subject: [PATCH 08/10] versions: Upgrade Alpine, using minor version - Upgrade Alpine guest rootfs to 3.15 - Specify a minor version rather than patch level as the Alpine repositories use that. Signed-off-by: Jakob Naucke --- tools/osbuilder/rootfs-builder/alpine/Dockerfile.in | 2 +- tools/osbuilder/rootfs-builder/alpine/config.sh | 2 +- versions.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/osbuilder/rootfs-builder/alpine/Dockerfile.in b/tools/osbuilder/rootfs-builder/alpine/Dockerfile.in index 4da311aa06..a7b8737a11 100644 --- a/tools/osbuilder/rootfs-builder/alpine/Dockerfile.in +++ b/tools/osbuilder/rootfs-builder/alpine/Dockerfile.in @@ -4,7 +4,7 @@ # SPDX-License-Identifier: Apache-2.0 ARG IMAGE_REGISTRY=docker.io -FROM ${IMAGE_REGISTRY}/alpine:3.13.5 +FROM ${IMAGE_REGISTRY}/alpine:3.15 RUN apk update && apk add \ apk-tools-static \ diff --git a/tools/osbuilder/rootfs-builder/alpine/config.sh b/tools/osbuilder/rootfs-builder/alpine/config.sh index 4d6ed2265a..105ecd8fbc 100644 --- a/tools/osbuilder/rootfs-builder/alpine/config.sh +++ b/tools/osbuilder/rootfs-builder/alpine/config.sh @@ -5,7 +5,7 @@ OS_NAME="Alpine" -OS_VERSION=${OS_VERSION:-latest-stable} +OS_VERSION=${OS_VERSION:-3.15} BASE_PACKAGES="alpine-base" diff --git a/versions.yaml b/versions.yaml index fa403c7e79..e1bc5f3c5c 100644 --- a/versions.yaml +++ b/versions.yaml @@ -139,7 +139,7 @@ assets: architecture: aarch64: name: &default-initrd-name "alpine" - version: &default-initrd-version "3.13.5" + version: &default-initrd-version "3.15" # Do not use Alpine on ppc64le & s390x, the agent cannot use musl because # there is no such Rust target ppc64le: From f3103696980f13ca505686e028381b5737b98ff7 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Wed, 8 Dec 2021 14:23:23 -0600 Subject: [PATCH 09/10] docs: fix check-markdown test Unit-Test-Advice.md was moved to kata-containers repo but URLs pointing to that document were not updated. This patch updates these URLs. Depends-on: github.com/kata-containers/tests#4273 fixes #3240 Signed-off-by: Julio Montes --- docs/code-pr-advice.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/code-pr-advice.md b/docs/code-pr-advice.md index 78413ccf23..ac0ea76afb 100644 --- a/docs/code-pr-advice.md +++ b/docs/code-pr-advice.md @@ -165,7 +165,7 @@ Ensure any new trace spans added to the code are completed. Where possible, code changes should be accompanied by unit tests. Consider using the standard -[table-based approach](https://github.com/kata-containers/tests/blob/main/Unit-Test-Advice.md) +[table-based approach](Unit-Test-Advice.md) as it encourages you to make functions small and simple, and also allows you to think about what types of value to test. From 2ebaaac73d623c103e7869e68150dfb2e5ee1be6 Mon Sep 17 00:00:00 2001 From: Snir Sheriber Date: Wed, 8 Dec 2021 16:45:51 +0200 Subject: [PATCH 10/10] osbuilder: be runtime consistent also with podman build Use the same runtime used for podman run also for the podman build cmd Additionally remove "docker" from the docker_run_args variable Fixes: #3239 Signed-off-by: Snir Sheriber --- .../osbuilder/image-builder/image_builder.sh | 9 ++++--- tools/osbuilder/rootfs-builder/rootfs.sh | 27 ++++++++++--------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/tools/osbuilder/image-builder/image_builder.sh b/tools/osbuilder/image-builder/image_builder.sh index a1cf97eae4..36c8a2211e 100755 --- a/tools/osbuilder/image-builder/image_builder.sh +++ b/tools/osbuilder/image-builder/image_builder.sh @@ -137,13 +137,16 @@ build_with_container() { image_dir=$(readlink -f "$(dirname "${image}")") image_name=$(basename "${image}") - REGISTRY_ARG="" + engine_build_args="" if [ -n "${IMAGE_REGISTRY}" ]; then - REGISTRY_ARG="--build-arg IMAGE_REGISTRY=${IMAGE_REGISTRY}" + engine_build_args+=" --build-arg IMAGE_REGISTRY=${IMAGE_REGISTRY}" + fi + if [ -n "${USE_PODMAN}" ]; then + engine_build_args+=" --runtime ${DOCKER_RUNTIME}" fi "${container_engine}" build \ - ${REGISTRY_ARG} \ + ${engine_build_args} \ --build-arg http_proxy="${http_proxy}" \ --build-arg https_proxy="${https_proxy}" \ -t "${container_image_name}" "${script_dir}" diff --git a/tools/osbuilder/rootfs-builder/rootfs.sh b/tools/osbuilder/rootfs-builder/rootfs.sh index 4f0dd45c59..1697e0a82d 100755 --- a/tools/osbuilder/rootfs-builder/rootfs.sh +++ b/tools/osbuilder/rootfs-builder/rootfs.sh @@ -353,23 +353,24 @@ build_rootfs_distro() info "build directly" build_rootfs ${ROOTFS_DIR} else + engine_build_args="" if [ -n "${USE_DOCKER}" ]; then container_engine="docker" elif [ -n "${USE_PODMAN}" ]; then container_engine="podman" + engine_build_args+=" --runtime ${DOCKER_RUNTIME}" fi image_name="${distro}-rootfs-osbuilder" - REGISTRY_ARG="" if [ -n "${IMAGE_REGISTRY}" ]; then - REGISTRY_ARG="--build-arg IMAGE_REGISTRY=${IMAGE_REGISTRY}" + engine_build_args+=" --build-arg IMAGE_REGISTRY=${IMAGE_REGISTRY}" fi # setup to install rust here generate_dockerfile "${distro_config_dir}" "$container_engine" build \ - ${REGISTRY_ARG} \ + ${engine_build_args} \ --build-arg http_proxy="${http_proxy}" \ --build-arg https_proxy="${https_proxy}" \ -t "${image_name}" "${distro_config_dir}" @@ -377,21 +378,21 @@ build_rootfs_distro() # fake mapping if KERNEL_MODULES_DIR is unset kernel_mod_dir=${KERNEL_MODULES_DIR:-${ROOTFS_DIR}} - docker_run_args="" - docker_run_args+=" --rm" + engine_run_args="" + engine_run_args+=" --rm" # apt sync scans all possible fds in order to close them, incredibly slow on VMs - docker_run_args+=" --ulimit nofile=262144:262144" - docker_run_args+=" --runtime ${DOCKER_RUNTIME}" + engine_run_args+=" --ulimit nofile=262144:262144" + engine_run_args+=" --runtime ${DOCKER_RUNTIME}" if [ -z "${AGENT_SOURCE_BIN}" ] ; then - docker_run_args+=" -v ${GOPATH_LOCAL}:${GOPATH_LOCAL} --env GOPATH=${GOPATH_LOCAL}" + engine_run_args+=" -v ${GOPATH_LOCAL}:${GOPATH_LOCAL} --env GOPATH=${GOPATH_LOCAL}" else - docker_run_args+=" --env AGENT_SOURCE_BIN=${AGENT_SOURCE_BIN}" - docker_run_args+=" -v ${AGENT_SOURCE_BIN}:${AGENT_SOURCE_BIN}" - docker_run_args+=" -v ${GOPATH_LOCAL}:${GOPATH_LOCAL} --env GOPATH=${GOPATH_LOCAL}" + engine_run_args+=" --env AGENT_SOURCE_BIN=${AGENT_SOURCE_BIN}" + engine_run_args+=" -v ${AGENT_SOURCE_BIN}:${AGENT_SOURCE_BIN}" + engine_run_args+=" -v ${GOPATH_LOCAL}:${GOPATH_LOCAL} --env GOPATH=${GOPATH_LOCAL}" fi - docker_run_args+=" $(docker_extra_args $distro)" + engine_run_args+=" $(docker_extra_args $distro)" # Relabel volumes so SELinux allows access (see docker-run(1)) if command -v selinuxenabled > /dev/null && selinuxenabled ; then @@ -432,7 +433,7 @@ build_rootfs_distro() -v "${ROOTFS_DIR}":"/rootfs" \ -v "${script_dir}/../scripts":"/scripts" \ -v "${kernel_mod_dir}":"${kernel_mod_dir}" \ - $docker_run_args \ + $engine_run_args \ ${image_name} \ bash /kata-containers/tools/osbuilder/rootfs-builder/rootfs.sh "${distro}"