-
Notifications
You must be signed in to change notification settings - Fork 93
Collect pod resource usage and cgroup metrics in must-gather #2494
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,102 @@ get_log_collection_args() { | |
| fi | ||
| } | ||
|
|
||
| function getPodResources() { | ||
| local ns="${1}" | ||
| local podName="${2}" | ||
| local container="${3}" | ||
| local podDirName="${podName##"pod/"}" | ||
|
|
||
| local resourcePath=${BASE_COLLECTION_PATH}/namespaces/${ns}/pods/${podDirName}/${container}/${container}/resources | ||
| mkdir -p "${resourcePath}" | ||
|
|
||
| echo "Collecting resource usage for pod ${podName} in ${ns}" | ||
|
|
||
| # Memory Info | ||
| oc exec -n "${ns}" "${podName}" -c "${container}" -- bash -c ' | ||
| curr=$(cat /sys/fs/cgroup/memory.current) | ||
| max=$(cat /sys/fs/cgroup/memory.max) | ||
| curr_kb=$((curr / 1024)) | ||
| curr_mb=$((curr / 1024 / 1024)) | ||
|
|
||
| if [ "$max" = "max" ]; then | ||
| limit_str="max (uncapped)" | ||
| else | ||
| max_kb=$((max / 1024)) | ||
| max_mb=$((max / 1024 / 1024)) | ||
| limit_str="$max bytes ($max_kb KiB / $max_mb MiB)" | ||
| fi | ||
|
|
||
| echo "Usage: $curr bytes ($curr_kb KiB / $curr_mb MiB) - Limit: $limit_str" > /tmp/memory.info | ||
| ' 2>/dev/null | ||
| oc exec -n "${ns}" "${podName}" -c "${container}" -- cat /tmp/memory.info > "${resourcePath}/memory.info" 2>&1 | ||
|
|
||
| # Get previous container termination info | ||
| oc get pod "${podName##"pod/"}" -n "${ns}" -o jsonpath="{.status.containerStatuses[?(@.name=='${container}')].lastState}" > "${resourcePath}/last.terminated" 2>/dev/null | ||
| if [ ! -s "${resourcePath}/last.terminated" ]; then | ||
| echo "No previous termination recorded" > "${resourcePath}/last.terminated" | ||
| fi | ||
|
|
||
| # Memory pressure | ||
| oc exec -n "${ns}" "${podName}" -c "${container}" -- bash -c ' | ||
| if [ -f /sys/fs/cgroup/memory.pressure ]; then | ||
| cat /sys/fs/cgroup/memory.pressure | ||
| else | ||
| echo "PSI not available on this node (kernel psi=1 not enabled)" | ||
| fi | ||
| ' > "${resourcePath}/memory.pressure" 2>&1 | ||
|
|
||
| # CPU usage | ||
| oc exec -n "${ns}" "${podName}" -c "${container}" -- bash -c " | ||
| declare -A cpu_descriptions | ||
| cpu_descriptions=( | ||
| [usage_usec]='Total CPU time consumed (user + system)' | ||
| [user_usec]='Time spent in user-space (app logic)' | ||
| [system_usec]='Time spent in kernel-space (syscalls, I/O)' | ||
| [core_sched.force_idle_usec]='Time CPU forced idle for security (cross-HT side-channel protection)' | ||
| [nr_periods]='Number of CPU quota enforcement periods (0 = no quota set)' | ||
| [nr_throttled]='Times the container was throttled for exceeding CPU quota' | ||
| [throttled_usec]='Total time spent throttled/waiting for CPU quota' | ||
| [nr_bursts]='Times container used burst CPU capacity beyond quota' | ||
| [burst_usec]='Total time spent using burst CPU capacity' | ||
| ) | ||
|
|
||
| cat /sys/fs/cgroup/cpu.stat | while read line; do | ||
| val=\$(echo \$line | awk '{print \$2}') | ||
| key=\$(echo \$line | awk '{print \$1}') | ||
| desc=\${cpu_descriptions[\$key]} | ||
| if [[ \$key == *_usec ]] && [ \$val -ge 1000000 ]; then | ||
| sec=\$(awk \"BEGIN {printf \\\"%.2f\\\", \$val/1000000}\") | ||
| echo \"\$line (\$sec s) | \$desc\" | ||
| else | ||
| echo \"\$line | \$desc\" | ||
| fi | ||
| done | ||
| " > "${resourcePath}/cpu.stat" 2>&1 | ||
|
Comment on lines
+64
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Inspect the Skupper container images used across pods to check shell availability assumptions.
rg -n 'FROM ' --type=dockerfile
fd -e DockerfileRepository: skupperproject/skupper Length of output: 195 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== relevant call sites ==\n'
rg -n 'getSkupperResourcesInNamespace|getPodResources|bash -c|declare -A|cpu\.stat' scripts/skupper-gather.sh
printf '\n== file map ==\n'
ast-grep outline scripts/skupper-gather.sh --view expanded
printf '\n== container/image hints in repo ==\n'
rg -n 'Dockerfile\.(kube-adaptor|controller|cli|system-controller|network-observer)|FROM ' -g '!**/node_modules/**' .Repository: skupperproject/skupper Length of output: 2093 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1,170p' scripts/skupper-gather.sh | cat -nRepository: skupperproject/skupper Length of output: 7333 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== script excerpt ==\n'
sed -n '1,170p' scripts/skupper-gather.sh | cat -nRepository: skupperproject/skupper Length of output: 7355
|
||
|
|
||
| # CPU limit | ||
| oc exec -n "${ns}" "${podName}" -c "${container}" -- bash -c ' | ||
| cpu_max=$(cat /sys/fs/cgroup/cpu.max) | ||
| quota=$(echo $cpu_max | awk "{print \$1}") | ||
| period=$(echo $cpu_max | awk "{print \$2}") | ||
| if [ "$quota" = "max" ]; then | ||
| echo "CPU Limit: uncapped (no quota set)" | ||
| else | ||
| cores=$(awk "BEGIN {printf \"%.2f\", $quota/$period}") | ||
| echo "CPU Limit: $quota/$period us = $cores cores" | ||
| fi | ||
| ' > "${resourcePath}/cpu.max" 2>&1 | ||
|
|
||
| # CPU pressure | ||
| oc exec -n "${ns}" "${podName}" -c "${container}" -- bash -c ' | ||
| if [ -f /sys/fs/cgroup/cpu.pressure ]; then | ||
| cat /sys/fs/cgroup/cpu.pressure | ||
| else | ||
| echo "PSI not available on this node (kernel psi=1 not enabled)" | ||
| fi | ||
| ' > "${resourcePath}/cpu.pressure" 2>&1 | ||
| } | ||
|
|
||
| function getSkupperRouterSkstatInNamespace() { | ||
| local routerNamespace="${1}" | ||
| local flags=("g" "c" "l" "n" "e" "a" "m" "p") | ||
|
|
@@ -28,7 +124,17 @@ function getSkupperRouterSkstatInNamespace() { | |
| for flag in "${flags[@]}"; do | ||
| oc exec -n "${sitens}" "${routerName}" -c router -- skstat -"${flag}" > "${logPath}/skstat.${flag}" 2>&1 | ||
| done | ||
| done | ||
| done | ||
| } | ||
|
|
||
| function getSkupperResourcesInNamespace() { | ||
| local ns="${1}" | ||
|
|
||
| for podName in $(oc get pods -n "${ns}" -l app.kubernetes.io/part-of=skupper -oname); do | ||
| for container in $(oc get "${podName}" -n "${ns}" -o jsonpath='{.spec.containers[*].name}'); do | ||
| getPodResources "${ns}" "${podName}" "${container}" | ||
| done | ||
| done | ||
| } | ||
|
|
||
| function getCRDs() { | ||
|
|
@@ -113,12 +219,14 @@ function main() { | |
| inspectNamespace "${sitens}" | ||
|
|
||
| getSkupperRouterSkstatInNamespace "${sitens}" | ||
| getSkupperResourcesInNamespace "${sitens}" | ||
| done | ||
|
|
||
| # iterate over all namespaces which have AttachedConnectors | ||
| for acns in $(oc get attachedconnector -A -o=jsonpath="{.items[*].metadata.namespace}"); do | ||
| echo "Inspecting AttachedConnector in ${acns} namespace" | ||
| inspectNamespace "${acns}" | ||
| getSkupperResourcesInNamespace "${acns}" | ||
| done | ||
|
|
||
| echo | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Avoid the intermediate
/tmpfile and secondoc execround trip.Unlike the other cgroup blocks (pressure, cpu.stat, cpu.max) which write directly to stdout and are captured via output redirection in a single
oc exec, memory.info is written to a hardcoded/tmp/memory.infoinside the container and then read back with a secondoc exec. Static analysis flags this as an insecure/predictable temp file (CWE-377), and it also doubles the exec calls needlessly.🛡️ Proposed fix: write directly to stdout like the other blocks
📝 Committable suggestion
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 46-46: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g.
tmpfile="$(mktemp)"(ormktemp -dfor directories) and reference "$tmpfile".Context: /tmp/memory.info
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
Source: Linters/SAST tools