From 770492b7c72664fc12c97bb11175fa9d5fdbb067 Mon Sep 17 00:00:00 2001 From: librelad Date: Thu, 20 Aug 2026 01:35:19 +0100 Subject: [PATCH] Allocate IPs per missing service, not all-or-nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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_ stayed unfilled, the literal IP_DATA_ 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 --- scripts/app/install/app_install.sh | 8 +- scripts/network/ip/ip_allocation.sh | 5 + scripts/network/ip/ip_find_available.sh | 15 ++- scripts/network/ip/ip_replace_tags.sh | 119 +++++++++++++----------- 4 files changed, 93 insertions(+), 54 deletions(-) diff --git a/scripts/app/install/app_install.sh b/scripts/app/install/app_install.sh index 01f04d2..e4ffed7 100644 --- a/scripts/app/install/app_install.sh +++ b/scripts/app/install/app_install.sh @@ -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 diff --git a/scripts/network/ip/ip_allocation.sh b/scripts/network/ip/ip_allocation.sh index 8c64903..4a1ac94 100755 --- a/scripts/network/ip/ip_allocation.sh +++ b/scripts/network/ip/ip_allocation.sh @@ -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);" diff --git a/scripts/network/ip/ip_find_available.sh b/scripts/network/ip/ip_find_available.sh index d717083..9f539fa 100755 --- a/scripts/network/ip/ip_find_available.sh +++ b/scripts/network/ip/ip_find_available.sh @@ -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 diff --git a/scripts/network/ip/ip_replace_tags.sh b/scripts/network/ip/ip_replace_tags.sh index c678233..040e6a1 100755 --- a/scripts/network/ip/ip_replace_tags.sh +++ b/scripts/network/ip/ip_replace_tags.sh @@ -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_ then stayed unfilled, the literal IP_DATA_ + # 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_ — 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"