Allocate IPs per missing service, not all-or-nothing

Not a flake. IP allocation lived in the else-branch of "did the database return
any rows for this app", so it ran only when the app held ZERO rows. An app with
even one row skipped the loop entirely, and a service without a row never got an
IP and never would. Its IP_TAG_<n> stayed unfilled, the literal IP_DATA_<n>
reached the compose, and docker refused the app with

  invalid IPv4 address: ParseAddr("IP_DATA_3")

which surfaced as "no container started (image pull failed?)". Nothing repaired
it: reinstalling re-ran the same skip, so the app stayed broken until someone
uninstalled it and wiped the rows.

Partial state is not exotic — an app that GAINS a service in a later version hits
this on its very next install, because the old services still hold rows. That is
the case worth worrying about; Stoat only got there by being installed and
uninstalled repeatedly.

Reproduced deterministically by deleting one row from a healthy 16-service Stoat:
the install reported "No IP allocated for service: stoat-rabbit" as a NOTICE,
then "Success: Updated 15 IP tag system", then failed at compose. After the fix
the same broken state self-heals — "Allocated IP: stoat/stoat-rabbit" — with no
uninstall.

Three more bugs in the same path, all found while tracing it:

- ipFindAvailable tested pool membership with a substring match against the
  newline-joined list of allocated IPs, so .4 read as taken whenever .46 or .147
  existed. Demonstrated: with 3 addresses allocated it excluded 5. Harmless at
  low occupancy, but it silently shrinks the pool as it fills and would report
  exhaustion early. Now an exact whole-line match.

- ipFindAvailable set available_ip="" on an exhausted pool and carried on to
  index the empty array, where RANDOM % 0 is a division-by-zero that would bury
  the real message. ipAllocation did the same and still ran its INSERT, writing
  a row with an empty resource_value — which then satisfied "this service has an
  allocation" forever after, making the service unrepairable. Both now return.

- first_allocated_ip was only assigned inside the allocate branch, so on every
  reinstall (where rows already exist) it came out empty and the trusted-domains
  list shipped with a hole. Now taken from the mapping.

An unfilled tag is also an error rather than a notice now: the compose is
unshippable at that point, and reporting "Success: updated 15 IP tags" is how
this reached the user as a confusing pull failure several steps later. The
install backstop no longer guesses "(image pull failed?)" either — that guess
was written for one cause and misdirects for every other.

Verified: clean install allocates all 16 with no unfilled tags, a deliberately
broken row self-heals, and no duplicate IPs exist across any app.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
librelad 2026-08-20 01:35:19 +01:00
parent aa4f1f1d9c
commit 770492b7c7
4 changed files with 93 additions and 54 deletions

View File

@ -193,7 +193,13 @@ installApp()
# still counts; only a total absence is treated as failure.
if declare -F dockerCommandRun >/dev/null 2>&1 \
&& ! dockerCommandRun "docker ps -a --filter label=com.docker.compose.project=$app_name --format '{{.Names}}' 2>/dev/null" 2>/dev/null | grep -q '[^[:space:]]'; then
isError "$app_name: no container started (image pull failed?) — not installed."
# Deliberately does not name a cause. "(image pull failed?)" was a
# guess carried over from the case this backstop was written for, and
# it actively misdirects for every other one — an unsubstituted tag
# that made compose reject the file reads as a registry problem, and
# the real error is further up the log.
isError "$app_name: no container started — not installed."
isNotice "The compose output above says why. Common causes: an image that could not be pulled, or a compose the daemon rejected (e.g. an unsubstituted LibrePortal tag)."
eval "$app_slug=n"
return 1
fi

View File

@ -44,6 +44,11 @@ ipAllocation()
if [[ -z "$available_ip" ]]; then
isError "No available IP addresses in pool"
allocated_ip=""
# Returning matters: without it the INSERT below still ran, writing a row
# with an empty resource_value. That row then satisfied "this service has
# an allocation" on every later lookup while carrying no address, so the
# service could never be repaired.
return 1
fi
local sql="INSERT INTO network_resources (app_name, resource_type, resource_value, service_name, status, created_date, created_time) VALUES ('$app_name', 'ip', '$available_ip', '$service_name', 'active', CURRENT_DATE, CURRENT_TIME);"

View File

@ -12,7 +12,16 @@ ipFindAvailable()
if [[ $? -ne 0 ]]; then
isError "Database query failed while checking existing IPs"
available_ip=""
return 1
fi
# Exact whole-line matching. The membership test used to be a substring check
# against the newline-joined list, so .4 read as taken whenever .46 or .147
# existed — silently shrinking a /24 pool to the addresses that happen not to
# be a prefix of another. Wrapping both sides in newlines makes the match
# exact, which is also why the delimiters are added rather than compared
# element by element: it stays one string test.
existing_ips=$'\n'"${existing_ips}"$'\n'
# Create an array to store available IPs
local available_ip_pool=()
@ -29,7 +38,7 @@ ipFindAvailable()
local test_ip="${subnet_base}.${i}"
# Check if IP is already allocated
if [[ "$existing_ips" != *"$test_ip"* ]]; then
if [[ "$existing_ips" != *$'\n'"$test_ip"$'\n'* ]]; then
available_ip_pool+=("$test_ip")
fi
done
@ -38,6 +47,10 @@ ipFindAvailable()
if [[ ${#available_ip_pool[@]} -eq 0 ]]; then
isError "No available IP addresses in subnet $CFG_NETWORK_SUBNET and all expansion ranges exhausted"
available_ip=""
# Returning matters: the line below indexes the pool, and RANDOM % 0 is a
# division-by-zero that would bury the real "pool exhausted" message under
# a bash arithmetic error.
return 1
fi
# Randomly select an IP from the available pool

View File

@ -7,78 +7,93 @@ ipUpdateComposeTags()
local app_name="$1"
local full_file_path="$2"
local ip_mapping_result=""
if [[ "$LIBREPORTAL_RESET_NETWORK" != "1" ]]; then
local sql="SELECT service_name, resource_value FROM network_resources WHERE app_name = '$app_name' AND resource_type = 'ip' AND status = 'active';"
ip_mapping_result=$(sqlite3 "$docker_dir/$db_file" "$sql" 2>/dev/null)
fi
declare -A ip_mapping
# Only process if we got results
if [[ -n "$ip_mapping_result" ]]; then
while IFS='|' read -r service_name resource_value; do
# Skip empty service names
if [[ -n "$service_name" && -n "$resource_value" ]]; then
ip_mapping["$service_name"]="$resource_value"
fi
done <<< "$ip_mapping_result"
else
# Get service names from SERVICE_TAG_N tags in docker-compose.yml
local compose_services=()
if [[ -f "$full_file_path" ]]; then
# Extract service names from SERVICE_TAG_N tags
for i in {1..20}; do
local service_tag="SERVICE_TAG_$i"
local compose_service=$(grep "#LIBREPORTAL|$service_tag|" "$full_file_path" | awk -F'|' '{print $3}' | head -1)
if [[ -n "$compose_service" ]]; then
compose_services+=("$compose_service")
# Existing allocations for this app. Skipped entirely on a full network reset,
# so every service falls through to ipAllocation and re-rolls.
if [[ "$LIBREPORTAL_RESET_NETWORK" != "1" ]]; then
local sql="SELECT service_name, resource_value FROM network_resources WHERE app_name = '$app_name' AND resource_type = 'ip' AND status = 'active';"
local ip_mapping_result
ip_mapping_result=$(sqlite3 "$docker_dir/$db_file" "$sql" 2>/dev/null)
if [[ -n "$ip_mapping_result" ]]; then
while IFS='|' read -r service_name resource_value; do
if [[ -n "$service_name" && -n "$resource_value" ]]; then
ip_mapping["$service_name"]="$resource_value"
fi
done
done <<< "$ip_mapping_result"
fi
# If no service tags found, no services to process
if [[ ${#compose_services[@]} -eq 0 ]]; then
isNotice "No service tags found, skipping IP allocation"
fi
# Allocate IPs for each service
service_counter=1
first_allocated_ip="" # Global variable for trusted domains processor
for compose_service in "${compose_services[@]}"; do
ipAllocation "$app_name" "$compose_service"
local service_allocated_ip="$allocated_ip" # Capture result immediately
# Set global first allocated IP (for trusted domains processor)
if [[ "$service_counter" -eq 1 && -n "$service_allocated_ip" ]]; then
first_allocated_ip="$service_allocated_ip"
fi
if [[ -n "$service_allocated_ip" ]]; then
ip_mapping["$compose_service"]="$service_allocated_ip"
fi
((service_counter++))
fi
# Every service the compose declares, in tag order.
local compose_services=()
local i compose_service
if [[ -f "$full_file_path" ]]; then
for i in {1..20}; do
compose_service=$(grep "#LIBREPORTAL|SERVICE_TAG_$i|" "$full_file_path" 2>/dev/null | awk -F'|' '{print $3}' | head -1)
[[ -n "$compose_service" ]] && compose_services+=("$compose_service")
done
fi
if [[ ${#compose_services[@]} -eq 0 ]]; then
isNotice "No service tags found, skipping IP allocation"
fi
# Allocate per service, for anything that does not already have a row.
#
# This used to be the else-branch of "did the database return any rows at
# all", which made allocation all-or-nothing: an app holding even one row
# skipped this loop entirely, so a service WITHOUT a row never got an IP and
# never would. Its IP_TAG_<n> then stayed unfilled, the literal IP_DATA_<n>
# reached the compose, and docker refused the app with
# invalid IPv4 address: ParseAddr("IP_DATA_3")
# surfacing to the user as "no container started (image pull failed?)".
#
# Partial state is not exotic: an app that GAINS a service in a later version
# hits it on the very next install, because the old services still hold rows.
# Nothing repaired it either — reinstalling re-ran the same skip, so the app
# stayed broken until someone uninstalled it and wiped the rows.
first_allocated_ip=""
for compose_service in "${compose_services[@]}"; do
if [[ -z "${ip_mapping[$compose_service]:-}" ]]; then
ipAllocation "$app_name" "$compose_service"
[[ -n "$allocated_ip" ]] && ip_mapping["$compose_service"]="$allocated_ip"
fi
# Feeds tagsProcessorTrustedDomains. Set from the mapping rather than only
# from a fresh allocation: it used to be assigned inside the allocate
# branch, so on every reinstall (where rows already existed) it came out
# empty and the trusted-domains list shipped with a hole in it.
[[ -z "$first_allocated_ip" ]] && first_allocated_ip="${ip_mapping[$compose_service]:-}"
done
# Walk SERVICE_TAG_N in numeric order so IP_TAG_i lines up with service i
# deterministically (associative array key order is hash-random).
local replaced_count=0
local missing_services=()
for i in {1..20}; do
local service_tag="SERVICE_TAG_$i"
local compose_service
compose_service=$(grep "#LIBREPORTAL|$service_tag|" "$full_file_path" 2>/dev/null \
compose_service=$(grep "#LIBREPORTAL|SERVICE_TAG_$i|" "$full_file_path" 2>/dev/null \
| awk -F'|' '{print $3}' | head -1)
[[ -z "$compose_service" ]] && continue
local allocated_ip="${ip_mapping[$compose_service]}"
local allocated_ip="${ip_mapping[$compose_service]:-}"
if [[ -n "$allocated_ip" ]]; then
tagsManagerUpdateUniversalTag "$full_file_path" "IP_TAG_$i" "$allocated_ip"
((replaced_count++))
else
isNotice "No IP allocated for service: $compose_service (IP_TAG_$i)"
missing_services+=("$compose_service (IP_TAG_$i)")
fi
done
# An unfilled tag is fatal, not a notice. The compose is now unshippable —
# docker rejects the literal IP_DATA_<n> — so saying "Success: updated 15 IP
# tags" and carrying on is how this surfaced as a confusing pull failure
# several steps later instead of as the allocation problem it is.
if [[ ${#missing_services[@]} -gt 0 ]]; then
isError "No IP allocated for ${#missing_services[@]} service(s) of $app_name — the compose cannot start:"
local miss
for miss in "${missing_services[@]}"; do echo " $miss"; done
isNotice "The IP pool may be exhausted, or the database rows for $app_name may be inconsistent."
return 1
fi
ip_update_result="Updated $replaced_count IP tag system"
isSuccessful "$ip_update_result"