Support arm64 builds with multipass #307

jrajahalme · 2021-12-15T03:32:06Z

Add 'multipass' make target for creating Cilium Development VMs with Canonical Multipass. As on now there is no vagrant provider for multipass, so the resulting images do not work with vagrant, but can be locally created and used directly with multipass.

Example use:

NETNEXT=true VM_NAME=netnext VM_CPUS=8 VM_MEMORY=8G make multipass

This will launch a new VM with Ubuntu 20.04 and run the standard Cilium VM provisioning scripts on it, followed by mounting the Cilium repos from the host to the VM (at /home/ubuntu/go/src/github.com/cilium) using NFS. Adjusting NFS exports is done with 'sudo', so the password will be asked for when needed.

See the Makefile for all the supported environment variables.

This has been tested on MacOS on Apple Silicon (arm64), but should also work on the Intel models (amd64). Multipass should also work on Linux, but that has not been tested.

To enable this dependencies have been bumped to versions with arm64 support and virtual box guest additions install has been made conditional.

tklauser · 2021-12-15T10:02:49Z

build-me-please

cilium-ubuntu-4.19.json

Move 'kernel-patches' directory to 'provision/kernel-patches', and copy files to '/tmp/provision/kernel-patches' so that we do not accidentally apply patches not in 'provision/kernel-patches'. Signed-off-by: Jarno Rajahalme <[email protected]>

jrajahalme · 2021-12-15T17:01:26Z

build-me-please

jrajahalme · 2022-01-03T14:32:04Z

build-me-please

Remove kernel patch that has already been applied in bpf-next. Signed-off-by: Jarno Rajahalme <[email protected]>

Use versions that have arm64 images available. Signed-off-by: Jarno Rajahalme <[email protected]>

jrajahalme · 2022-01-03T17:04:40Z

build-me-please

jrajahalme · 2022-01-03T17:35:07Z

build-me-please

Default to "amd64", but support "arm64" as well. Remove install of gcc-multilib, as it is not available for arm64, and apparently not deeded with amd64 either. Signed-off-by: Jarno Rajahalme <[email protected]>

Use $HOME_DIR exclusively instead of $HOME to refer to the target user home directory, as sudo can override $HOME with 'root' depending on the execution context. Signed-off-by: Jarno Rajahalme <[email protected]>

Pass the install user name via $USERNAME, so that same scripts can be used for different default user names (e.g., 'ubuntu' in addition to 'vagrant'). Signed-off-by: Jarno Rajahalme <[email protected]>

Make sure the home directory is owned by the target ${USERNAME} before trying to create the 'go' directory on it. Not sure if needed still, but failed due to this at one point. Signed-off-by: Jarno Rajahalme <[email protected]>

Add export for GOPATH in .profile so that make targets that depend on it being set work as expected (e.g., 'make generate-k8s-api'). Signed-off-by: Jarno Rajahalme <[email protected]>

Add -f to 'ln -s' commands so that they do not fail if executed again. This is helpful when improving the provisioning scripts. Signed-off-by: Jarno Rajahalme <[email protected]>

Append to SSH authorized keys instead of overwriting them to keep any authorized keys set previously. This allows building of local images that have local users public key as an authorized key for local testing. Signed-off-by: Jarno Rajahalme <[email protected]>

Only install VirtualBox guest additions if VirtualBox is detected via "$HOME_DIR/.vbox_version". Signed-off-by: Jarno Rajahalme <[email protected]>

Install docker only if not already installed. This allows building on top of image stages that already install docker before these provisioning steps. Signed-off-by: Jarno Rajahalme <[email protected]>

…the rest Add script provision/provision-kernel.sh for provisioning an old or new kernel, and provision/provision.sh for provisioning everything after the kernel reboot. Add kernel modules needed by kube-proxy 1.19.11 to provision/ubuntu/kernel-next.sh. Signed-off-by: Jarno Rajahalme <[email protected]>

jrajahalme · 2022-01-03T18:27:56Z

build-me-please

Changes made :-)

tklauser · 2022-01-07T09:15:41Z

Looks like this needs another rebase after #309 being merged.

Use "sudo reboot" also for old kernel version install like for net-next build so that we can scan for it the same way from the console output. Signed-off-by: Jarno Rajahalme <[email protected]>

Add 'multipass' target for creating Cilium Development VMs with Canonical Multipass. Example use: NETNEXT=true VM_NAME=netnext VM_CPUS=8 VM_MEMORY=8G make multipass This will launch a new VM with Ubuntu 20.04 and run the standard Cilium VM provisioning scripts on it, followed by mounting the Cilium repos from the host to the VM (at /home/ubuntu/go/src/github.com/cilium) using NFS. See the Makefile for all the supported environment variables. This has been tested on MacOS on Apple Silicon (arm64), but should also work on the Intel models (amd64):. Multipass should also work on Linux, but that has not been tested. Install the package needed (nfs-common) for mounting NFS shares to the dev VM. Signed-off-by: Jarno Rajahalme <[email protected]>

tklauser · 2022-01-13T21:22:37Z

build-me-please

gentoo-root · 2023-05-26T16:34:41Z

provision/ubuntu/kernel-next.sh

-git am /tmp/provision/kernel-patches/*.patch
+if [ -d /tmp/provision/kernel-patches ]; then
+	for patch in /tmp/provision/kernel-patches/*.patch; do
+		git am $patch


This will still fail if the directory exists, but it's empty.

Besides that, it kind of doesn't make sense to me when you keep this part, but remove copying the directory from cilium-ubuntu-next.json. If the directory isn't even copied, what's the point of this loop? If you want to keep the boilerplate for potential future patches, then it would make sense to keep the lines in cilium-ubuntu-next.json, just keep the empty directory for now.

gentoo-root · 2023-05-26T16:38:47Z

cilium-ubuntu-4.19.json

-        "CILIUM_BRANCH={{ user `CILIUM_BRANCH` }}",
-        "NAME_PREFIX={{ user `NAME_PREFIX` }}"
+          "VM_ARCH=amd64",
+          "ENV_FILEPATH=/tmp/env.bash",


Nit: unintentional indentation change for three existing lines? It seems like most of this file is indented with two spaces.

Same for the files below.

gentoo-root · 2023-05-26T16:42:07Z

provision/env.bash

-export HOME_DIR=/home/vagrant
-export HOME=/home/vagrant
-export GOPATH="${HOME}/go"
+export HOME_DIR=/home/${USERNAME}


Not sure if $USERNAME can contain whitespace, but it would be safer to use double quotes around it. Same for $HOME_DIR in the next file below.

gentoo-root · 2023-05-26T16:45:13Z

provision/ubuntu/install.sh

 ISO="VBoxGuestAdditions_$VER.iso";

 # Validate that custom GuestAdditions are needed
 if [[ -n "${GUESTADDITIONS}" ]]; then
-    cd $HOME_DIR
+    cd ${HOME_DIR}


What's the purpose of this change? If you touch this, I suggest adding double quotes around as the general good practice to avoid surprises if the variable ever contains whitespace. Curly braces don't add any extra safety.

gentoo-root · 2023-05-26T16:46:51Z

provision/golang.sh

@@ -4,6 +4,7 @@ source "${ENV_FILEPATH}"

 set -e

+sudo -E chown ${USERNAME}:${USERNAME} ${HOME_DIR}


Why is -E needed? Doesn't look necessary to me, chown shouldn't use any env vars.

gentoo-root · 2023-05-26T16:49:18Z

provision/golang.sh

@@ -12,3 +12,5 @@ sudo -u ${USERNAME} -E bash -c "mkdir -p ${GOPATH} && \
 curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sudo sh -s -- -b ${GOPATH}/bin/ v1.42.1

 sudo -E ln -s "${GOPATH}/bin/"* /usr/bin
+
+if ! grep "export GOPATH=" ${HOME_DIR}/.profile ; then echo "export GOPATH=${GOPATH}" >>${HOME_DIR}/.profile; fi


grep ... || echo ... >> ... would be more compact if you want a one-liner.

grep -q is also a good idea to suppress unneeded output.

gentoo-root · 2023-05-26T16:49:51Z

provision/golang.sh

@@ -11,6 +11,6 @@ sudo -u ${USERNAME} -E bash -c "mkdir -p ${GOPATH} && \

 curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sudo sh -s -- -b ${GOPATH}/bin/ v1.42.1

-sudo -E ln -s "${GOPATH}/bin/"* /usr/bin
+sudo -E ln -sf "${GOPATH}/bin/"* /usr/bin


-E doesn't seem necessary here as well.

gentoo-root · 2023-05-26T16:51:57Z

provision/vagrant.sh

@@ -9,9 +9,11 @@ source "${ENV_FILEPATH}"
 pubkey_url="https://raw.githubusercontent.com/hashicorp/vagrant/master/keys/vagrant.pub";
 mkdir -p "${HOME_DIR}/.ssh";
 if command -v wget >/dev/null 2>&1; then
-    wget --no-check-certificate "${pubkey_url}" -O "${HOME_DIR}/.ssh/authorized_keys";
+    wget --no-check-certificate "${pubkey_url}" -O vagrant.pub;
+    cat vagrant.pub >> "${HOME_DIR}/.ssh/authorized_keys";


A temporary file is not needed, wget supports -O - to write output to stdout, you can use the same pipeline as with curl:

wget --no-check-certificate "${pubkey_url}" -O - >> "${HOME_DIR}/.ssh/authorized_keys"

gentoo-root · 2023-05-26T17:00:29Z

Makefile

+
+# share parent if it is named "cilium", so that it can be correctly mounted
+# as ~/go/src/github.com/cilium"
+ifeq (cilium,$(notdir $(abspath $(ROOT_DIR)/..)))


I'm not sure I get the logic here...

If the parent directory is called "cilium", mount the whole github.com, but if it's not "cilium", mount only github.com/cilium... what's the idea of this?

gentoo-root · 2023-05-26T17:11:53Z

Makefile

+nfs-export: HOST_MASK ?= 255.255.255.0
+nfs-export: ETC_EXPORTS ?= $(realpath $(SHARE_SOURCE)) -mapall=$(shell whoami) -alldirs -network $(HOST_NETWORK) -mask $(HOST_MASK)
+nfs-export:
+	@if ! grep "$(ETC_EXPORTS)" /etc/exports; then \


Not sure about macOS, but at least my Linux supports /etc/exports.d, where you can put multiple files. It has the advantage that the file can be owned by the corresponding user or group (therefore, sudo is not needed), and different applications have their own files, not competing over the single /etc/exports and not needing to invent ingenious ways to mark ownership of the lines of that file.

Vagrant that doesn't support /etc/exports.d is my personal pain. This script is written by us, so we can make it good from the beginning. Could you change it to a dedicated file under /etc/exports.d (e.g., /etc/exports.d/cilium-multipass, or whatever name you prefer) and use sudo only if a regular write as the current user failed?

Another note: grep should be grep -Fx to match the whole lines and not try to treat it as a regexp.

jrajahalme requested a review from a team as a code owner December 15, 2021 03:32

jrajahalme requested review from tklauser and removed request for a team December 15, 2021 03:32

tklauser added the needs-rebase label Dec 15, 2021

tklauser previously requested changes Dec 15, 2021

View reviewed changes

cilium-ubuntu-4.19.json Outdated Show resolved Hide resolved

kernel-patches: Move to provision/

879c49a

Move 'kernel-patches' directory to 'provision/kernel-patches', and copy files to '/tmp/provision/kernel-patches' so that we do not accidentally apply patches not in 'provision/kernel-patches'. Signed-off-by: Jarno Rajahalme <[email protected]>

jrajahalme force-pushed the pr/jrajahalme/arm64-multipass branch from d6acbaf to 35f91ff Compare December 15, 2021 17:00

jrajahalme requested a review from tklauser December 15, 2021 17:01

tklauser removed the needs-rebase label Dec 15, 2021

provision: Remove already applied kernel patch

a308e75

Remove kernel patch that has already been applied in bpf-next. Signed-off-by: Jarno Rajahalme <[email protected]>

jrajahalme force-pushed the pr/jrajahalme/arm64-multipass branch 2 times, most recently from 1f4ce92 to c99e02f Compare January 3, 2022 16:29

build: Bump versions for arm64

3753f78

Use versions that have arm64 images available. Signed-off-by: Jarno Rajahalme <[email protected]>

jrajahalme force-pushed the pr/jrajahalme/arm64-multipass branch from c99e02f to 4a66df8 Compare January 3, 2022 16:45

jrajahalme added 10 commits January 3, 2022 10:23

provision: Generalize CPU architecture

3b2ef56

Default to "amd64", but support "arm64" as well. Remove install of gcc-multilib, as it is not available for arm64, and apparently not deeded with amd64 either. Signed-off-by: Jarno Rajahalme <[email protected]>

provision: Do not use $HOME

1923009

Use $HOME_DIR exclusively instead of $HOME to refer to the target user home directory, as sudo can override $HOME with 'root' depending on the execution context. Signed-off-by: Jarno Rajahalme <[email protected]>

provision: Generalize user name

d7bcfb9

Pass the install user name via $USERNAME, so that same scripts can be used for different default user names (e.g., 'ubuntu' in addition to 'vagrant'). Signed-off-by: Jarno Rajahalme <[email protected]>

provision: Export GOPATH

48bda45

Add export for GOPATH in .profile so that make targets that depend on it being set work as expected (e.g., 'make generate-k8s-api'). Signed-off-by: Jarno Rajahalme <[email protected]>

provision: Force links

957b60b

Add -f to 'ln -s' commands so that they do not fail if executed again. This is helpful when improving the provisioning scripts. Signed-off-by: Jarno Rajahalme <[email protected]>

provision: Install VirtualBox guest additions conditionally

73f2968

Only install VirtualBox guest additions if VirtualBox is detected via "$HOME_DIR/.vbox_version". Signed-off-by: Jarno Rajahalme <[email protected]>

provision: Only install docker if not already installed

52087af

Install docker only if not already installed. This allows building on top of image stages that already install docker before these provisioning steps. Signed-off-by: Jarno Rajahalme <[email protected]>

jrajahalme force-pushed the pr/jrajahalme/arm64-multipass branch from 5087f1e to cdaedae Compare January 3, 2022 18:27

tklauser added the needs-rebase label Jan 7, 2022

jrajahalme added 2 commits January 13, 2022 18:06

ubuntu: Use "sudo reboot" for both kernel installs

8762b90

Use "sudo reboot" also for old kernel version install like for net-next build so that we can scan for it the same way from the console output. Signed-off-by: Jarno Rajahalme <[email protected]>

jrajahalme force-pushed the pr/jrajahalme/arm64-multipass branch from cdaedae to 4f9f922 Compare January 13, 2022 16:57

tklauser removed the needs-rebase label Jan 13, 2022

tklauser approved these changes Jan 14, 2022

View reviewed changes

tklauser added the needs-rebase label Jan 24, 2022

nbusseneau requested review from a team as code owners May 26, 2023 15:43

nbusseneau requested review from gentoo-root and lmb and removed request for a team May 26, 2023 15:43

gentoo-root reviewed May 26, 2023

View reviewed changes

lmb approved these changes May 30, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support arm64 builds with multipass #307

Support arm64 builds with multipass #307

jrajahalme commented Dec 15, 2021

tklauser commented Dec 15, 2021

jrajahalme commented Dec 15, 2021

jrajahalme commented Jan 3, 2022

jrajahalme commented Jan 3, 2022

jrajahalme commented Jan 3, 2022

jrajahalme commented Jan 3, 2022

tklauser commented Jan 7, 2022 •

edited

Loading

tklauser commented Jan 13, 2022

gentoo-root May 26, 2023

gentoo-root May 26, 2023

gentoo-root May 26, 2023

gentoo-root May 26, 2023

gentoo-root May 26, 2023

gentoo-root May 26, 2023

gentoo-root May 26, 2023

gentoo-root May 26, 2023

gentoo-root May 26, 2023

gentoo-root May 26, 2023

		@@ -4,6 +4,7 @@ source "${ENV_FILEPATH}"

		set -e

		sudo -E chown ${USERNAME}:${USERNAME} ${HOME_DIR}

Support arm64 builds with multipass #307

Are you sure you want to change the base?

Support arm64 builds with multipass #307

Conversation

jrajahalme commented Dec 15, 2021

tklauser commented Dec 15, 2021

jrajahalme commented Dec 15, 2021

jrajahalme commented Jan 3, 2022

jrajahalme commented Jan 3, 2022

jrajahalme commented Jan 3, 2022

jrajahalme commented Jan 3, 2022

tklauser commented Jan 7, 2022 • edited Loading

tklauser commented Jan 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tklauser commented Jan 7, 2022 •

edited

Loading