Skip to content

fix: separate UI customisation from pinning, stop silent config loss #4

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
catinspace-au merged 1 commit into from
Jul 26, 2026
Merged

fix: separate UI customisation from pinning, stop silent config loss #4

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
21 changes: 18 additions & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,15 +129,30 @@ Conservative update policy with n-0.1 minor version pinning. Keeps you one minor
**Commands:**

```bash
sudo ./proxmox-update-policy.sh enable # Enable with UI patches
sudo ./proxmox-update-policy.sh enable --no-ui # Enable without UI patches
sudo ./proxmox-update-policy.sh disable # Disable and restore UI
sudo ./proxmox-update-policy.sh enable # Pin + suppress warnings
sudo ./proxmox-update-policy.sh enable --no-ui # Pin only, keep warnings
sudo ./proxmox-update-policy.sh ui-only # Suppress warnings, pin nothing
sudo ./proxmox-update-policy.sh ui-disable # Restore warnings, keep pinning
sudo ./proxmox-update-policy.sh disable # Remove both
sudo ./proxmox-update-policy.sh status # Show policy and versions
sudo ./proxmox-update-policy.sh update # Refresh pinning
sudo ./proxmox-update-policy.sh cron-enable # Install daily cron
sudo ./proxmox-update-policy.sh cron-disable # Remove cron
```

**Pinning and the UI customisation are independent.** A host can hold packages
back, suppress the subscription warnings, both, or neither:

| Want | Command |
|---|---|
| Both | `enable` |
| Pin only, keep the warnings | `enable --no-ui` |
| Warnings gone, track latest | `ui-only` |
| Neither | `disable` |

`ui-only` is the right choice for a host that should stay current -- a lab or
a personal box -- but without Proxmox telling you off about it on every login.

**UI customisations:**

When enabled, the web interface reports the update policy that is actually in
Expand Down
49 changes: 48 additions & 1 deletion postinstall/proxmox-cleanup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ OLD_SYSCTL=(
)

FOUND=0
KEEP=0
ACTIONS=()

#---------------------------------------------------------------
Expand All @@ -107,6 +108,29 @@ has_readonly_keys() {
echo "$content" | grep -qE '^[[:space:]]*(kernel\.random\.entropy_avail|kernel\.random\.(boot_id|uuid)|fs\.dentry-state|fs\.file-nr)[[:space:]]*='
}

sysctl_keys() {
# Settable keys in sysctl.d content ($1), one per line, sorted.
# Commented lines are inert and must not count.
local content="$1"
echo "$content" \
| grep -oE '^[[:space:]]*[a-z][a-z0-9._-]*[[:space:]]*=' \
| tr -d ' =' \
| sort -u
}

orphaned_keys() {
# Keys set by the superseded file ($1) that the replacement ($2) does NOT
# set, one per line.
#
# Removing a file purely because a newer generation exists loses whatever
# the newer one never picked up. On a real host the superseded tweaks file
# carried five keys the replacement had no equivalent for -- dirty
# writeback intervals, keepalive probe tuning, an inotify limit -- and all
# five would have gone silently. A backup makes that recoverable, not safe.
local old_content="$1" new_content="$2"
comm -23 <(sysctl_keys "$old_content") <(sysctl_keys "$new_content")
}

network_tier_from_path() {
# 99-proxmox-network-10gbe.conf -> 10gbe
local path="$1" base
Expand Down Expand Up @@ -158,8 +182,28 @@ scan() {
note high "$f" \
"sets read-only kernel statistics -- errors on every boot"
else
# Only call it superseded if the replacement actually covers it.
local current_content orphans
current_content=$(cat /etc/sysctl.d/98-proxmox-optimize.conf 2>/dev/null || echo "")
orphans=$(orphaned_keys "$content" "$current_content")

if [ -n "$orphans" ]; then
KEEP=$((KEEP + 1))
# Counts as "something to report", or the no-findings fallback
# prints "none" directly underneath this entry.
found_sysctl=1
printf ' %b[KEEP]%b %s\n' "$CYAN" "$NC" "$f"
printf ' overrides 98-proxmox-optimize.conf, but sets %s key(s) it does not:\n' \
"$(echo "$orphans" | wc -l)"
while IFS= read -r orphan_key; do
printf ' %s\n' "$orphan_key"
done <<< "$orphans"
printf ' NOT removed -- fold these into 98-proxmox-optimize.conf first.\n'
continue
fi

note med "$f" \
"superseded by 98-proxmox-optimize.conf, which it currently overrides by sort order"
"superseded by 98-proxmox-optimize.conf, which covers every key it sets"
fi
found_sysctl=1
done
Expand Down Expand Up @@ -209,6 +253,9 @@ scan() {
else
echo -e "${YELLOW}${FOUND} superseded artefact(s) found.${NC}"
fi
if [ "$KEEP" -gt 0 ]; then
echo -e "${CYAN}${KEEP} file(s) kept: they still set keys nothing else does.${NC}"
fi
}

#---------------------------------------------------------------
Expand Down
125 changes: 106 additions & 19 deletions postinstall/proxmox-disk-errors.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@
# completely unrelated, healthy pool lost their I/O. SMART reported PASSED
# throughout.
#
# Generalised from the hardening Derek wrote by hand during that incident.
# Two details are his and were reverted to after the generated versions
# proved worse: addressing drives by /dev/disk/by-id rather than sdX, and
# matching the udev rule on ID_MODEL rather than per-drive serial. Both
# matter for the same reason -- the protection has to survive a reboot or a
# disk swap without anyone remembering to re-run this.
#
# SAFETY -- READ THIS:
# SCT ERC is only correct BECAUSE there is redundancy. Telling a drive in a
# NON-redundant pool to give up early tells it to abandon data it might
Expand Down Expand Up @@ -179,6 +186,39 @@ base_device() {
return 0
}

stable_device_path() {
# A /dev/disk/by-id path for $1, or /dev/$1 if none exists.
#
# Reverted to Derek's original approach from the incident scripts, which
# addressed drives by /dev/disk/by-id. The generated version used bare sdX
# names and was worse: kernel names are assigned in discovery order and are
# NOT stable across reboots, so a boot-time unit that reasserts drive
# settings by kernel name can target a different disk than the one it was
# written for. On a host where some drives take the setting and others
# cannot, that is exactly how the wrong drive ends up unprotected.
#
# ata-/nvme- links carry model and serial and are preferred; wwn- links are
# stable too but opaque, so they are only a fallback.
local dev="$1" link base fallback=""

for link in /dev/disk/by-id/*; do
[ -e "$link" ] || continue
base=$(basename "$link")
case "$base" in *-part[0-9]*) continue ;; esac
[ "$(readlink -f "$link")" = "/dev/$dev" ] || continue
case "$base" in
wwn-*) [ -z "$fallback" ] && fallback="$link" ;;
*) echo "$link"; return 0 ;;
esac
done

if [ -n "$fallback" ]; then
echo "$fallback"
return 0
fi
echo "/dev/$dev"
}

pool_members() {
# Base device names backing pool $1, one per line, deduplicated.
local pool="$1" member dev
Expand Down Expand Up @@ -285,26 +325,48 @@ write_erc_service() {
return 0
fi

# Resolve to stable by-id paths at generation time. The helper must not
# depend on kernel names surviving a reboot -- see stable_device_path.
local dev paths=()
for dev in "${devices[@]}"; do
paths+=("$(stable_device_path "$dev")")
done

# SCT ERC is volatile: it is lost on a power cycle (though usually not on a
# warm reboot), so it must be reasserted at every boot rather than set once.
cat > "$ERC_HELPER" << EOF
{
cat << EOF
#!/bin/bash
# Reassert SCT ERC on redundant pool members.
# Installed by: proxmox-disk-errors.sh -- do not edit, changes are overwritten.
#
# SCT ERC does not persist across a power cycle. Without this, the setting
# silently reverts to the factory default (usually Disabled) and the drive goes
# back to retrying without bound.
#
# Drives are addressed by /dev/disk/by-id, not sdX: kernel names are assigned
# in discovery order and a drive that moves between boots would otherwise have
# the setting applied to whatever took its old name.

EOF
for dev in "${paths[@]}"; do
printf 'for_each_drive+=("%s")\n' "$dev"
done
cat << EOF

for dev in ${devices[*]}; do
[ -b "/dev/\$dev" ] || continue
smartctl -l scterc,${ERC_DECISECONDS},${ERC_DECISECONDS} "/dev/\$dev" >/dev/null 2>&1 \\
&& logger -t zfs-disk-erc "SCT ERC ${ERC_DECISECONDS}ds set on \$dev" \\
|| logger -t zfs-disk-erc "failed to set SCT ERC on \$dev"
for drive in "\${for_each_drive[@]}"; do
# A drive that has been pulled or replaced must not fail the unit.
[ -e "\$drive" ] || { logger -t zfs-disk-erc "absent, skipped: \$drive"; continue; }
if smartctl -l scterc,${ERC_DECISECONDS},${ERC_DECISECONDS} "\$drive" >/dev/null 2>&1; then
logger -t zfs-disk-erc "SCT ERC ${ERC_DECISECONDS}ds set on \$drive"
else
logger -t zfs-disk-erc "failed to set SCT ERC on \$drive"
fi
done

exit 0
EOF
} > "$ERC_HELPER"
chmod +x "$ERC_HELPER"

cat > "$ERC_SERVICE" << EOF
Expand Down Expand Up @@ -349,24 +411,49 @@ write_udev_rule() {
echo "# up on their behalf. The 30s default is far longer than any healthy SATA"
echo "# I/O; ${NO_SCT_TIMEOUT}s converts a stall into an error ZFS can actually repair from."
echo "#"
echo "# Matched on serial so the rule survives device renaming (sdc is not stable)."
echo "# Matched on MODEL, not serial -- reverted to Derek's original incident"
echo "# rule, which matched ENV{ID_MODEL} and worked better than the generated"
echo "# per-serial version that briefly replaced it."
echo "#"
echo "# Whether a drive supports SCT ERC is a property of the MODEL, so a"
echo "# replacement of the same model needs the same backstop -- and would not"
echo "# get it from a serial-specific rule until somebody remembered to re-run"
echo "# this script. Fitting a replacement disk is exactly the moment nobody is"
echo "# thinking about kernel timeouts."
echo "#"
echo "# DEVTYPE==\"disk\" restricts this to whole disks. A partition carries the"
echo "# same serial but has no device/timeout attribute of its own -- without the"
echo "# restriction udev retries the assignment against every partition and logs a"
echo "# failure for each."
echo "# same model but has no device/timeout attribute of its own -- without the"
echo "# restriction udev retries the assignment against every partition and logs"
echo "# a failure for each."
echo ""

# One rule per distinct model, rather than one per drive.
local models=() model seen
for dev in "${devices[@]}"; do
serial=$(udevadm info --query=property --name="/dev/$dev" 2>/dev/null | \
sed -n 's/^ID_SERIAL_SHORT=//p' | head -1)
if [ -n "$serial" ]; then
echo "ACTION==\"add|change\", SUBSYSTEM==\"block\", ENV{DEVTYPE}==\"disk\", ENV{ID_SERIAL_SHORT}==\"${serial}\", ATTR{device/timeout}=\"${NO_SCT_TIMEOUT}\""
else
# No stable serial exposed; fall back to the kernel name and say
# so, rather than silently writing a rule that matches nothing.
echo "# WARNING: no ID_SERIAL_SHORT for ${dev}, matching on kernel name (not stable across reboots)"
echo "ACTION==\"add|change\", SUBSYSTEM==\"block\", ENV{DEVTYPE}==\"disk\", KERNEL==\"${dev}\", ATTR{device/timeout}=\"${NO_SCT_TIMEOUT}\""
model=$(udevadm info --query=property --name="/dev/$dev" 2>/dev/null | \
sed -n 's/^ID_MODEL=//p' | head -1)
if [ -z "$model" ]; then
serial=$(udevadm info --query=property --name="/dev/$dev" 2>/dev/null | \
sed -n 's/^ID_SERIAL_SHORT=//p' | head -1)
if [ -n "$serial" ]; then
echo "# ${dev}: no ID_MODEL exposed, falling back to serial"
echo "ACTION==\"add|change\", SUBSYSTEM==\"block\", ENV{DEVTYPE}==\"disk\", ENV{ID_SERIAL_SHORT}==\"${serial}\", ATTR{device/timeout}=\"${NO_SCT_TIMEOUT}\""
else
echo "# WARNING: no ID_MODEL or ID_SERIAL_SHORT for ${dev}; kernel name is not stable across reboots"
echo "ACTION==\"add|change\", SUBSYSTEM==\"block\", ENV{DEVTYPE}==\"disk\", KERNEL==\"${dev}\", ATTR{device/timeout}=\"${NO_SCT_TIMEOUT}\""
fi
continue
fi

seen=0
for m in "${models[@]:-}"; do
[ "$m" = "$model" ] && { seen=1; break; }
done
[ "$seen" -eq 1 ] && continue
models+=("$model")

echo "# ${model} -- no SCT ERC support"
echo "ACTION==\"add|change\", SUBSYSTEM==\"block\", ENV{DEVTYPE}==\"disk\", ENV{ID_MODEL}==\"${model}\", ATTR{device/timeout}=\"${NO_SCT_TIMEOUT}\""
done
} > "$UDEV_RULE"

Expand Down
10 changes: 10 additions & 0 deletions postinstall/proxmox-optimize.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,11 +219,21 @@ net.core.netdev_max_backlog=8192
net.core.somaxconn=8192
net.ipv4.tcp_fin_timeout=30
net.ipv4.tcp_keepalive_time=300
net.ipv4.tcp_keepalive_probes=3
net.ipv4.tcp_keepalive_intvl=30
net.ipv4.tcp_tw_reuse=1

# File System
fs.file-max=2097152
fs.inotify.max_user_watches=524288
fs.inotify.max_user_instances=512

# Deliberately NOT set here: vm.dirty_expire_centisecs and
# vm.dirty_writeback_centisecs. Raising them keeps dirty pages in memory
# longer, which trades a measurable amount of unflushed data against a small
# throughput gain. That is a defensible choice on a host with a UPS and a
# reckless one without, so it is left to the operator rather than shipped as a
# default -- this file otherwise holds nothing that risks data on power loss.
EOF

# Bridge netfilter: NOT a Proxmox requirement, despite being widely repeated as
Expand Down
52 changes: 50 additions & 2 deletions postinstall/proxmox-power-management.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,18 @@ for host in /sys/class/scsi_host/host*/; do
CURRENT=$(tr -d ' ' < "${host}link_power_management_policy" 2>/dev/null)

if [[ " $ZFS_HOSTS " == *" $HOSTNAME_ID "* ]]; then
echo " ${HOSTNAME_ID}: backs a ZFS pool, leaving at '$CURRENT'"
# Assert max_performance rather than merely skipping. Skipping
# prevents this script making things worse, but leaves a controller
# an EARLIER run already put into a power-saving state -- which is
# the situation on any host that ran a previous version. The disks
# holding the VMs are the last place to save a watt.
if [[ "$CURRENT" == "max_performance" ]] || [[ "$CURRENT" == "keep_firmware_settings" ]]; then
echo " ${HOSTNAME_ID}: backs a ZFS pool, already '$CURRENT'"
elif echo "max_performance" > "${host}link_power_management_policy" 2>/dev/null; then
echo " ${HOSTNAME_ID}: backs a ZFS pool, '$CURRENT' -> max_performance"
else
echo -e " ${HOSTNAME_ID}: ${YELLOW}backs a ZFS pool but could not clear '$CURRENT'${NC}"
fi
SATA_SKIPPED=$((SATA_SKIPPED + 1))
continue
fi
Expand Down Expand Up @@ -360,7 +371,7 @@ done
set -e

[ $SATA_COUNT -gt 0 ] && echo "Configured $SATA_COUNT SATA controllers" || echo "No SATA controllers needed changes"
[ $SATA_SKIPPED -gt 0 ] && echo "Skipped $SATA_SKIPPED controller(s) backing ZFS pools"
[ "$SATA_SKIPPED" -gt 0 ] && echo "$SATA_SKIPPED controller(s) backing ZFS pools held at max_performance"

#############################################
# Network Power Management
Expand Down Expand Up @@ -581,6 +592,43 @@ for usb in /sys/bus/usb/devices/*/power/control; do
fi
fi
done

# Hold SATA controllers that back a ZFS pool at max_performance.
#
# The sysfs link power policy does not persist across a reboot, so setting it
# once at install time is not enough -- it reverts to whatever the kernel
# defaults to for that controller. Reasserting it here is what makes it stick.
#
# The disks holding the VMs are the last place to save a watt: link power
# management parks the SATA link between commands and adds wake latency to
# every I/O, which works directly against the bounded retry times configured by
# proxmox-disk-errors.sh.
ZFS_HOSTS=""
if command -v zpool >/dev/null 2>&1; then
for pool in $(zpool list -H -o name 2>/dev/null); do
while read -r member; do
[ -z "$member" ] && continue
memberdev=$(readlink -f "$member" 2>/dev/null)
memberdev=${memberdev#/dev/}
memberdev=$(echo "$memberdev" | sed -E 's/(nvme[0-9]+n[0-9]+)p[0-9]+$/\1/; s/^([a-z]+)[0-9]+$/\1/')
hostpath=$(readlink -f "/sys/block/${memberdev}/device" 2>/dev/null) || continue
hostid=$(echo "$hostpath" | grep -oE 'host[0-9]+' | head -1)
[ -n "$hostid" ] && ZFS_HOSTS="$ZFS_HOSTS $hostid"
done < <(zpool list -vHP "$pool" 2>/dev/null | awk '$1 ~ /^\/dev\// {print $1}')
done
fi

for host in /sys/class/scsi_host/host*/; do
[ -f "${host}link_power_management_policy" ] || continue
hostid=$(basename "$host")
case " $ZFS_HOSTS " in
*" $hostid "*)
CURRENT=$(tr -d ' ' < "${host}link_power_management_policy" 2>/dev/null)
[ "$CURRENT" != "max_performance" ] && \
echo "max_performance" > "${host}link_power_management_policy" 2>/dev/null || true
;;
esac
done
EOF

chmod +x /usr/local/bin/proxmox-power-apply.sh
Expand Down
Loading