Skip to content

fix(k3s): keep reserve setup best effort #2127

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

Merged
shunkakinoki merged 1 commit into from
Jul 19, 2026
Merged

fix(k3s): keep reserve setup best effort #2127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions config/k3s/kubelet.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ 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
16 changes: 12 additions & 4 deletions home-manager/services/k3s/activate.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ require_sudo() {
echo "Warning: sudo not found, skipping k3s system setup" >&2
return 1
fi
return 0
}

configure_root_ext4_reserve() {
local root_source root_fs_type block_count reserved_blocks target_reserved_blocks
local root_source root_fs_type filesystem_info block_count reserved_blocks target_reserved_blocks
local target_reserved_percent=1

root_source="$(@findmnt@ --noheadings --output SOURCE --target /)"
Expand All @@ -53,10 +54,14 @@ configure_root_ext4_reserve() {
fi

require_sudo || return 0
if ! filesystem_info="$(run_sudo @tune2fs@ -l "$root_source")"; then

@gemini-code-assist gemini-code-assist Bot Jul 19, 2026

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

There is a critical bug in the require_sudo function (defined on lines 33-38) that prevents this entire block (and other sudo-dependent parts of the script) from executing when sudo is available.

In Bash, if a function does not end with an explicit return statement, its exit status is that of the last command executed. In require_sudo:

require_sudo() {
  if [ "${#sudo_cmd[@]}" -eq 0 ]; then
    echo "Warning: sudo not found, skipping k3s system setup" >&2
    return 1
  fi
}

When sudo is available, the if condition [ "${#sudo_cmd[@]}" -eq 0 ] evaluates to false (exit status 1), the body is skipped, and the function exits. Since the [ test was the last command executed, the function returns 1.

As a result, require_sudo || return 0 on line 55 will always evaluate to true and return early, skipping the filesystem inspection entirely. Similarly, require_sudo || exit 0 on lines 84 and 92 will cause the script to exit prematurely.

To fix this, please add an explicit return 0 at the end of require_sudo.

@shunkakinoki shunkakinoki Jul 19, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 83bfbfb with an explicit success return and a focused regression assertion; the native Kyber activation-package build also succeeds.

@cubic-dev-ai cubic-dev-ai Bot Jul 19, 2026

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: The new best-effort behavior is unverified: a future change could make an inspection or mutation failure abort activation again without CI detecting it. A focused shellspec fixture that stubs tune2fs should cover both failures and assert activation continues while emitting the warning.

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 56:

<comment>The new best-effort behavior is unverified: a future change could make an inspection or mutation failure abort activation again without CI detecting it. A focused shellspec fixture that stubs `tune2fs` should cover both failures and assert activation continues while emitting the warning.</comment>

<file context>
@@ -53,10 +53,14 @@ configure_root_ext4_reserve() {
   fi
 
   require_sudo || return 0
+  if ! filesystem_info="$(run_sudo @tune2fs@ -l "$root_source")"; then
+    echo "Warning: unable to inspect ext4 reserve on $root_source" >&2
+    return 0
</file context>

echo "Warning: unable to inspect ext4 reserve on $root_source" >&2
return 0
fi
# shellcheck disable=SC2016
block_count="$(run_sudo @tune2fs@ -l "$root_source" 2>/dev/null | @awk@ -F: '/^Block count:/ { gsub(/[[:space:]]/, "", $2); print $2 }')"
block_count="$(@awk@ -F: '/^Block count:/ { gsub(/[[:space:]]/, "", $2); print $2 }' <<<"$filesystem_info")"
# shellcheck disable=SC2016
reserved_blocks="$(run_sudo @tune2fs@ -l "$root_source" 2>/dev/null | @awk@ -F: '/^Reserved block count:/ { gsub(/[[:space:]]/, "", $2); print $2 }')"
reserved_blocks="$(@awk@ -F: '/^Reserved block count:/ { gsub(/[[:space:]]/, "", $2); print $2 }' <<<"$filesystem_info")"
if [ -z "$block_count" ] || [ -z "$reserved_blocks" ]; then
echo "Warning: unable to inspect ext4 reserve on $root_source" >&2
return 0
Expand All @@ -67,7 +72,10 @@ configure_root_ext4_reserve() {
return 0
fi

run_sudo @tune2fs@ -m "$target_reserved_percent" "$root_source"
if ! run_sudo @tune2fs@ -m "$target_reserved_percent" "$root_source"; then
echo "Warning: unable to configure ext4 reserve on $root_source" >&2
return 0
fi
echo "Configured $root_source ext4 reserved blocks to ${target_reserved_percent}%"
}

Expand Down
1 change: 1 addition & 0 deletions named-hosts/kyber/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ and CRI health before restarting services:

```bash
df -h /
cat /proc/pressure/io
sudo tune2fs -l "$(findmnt -n -o SOURCE /)" | grep -E 'Block count|Reserved block count'
sudo journalctl -u k3s --since '30 minutes ago' | grep -E 'image garbage collection|DiskPressure|deadline exceeded'
sudo k3s crictl info
Expand Down
8 changes: 7 additions & 1 deletion spec/k3s_service_activate_spec.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ It 'warns when sudo is unavailable'
When run bash -c "grep 'sudo not found' '$SCRIPT'"
The output should include 'sudo not found'
End

It 'returns success when sudo is available'
When run bash -c "sed -n '/^require_sudo()/,/^}/p' '$SCRIPT' | grep -xF ' return 0'"
The output should include 'return 0'
The status should be success
End
End

Describe 'k3s setup'
Expand All @@ -77,7 +83,7 @@ The output should include 'DRY_RUN_CMD'
End

It 'keeps one percent of the ext4 root volume reserved'
When run bash -c "grep 'target_reserved_percent=1' '$SCRIPT'"
When run bash -c "grep -xF ' local target_reserved_percent=1' '$SCRIPT'"
The output should include 'target_reserved_percent=1'
End

Expand Down
Loading