Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion config/k3s/containerd.mount
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Description=Dedicated containerd filesystem for K3s
Before=k3s.service

[Mount]
What=/dev/disk/by-label/k3s-containerd
What=/dev/disk/by-uuid/90f29a7b-38ff-460b-b534-92a02f1412ec
Where=/var/lib/rancher/k3s/agent/containerd
Type=ext4
Options=noatime
Expand Down
49 changes: 49 additions & 0 deletions config/k3s/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,35 @@ let
serverActivateScript = pkgs.replaceVars ./activate.sh {
inherit galacticaAuthorizedKey;
};
alertScript = pkgs.writeShellApplication {
name = "kyber-host-alert";
runtimeInputs = [ pkgs.util-linux ];
text = builtins.readFile ./kyber-host-alert.sh;
};
healthCheckScript = pkgs.writeShellApplication {
name = "kyber-host-health";
runtimeInputs = [
alertScript
pkgs.coreutils
pkgs.gawk
pkgs.gnugrep
pkgs.k3s
pkgs.procps
pkgs.systemd
pkgs.util-linux
];
text = builtins.readFile ./kyber-host-health.sh;
};
healthCheckService = pkgs.replaceVars ./kyber-host-health.service {
inherit healthCheckScript;
};
smartdConfig = pkgs.replaceVars ./kyber-smartd.conf {
inherit alertScript;
};
smartdService = pkgs.replaceVars ./kyber-smartd.service {
smartd = "${pkgs.smartmontools}/bin/smartd";
inherit smartdConfig;
};
in
{
home.file.".config/k3s/config.yaml" = lib.mkIf isKyber {
Expand All @@ -38,6 +67,26 @@ in
force = true;
};

home.file.".config/k3s/journald.conf.d/10-kyber-limits.conf" = lib.mkIf isKyber {
source = ./journald.conf;
force = true;
};

home.file.".config/k3s/kyber-host-health.service" = lib.mkIf isKyber {
source = healthCheckService;
force = true;
};

home.file.".config/k3s/kyber-host-health.timer" = lib.mkIf isKyber {
source = ./kyber-host-health.timer;
force = true;
};

home.file.".config/k3s/kyber-smartd.service" = lib.mkIf isKyber {
source = smartdService;
force = true;
};

home.sessionVariables = lib.mkIf (isKyber || isGalactica) {
KUBECONFIG = kubeconfig;
};
Expand Down
7 changes: 7 additions & 0 deletions config/k3s/journald.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[Journal]
Storage=persistent
SystemMaxUse=2G
SystemKeepFree=10G
RuntimeMaxUse=256M
MaxRetentionSec=7day
MaxFileSec=1day
28 changes: 18 additions & 10 deletions config/k3s/kubelet.conf
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
apiVersion: kubelet.config.k8s.io/v1beta1
kind: KubeletConfiguration
# Keep limited parallelism for faster cold starts without allowing a full-node
# restart to saturate containerd, disk I/O, and CRI request deadlines.
serializeImagePulls: false
maxParallelImagePulls: 2
# Make the single-node disk contract explicit. The ext4 root reserve is managed
# by the Kyber activation script, keeping ordinary usage below the low watermark
# while kubelet remains the sole owner of image and container garbage collection.
# These are kubelet's defaults, pinned here so the host headroom contract is visible.
imageGCHighThresholdPercent: 85
imageGCLowThresholdPercent: 80
# Serialize image pulls on the single Kyber containerd SSD. A broad restart must
# not fan out concurrent downloads, unpacking, and snapshot writes.
serializeImagePulls: true
# Start image garbage collection early and preserve enough space for image
# unpacking before kubelet reaches an eviction threshold.
imageGCHighThresholdPercent: 70
imageGCLowThresholdPercent: 60
# Keep at least twenty percent free on both the root/control-plane filesystem
# and the dedicated image filesystem. Kubelet remains the sole CRI collector.
evictionHard:
memory.available: "500Mi"
nodefs.available: "20%"
imagefs.available: "20%"
nodefs.inodesFree: "10%"
imagefs.inodesFree: "10%"
# Bound the CRI-managed container log files using kubelet's native rotation.
containerLogMaxSize: 10Mi
containerLogMaxFiles: 3
9 changes: 9 additions & 0 deletions config/k3s/kyber-host-alert.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/usr/bin/env bash
set -euo pipefail

source_name="${1:-${SMARTD_DEVICE:-kyber-host-health}}"
message="${2:-${SMARTD_MESSAGE:-Kyber host reliability alert}}"
alert="${source_name}: ${message}"

logger --priority daemon.alert --tag kyber-host-health -- "$alert"
printf 'KYBER ALERT: %s\n' "$alert" | wall --nobanner || true
7 changes: 7 additions & 0 deletions config/k3s/kyber-host-health.service
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[Unit]
Description=Check Kyber storage and CRI reliability
After=var-lib-rancher-k3s-agent-containerd.mount k3s.service

[Service]
Type=oneshot
ExecStart=@healthCheckScript@/bin/kyber-host-health
121 changes: 121 additions & 0 deletions config/k3s/kyber-host-health.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
#!/usr/bin/env bash
set -euo pipefail

readonly EXPECTED_CONTAINERD_UUID="90f29a7b-38ff-460b-b534-92a02f1412ec"
readonly CONTAINERD_MOUNT="/var/lib/rancher/k3s/agent/containerd"
readonly STATE_DIR="/run/kyber-host-health"
readonly D_STATE_THRESHOLD=3
readonly D_STATE_SUSTAINED_SAMPLES=5
readonly IO_SOME_AVG300_THRESHOLD=20
readonly IO_FULL_AVG300_THRESHOLD=10
readonly IMAGEFS_USAGE_THRESHOLD=70
readonly CRI_LATENCY_THRESHOLD_SECONDS=5
readonly CRI_ERROR_THRESHOLD=5

install -d --mode 0755 "$STATE_DIR"

set_alert() {
local key="$1"
local message="$2"
local marker="$STATE_DIR/${key}.alerted"

if [ ! -e "$marker" ]; then
kyber-host-alert "$key" "$message"
: >"$marker"
fi
}

clear_alert() {
local key="$1"
local marker="$STATE_DIR/${key}.alerted"

if [ -e "$marker" ]; then
logger --priority daemon.notice --tag kyber-host-health -- "$key recovered"
rm -f "$marker"
fi
}

check_io_pressure() {
local some_avg300 full_avg300
Comment on lines +38 to +39

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If /proc/pressure/io does not exist (e.g., on kernels where PSI is disabled or unsupported), the awk command will fail. Because set -e is enabled, this failure will prematurely abort the entire health check script, preventing subsequent checks from running.

Adding a guard to check for the existence of /proc/pressure/io ensures the script handles this state gracefully.

Suggested change
check_io_pressure() {
local some_avg300 full_avg300
check_io_pressure() {
if [ ! -f /proc/pressure/io ]; then
return 0
fi
local some_avg300 full_avg300
References
  1. When 'set -e' is enabled in Bash scripts, ensure that individual command failures do not prematurely abort the entire script. Handle potential failures gracefully.


# shellcheck disable=SC2016
some_avg300="$(awk '$1 == "some" { for (i = 1; i <= NF; i++) if ($i ~ /^avg300=/) { sub(/^avg300=/, "", $i); print $i } }' /proc/pressure/io)"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Guard /proc/pressure/io before running awk. On hosts without PSI support, this command exits non-zero and set -e stops the health script before the remaining checks run.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/k3s/kyber-host-health.sh, line 42:

<comment>Guard `/proc/pressure/io` before running `awk`. On hosts without PSI support, this command exits non-zero and `set -e` stops the health script before the remaining checks run.</comment>

<file context>
@@ -0,0 +1,121 @@
+  local some_avg300 full_avg300
+
+  # shellcheck disable=SC2016
+  some_avg300="$(awk '$1 == "some" { for (i = 1; i <= NF; i++) if ($i ~ /^avg300=/) { sub(/^avg300=/, "", $i); print $i } }' /proc/pressure/io)"
+  # shellcheck disable=SC2016
+  full_avg300="$(awk '$1 == "full" { for (i = 1; i <= NF; i++) if ($i ~ /^avg300=/) { sub(/^avg300=/, "", $i); print $i } }' /proc/pressure/io)"
</file context>

# shellcheck disable=SC2016
full_avg300="$(awk '$1 == "full" { for (i = 1; i <= NF; i++) if ($i ~ /^avg300=/) { sub(/^avg300=/, "", $i); print $i } }' /proc/pressure/io)"
Comment on lines +38 to +44

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Gracefully handle missing PSI file to prevent premature script failure.

If the system lacks PSI capabilities or /proc/pressure/io is not readable, awk will throw a fatal error. Due to set -e, this will cause the entire health check script to crash, preventing subsequent checks (like check_d_state and check_image_filesystem) from running.

Add a readability check to return early if the file does not exist.

🛠️ Proposed fix
 check_io_pressure() {
+  if [ ! -r /proc/pressure/io ]; then
+    return 0
+  fi
   local some_avg300 full_avg300
 
   # shellcheck disable=SC2016
📝 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
check_io_pressure() {
local some_avg300 full_avg300
# shellcheck disable=SC2016
some_avg300="$(awk '$1 == "some" { for (i = 1; i <= NF; i++) if ($i ~ /^avg300=/) { sub(/^avg300=/, "", $i); print $i } }' /proc/pressure/io)"
# shellcheck disable=SC2016
full_avg300="$(awk '$1 == "full" { for (i = 1; i <= NF; i++) if ($i ~ /^avg300=/) { sub(/^avg300=/, "", $i); print $i } }' /proc/pressure/io)"
check_io_pressure() {
if [ ! -r /proc/pressure/io ]; then
return 0
fi
local some_avg300 full_avg300
# shellcheck disable=SC2016
some_avg300="$(awk '$1 == "some" { for (i = 1; i <= NF; i++) if ($i ~ /^avg300=/) { sub(/^avg300=/, "", $i); print $i } }' /proc/pressure/io)"
# shellcheck disable=SC2016
full_avg300="$(awk '$1 == "full" { for (i = 1; i <= NF; i++) if ($i ~ /^avg300=/) { sub(/^avg300=/, "", $i); print $i } }' /proc/pressure/io)"
🤖 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 `@config/k3s/kyber-host-health.sh` around lines 38 - 44, Update
check_io_pressure to test whether /proc/pressure/io is readable before invoking
either awk command; if it is unavailable, return successfully from the function
so set -e does not terminate the script and subsequent health checks still run.


if awk -v some="$some_avg300" -v full="$full_avg300" -v some_limit="$IO_SOME_AVG300_THRESHOLD" -v full_limit="$IO_FULL_AVG300_THRESHOLD" 'BEGIN { exit !(some >= some_limit || full >= full_limit) }'; then
set_alert "io-pressure" "sustained I/O PSI is elevated (some avg300=${some_avg300}, full avg300=${full_avg300})"
else
clear_alert "io-pressure"
fi
}

check_d_state() {
local count_file="$STATE_DIR/d-state.samples"
local d_state_count previous_samples=0 samples=0

d_state_count="$(ps --no-headers -eo stat= | awk '$1 ~ /^D/ { count++ } END { print count + 0 }')"
if [ -r "$count_file" ]; then
read -r previous_samples <"$count_file" || previous_samples=0
fi

if [ "$d_state_count" -ge "$D_STATE_THRESHOLD" ]; then
samples=$((previous_samples + 1))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Latent: corrupted state file crashes the health check.

previous_samples is read from /run/kyber-host-health/d-state.samples and fed directly into $((previous_samples + 1)). If that file ever contains non-numeric content (a partial write, a manual edit, or a truncated line), arithmetic evaluation fails and, under set -euo pipefail at the top of the file, the whole script exits non-zero — no subsequent check runs, and since the monitor is Type=oneshot the timer simply logs a failed unit and moves on. On tmpfs the file is normally rewritten on every run, so this is dormant today, but any interrupted write leaves the reliability monitor silently disabled until reboot.

A one-line guard makes this resilient:

if [ -r "$count_file" ]; then
  read -r previous_samples <"$count_file" || previous_samples=0
  [[ "$previous_samples" =~ ^[0-9]+$ ]] || previous_samples=0
fi

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P3: Validate previous_samples before arithmetic. A malformed state file can break this calculation and terminate the health run early under set -e.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/k3s/kyber-host-health.sh, line 63:

<comment>Validate `previous_samples` before arithmetic. A malformed state file can break this calculation and terminate the health run early under `set -e`.</comment>

<file context>
@@ -0,0 +1,121 @@
+  fi
+
+  if [ "$d_state_count" -ge "$D_STATE_THRESHOLD" ]; then
+    samples=$((previous_samples + 1))
+  fi
+  printf '%s\n' "$samples" >"$count_file"
</file context>

fi
printf '%s\n' "$samples" >"$count_file"

if [ "$samples" -ge "$D_STATE_SUSTAINED_SAMPLES" ]; then
set_alert "d-state" "${d_state_count} processes have remained in uninterruptible sleep for ${samples} consecutive samples"
elif [ "$samples" -eq 0 ]; then
clear_alert "d-state"
fi
}

check_image_filesystem() {
local mounted_source mounted_uuid usage_percent

if ! findmnt --mountpoint "$CONTAINERD_MOUNT" >/dev/null 2>&1; then
set_alert "image-filesystem" "$CONTAINERD_MOUNT is not mounted"
return
fi

mounted_source="$(findmnt --noheadings --output SOURCE --target "$CONTAINERD_MOUNT")"
mounted_uuid="$(blkid --match-tag UUID --output value "$mounted_source")"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: An unreadable or unsupported mounted source makes blkid terminate this script instead of producing the image-filesystem alert. Treat an empty UUID as a mismatch so the failed identity check remains visible and later health checks still run.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/k3s/kyber-host-health.sh, line 83:

<comment>An unreadable or unsupported mounted source makes `blkid` terminate this script instead of producing the image-filesystem alert. Treat an empty UUID as a mismatch so the failed identity check remains visible and later health checks still run.</comment>

<file context>
@@ -0,0 +1,121 @@
+  fi
+
+  mounted_source="$(findmnt --noheadings --output SOURCE --target "$CONTAINERD_MOUNT")"
+  mounted_uuid="$(blkid --match-tag UUID --output value "$mounted_source")"
+  if [ "$mounted_uuid" != "$EXPECTED_CONTAINERD_UUID" ]; then
+    set_alert "image-filesystem" "$CONTAINERD_MOUNT has UUID $mounted_uuid, expected $EXPECTED_CONTAINERD_UUID"
</file context>
Suggested change
mounted_uuid="$(blkid --match-tag UUID --output value "$mounted_source")"
mounted_uuid="$(blkid --match-tag UUID --output value "$mounted_source" 2>/dev/null || true)"

if [ "$mounted_uuid" != "$EXPECTED_CONTAINERD_UUID" ]; then
Comment on lines +82 to +84

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Prevent script termination if the filesystem lacks a UUID.

If the mounted device lacks a UUID (e.g., due to corruption or unexpected filesystem type), blkid will exit with status 2. Under set -e, this will crash the script before it can trigger the alert or proceed to other health checks.

Append || true so that an empty mounted_uuid correctly triggers the mismatch alert.

🛠️ Proposed fix
   mounted_source="$(findmnt --noheadings --output SOURCE --target "$CONTAINERD_MOUNT")"
-  mounted_uuid="$(blkid --match-tag UUID --output value "$mounted_source")"
+  mounted_uuid="$(blkid --match-tag UUID --output value "$mounted_source" || true)"
   if [ "$mounted_uuid" != "$EXPECTED_CONTAINERD_UUID" ]; then
📝 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
mounted_source="$(findmnt --noheadings --output SOURCE --target "$CONTAINERD_MOUNT")"
mounted_uuid="$(blkid --match-tag UUID --output value "$mounted_source")"
if [ "$mounted_uuid" != "$EXPECTED_CONTAINERD_UUID" ]; then
mounted_source="$(findmnt --noheadings --output SOURCE --target "$CONTAINERD_MOUNT")"
mounted_uuid="$(blkid --match-tag UUID --output value "$mounted_source" || true)"
if [ "$mounted_uuid" != "$EXPECTED_CONTAINERD_UUID" ]; then
🤖 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 `@config/k3s/kyber-host-health.sh` around lines 82 - 84, Update the
mounted_uuid assignment in the health-check script to tolerate blkid failure by
appending the requested non-failing fallback. Preserve an empty mounted_uuid so
the existing comparison against EXPECTED_CONTAINERD_UUID triggers the mismatch
alert instead of terminating under set -e.

set_alert "image-filesystem" "$CONTAINERD_MOUNT has UUID $mounted_uuid, expected $EXPECTED_CONTAINERD_UUID"
return
fi
Comment on lines +82 to +87

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using blkid to query the UUID of the mounted source device can be fragile if the device is temporarily inaccessible or if the blkid cache is stale.

We can retrieve the UUID directly and robustly using findmnt --output UUID, which also handles potential command failures gracefully with || true to prevent set -e from aborting the script.

Suggested change
mounted_source="$(findmnt --noheadings --output SOURCE --target "$CONTAINERD_MOUNT")"
mounted_uuid="$(blkid --match-tag UUID --output value "$mounted_source")"
if [ "$mounted_uuid" != "$EXPECTED_CONTAINERD_UUID" ]; then
set_alert "image-filesystem" "$CONTAINERD_MOUNT has UUID $mounted_uuid, expected $EXPECTED_CONTAINERD_UUID"
return
fi
mounted_uuid="$(findmnt --noheadings --output UUID --target "$CONTAINERD_MOUNT" 2>/dev/null || true)"
if [ -z "$mounted_uuid" ] || [ "$mounted_uuid" != "$EXPECTED_CONTAINERD_UUID" ]; then
set_alert "image-filesystem" "$CONTAINERD_MOUNT has UUID ${mounted_uuid:-unknown}, expected $EXPECTED_CONTAINERD_UUID"
return
fi
References
  1. When 'set -e' is enabled in Bash scripts, ensure that individual command failures do not prematurely abort the entire script. Handle potential failures gracefully.


usage_percent="$(df --output=pcent "$CONTAINERD_MOUNT" | tail -n 1 | tr -cd '0-9')"
if [ "$usage_percent" -ge "$IMAGEFS_USAGE_THRESHOLD" ]; then
set_alert "image-filesystem" "containerd image filesystem usage is ${usage_percent}% (threshold ${IMAGEFS_USAGE_THRESHOLD}%)"
else
clear_alert "image-filesystem"
fi
}

check_cri() {
local started_at finished_at latency_seconds error_count

started_at="$(date +%s)"
if ! timeout 15 k3s crictl info >/dev/null 2>&1; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: A hung k3s crictl that ignores or blocks TERM can keep this health service active past its intended 15-second bound, suppressing subsequent timer probes. Add a short kill-after grace period to enforce a real upper limit.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/k3s/kyber-host-health.sh, line 101:

<comment>A hung `k3s crictl` that ignores or blocks TERM can keep this health service active past its intended 15-second bound, suppressing subsequent timer probes. Add a short kill-after grace period to enforce a real upper limit.</comment>

<file context>
@@ -0,0 +1,121 @@
+  local started_at finished_at latency_seconds error_count
+
+  started_at="$(date +%s)"
+  if ! timeout 15 k3s crictl info >/dev/null 2>&1; then
+    set_alert "cri-health" "k3s crictl info failed or exceeded 15 seconds"
+    return
</file context>
Suggested change
if ! timeout 15 k3s crictl info >/dev/null 2>&1; then
if ! timeout -k 1s 15s k3s crictl info >/dev/null 2>&1; then

set_alert "cri-health" "k3s crictl info failed or exceeded 15 seconds"
return
fi
finished_at="$(date +%s)"
latency_seconds=$((finished_at - started_at))

error_count="$(journalctl --unit k3s --since '5 minutes ago' --no-pager --quiet 2>/dev/null |
grep -Eci 'DeadlineExceeded|deadline exceeded|FailedPrecondition|failed precondition|reserved (container )?name|failed to (create|stop|remove).*(sandbox|container)|cgroup.*(busy|failed)' || true)"

if [ "$latency_seconds" -ge "$CRI_LATENCY_THRESHOLD_SECONDS" ] || [ "$error_count" -ge "$CRI_ERROR_THRESHOLD" ]; then
set_alert "cri-health" "CRI latency was ${latency_seconds}s with ${error_count} lifecycle errors in the last five minutes"
else
clear_alert "cri-health"
fi
}

check_io_pressure
check_d_state
check_image_filesystem
check_cri
12 changes: 12 additions & 0 deletions config/k3s/kyber-host-health.timer
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[Unit]
Description=Run Kyber storage and CRI reliability checks

[Timer]
OnBootSec=5min
OnUnitActiveSec=1min
RandomizedDelaySec=15s
Persistent=true
Unit=kyber-host-health.service

[Install]
WantedBy=timers.target
4 changes: 4 additions & 0 deletions config/k3s/kyber-smartd.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Monitor all SMART-capable host disks. The filesystem UUID pin remains the
# authoritative identity for the containerd mount; smartd monitors physical
# devices because SMART data belongs to the whole SSD, not its partition.
DEVICESCAN -a -n standby,q -s (S/../.././02|L/../01/./03) -W 4,50,60 -m <nomailer> -M exec @alertScript@/bin/kyber-host-alert
14 changes: 14 additions & 0 deletions config/k3s/kyber-smartd.service
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[Unit]
Description=SMART monitoring for Kyber disks
Documentation=man:smartd(8) man:smartd.conf(5)
Wants=var-lib-rancher-k3s-agent-containerd.mount
After=var-lib-rancher-k3s-agent-containerd.mount

[Service]
Type=simple
ExecStart=@smartd@ --no-fork --quit=never --configfile=@smartdConfig@
Restart=on-failure
RestartSec=30s

[Install]
WantedBy=multi-user.target
68 changes: 57 additions & 11 deletions home-manager/services/k3s/activate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,18 @@ set -euo pipefail
SERVICE_FILE="$1"
KUBE_DIR="$2"
MOUNT_FILE="$3"
JOURNALD_FILE="$4"
HEALTH_SERVICE_FILE="$5"
HEALTH_TIMER_FILE="$6"
SMARTD_SERVICE_FILE="$7"
SYSTEM_SERVICE="/etc/systemd/system/k3s.service"
SYSTEM_MOUNT="/etc/systemd/system/var-lib-rancher-k3s-agent-containerd.mount"
SYSTEM_JOURNALD="/etc/systemd/journald.conf.d/10-kyber-limits.conf"
SYSTEM_HEALTH_SERVICE="/etc/systemd/system/kyber-host-health.service"
SYSTEM_HEALTH_TIMER="/etc/systemd/system/kyber-host-health.timer"
SYSTEM_SMARTD_SERVICE="/etc/systemd/system/kyber-smartd.service"
MOUNT_POINT="/var/lib/rancher/k3s/agent/containerd"
EXPECTED_CONTAINERD_UUID="90f29a7b-38ff-460b-b534-92a02f1412ec"
K3S_KUBECONFIG="/etc/rancher/k3s/k3s.yaml"

sudo_cmd=()
Expand Down Expand Up @@ -41,6 +50,20 @@ require_sudo() {
return 0
}

sync_root_file() {
local source="$1"
local target="$2"

if [ ! -f "$source" ] || @diff@ -q "$source" "$target" >/dev/null 2>&1; then
return 1
fi

require_sudo || return 1
run_sudo mkdir -p "$(dirname "$target")"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1: A failed privileged copy is reported as success, so activation can reload and enable stale or missing K3s/health/SMART units after a disk-full or permission failure. Make each install command terminate the activation on failure rather than falling through to return 0.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/services/k3s/activate.sh, line 62:

<comment>A failed privileged copy is reported as success, so activation can reload and enable stale or missing K3s/health/SMART units after a disk-full or permission failure. Make each install command terminate the activation on failure rather than falling through to `return 0`.</comment>

<file context>
@@ -41,6 +50,20 @@ require_sudo() {
+  fi
+
+  require_sudo || return 1
+  run_sudo mkdir -p "$(dirname "$target")"
+  run_sudo cp -f "$source" "$target"
+  return 0
</file context>

run_sudo cp -f "$source" "$target"
return 0
}

Comment on lines +53 to +66

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Propagate command failures in sync_root_file.

Because sync_root_file is evaluated as the condition of an if statement later in the script (e.g. if sync_root_file ...; then), Bash temporarily suspends set -e execution inside the function body. If run_sudo mkdir or run_sudo cp fails, the script will not abort and will silently proceed to return 0, falsely reporting a successful synchronization.

Append || return 1 to ensure failures are correctly propagated.

🐛 Proposed fix
   require_sudo || return 1
-  run_sudo mkdir -p "$(dirname "$target")"
-  run_sudo cp -f "$source" "$target"
-  return 0
+  run_sudo mkdir -p "$(dirname "$target")" || return 1
+  run_sudo cp -f "$source" "$target" || return 1
+  return 0
 }
📝 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
sync_root_file() {
local source="$1"
local target="$2"
if [ ! -f "$source" ] || @diff@ -q "$source" "$target" >/dev/null 2>&1; then
return 1
fi
require_sudo || return 1
run_sudo mkdir -p "$(dirname "$target")"
run_sudo cp -f "$source" "$target"
return 0
}
sync_root_file() {
local source="$1"
local target="$2"
if [ ! -f "$source" ] || `@diff`@ -q "$source" "$target" >/dev/null 2>&1; then
return 1
fi
require_sudo || return 1
run_sudo mkdir -p "$(dirname "$target")" || return 1
run_sudo cp -f "$source" "$target" || return 1
return 0
}
🤖 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 `@home-manager/services/k3s/activate.sh` around lines 53 - 66, Update
sync_root_file so both run_sudo mkdir and run_sudo cp explicitly append “||
return 1”, ensuring command failures propagate even when the function is used as
an if condition; retain the existing successful return 0 behavior.

configure_root_ext4_reserve() {
local root_source root_fs_type filesystem_info block_count reserved_blocks target_reserved_blocks
local target_reserved_percent=1
Expand Down Expand Up @@ -84,6 +107,16 @@ configure_root_ext4_reserve() {

configure_root_ext4_reserve

if @findmnt@ --mountpoint "$MOUNT_POINT" >/dev/null 2>&1; then
mounted_source="$(@findmnt@ --noheadings --output SOURCE --target "$MOUNT_POINT")"
mounted_uuid="$(@blkid@ --match-tag UUID --output value "$mounted_source")"
Comment on lines +111 to +112

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Running blkid on a block device as a non-root user during Home Manager activation will fail or return an empty string due to permission restrictions on raw block devices. Since Home Manager activation runs as the unprivileged user, this will cause the activation script to abort or fail with an unexpected UUID error.

Using findmnt to retrieve the UUID directly is safe, does not require root privileges, and avoids the dependency on blkid entirely.

Suggested change
mounted_source="$(@findmnt@ --noheadings --output SOURCE --target "$MOUNT_POINT")"
mounted_uuid="$(@blkid@ --match-tag UUID --output value "$mounted_source")"
mounted_uuid="$(@findmnt@ --noheadings --output UUID --target "$MOUNT_POINT")"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bare blkid will read empty for non-root users on Ubuntu, tripping the guard.

On Ubuntu the containerd block device (/dev/sda*) is brw-rw---- root disk, and blkid only returns cached info for devices readable by the invoking user. Since home-manager activation runs as the user, this call typically returns an empty string, and the check on line 113 then compares "" != "90f29a7b-..." — which is true — so activation aborts with unexpected containerd filesystem UUID: (empty), even when the disk is correctly mounted.

The same script already reaches for run_sudo @tune2fs@ -l "$root_source" in configure_root_ext4_reserve for exactly this reason, and prepare-containerd-disk.sh uses sudo blkid for the equivalent probe. Suggest:

if @findmnt@ --mountpoint "$MOUNT_POINT" >/dev/null 2>&1; then
  mounted_source="$(@findmnt@ --noheadings --output SOURCE --target "$MOUNT_POINT")"
  require_sudo || exit 0
  mounted_uuid="$(run_sudo @blkid@ --match-tag UUID --output value "$mounted_source")"
  ...
fi

The existing spec assertion grep -q 'mounted_uuid=.*@blkid@' in spec/k3s_service_activate_spec.sh still matches after the change.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1: This UUID probe uses @blkid@ before privilege escalation. During user-scoped activation, blkid can return no data or non-zero, which can trigger set -e or an empty-value mismatch and fail activation even when the mount is correct. Running this probe via run_sudo after require_sudo avoids that failure mode.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/services/k3s/activate.sh, line 112:

<comment>This UUID probe uses `@blkid@` before privilege escalation. During user-scoped activation, `blkid` can return no data or non-zero, which can trigger `set -e` or an empty-value mismatch and fail activation even when the mount is correct. Running this probe via `run_sudo` after `require_sudo` avoids that failure mode.</comment>

<file context>
@@ -84,6 +107,16 @@ configure_root_ext4_reserve() {
 
+if @findmnt@ --mountpoint "$MOUNT_POINT" >/dev/null 2>&1; then
+  mounted_source="$(@findmnt@ --noheadings --output SOURCE --target "$MOUNT_POINT")"
+  mounted_uuid="$(@blkid@ --match-tag UUID --output value "$mounted_source")"
+  if [ "$mounted_uuid" != "$EXPECTED_CONTAINERD_UUID" ]; then
+    echo "Refusing to run k3s with unexpected containerd filesystem UUID: $mounted_uuid" >&2
</file context>
Suggested change
mounted_uuid="$(@blkid@ --match-tag UUID --output value "$mounted_source")"
require_sudo || exit 0
mounted_uuid="$(run_sudo @blkid@ --match-tag UUID --output value "$mounted_source")"

if [ "$mounted_uuid" != "$EXPECTED_CONTAINERD_UUID" ]; then
echo "Refusing to run k3s with unexpected containerd filesystem UUID: $mounted_uuid" >&2
echo "Expected $EXPECTED_CONTAINERD_UUID at $MOUNT_POINT" >&2
exit 1
fi
fi

Comment on lines +110 to +119

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Run blkid with root privileges.

By default, reading block device attributes with blkid requires root privileges. Because this script executes as a normal user during Home Manager activation, blkid will fail with an exit code of 2 and produce empty output. The set -e policy will immediately catch the failure inside the command substitution and abort the activation.

🐛 Proposed fix to use `run_sudo`
 if `@findmnt`@ --mountpoint "$MOUNT_POINT" >/dev/null 2>&1; then
   mounted_source="$(`@findmnt`@ --noheadings --output SOURCE --target "$MOUNT_POINT")"
-  mounted_uuid="$(`@blkid`@ --match-tag UUID --output value "$mounted_source")"
+  mounted_uuid="$(run_sudo `@blkid`@ --match-tag UUID --output value "$mounted_source")"
   if [ "$mounted_uuid" != "$EXPECTED_CONTAINERD_UUID" ]; then
📝 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
if @findmnt@ --mountpoint "$MOUNT_POINT" >/dev/null 2>&1; then
mounted_source="$(@findmnt@ --noheadings --output SOURCE --target "$MOUNT_POINT")"
mounted_uuid="$(@blkid@ --match-tag UUID --output value "$mounted_source")"
if [ "$mounted_uuid" != "$EXPECTED_CONTAINERD_UUID" ]; then
echo "Refusing to run k3s with unexpected containerd filesystem UUID: $mounted_uuid" >&2
echo "Expected $EXPECTED_CONTAINERD_UUID at $MOUNT_POINT" >&2
exit 1
fi
fi
if `@findmnt`@ --mountpoint "$MOUNT_POINT" >/dev/null 2>&1; then
mounted_source="$(`@findmnt`@ --noheadings --output SOURCE --target "$MOUNT_POINT")"
mounted_uuid="$(run_sudo `@blkid`@ --match-tag UUID --output value "$mounted_source")"
if [ "$mounted_uuid" != "$EXPECTED_CONTAINERD_UUID" ]; then
echo "Refusing to run k3s with unexpected containerd filesystem UUID: $mounted_uuid" >&2
echo "Expected $EXPECTED_CONTAINERD_UUID at $MOUNT_POINT" >&2
exit 1
fi
fi
🤖 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 `@home-manager/services/k3s/activate.sh` around lines 110 - 119, Update the
mounted filesystem UUID lookup in the mount-check block to invoke blkid through
the existing run_sudo helper, ensuring it runs with root privileges while
preserving the current UUID comparison and refusal behavior.

if [ -f "$MOUNT_FILE" ] && ! @findmnt@ --mountpoint "$MOUNT_POINT" >/dev/null 2>&1; then
if @systemctl@ is-active --quiet k3s; then
echo "Refusing to mount the containerd SSD while k3s is running" >&2
Expand All @@ -105,22 +138,27 @@ if [ -f "$MOUNT_FILE" ] && ! @findmnt@ --mountpoint "$MOUNT_POINT" >/dev/null 2>
fi

systemd_changed=0
if [ -f "$MOUNT_FILE" ] && ! @diff@ -q "$MOUNT_FILE" "$SYSTEM_MOUNT" >/dev/null 2>&1; then
require_sudo || exit 0
run_sudo cp -f "$MOUNT_FILE" "$SYSTEM_MOUNT"
systemd_changed=1
fi

if [ -f "$SERVICE_FILE" ] && ! @diff@ -q "$SERVICE_FILE" "$SYSTEM_SERVICE" >/dev/null 2>&1; then
require_sudo || exit 0
run_sudo cp -f "$SERVICE_FILE" "$SYSTEM_SERVICE"
systemd_changed=1
fi
for systemd_file_pair in \
"$MOUNT_FILE:$SYSTEM_MOUNT" \
"$SERVICE_FILE:$SYSTEM_SERVICE" \
"$HEALTH_SERVICE_FILE:$SYSTEM_HEALTH_SERVICE" \
"$HEALTH_TIMER_FILE:$SYSTEM_HEALTH_TIMER" \
"$SMARTD_SERVICE_FILE:$SYSTEM_SMARTD_SERVICE"; do
source_file="${systemd_file_pair%%:*}"
target_file="${systemd_file_pair#*:}"
if sync_root_file "$source_file" "$target_file"; then
systemd_changed=1
fi
done

if [ "$systemd_changed" -eq 1 ]; then
run_sudo @systemctl@ daemon-reload
fi
Comment on lines +141 to 156

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Initialize systemd_changed to avoid an unbound variable crash.

If none of the systemd files require synchronization, the for loop completes without assigning a value to systemd_changed. Evaluating [ "$systemd_changed" -eq 1 ] will then trigger an "unbound variable" error because of the set -u policy, crashing the script during steady-state (idempotent) runs.

🐛 Proposed fix
+systemd_changed=0
 for systemd_file_pair in \
   "$MOUNT_FILE:$SYSTEM_MOUNT" \
📝 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
for systemd_file_pair in \
"$MOUNT_FILE:$SYSTEM_MOUNT" \
"$SERVICE_FILE:$SYSTEM_SERVICE" \
"$HEALTH_SERVICE_FILE:$SYSTEM_HEALTH_SERVICE" \
"$HEALTH_TIMER_FILE:$SYSTEM_HEALTH_TIMER" \
"$SMARTD_SERVICE_FILE:$SYSTEM_SMARTD_SERVICE"; do
source_file="${systemd_file_pair%%:*}"
target_file="${systemd_file_pair#*:}"
if sync_root_file "$source_file" "$target_file"; then
systemd_changed=1
fi
done
if [ "$systemd_changed" -eq 1 ]; then
run_sudo @systemctl@ daemon-reload
fi
systemd_changed=0
for systemd_file_pair in \
"$MOUNT_FILE:$SYSTEM_MOUNT" \
"$SERVICE_FILE:$SYSTEM_SERVICE" \
"$HEALTH_SERVICE_FILE:$SYSTEM_HEALTH_SERVICE" \
"$HEALTH_TIMER_FILE:$SYSTEM_HEALTH_TIMER" \
"$SMARTD_SERVICE_FILE:$SYSTEM_SMARTD_SERVICE"; do
source_file="${systemd_file_pair%%:*}"
target_file="${systemd_file_pair#*:}"
if sync_root_file "$source_file" "$target_file"; then
systemd_changed=1
fi
done
if [ "$systemd_changed" -eq 1 ]; then
run_sudo `@systemctl`@ daemon-reload
fi
🤖 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 `@home-manager/services/k3s/activate.sh` around lines 141 - 156, Initialize
systemd_changed to 0 before the systemd_file_pair loop. Keep sync_root_file
setting it to 1 when any systemd file changes, so the daemon-reload condition
remains unchanged and idempotent runs do not fail under set -u.


if sync_root_file "$JOURNALD_FILE" "$SYSTEM_JOURNALD"; then
run_sudo @systemctl@ try-restart systemd-journald.service
fi

if [ -f "$MOUNT_FILE" ]; then
require_sudo || exit 0
run_sudo mkdir -p "$MOUNT_POINT"
Expand All @@ -129,6 +167,14 @@ fi

run_sudo @systemctl@ enable --now k3s

if [ -f "$SMARTD_SERVICE_FILE" ]; then
run_sudo @systemctl@ enable --now kyber-smartd.service

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Updated SMART monitoring configuration never takes effect while kyber-smartd.service is already active: enable --now starts inactive units but does not restart the running smartd process. Track a changed smartd unit and restart it after daemon-reload (while preserving first-install start behavior).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/services/k3s/activate.sh, line 171:

<comment>Updated SMART monitoring configuration never takes effect while `kyber-smartd.service` is already active: `enable --now` starts inactive units but does not restart the running smartd process. Track a changed smartd unit and restart it after `daemon-reload` (while preserving first-install start behavior).</comment>

<file context>
@@ -129,6 +167,14 @@ fi
 run_sudo @systemctl@ enable --now k3s
 
+if [ -f "$SMARTD_SERVICE_FILE" ]; then
+  run_sudo @systemctl@ enable --now kyber-smartd.service
+fi
+
</file context>

fi

if [ -f "$HEALTH_TIMER_FILE" ]; then
run_sudo @systemctl@ enable --now kyber-host-health.timer
fi

if [ -f "$K3S_KUBECONFIG" ]; then
run mkdir -p "$KUBE_DIR"
require_sudo || exit 0
Expand Down
Loading
Loading