On 5/18/26 1:28 PM, Eelco Chaudron via dev wrote:
> Replace Cirrus CI with GitHub Actions running FreeBSD inside QEMU/KVM
> VMs on ubuntu-24.04 runners, testing with both gcc and clang.  VM images
> are pre-built from official FreeBSD BASIC-CLOUDINIT releases, provisioned
> via nuageinit, and cached between runs to keep build times short.
> 
> FreeBSD versions are updated from 13.5/14.3 to 14.4/15.0, which are the
> current supported releases and the first to include the nuageinit support
> required for automated VM provisioning.

Thanks, Eelco for working on this!

It looks fine conceptually, but I think a lot can be simplified/improved.

One general thought here is that while it was OK to waste a little bit
of resources while gaining a bit of coverage by running the full matrix
of 2 FreeBSD releases and two compilers, it doesn't sound good when we're
sharing capacity with the rest of the workflows in GitHub Actions.
So, I'd suggest we just pick one latest release and one compiler - clang,
which is native to FreeBSD.  This will save us a lot of time on CI runs
without significantly degrading the coverage as most of what we need to
cover is system headers and other things like that that are not really
compiler-dependent.

Additionally, this allows to combine the cache and the build workflows
into one, simplifying the operations.

Some more comments below.

> 
> Assisted-by: Claude Sonnet 4.6, OpenCode
> Signed-off-by: Eelco Chaudron <[email protected]>
> ---
>  .ci/freebsd-build.sh          | 141 ++++++++++++++++
>  .ci/freebsd-prepare-image.sh  | 154 ++++++++++++++++++
>  .ci/freebsd-vm.sh             | 291 ++++++++++++++++++++++++++++++++++
>  .github/workflows/freebsd.yml | 160 +++++++++++++++++++
>  Makefile.am                   |   4 +
>  README.rst                    |   4 +-
>  utilities/checkpatch_dict.txt |   2 +
>  7 files changed, 754 insertions(+), 2 deletions(-)
>  create mode 100755 .ci/freebsd-build.sh
>  create mode 100755 .ci/freebsd-prepare-image.sh
>  create mode 100755 .ci/freebsd-vm.sh
>  create mode 100755 .github/workflows/freebsd.yml
> 
> diff --git a/.ci/freebsd-build.sh b/.ci/freebsd-build.sh
> new file mode 100755
> index 000000000..f1cb41650
> --- /dev/null
> +++ b/.ci/freebsd-build.sh
> @@ -0,0 +1,141 @@
> +#!/bin/bash
> +# Builds and tests Open vSwitch inside a FreeBSD QEMU VM.
> +#
> +# Steps mirror the original Cirrus CI configuration:
> +#   configure -> ./boot.sh && ./configure CC=<compiler> ...
> +#   build     -> gmake -j8
> +#   check     -> gmake -j8 check TESTSUITEFLAGS=-j8 RECHECK=yes
> +#
> +# All CI dependencies are pre-installed in the cached image by nuageinit
> +# during the prepare phase (see freebsd-prepare-image.sh).
> +#
> +# Required environment variables:
> +#   FREEBSD_VER   - FreeBSD version string, e.g. "14.4" or "15.0"
> +#   COMPILER      - compiler to use: "gcc" or "clang"
> +#
> +# The cached image freebsd-<FREEBSD_VER>.qcow2 must exist in the current
> +# directory before this script is called (restored from actions/cache by the
> +# workflow).

This comment goes into technical details that it probably shouldn't go into.
E.g. the dependency instalation via nuageinit is not relevant.  The required
env variables are also right below here with the help messages on them.

> +
> +set -o errexit
> +set -o xtrace
> +
> +FREEBSD_VER="${FREEBSD_VER:?Must set FREEBSD_VER (e.g. 14.4)}"
> +COMPILER="${COMPILER:?Must set COMPILER (gcc or clang)}"
> +
> +BASE_IMG="freebsd-${FREEBSD_VER}.qcow2"
> +RUN_IMG="freebsd-run.qcow2"
> +
> +if [ ! -f "${BASE_IMG}" ]; then
> +    echo "ERROR: ${BASE_IMG} not found." \
> +         "Ensure the cache was restored before calling this script." >&2
> +    exit 1
> +fi
> +
> +# ---------------------------------------------------------------------------
> +# 1. Install host-side tools
> +# ---------------------------------------------------------------------------

These block comments, IMO, make the code hard to read and do not bring any
real value.  Mostly they are stating the obvious and should just be removed.
A few can be simplified and re-written as normal comments.  The numbers are
also not a good thing to have in general, since they will have to be updated
every time somethign changes.

> +sudo apt-get update -qq
> +sudo apt-get install -y \
> +    qemu-system-x86 qemu-utils \
> +    genisoimage \
> +    rsync openssh-client \
> +    ovmf

This is better done as part of the workflow yaml instead of a script.  It's
fine to install a little more than necessary if both the omage build and the
OVS build require slightly different sets.

> +
> +# ---------------------------------------------------------------------------
> +# 2. Source VM utilities
> +# ---------------------------------------------------------------------------
> +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
> +# shellcheck source=freebsd-vm.sh
> +. "${SCRIPT_DIR}/freebsd-vm.sh"
> +
> +# ---------------------------------------------------------------------------
> +# 3. Generate an ephemeral SSH key for this CI run
> +# ---------------------------------------------------------------------------
> +KEY_DIR="$(mktemp -d)"
> +SSH_KEY="${KEY_DIR}/id_ed25519"
> +ssh-keygen -t ed25519 -f "${SSH_KEY}" -N "" -q
> +export FREEBSD_SSH_KEY="${SSH_KEY}"
> +
> +# ---------------------------------------------------------------------------
> +# 4. Create a COW overlay on top of the cached base image
> +# ---------------------------------------------------------------------------
> +# COW overlay keeps the cached base image unmodified.
> +echo "==> Creating COW overlay image ..."

We're running with -x where all the commands are logged anyway, there is no
need for most of these status messages.

> +qemu-img create -f qcow2 -F qcow2 -b "$(realpath "${BASE_IMG}")" "${RUN_IMG}"
> +
> +# ---------------------------------------------------------------------------
> +# 5. Create a seed ISO with a fresh instance-id
> +# ---------------------------------------------------------------------------
> +SEED_INSTANCE_ID="freebsd-build-${FREEBSD_VER}-${COMPILER}-$(date +%s%N)"

The date here doesn't seem to be needed.  You're using the /firstboot which will
always trigger the nuageinit, the instance id should not make any difference.

> +freebsd_create_seed \
> +    "${SSH_KEY}.pub" \
> +    "${SEED_INSTANCE_ID}" \
> +    /tmp/freebsd-seed-build \
> +    /tmp/freebsd-seed-build.iso
> +
> +# ---------------------------------------------------------------------------
> +# 6. Boot the VM
> +# ---------------------------------------------------------------------------
> +echo "==> Booting FreeBSD ${FREEBSD_VER} (compiler: ${COMPILER}) ..."
> +
> +BUILD_OVMF_VARS="/tmp/freebsd-ovmf-vars-build.fd"
> +cp "${FREEBSD_OVMF_VARS}" "${BUILD_OVMF_VARS}"
> +
> +freebsd_start_vm \
> +    "${RUN_IMG}" /tmp/freebsd-seed-build.iso 4096 4 "${BUILD_OVMF_VARS}"
> +
> +cleanup() {
> +    # Retrieve logs before stopping the VM (|| true handles boot failures).
> +    echo "==> Retrieving logs ..."
> +    mkdir -p tests
> +    freebsd_rsync_from /root/ovs/config.log        ./      2>/dev/null || 
> true

This is one space off.

> +    freebsd_rsync_from /root/ovs/tests/testsuite.log tests/ 2>/dev/null || 
> true
> +    freebsd_rsync_from /root/ovs/tests/testsuite.dir tests/ 2>/dev/null || 
> true

We should also get nuageinit logs from the VM, otherwise we have no visibility
into what it is doing.

> +    freebsd_stop_vm
> +    rm -rf "${KEY_DIR}" "${RUN_IMG}" "${BUILD_OVMF_VARS}" \
> +           /tmp/freebsd-seed-build /tmp/freebsd-seed-build.iso
> +}
> +trap cleanup EXIT
> +
> +# ---------------------------------------------------------------------------
> +# 7. Wait for SSH, then wait for firstboot to complete
> +# ---------------------------------------------------------------------------
> +# Wait for SSH, then ensure all firstboot rc scripts (including the sshd
> +# restart runcmd) have completed before proceeding.
> +# SSH wait: 20 x 10s = 200s.  Firstboot wait: 30 x 5s = 150s.

Too much of unnecessary info.

> +freebsd_wait_ssh 20 10
> +freebsd_wait_firstboot_complete 30 5
> +
> +# ---------------------------------------------------------------------------
> +# 8. Sync source tree into the VM
> +# ---------------------------------------------------------------------------
> +echo "==> Syncing source tree to VM ..."
> +freebsd_ssh "mkdir -p /root/ovs"
> +freebsd_rsync_to "$(pwd)/" /root/ovs/
> +
> +# ---------------------------------------------------------------------------
> +# 9. Configure
> +# ---------------------------------------------------------------------------
> +echo "==> Configuring (CC=${COMPILER}) ..."
> +freebsd_ssh "cd /root/ovs && \
> +    ./boot.sh && \
> +    ./configure CC=${COMPILER} CFLAGS='-g -O2 -Wall' MAKE=gmake \
> +        --enable-Werror \
> +    || { cat config.log; exit 1; }"

No need to cat, we can properly get the log files as artifacts.  We couldn't
in Cirrus, so we had the cat.

> +
> +# ---------------------------------------------------------------------------
> +# 10. Build
> +# ---------------------------------------------------------------------------
> +echo "==> Building ..."
> +freebsd_ssh "cd /root/ovs && gmake -j8"
> +
> +# ---------------------------------------------------------------------------
> +# 11. Test
> +# ---------------------------------------------------------------------------
> +echo "==> Running test suite ..."
> +freebsd_ssh "cd /root/ovs && \
> +    gmake -j8 check TESTSUITEFLAGS=-j8 RECHECK=yes \
> +    || { cat ./tests/testsuite.log; exit 1; }"

No reason to keep the build and the test as separate commands.  It made sense
in Cirrus because they were separate steps in the job, so proerly separated in
the UI, but it will not be the case here.  And no need to cat as well.

> +
> +echo "==> FreeBSD ${FREEBSD_VER} ${COMPILER} build complete."
> diff --git a/.ci/freebsd-prepare-image.sh b/.ci/freebsd-prepare-image.sh
> new file mode 100755
> index 000000000..5cbbb7960
> --- /dev/null
> +++ b/.ci/freebsd-prepare-image.sh
> @@ -0,0 +1,154 @@
> +#!/bin/bash
> +# Prepares a FreeBSD QEMU VM image with all CI dependencies pre-installed.
> +#
> +# Usage: freebsd-prepare-image.sh <freebsd_version>
> +#   freebsd_version  e.g. "14.4" or "15.0"
> +#
> +# Downloads the FreeBSD BASIC-CLOUDINIT qcow2 image, verifies its SHA256,
> +# grows the disk, runs the two-boot nuageinit sequence (SSH key injection,
> +# package install, PermitRootLogin), restores /firstboot for build job boots,
> +# and compresses the result for caching.  Boot sequence details: 
> freebsd-vm.sh.
> +#
> +# Output file: freebsd-<version>.qcow2  (in the current directory)

The comment may probbaly be a littel simplified, as well.

> +
> +set -o errexit
> +set -o xtrace
> +
> +# Capture the exact command that triggers errexit so the EXIT trap can print
> +# a clear "FAILED: <cmd>" line.  Without this the failure is buried in
> +# xtrace noise.
> +_FAILED_CMD=""
> +trap '_FAILED_CMD="${BASH_COMMAND}"' ERR

This seems unnecessary.

> +
> +FREEBSD_VER="${1:?Usage: $0 <version>  (e.g. 14.4)}"
> +
> +RELEASE="${FREEBSD_VER}-RELEASE"
> +ARCH="amd64"
> +BASE_URL="https://download.freebsd.org/releases/VM-IMAGES";
> +BASE_URL="${BASE_URL}/${RELEASE}/${ARCH}/Latest"
> +
> +IMG_NAME="FreeBSD-${RELEASE}-${ARCH}-BASIC-CLOUDINIT-ufs.qcow2"
> +IMG_XZ="${IMG_NAME}.xz"
> +OUT_IMG="freebsd-${FREEBSD_VER}.qcow2"
> +
> +# ---------------------------------------------------------------------------
> +# 1. Install host-side tools
> +# ---------------------------------------------------------------------------
> +sudo apt-get update -qq
> +sudo apt-get install -y \
> +    qemu-system-x86 qemu-utils \
> +    genisoimage \
> +    wget xz-utils \
> +    ovmf
> +
> +# ---------------------------------------------------------------------------
> +# 2. Download and verify the FreeBSD image
> +# ---------------------------------------------------------------------------
> +echo "==> Fetching CHECKSUM.SHA256 ..."

Same general comments as for the other script.

> +wget -q "${BASE_URL}/CHECKSUM.SHA256" -O freebsd-checksum.txt
> +
> +echo "==> Downloading ${IMG_XZ} ..."
> +wget -q "${BASE_URL}/${IMG_XZ}" -O "${IMG_XZ}"
> +
> +echo "==> Verifying image integrity ..."
> +expected_sha=$(grep "(${IMG_XZ})" freebsd-checksum.txt | awk '{print $NF}')
> +actual_sha=$(sha256sum "${IMG_XZ}" | awk '{print $1}')
> +if [ "${expected_sha}" != "${actual_sha}" ]; then
> +    echo "ERROR: SHA256 mismatch for ${IMG_XZ}" >&2
> +    echo "  expected: ${expected_sha}" >&2
> +    echo "  actual:   ${actual_sha}" >&2
> +    exit 1
> +fi
> +
> +echo "==> Extracting image ..."
> +xz --decompress --keep "${IMG_XZ}"
> +mv "${IMG_NAME}" "${OUT_IMG}"
> +rm -f "${IMG_XZ}"
> +
> +# ---------------------------------------------------------------------------
> +# 3. Grow the disk to make room for packages
> +# ---------------------------------------------------------------------------
> +echo "==> Resizing disk to +8G ..."
> +qemu-img resize "${OUT_IMG}" +8G
> +
> +# ---------------------------------------------------------------------------
> +# 4. Generate an ephemeral SSH key for this preparation run
> +# ---------------------------------------------------------------------------
> +PREP_KEY_DIR="$(mktemp -d)"
> +PREP_KEY="${PREP_KEY_DIR}/id_ed25519"
> +ssh-keygen -t ed25519 -f "${PREP_KEY}" -N "" -q
> +
> +# ---------------------------------------------------------------------------
> +# 5. Source VM utilities
> +# ---------------------------------------------------------------------------
> +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
> +# shellcheck source=freebsd-vm.sh
> +. "${SCRIPT_DIR}/freebsd-vm.sh"
> +export FREEBSD_SSH_KEY="${PREP_KEY}"
> +
> +# ---------------------------------------------------------------------------
> +# 6. Boot the VM with a seed ISO
> +# ---------------------------------------------------------------------------
> +SEED_INSTANCE_ID="freebsd-prepare-${FREEBSD_VER}-$(date +%s)"
> +freebsd_create_seed \
> +    "${PREP_KEY}.pub" \
> +    "${SEED_INSTANCE_ID}" \
> +    /tmp/freebsd-seed-prepare \
> +    /tmp/freebsd-seed-prepare.iso
> +
> +echo "==> Booting FreeBSD ${FREEBSD_VER} for image preparation ..."
> +
> +PREP_OVMF_VARS="/tmp/freebsd-ovmf-vars-prepare.fd"
> +cp "${FREEBSD_OVMF_VARS}" "${PREP_OVMF_VARS}"
> +
> +freebsd_start_vm \
> +    "${OUT_IMG}" /tmp/freebsd-seed-prepare.iso 4096 4 "${PREP_OVMF_VARS}"
> +
> +_prep_cleanup() {
> +    local rc=$?
> +    if [ "${rc}" -ne 0 ]; then
> +        echo "### PREPARE FAILED (rc=${rc}): ${_FAILED_CMD} ###" >&2
> +    fi
> +    freebsd_stop_vm

Should also collect nuagenint and qemu console logs.

> +    rm -rf "${PREP_KEY_DIR}" "${PREP_OVMF_VARS}" \
> +           /tmp/freebsd-seed-prepare /tmp/freebsd-seed-prepare.iso
> +}
> +trap _prep_cleanup EXIT

Instead of changing the traps it probably better to just have a global
variable that says if VM is running or not and do slightly different
things depending on it.  Different traps are confusing.

> +
> +# ---------------------------------------------------------------------------
> +# 7. Wait for SSH, then wait for firstboot to complete
> +# ---------------------------------------------------------------------------
> +# Covers two boots: Boot 1 (freebsd-update + reboot) + Boot 2 (packages +
> +# runcmds).  Must finish before "touch /firstboot" (step 8).
> +# SSH wait: 90 x 10s = 900s.  Firstboot wait: 12 x 10s = 120s.
> +freebsd_wait_ssh 90 10
> +freebsd_wait_firstboot_complete 12 10
> +
> +# ---------------------------------------------------------------------------
> +# 8. Restore /firstboot so build job boots re-run nuageinit
> +# ---------------------------------------------------------------------------
> +# Restore /firstboot so nuageinit runs on build job boots to inject
> +# per-job SSH keys.
> +echo "==> Restoring /firstboot for build job boots ..."
> +freebsd_ssh "touch /firstboot"
> +
> +echo "==> Shutting down FreeBSD ${FREEBSD_VER} ..."
> +freebsd_stop_vm
> +_prep_cleanup() {
> +    rm -rf "${PREP_KEY_DIR}" "${PREP_OVMF_VARS}" \
> +           /tmp/freebsd-seed-prepare /tmp/freebsd-seed-prepare.iso
> +}
> +trap _prep_cleanup EXIT
> +
> +# ---------------------------------------------------------------------------
> +# 9. Compress the prepared image for caching
> +# ---------------------------------------------------------------------------
> +echo "==> Compressing prepared image ..."
> +COMPRESSED="${OUT_IMG}.compressed"
> +qemu-img convert -c -O qcow2 "${OUT_IMG}" "${COMPRESSED}"
> +mv "${COMPRESSED}" "${OUT_IMG}"
> +
> +FINAL_SIZE=$(du -sh "${OUT_IMG}" | cut -f1)
> +echo "==> Image preparation complete."
> +echo "    Output : ${OUT_IMG}"
> +echo "    Size   : ${FINAL_SIZE}"

This log seems unnecessary.  We can see the sizes when downloading and when
uploading the artifacts.

> diff --git a/.ci/freebsd-vm.sh b/.ci/freebsd-vm.sh
> new file mode 100755
> index 000000000..13394efba
> --- /dev/null
> +++ b/.ci/freebsd-vm.sh
> @@ -0,0 +1,291 @@
> +#!/bin/bash
> +# FreeBSD QEMU VM management utilities.
> +#
> +# Source this file from other scripts; do NOT execute it directly.
> +#
> +# Required environment variables (set before sourcing or calling functions):
> +#   FREEBSD_SSH_KEY   - path to the private SSH key for VM access
> +#
> +# Optional environment variables:
> +#   FREEBSD_SSH_PORT  - host port forwarded to VM's SSH (default: 2222)
> +#   FREEBSD_VM_PIDFILE - path for the QEMU pid file
> +#                        (default: /tmp/freebsd-vm.pid)
> +
> +FREEBSD_SSH_PORT="${FREEBSD_SSH_PORT:-2222}"
> +FREEBSD_VM_PIDFILE="${FREEBSD_VM_PIDFILE:-/tmp/freebsd-vm.pid}"
> +
> +# OVMF firmware paths (Ubuntu 24.04, package: ovmf).
> +FREEBSD_OVMF_CODE="/usr/share/OVMF/OVMF_CODE_4M.fd"
> +FREEBSD_OVMF_VARS="/usr/share/OVMF/OVMF_VARS_4M.fd"
> +
> +# SSH options for VM communication.  Host key checking disabled (ephemeral 
> VM).
> +_freebsd_ssh_opts() {
> +    echo "-p ${FREEBSD_SSH_PORT}"
> +    echo "-i ${FREEBSD_SSH_KEY}"
> +    echo "-o StrictHostKeyChecking=no"
> +    echo "-o UserKnownHostsFile=/dev/null"
> +    echo "-o ConnectTimeout=5"
> +    echo "-o BatchMode=yes"
> +    echo "-o ServerAliveInterval=15"
> +    echo "-o ServerAliveCountMax=4"
> +    echo "-o LogLevel=ERROR"
> +}

This doesn't look great.  It seems better to just have an array of options and
then use it in the calls:
  
_FREEBSD_SSH_OPTS=(
    -p "$FREEBSD_SSH_PORT"
    -o StrictHostKeyChecking=no
    -o UserKnownHostsFile=/dev/null
    -o ConnectTimeout=5
    -o BatchMode=yes
    -o ServerAliveInterval=15
    -o ServerAliveCountMax=4
    -o LogLevel=ERROR
)

freebsd_ssh() {
    ssh "${_FREEBSD_SSH_OPTS[@]}" -i "${FREEBSD_SSH_KEY}" \
        root@localhost "$@"
}

> +
> +# freebsd_kvm_opts
> +# Emits KVM flags if /dev/kvm is available, falls back to software emulation.
> +freebsd_kvm_opts() {
> +    if [ -e /dev/kvm ] && [ -r /dev/kvm ]; then
> +        echo "-enable-kvm -cpu host"
> +    else
> +        echo "WARNING: /dev/kvm not available; falling back to software" \
> +             "emulation.  Build times will be significantly longer." >&2
> +        echo "-cpu qemu64"
> +    fi

We shouldn't need a fallback, KVM should always be available.

> +}
> +
> +# freebsd_start_vm <image.qcow2> [seed.iso] [mem_mb] [cpus] [ovmf_vars.fd]
> +# Boots a FreeBSD VM in the background.
> +#   image.qcow2  - disk image (may be a COW overlay)
> +#   seed.iso     - optional cloud-init NoCloud seed ISO
> +#   mem_mb       - RAM in megabytes (default: 4096)
> +#   cpus         - vCPU count (default: 4)
> +#   ovmf_vars.fd - path to a *writable* copy of FREEBSD_OVMF_VARS for UEFI
> +#                  boot.  FreeBSD BASIC-CLOUDINIT images use GPT/EFI and
> +#                  require UEFI firmware.  When absent the VM falls back to
> +#                  SeaBIOS (legacy BIOS).
> +freebsd_start_vm() {
> +    local img_file="${1:?freebsd_start_vm: image file required}"
> +    local seed_iso="${2:-}"
> +    local mem="${3:-4096}"
> +    local cpus="${4:-4}"
> +    local ovmf_vars="${5:-}"
> +
> +    local seed_args=""
> +    if [ -n "${seed_iso}" ]; then
> +        # Attach seed ISO via AHCI controller — AHCI is probed during PCI
> +        # bus scan, early enough for nuageinit.  FreeBSD creates
> +        # /dev/iso9660/cidata from the volume ID.
> +        seed_args="-device ahci,id=ahci0"
> +        seed_args="${seed_args} -drive"
> +        seed_args="${seed_args} if=none,id=seed_drive"
> +        seed_args="${seed_args},file=${seed_iso}"
> +        seed_args="${seed_args},format=raw,media=cdrom,readonly=on"
> +        seed_args="${seed_args} -device ide-cd,bus=ahci0.0,drive=seed_drive"
> +    fi
> +
> +    # UEFI firmware (OVMF).  FreeBSD BASIC-CLOUDINIT images require GPT/EFI
> +    # boot.  The vars file must be a writable per-VM copy.
> +    local ovmf_args=""
> +    if [ -n "${ovmf_vars}" ]; then
> +        ovmf_args="-drive"
> +        ovmf_args="${ovmf_args} if=pflash,format=raw"
> +        ovmf_args="${ovmf_args},readonly=on,file=${FREEBSD_OVMF_CODE}"
> +        ovmf_args="${ovmf_args} -drive 
> if=pflash,format=raw,file=${ovmf_vars}"
> +    fi
> +
> +    # Build the full QEMU command into an array.
> +    # shellcheck disable=SC2046
> +    local qemu_cmd
> +    qemu_cmd=(
> +        qemu-system-x86_64
> +        $(freebsd_kvm_opts)
> +        -m "${mem}"
> +        -smp "${cpus}"
> +        -nographic
> +        -netdev "user,id=net0,hostfwd=tcp::${FREEBSD_SSH_PORT}-:22"
> +        -device virtio-net-pci,netdev=net0
> +        -drive "file=${img_file},if=virtio,format=qcow2,cache=unsafe"
> +        -device virtio-rng-pci
> +        -pidfile "${FREEBSD_VM_PIDFILE}"
> +    )
> +
> +    if [ -n "${seed_args}" ]; then
> +        # shellcheck disable=SC2206
> +        qemu_cmd+=( ${seed_args} )
> +    fi
> +    if [ -n "${ovmf_args}" ]; then
> +        # shellcheck disable=SC2206
> +        qemu_cmd+=( ${ovmf_args} )
> +    fi
> +
> +    "${qemu_cmd[@]}" > /tmp/freebsd-vm.log 2>&1 &
> +
> +    echo "FreeBSD VM launched (PID $!); log: /tmp/freebsd-vm.log"

This function is overcomplicated.  We don't really need any fallbacks, we should
be able to hardcode most of the options like memory and cpu count.  Seed iso and
the ovmf_vars can be required, as all callers have them.

> +}
> +
> +# freebsd_wait_ssh [max_attempts] [delay_seconds]
> +# Polls SSH port until the VM responds or we exhaust attempts.
> +# Returns 0 on success, 1 on timeout.
> +freebsd_wait_ssh() {
> +    local max_attempts="${1:-20}"
> +    local delay="${2:-10}"
> +
> +    echo "Waiting for SSH on localhost:${FREEBSD_SSH_PORT} ..."
> +    local i
> +    for i in $(seq 1 "${max_attempts}"); do
> +        # shellcheck disable=SC2046
> +        if ssh $(tr '\n' ' ' < <(_freebsd_ssh_opts)) \
> +               root@localhost true 2>/dev/null; then
> +            echo "SSH ready after attempt ${i}."
> +            return 0
> +        fi
> +
> +        echo "  attempt ${i}/${max_attempts}, retrying in ${delay}s ..."
> +
> +        if [ "${i}" != "${max_attempts}" ]; then
> +            sleep "${delay}"
> +        fi
> +    done
> +
> +    echo "ERROR: SSH did not become available after" \
> +         "$((max_attempts * delay))s." >&2
> +    return 1
> +}
> +
> +# freebsd_wait_firstboot_complete [max_attempts] [delay_seconds]
> +# Polls until /firstboot is absent, which means rc.d/firstboot has finished 
> all
> +# firstboot-keyed rc scripts — including nuageinit_user_data_script and its
> +# "sshd onerestart" runcmd.  Call after freebsd_wait_ssh; without this, SSH
> +# commands can be killed mid-flight by that sshd restart.

A little too many details here.

> +freebsd_wait_firstboot_complete() {
> +    local max_attempts="${1:-30}"
> +    local delay="${2:-5}"
> +
> +    echo "Waiting for firstboot to complete (/firstboot to be removed) ..."
> +    local i
> +    for i in $(seq 1 "${max_attempts}"); do
> +        if freebsd_ssh "test ! -f /firstboot" 2>/dev/null; then
> +            echo "Firstboot sequence complete (attempt ${i})."
> +            return 0
> +        fi
> +        echo "  /firstboot still present (attempt ${i}/${max_attempts})," \
> +             "waiting ${delay}s ..."
> +        sleep "${delay}"
> +    done
> +
> +    echo "ERROR: /firstboot still present after" \
> +         "$((max_attempts * delay))s." >&2
> +    return 1
> +}
> +
> +# freebsd_ssh <command ...>
> +# Runs a command inside the VM as root.
> +freebsd_ssh() {
> +    # shellcheck disable=SC2046
> +    ssh $(tr '\n' ' ' < <(_freebsd_ssh_opts)) root@localhost "$@"
> +}
> +
> +# freebsd_rsync_to <local_src> <vm_dst>
> +# Rsyncs from the host into the VM.
> +freebsd_rsync_to() {
> +    local src="${1:?freebsd_rsync_to: source required}"
> +    local dst="${2:?freebsd_rsync_to: destination required}"
> +    # shellcheck disable=SC2046
> +    rsync -az --delete \
> +        -e "ssh $(tr '\n' ' ' < <(_freebsd_ssh_opts))" \
> +        "${src}" "root@localhost:${dst}"
> +}
> +
> +# freebsd_rsync_from <vm_src> <local_dst>
> +# Rsyncs from the VM back to the host.
> +freebsd_rsync_from() {
> +    local src="${1:?freebsd_rsync_from: source required}"
> +    local dst="${2:?freebsd_rsync_from: destination required}"
> +    # shellcheck disable=SC2046
> +    rsync -az \
> +        -e "ssh $(tr '\n' ' ' < <(_freebsd_ssh_opts))" \
> +        "root@localhost:${src}" "${dst}"
> +}
> +
> +# freebsd_stop_vm
> +# Gracefully shuts down the VM; kills QEMU if it does not exit in time.
> +freebsd_stop_vm() {
> +    if [ ! -f "${FREEBSD_VM_PIDFILE}" ]; then
> +        return 0
> +    fi
> +
> +    local pid
> +    pid=$(cat "${FREEBSD_VM_PIDFILE}" 2>/dev/null) || return 0
> +
> +    echo "Shutting down FreeBSD VM (PID ${pid}) ..."
> +    freebsd_ssh "shutdown -p now" 2>/dev/null || true
> +
> +    local i
> +    for i in $(seq 1 30); do
> +        kill -0 "${pid}" 2>/dev/null || {
> +            echo "VM exited cleanly."
> +            rm -f "${FREEBSD_VM_PIDFILE}"
> +            return 0
> +        }
> +        sleep 2
> +    done
> +
> +    echo "VM did not stop in time; killing QEMU."
> +    kill "${pid}" 2>/dev/null || true
> +    rm -f "${FREEBSD_VM_PIDFILE}"
> +}
> +
> +# freebsd_create_seed <pubkey_file> <instance_id> <work_dir> <output_iso>
> +#                     [label]
> +# Creates a NoCloud seed ISO that injects <pubkey_file> into root
> +# authorized_keys
> +# and installs CI dependencies via nuageinit.
> +#
> +# label (optional, default: cidata):
> +#   ISO 9660 volume ID passed to genisoimage/mkisofs.  nuageinit on FreeBSD
> +#   14.x+ BASIC-CLOUDINIT images recognises the "cidata" NoCloud label.
> +freebsd_create_seed() {
> +    local pub_key_file="${1:?freebsd_create_seed: public key file required}"
> +    local instance_id="${2:?freebsd_create_seed: instance-id required}"
> +    local work_dir="${3:-/tmp/freebsd-seed}"
> +    local out_iso="${4:-/tmp/freebsd-seed.iso}"
> +    local label="${5:-cidata}"
> +
> +    local pub_key
> +    pub_key=$(cat "${pub_key_file}")
> +
> +    mkdir -p "${work_dir}"
> +
> +    cat > "${work_dir}/meta-data" <<EOF
> +instance-id: ${instance_id}
> +local-hostname: freebsd-ci
> +EOF
> +
> +    # py311-* names match FreeBSD 14/15 default Python 3.11.
> +    cat > "${work_dir}/user-data" <<EOF
> +#cloud-config
> +users:
> +  - name: root
> +    ssh_authorized_keys:
> +      - ${pub_key}
> +package_update: true
> +packages:
> +  - automake
> +  - libtool
> +  - gmake
> +  - gcc
> +  - openssl
> +  - python3
> +  - rsync
> +  - py311-sphinx
> +  - py311-netaddr
> +  - py311-pyparsing

It would be better to define the list in the workflow and use the env
variable to genere these here.  Also, nuageinint doesn't fail if it
can't install some of the packages, we need an extra check that all of
these packages are actually installed.  For this we'll need the list
defined externally as well.

And we probbaly don't want to run pkg update/install during the build
phase, only when we're preparing the image.  This will save us from
broken repository updates that sometimes happen with FreeBSD images.

> +runcmd:
> +  - printf '\nPermitRootLogin yes\n' >> /etc/ssh/sshd_config
> +  - grep -q kern.coredump /etc/sysctl.conf || echo 'kern.coredump=0' >> 
> /etc/sysctl.conf
> +  - sysctl -w kern.coredump=0 || true
> +  - service sshd onerestart || true
> +EOF
> +
> +    if command -v genisoimage >/dev/null 2>&1; then
> +        genisoimage -output "${out_iso}" \
> +            -volid "${label}" -rational-rock -joliet \
> +            "${work_dir}/user-data" "${work_dir}/meta-data" 2>/dev/null
> +    else
> +        mkisofs -output "${out_iso}" \
> +            -volid "${label}" -rational-rock -joliet \
> +            "${work_dir}/user-data" "${work_dir}/meta-data"

This fallback is not needed, we have genisoimage installed.

> +    fi
> +
> +    echo "Seed ISO created: ${out_iso} (label: ${label})"
> +}
> diff --git a/.github/workflows/freebsd.yml b/.github/workflows/freebsd.yml
> new file mode 100755
> index 000000000..3d57d9c03
> --- /dev/null
> +++ b/.github/workflows/freebsd.yml
> @@ -0,0 +1,160 @@
> +name: FreeBSD Build and Test
> +
> +# Triggers match the existing build-and-test.yml for consistency.
> +on: [push, pull_request]
> +
> +jobs:
> +
> +  # 
> ---------------------------------------------------------------------------
> +  # build-freebsd-image
> +  # 
> ---------------------------------------------------------------------------
> +  # Downloads the official FreeBSD BASIC-CLOUDINIT image, boots it with QEMU,
> +  # lets nuageinit install all CI dependencies and configure SSH access, then
> +  # compresses and caches the result.
> +  #
> +  # On a cache hit the job completes in under 1 minute.  The actual image
> +  # preparation only runs when the cache key changes, i.e. when the FreeBSD
> +  # version, the prepare/VM-utility scripts, or the upstream image itself
> +  # changes.
> +  #
> +  # The cache key is re-derived independently in build-freebsd, so no job
> +  # output is needed to pass the key between jobs.
> +  # 
> ---------------------------------------------------------------------------

Too much text, can be mostly dropped.

> +  build-freebsd-image:
> +    name: freebsd ${{ matrix.freebsd_version }} image
> +    runs-on: ubuntu-24.04
> +    timeout-minutes: 45
> +
> +    strategy:
> +      fail-fast: false
> +      matrix:
> +        freebsd_version: ["14.4", "15.0"]
> +
> +    steps:
> +    - name: checkout
> +      uses: actions/checkout@v6
> +
> +    # The cache key covers three inputs:
> +    #   - the FreeBSD version (one image per version)
> +    #   - SHA-256 of the scripts that control what ends up in the image
> +    #   - SHA-256 of the upstream image file from FreeBSD's CHECKSUM.SHA256
> +    # Any change to the scripts or a FreeBSD image re-spin busts the cache.
> +    - name: generate image cache key
> +      id: gen_key
> +      run: |
> +        img_xz="FreeBSD-${{ matrix.freebsd_version 
> }}-RELEASE-amd64-BASIC-CLOUDINIT-ufs.qcow2.xz"
> +        base_url="https://download.freebsd.org/releases/VM-IMAGES/${{ 
> matrix.freebsd_version }}-RELEASE/amd64/Latest"
> +        wget -q "${base_url}/CHECKSUM.SHA256" -O 
> freebsd-upstream-checksum.txt

This is an external network request.  We don't really need this, we can use
the same image until the version changes.

> +        image_sha=$(grep "(${img_xz})" freebsd-upstream-checksum.txt | awk 
> '{print $NF}')
> +        scripts_hash=$(cat .ci/freebsd-prepare-image.sh .ci/freebsd-vm.sh \
> +                         | sha256sum | cut -d' ' -f1)

May be good to include the workflow file itself.

> +        combined=$(printf '%s-%s' "${image_sha}" "${scripts_hash}" \
> +                    | sha256sum | cut -d' ' -f1)
> +        echo "key=freebsd-${{ matrix.freebsd_version }}-${combined}" \
> +          >> "$GITHUB_OUTPUT"
> +
> +    # /dev/kvm is owned by group kvm (mode 0660) on GitHub-hosted runners.
> +    # The runner user is not in that group, so we make it world-accessible.
> +    # QEMU requires this for KVM acceleration.
> +    - name: enable KVM access
> +      run: |
> +        echo 'KERNEL=="kvm", GROUP="kvm", MODE="0666", 
> OPTIONS+="static_node=kvm"' \
> +          | sudo tee /etc/udev/rules.d/99-kvm4all.rules
> +        sudo udevadm control --reload-rules
> +        sudo udevadm trigger --name-match=kvm
> +
> +    - name: restore image cache
> +      id: image_cache
> +      uses: actions/cache@v5
> +      with:
> +        path: freebsd-${{ matrix.freebsd_version }}.qcow2
> +        key:  ${{ steps.gen_key.outputs.key }}
> +
> +    - name: prepare FreeBSD ${{ matrix.freebsd_version }} image
> +      if: steps.image_cache.outputs.cache-hit != 'true'
> +      run: ./.ci/freebsd-prepare-image.sh ${{ matrix.freebsd_version }}
> +
> +  # 
> ---------------------------------------------------------------------------
> +  # build-freebsd
> +  # 
> ---------------------------------------------------------------------------
> +  # Restores the prepared VM image, boots it with QEMU using a COW overlay so
> +  # the cached base is never modified, rsyncs the source tree into the VM, 
> and
> +  # runs configure -> gmake -j8 -> gmake check.
> +  # 
> ---------------------------------------------------------------------------

Same here.

> +  build-freebsd:
> +    name: freebsd ${{ matrix.freebsd_version }} ${{ matrix.compiler }}
> +    needs: build-freebsd-image
> +    runs-on: ubuntu-24.04
> +    timeout-minutes: 60
> +
> +    strategy:
> +      fail-fast: false
> +      matrix:
> +        include:
> +          - freebsd_version: "14.4"
> +            compiler: gcc
> +          - freebsd_version: "14.4"
> +            compiler: clang
> +          - freebsd_version: "15.0"
> +            compiler: gcc
> +          - freebsd_version: "15.0"
> +            compiler: clang

I believe, this matrix can be done with two arrays.  But as mentioned above,
we should just reduce it to one job and combine it with the image build.

> +
> +    env:
> +      FREEBSD_VER: ${{ matrix.freebsd_version }}
> +      COMPILER:    ${{ matrix.compiler }}
> +
> +    steps:
> +    - name: checkout
> +      uses: actions/checkout@v6
> +
> +    # Re-derive the same cache key used by build-freebsd-image.
> +    - name: generate image cache key
> +      id: gen_key
> +      run: |
> +        img_xz="FreeBSD-${{ matrix.freebsd_version 
> }}-RELEASE-amd64-BASIC-CLOUDINIT-ufs.qcow2.xz"
> +        base_url="https://download.freebsd.org/releases/VM-IMAGES/${{ 
> matrix.freebsd_version }}-RELEASE/amd64/Latest"
> +        wget -q "${base_url}/CHECKSUM.SHA256" -O 
> freebsd-upstream-checksum.txt
> +        image_sha=$(grep "(${img_xz})" freebsd-upstream-checksum.txt | awk 
> '{print $NF}')
> +        scripts_hash=$(cat .ci/freebsd-prepare-image.sh .ci/freebsd-vm.sh \
> +                         | sha256sum | cut -d' ' -f1)
> +        combined=$(printf '%s-%s' "${image_sha}" "${scripts_hash}" \
> +                    | sha256sum | cut -d' ' -f1)
> +        echo "key=freebsd-${{ matrix.freebsd_version }}-${combined}" \
> +          >> "$GITHUB_OUTPUT"

This will go away if the image build and the OVS build are in the same job.

> +
> +    - name: enable KVM access
> +      run: |
> +        echo 'KERNEL=="kvm", GROUP="kvm", MODE="0666", 
> OPTIONS+="static_node=kvm"' \
> +          | sudo tee /etc/udev/rules.d/99-kvm4all.rules
> +        sudo udevadm control --reload-rules
> +        sudo udevadm trigger --name-match=kvm
> +
> +    # Restore the prepared image written by build-freebsd-image.  If the 
> cache
> +    # is absent (e.g. eviction) the build step will fail with a clear error
> +    # message pointing to the missing image file.
> +    - name: restore FreeBSD ${{ matrix.freebsd_version }} image
> +      uses: actions/cache/restore@v5
> +      with:
> +        path: freebsd-${{ matrix.freebsd_version }}.qcow2
> +        key:  ${{ steps.gen_key.outputs.key }}
> +
> +    - name: build and test
> +      run: ./.ci/freebsd-build.sh
> +
> +    - name: collect logs on failure
> +      if: failure() || cancelled()
> +      run: |
> +        mkdir -p logs
> +        cp config.log              logs/ 2>/dev/null || true
> +        cp tests/testsuite.log     logs/ 2>/dev/null || true

Need to capture nuageinit logs and the qemu logs.

> +        # testsuite.dir may contain socket files; tar handles them 
> gracefully.
> +        sudo tar -czf logs.tgz logs/ tests/testsuite.dir 2>/dev/null || \
> +        tar -czf logs.tgz logs/
> +
> +    - name: upload logs on failure
> +      if: failure() || cancelled()
> +      uses: actions/upload-artifact@v7
> +      with:
> +        name: logs-freebsd-${{ matrix.freebsd_version }}-${{ matrix.compiler 
> }}
> +        path: logs.tgz
> diff --git a/Makefile.am b/Makefile.am
> index a805f21d1..ed57ddac5 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -79,6 +79,9 @@ EXTRA_DIST = \
>       NOTICE \
>       .ci/dpdk-build.sh \
>       .ci/dpdk-prepare.sh \
> +     .ci/freebsd-build.sh \
> +     .ci/freebsd-prepare-image.sh \
> +     .ci/freebsd-vm.sh \
>       .ci/linux-build.sh \
>       .ci/linux-prepare.sh \
>       .ci/osx-build.sh \
> @@ -88,6 +91,7 @@ EXTRA_DIST = \
>       .cirrus.yml \
>       .editorconfig \
>       .github/workflows/build-and-test.yml \
> +     .github/workflows/freebsd.yml \
>       .readthedocs.yaml \
>       appveyor.yml \
>       boot.sh \
> diff --git a/README.rst b/README.rst
> index 649dc1d38..97c85bd95 100644
> --- a/README.rst
> +++ b/README.rst
> @@ -10,8 +10,8 @@ Open vSwitch
>      :target: https://github.com/openvswitch/ovs/actions
>  .. image:: 
> https://ci.appveyor.com/api/projects/status/github/openvswitch/ovs?branch=main&svg=true&retina=true
>      :target: https://ci.appveyor.com/project/blp/ovs/history
> -.. image:: https://api.cirrus-ci.com/github/openvswitch/ovs.svg
> -    :target: https://cirrus-ci.com/github/openvswitch/ovs
> +.. image:: 
> https://github.com/openvswitch/ovs/workflows/FreeBSD%20Build%20and%20Test/badge.svg
> +    :target: https://github.com/openvswitch/ovs/actions
>  .. image:: https://readthedocs.org/projects/openvswitch/badge/?version=latest
>      :target: https://docs.openvswitch.org/en/latest/
>  
> diff --git a/utilities/checkpatch_dict.txt b/utilities/checkpatch_dict.txt
> index c1f43e5af..cfeea6e8b 100644
> --- a/utilities/checkpatch_dict.txt
> +++ b/utilities/checkpatch_dict.txt
> @@ -186,6 +186,7 @@ nicira
>  nics
>  ns
>  nsec
> +nuageinit
>  num
>  numa
>  odp
> @@ -196,6 +197,7 @@ ofpbufs
>  ofport
>  ofproto
>  onwards
> +opencode

Not sure if we need this.

>  openflow
>  openssl
>  openvswitch

In the end, I fed my comments to claude and after some back and forth with it
and triel and error on GHA, we canme up with the follwoing variant of this 
patch:
  https://github.com/igsilya/ovs/commit/960a455a0d47520efab2ea4bbdcf8460bd3e697c
It is significantly shorter and easier to understand, IMO.

WDYT?

Feel free to take any part or entirety of it for the v2.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to