Skip to content

must-gather/gather_sriov: parallelize and optimize per-node SR-IOV collection #540

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gavrielg1 wants to merge 3 commits into
base: main
Choose a base branch
from
+173 −81
Open

must-gather/gather_sriov: parallelize and optimize per-node SR-IOV collection #540

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
95dd7a1
must-gather/gather_sriov: parallelize and optimize per-node SR-IOV co…
Apr 15, 2026
0c207b8
Update gather_sriov with systemd service collection
gavrielg1 May 26, 2026
cd13d11
Merge branch 'main' into improve-gather-sriov
gavrielg1 May 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
258 changes: 178 additions & 80 deletions collection-scripts/gather_sriov
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ get_operator_ns "sriov-network-operator"
get_log_collection_args
SRIOV_LOG_PATH="${BASE_COLLECTION_PATH}/namespaces/${operator_ns}"

# Kernel log window when MUST_GATHER_SINCE* / node_log_collection_args are unset (journalctl --since=…).
SRIOV_JOURNAL_SINCE="${SRIOV_JOURNAL_SINCE:-${SINCE_TIMEFRAME:--2d}}"

# resource list
resources=()

Expand All @@ -15,119 +18,214 @@ resources+=(ns/${operator_ns})
# sriovnetwork.openshift.io
resources+=(sriovnetworknodepolicies sriovnetworknodestates sriovnetworkpoolconfigs sriovnetworks sriovoperatorconfigs sriovibnetworks)

# run the collection of resources using must-gather
for resource in ${resources[@]}; do
oc adm inspect ${log_collection_args} --dest-dir must-gather --all-namespaces ${resource}
PIDS=()
for resource in "${resources[@]}"; do
oc adm inspect ${log_collection_args} --dest-dir must-gather --all-namespaces ${resource} &
PIDS+=($!)
done

CONFIG_DAEMON_PODS="${@:-$(oc -n ${operator_ns} get pods -l app=sriov-network-config-daemon -o jsonpath='{.items[*].metadata.name}')}"
PIDS=()
if [ "$#" -gt 0 ]; then
CONFIG_DAEMON_PODS=( "$@" )
else
if ! pod_names=$(oc -n "${operator_ns}" get pods -l app=sriov-network-config-daemon -o jsonpath='{.items[*].metadata.name}'); then
echo "ERROR: failed to list sriov-network-config-daemon pods" >&2
wait "${PIDS[@]}"
sync
exit 1
fi
read -r -a CONFIG_DAEMON_PODS <<< "${pod_names}"
fi

if [ "${#CONFIG_DAEMON_PODS[@]}" -eq 0 ]; then
echo "INFO: No sriov-network-config-daemon pods found; skipping per-node SR-IOV collection."
wait "${PIDS[@]}"
sync
exit 0
fi

function kernel_journal_since_args() {
if [ -n "${node_log_collection_args}" ]; then
echo "${node_log_collection_args}"
else
local s="${SRIOV_JOURNAL_SINCE}"
case "${s}" in
-*) echo "--since=${s}" ;;
*) echo "--since=-${s}" ;;
esac
fi
}

# gather_netns_ip_a runs ip netns, and for every net namespace runs `ip netns exec <id> ip a`
function gather_netns_ip_a() {
CONFIG_DAEMON_POD_LOG_PATH="${SRIOV_LOG_PATH}/pods/${1}"
NETNS_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/netns"
NETNS_IP_A_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/netns_ip_a"

# Filter out everything after network namespace name
oc exec -n ${operator_ns} ${1} -c sriov-network-config-daemon -- chroot /host /bin/bash -c "ip netns | sed "s/ .*//"" >"${NETNS_LOG_PATH}" 2>&1

while IFS= read -r id; do
echo "> ip netns exec ${id} ip a" >>"${NETNS_IP_A_LOG_PATH}" &&
oc exec -n ${operator_ns} "${1}" -c sriov-network-config-daemon -- chroot /host \
/bin/bash -c "ip netns exec ${id} ip a" >>"${NETNS_IP_A_LOG_PATH}" 2>&1
done <"${NETNS_LOG_PATH}"
local pod="$1"
local CONFIG_DAEMON_POD_LOG_PATH="${SRIOV_LOG_PATH}/pods/${pod}"
local NETNS_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/netns"
local NETNS_IP_A_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/netns_ip_a"

oc exec -n "${operator_ns}" "${pod}" -c sriov-network-config-daemon -- chroot /host \
/bin/bash -c "ip netns | sed 's/ .*//' " >"${NETNS_LOG_PATH}" 2>&1

oc exec -n "${operator_ns}" "${pod}" -c sriov-network-config-daemon -i -- chroot /host /bin/bash -s <<'NETNS_SCRIPT' >"${NETNS_IP_A_LOG_PATH}" 2>&1
for id in $(ip netns 2>/dev/null | sed 's/ .*//'); do
[ -z "${id}" ] && continue
echo "> ip netns exec ${id} ip a"
ip netns exec "${id}" ip a 2>/dev/null
done
NETNS_SCRIPT
}

function gather_ethtool() {
CONFIG_DAEMON_POD_LOG_PATH="${SRIOV_LOG_PATH}/pods/${1}"
ETHTOOL_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/ethtool"

# Get ip -o link show output.
OUT="$(oc exec -n ${operator_ns} ${1} -c sriov-network-config-daemon -- chroot /host \
/bin/bash -c "ip -o link show 2>/dev/null")"

# Cut long interfaces names.
INTERFACES="$(echo "$OUT" | awk -F': ' '{print $2}')"

# Run ethtool for each interface except for lo.
while IFS= read -r interface; do
if [ -z "$interface" ] || [ "$interface" = "lo" ] || [[ "$interface" == *"@"* ]]; then
continue
fi
echo "> ethtool -i ${interface}" >>"${ETHTOOL_LOG_PATH}"
oc exec -n ${operator_ns} ${1} -c sriov-network-config-daemon -- chroot /host \
/bin/bash -c "ethtool -i ${interface}" >>"${ETHTOOL_LOG_PATH}"
done <<<"$INTERFACES"
local pod="$1"
local CONFIG_DAEMON_POD_LOG_PATH="${SRIOV_LOG_PATH}/pods/${pod}"
local ETHTOOL_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/ethtool"

oc exec -n "${operator_ns}" "${pod}" -c sriov-network-config-daemon -i -- chroot /host /bin/bash -s <<'ETHTOOL_SCRIPT' >"${ETHTOOL_LOG_PATH}" 2>&1
while IFS= read -r interface; do
[ -z "${interface}" ] && continue
[ "${interface}" = lo ] && continue
[[ "${interface}" == *"@"* ]] && continue
echo "> ethtool -i ${interface}"
ethtool -i "${interface}" 2>/dev/null
done <<< "$(ip -o link show 2>/dev/null | awk -F': ' '{print $2}')"
ETHTOOL_SCRIPT
}

function gather_bf_info() {
CONFIG_DAEMON_POD_LOG_PATH="${SRIOV_LOG_PATH}/pods/${1}"
MSTCONFIG_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/mstconfig"
MSTFLINT_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/mstflint"
local pod="$1"
local CONFIG_DAEMON_POD_LOG_PATH="${SRIOV_LOG_PATH}/pods/${pod}"
local MSTCONFIG_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/mstconfig"
local MSTFLINT_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/mstflint"

devices=$(oc exec -n openshift-sriov-network-operator ${1} -c sriov-network-config-daemon -- mstconfig q 2>/dev/null | grep "Device:" | awk 'BEGIN {FS="/";} { print $6 }')
local devices
devices=$(oc exec -n "${operator_ns}" "${pod}" -c sriov-network-config-daemon -- mstconfig q 2>/dev/null | grep "Device:" | awk 'BEGIN {FS="/";} { print $6 }')
if [ -z "${devices}" ]; then
echo "No Mellanox devices detected by mstconfig" | tee -a "${MSTCONFIG_LOG_PATH}" "${MSTFLINT_LOG_PATH}" >/dev/null
return
fi
for device in $devices; do
oc exec -n ${operator_ns} ${1} -c sriov-network-config-daemon -- \
/bin/bash -c "mstconfig -e -d $device q" >>"${MSTCONFIG_LOG_PATH}"
oc exec -n "${operator_ns}" "${pod}" -c sriov-network-config-daemon -i -- \
/bin/bash -c 'while IFS= read -r d; do mstconfig -e -d "$d" q; done' \
>>"${MSTCONFIG_LOG_PATH}" 2>&1 <<< "${devices}"

oc exec -n ${operator_ns} ${1} -c sriov-network-config-daemon -- \
/bin/bash -c "mstflint -d $device q" >>"${MSTFLINT_LOG_PATH}"
done
oc exec -n "${operator_ns}" "${pod}" -c sriov-network-config-daemon -i -- \
/bin/bash -c 'while IFS= read -r d; do mstflint -d "$d" q; done' \
>>"${MSTFLINT_LOG_PATH}" 2>&1 <<< "${devices}"
}

for CONFIG_DAEMON_POD in ${CONFIG_DAEMON_PODS[@]}; do
# gather_hardware_info collects PCI and driver info.
function gather_hardware_info() {
local pod="$1"
local CONFIG_DAEMON_POD_LOG_PATH="${SRIOV_LOG_PATH}/pods/${pod}"
local LSPCI_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/lspci"
local IP_DETAIL_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/ip_link_d"

oc exec -n "${operator_ns}" "${pod}" -c sriov-network-config-daemon -- chroot /host \
/bin/bash -c 'command -v lspci >/dev/null 2>&1 && lspci -nnk 2>/dev/null || { test -x /usr/sbin/lspci && /usr/sbin/lspci -nnk 2>/dev/null; }' \
>"${LSPCI_LOG_PATH}" 2>&1

oc exec -n "${operator_ns}" "${pod}" -c sriov-network-config-daemon -- chroot /host \
/bin/bash -c 'echo "=== ip -d link ==="; \
ip -d link show 2>/dev/null; \
echo "=== ip -s link ==="; \
ip -s link show 2>/dev/null' \
>"${IP_DETAIL_LOG_PATH}" 2>&1
}

CONFIG_DAEMON_POD_LOG_PATH="${SRIOV_LOG_PATH}/pods/${CONFIG_DAEMON_POD}"
SNO_INITIAL_NODE_STATE_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/sno-initial-node-state.json"
KERNEL_CMDLINE_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/kernel-cmdline"
IP_LINK_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/ip_link"
DMESG_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/dmesg"
MULTUS_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/multus-log"
VAR_LIB_CNI_SRIOV_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/var-lib-cni-sriov"
function gather_sriov_service_journal() {
local pod="$1"
local CONFIG_DAEMON_POD_LOG_PATH="${SRIOV_LOG_PATH}/pods/${pod}"
local JOURNAL_LOG_DIR="${CONFIG_DAEMON_POD_LOG_PATH}/sriov-service-journal"

# Collect sno-initial-node-state.json.
oc exec -n ${operator_ns} "${CONFIG_DAEMON_POD}" -c sriov-network-config-daemon -- chroot /host \
/bin/bash -c "cat /tmp/sno-initial-node-state.json" >${SNO_INITIAL_NODE_STATE_LOG_PATH} &
PIDS+=($!)
SRIOV_SERVICES="sriov-config.service sriov-config-post-network.service"
MAX_BOOTS=5

# Collect kernel cmdline.
oc exec -n ${operator_ns} "${CONFIG_DAEMON_POD}" -c sriov-network-config-daemon -- chroot /host \
/bin/bash -c "cat /proc/cmdline" >"${KERNEL_CMDLINE_LOG_PATH}" &
PIDS+=($!)
boot_count=$(oc exec -n ${operator_ns} "${pod}" -c sriov-network-config-daemon -- chroot /host \
/bin/bash -c "journalctl --list-boots --no-pager 2>/dev/null | wc -l" 2>/dev/null) || true
boot_count=${boot_count//[!0-9]/}
boot_count=${boot_count:-1}

# Collect ip link.
oc exec -n ${operator_ns} "${CONFIG_DAEMON_POD}" -c sriov-network-config-daemon -- chroot /host \
/bin/bash -c "ip link" >"${IP_LINK_LOG_PATH}" 2>&1 &
PIDS+=($!)
if [ "${boot_count}" -gt "${MAX_BOOTS}" ]; then
boot_count=${MAX_BOOTS}
fi

# Collect dmesg.
oc exec -n ${operator_ns} "${CONFIG_DAEMON_POD}" -c sriov-network-config-daemon -- chroot /host \
/bin/bash -c "journalctl -k" >"${DMESG_LOG_PATH}" &
PIDS+=($!)
mkdir -p "${JOURNAL_LOG_DIR}"

for service in ${SRIOV_SERVICES}; do
service_name="${service%.service}"
for (( boot_idx=0; boot_idx<boot_count; boot_idx++ )); do
log_file="${JOURNAL_LOG_DIR}/${service_name}_boot-${boot_idx}.log"
oc exec -n ${operator_ns} "${pod}" -c sriov-network-config-daemon -- chroot /host \
/bin/bash -c "journalctl -u ${service} -b -${boot_idx} --no-pager 2>/dev/null" >"${log_file}" 2>&1 || true
if [ ! -s "${log_file}" ]; then
rm -f "${log_file}"
fi
done
done

# Collect var/log/multus.log if exists.
out=$(oc exec -n ${operator_ns} "${CONFIG_DAEMON_POD}" -c sriov-network-config-daemon -- chroot /host \
/bin/bash -c "cat var/log/multus.log" 2>/dev/null) && echo "$out" 1>"${MULTUS_LOG_PATH}" &
PIDS+=($!)
# Clean up directory if no logs were collected (systemd mode not enabled)
rmdir "${JOURNAL_LOG_DIR}" 2>/dev/null || true
}

function gather_sriov_for_pod() {
local gather_sriov_pids=()
local CONFIG_DAEMON_POD="$1"
local CONFIG_DAEMON_POD_LOG_PATH="${SRIOV_LOG_PATH}/pods/${CONFIG_DAEMON_POD}"
local SNO_INITIAL_NODE_STATE_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/sno-initial-node-state.json"
local KERNEL_CMDLINE_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/kernel-cmdline"
local DMESG_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/dmesg.gz"
local MULTUS_LOG_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/multus-log.gz"
local VAR_LIB_CNI_SRIOV_PATH="${CONFIG_DAEMON_POD_LOG_PATH}/var-lib-cni-sriov"

mkdir -p "${CONFIG_DAEMON_POD_LOG_PATH}"

local since_arg
since_arg=$(kernel_journal_since_args)

# Collect sno-initial-node-state.json.
oc exec -n "${operator_ns}" "${CONFIG_DAEMON_POD}" -c sriov-network-config-daemon -- chroot /host \
/bin/bash -c "cat /tmp/sno-initial-node-state.json" >"${SNO_INITIAL_NODE_STATE_LOG_PATH}" 2>&1 &
gather_sriov_pids+=($!)

# Collect kernel cmdline.
oc exec -n "${operator_ns}" "${CONFIG_DAEMON_POD}" -c sriov-network-config-daemon -- chroot /host \
/bin/bash -c "cat /proc/cmdline" >"${KERNEL_CMDLINE_LOG_PATH}" 2>&1 &
gather_sriov_pids+=($!)

# Collect dmesg logs.
( set -o pipefail; oc exec -n "${operator_ns}" "${CONFIG_DAEMON_POD}" -c sriov-network-config-daemon -- chroot /host \
/bin/bash -c "journalctl -k ${since_arg} --no-pager" 2>/dev/null | gzip >"${DMESG_LOG_PATH}" ) \
|| rm -f "${DMESG_LOG_PATH}" &
gather_sriov_pids+=($!)

# Collect multus.log if exists.
( set -o pipefail; oc exec -n "${operator_ns}" "${CONFIG_DAEMON_POD}" -c sriov-network-config-daemon -- chroot /host /bin/bash -c \
"test -f var/log/multus.log && cat var/log/multus.log" 2>/dev/null | gzip >"${MULTUS_LOG_PATH}" ) \
|| rm -f "${MULTUS_LOG_PATH}" &
Comment on lines +198 to +201

@coderabbitai coderabbitai Bot May 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use an absolute path for multus.log inside the chroot.

Line 200 checks var/log/multus.log, so this capture depends on the daemon container's working directory after chroot. If that directory is not /, the probe fails and multus-log.gz gets dropped even though /var/log/multus.log exists.

Suggested fix 
  ( set -o pipefail; oc exec -n "${operator_ns}" "${CONFIG_DAEMON_POD}" -c sriov-network-config-daemon -- chroot /host /bin/bash -c \
-		"test -f var/log/multus.log && cat var/log/multus.log" 2>/dev/null | gzip >"${MULTUS_LOG_PATH}" ) \
+		"test -f /var/log/multus.log && cat /var/log/multus.log" 2>/dev/null | gzip >"${MULTUS_LOG_PATH}" ) \
  		|| rm -f "${MULTUS_LOG_PATH}" &
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Collect multus.log if exists.
( set -o pipefail; oc exec -n "${operator_ns}" "${CONFIG_DAEMON_POD}" -c sriov-network-config-daemon -- chroot /host /bin/bash -c \
"test -f var/log/multus.log && cat var/log/multus.log" 2>/dev/null | gzip >"${MULTUS_LOG_PATH}" ) \
|| rm -f "${MULTUS_LOG_PATH}" &
# Collect multus.log if exists.
( set -o pipefail; oc exec -n "${operator_ns}" "${CONFIG_DAEMON_POD}" -c sriov-network-config-daemon -- chroot /host /bin/bash -c \
"test -f /var/log/multus.log && cat /var/log/multus.log" 2>/dev/null | gzip >"${MULTUS_LOG_PATH}" ) \
|| rm -f "${MULTUS_LOG_PATH}" &
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@collection-scripts/gather_sriov` around lines 198 - 201, The chrooted command
in the block that collects multus.log uses a relative path "var/log/multus.log",
which can fail if the chrooted process's working directory is not root; update
the chrooted test and cat invocations executed by the oc exec for
CONFIG_DAEMON_POD (container sriov-network-config-daemon in namespace
operator_ns) to use the absolute path /var/log/multus.log so MULTUS_LOG_PATH
reliably receives the gzipped content when the file exists.

gather_sriov_pids+=($!)

# Collect /var/lib/cni/sriov contents.
oc cp -n ${operator_ns} -c sriov-network-config-daemon "${CONFIG_DAEMON_POD}":/host/var/lib/cni/sriov "${VAR_LIB_CNI_SRIOV_PATH}" &
PIDS+=($!)
oc cp -n "${operator_ns}" -c sriov-network-config-daemon "${CONFIG_DAEMON_POD}":/host/var/lib/cni/sriov "${VAR_LIB_CNI_SRIOV_PATH}" &>/dev/null || true &
gather_sriov_pids+=($!)

gather_sriov_service_journal "${CONFIG_DAEMON_POD}" &
gather_sriov_pids+=($!)

gather_netns_ip_a "${CONFIG_DAEMON_POD}" &
PIDS+=($!)
gather_sriov_pids+=($!)

gather_ethtool "${CONFIG_DAEMON_POD}" &
PIDS+=($!)
gather_sriov_pids+=($!)

gather_bf_info "${CONFIG_DAEMON_POD}" &
PIDS+=($!)
gather_sriov_pids+=($!)

gather_hardware_info "${CONFIG_DAEMON_POD}" &
gather_sriov_pids+=($!)

wait "${gather_sriov_pids[@]}"
}

for CONFIG_DAEMON_POD in "${CONFIG_DAEMON_PODS[@]}"; do
gather_sriov_for_pod "${CONFIG_DAEMON_POD}" &
PIDS+=($!)
Comment on lines +226 to +228

@coderabbitai coderabbitai Bot May 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Bound the outer pod fan-out before this overwhelms larger clusters.

Each gather_sriov_for_pod already starts a large batch of background oc exec/oc cp calls. Launching one per daemon pod with no limit turns this into unbounded nested parallelism, which can easily swamp the API server on bigger fleets or during degraded must-gather runs.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@collection-scripts/gather_sriov` around lines 226 - 228, The loop that starts
gather_sriov_for_pod for each CONFIG_DAEMON_POD currently launches all pods in
parallel and can cause unbounded nested parallelism; introduce a concurrency
limit (e.g., MAX_POD_FANOUT) and throttle the loop so you only start a new
background gather_sriov_for_pod when the number of in-flight jobs is below that
limit—use the existing PIDS array (or job control with wait -n) to track running
jobs, wait for one to finish when the limit is reached, then push the new PID
into PIDS; update the loop that iterates CONFIG_DAEMON_PODS and any bookkeeping
around PIDS accordingly.

done

echo "INFO: Waiting for sriov info collection to complete ..."
Expand Down