SVI/ProxmoxVE
1
0
Fork 0
mirror of https://github.com/community-scripts/ProxmoxVE.git synced 2026-03-31 06:34:17 -04:00
Code Issues 2.6k Packages Projects Releases 10 Wiki Activity

[PR #8871] Refactor(tools.func): Add Retry Logic, OS-Upgrade Safety, Smart Version Detection + 10 Critical Bugfixes #7676

New Issue
Open
opened 2025-11-20 08:06:27 -05:00 by saavagebueno · 0 comments
saavagebueno commented 2025-11-20 08:06:27 -05:00
Owner
Copy Link

Original Pull Request: https://github.com/community-scripts/ProxmoxVE/pull/8871

State: closed
Merged: Yes

✍️ Description

  • Retry logic for APT operations (3 attempts, auto-cache refresh)
  • OS-upgrade-safe repository cleanup (all 3 keyring locations)
  • Smart version detection (already installed / upgrade / fresh install)
  • Enhanced error handling with validation and non-fatal fallbacks
  • Legacy cleanup (nvm, rbenv, rustup conflicts)

Fixes 10+ bugs: Node.js Debian conflicts, MySQL 8.0 on Debian 13+, PHP-FPM service conflicts, PostgreSQL backup/restore, repository keyring cleanup, GitHub API timeouts, and more.

AI-Summary

Click me

1. Unified Retry Logic for APT Operations

  • Added install_packages_with_retry() and upgrade_packages_with_retry() functions
  • Automatic retry (3 attempts) for transient APT/network failures
  • Automatic APT cache refresh between retries
  • Eliminates random installation failures in scripts

2. OS-Upgrade-Safe Repository Management

  • New prepare_repository_setup() function for unified repo cleanup
  • Cleanup from all 3 keyring locations (/etc/apt/keyrings, /usr/share/keyrings, /etc/apt/trusted.gpg.d)
  • Prevents "conflicting packages" errors after Debian/Ubuntu upgrades
  • cleanup_tool_keyrings() helper for multi-tool cleanup

3. Smart Version Detection & Upgrade Scenarios

All setup_* functions now handle 3 scenarios:

  1. Already at target version → Quick update with upgrade_packages_with_retry()
  2. Different version installed → Clean removal + fresh install
  3. Not installed → Fresh install with retry logic

This eliminates:

  • Unnecessary reinstalls when already at correct version
  • Version conflicts during upgrades
  • Repository mismatches

4. Enhanced Error Handling & Validation

  • Parameter validation in setup_deb822_repo() (fail-safe if GPG/repo URLs empty)
  • Version verification with verify_tool_version()
  • Improved error messages with context (e.g., "Failed to download from $URL")
  • Non-fatal fallbacks for optional operations (npm update, shell completions)

5. Legacy Installation Cleanup

  • New cleanup_legacy_install() function for nvm, rbenv, rustup, etc.
  • Prevents conflicts between system packages and user-space installers
  • Automatic cleanup of deprecated environment variables

6. Service Management Improvements

  • stop_all_services() with pattern matching (e.g., stops all php*-fpm services)
  • Prevents service conflicts during multi-version tool upgrades

7. Comprehensive Documentation

  • 32 lines of inline documentation at file header
  • Quick reference guide for core helpers
  • Usage examples for common patterns

🛠️ Functions Changed (Summary)

🆕 New Helper Functions

  • cleanup_tool_keyrings() - Remove keyrings from all 3 locations
  • stop_all_services() - Stop services by pattern
  • verify_tool_version() - Validate installed version
  • cleanup_legacy_install() - Remove nvm, rbenv, rustup, etc.
  • prepare_repository_setup() - Unified repo cleanup + APT validation
  • install_packages_with_retry() - Install with 3 retries
  • upgrade_packages_with_retry() - Upgrade with 3 retries
  • is_tool_installed() - Check if tool is installed (with version)
  • remove_old_tool_version() - Purge old tool version completely
  • should_update_tool() - Determine if update is needed
  • manage_tool_repository() - Unified repository management
  • _install_uvx_wrapper() - Helper for uvx setup

🔧 Refactored Setup Functions

Database Engines:

  • setup_mariadb() - 3-scenario logic, retry, better version resolution
  • setup_mysql() - Debian 13+ libaio1t64 fix, retry logic, fallback to MariaDB
  • setup_postgresql() - Backup/restore on upgrade, retry logic, module support
  • setup_mongodb() - Unified repo management, retry logic
  • setup_clickhouse() - 3-scenario logic, retry, version detection

Programming Languages:

  • setup_nodejs() - Complete rewrite: removes ALL Debian nodejs packages before NodeSource, prevents conflicts, retry logic
  • setup_php() - Stop all PHP-FPM services, cleanup, retry logic
  • setup_ruby() - Conditional rbenv/ruby-build download, retry logic
  • setup_rust() - 3-scenario logic, toolchain update
  • setup_go() - Version validation, clean upgrade, retry

Tools & Utilities:

  • setup_java() - Temurin JDK upgrade logic, retry
  • setup_uv() - i686 arch support, uvx wrapper, shell completions, Python version install
  • setup_yq() - Better error handling, GitHub API timeout
  • setup_composer() - Self-update logic for existing installations
  • setup_ffmpeg() - Binary fallback if source build fails
  • setup_gs() (Ghostscript) - Fallback logic if GitHub API fails
  • setup_hwaccel() - Improved GPU detection, error handling
  • setup_imagemagick() - Version tracking, upgrade messages
  • setup_adminer() - No functional changes
  • setup_local_ip_helper() - Update detection

🐛 Bugfixes

  1. Node.js Debian Package Conflicts (Critical Fix)

    • Removes ALL Debian nodejs packages (libnode*, node-*) before adding NodeSource
    • Prevents "trying to overwrite" errors on fresh installs
    • Fixes: node-undici, node-cjs, node-acorn conflicts

  2. MySQL 8.0 on Debian 13+ (Trixie/Sid)

    • Forces MySQL 8.4 LTS (libaio1t64 compatible)
    • Fallback to MariaDB if 8.4 fails
    • Prevents "libaio1 not found" errors

  3. PHP-FPM Service Conflicts

    • Now stops ALL php*-fpm services before upgrade (not just old version)
    • Prevents socket conflicts during version changes

  4. PostgreSQL Backup/Restore

    • Creates backup before major version upgrade
    • Attempts restore (with warning if fails)
    • Prevents data loss during upgrades

  5. Repository Keyring Cleanup

    • Cleans keyrings from all 3 locations (not just /etc/apt/keyrings)
    • Fixes "conflicting keys" after OS upgrades

  6. APT Working State Validation

    • ensure_apt_working() called before all operations
    • Fixes "dpkg interrupted" errors

  7. GitHub API Timeouts

    • All curl calls now have --max-time 15 timeout
    • Prevents indefinite hangs on slow networks
    • Fallback logic if API fails

  8. Empty Version Variables

    • Parameter validation in setup_deb822_repo()
    • Prevents creating invalid repo files

  9. Module Installation Failures

    • Non-fatal npm/gem/crate installs (warnings instead of errors)
    • Script continues even if optional modules fail

  10. Version Mismatch Warnings

    • verify_tool_version() warns if installed version differs from expected
    • Helps debug upgrade issues

📊 Statistics

  • File Changed: misc/tools.func
  • Lines Added: +1599
  • Lines Removed: -769
  • Net Change: +830 lines
  • Functions Added: 12 new helper functions
  • Functions Refactored: 20+ setup functions

🧪 Testing Performed

Test Environments

  • ✅ Debian 12 (Bookworm) - Fresh install
  • ✅ Debian 13 (Trixie) - Upgraded from Debian 12
  • ✅ Ubuntu 24.04 (Noble) - Fresh install

Tested Scenarios

  1. Fresh Installs: MariaDB, MySQL, PostgreSQL, MongoDB, Node.js, PHP, Ruby, Go, Rust
  2. Version Upgrades: PHP 8.2→8.3, Node 20→22, PostgreSQL 15→16, MariaDB 11.3→11.4
  3. OS Upgrades: Debian 12→13 (repository cleanup validation)
  4. Network Failures: Tested retry logic with iptables blocking
  5. Conflict Resolution: Node.js Debian package conflicts, MySQL libaio1t64

🔗 Related PR / Issue

Link: #

✅ Prerequisites (X in brackets)

  • Self-review completed – Code follows project standards.
  • Tested thoroughly – Changes work as expected.
  • No security risks – No hardcoded secrets, unnecessary privilege escalations, or permission issues.

🛠️ Type of Change (X in brackets)

  • 🐞 Bug fix – Resolves an issue without breaking functionality.
  • ✨ New feature – Adds new, non-breaking functionality.
  • 💥 Breaking change – Alters existing functionality in a way that may require updates.
  • 🆕 New script – A fully functional and tested script or script set.
  • 🌍 Website update – Changes to website-related JSON files or metadata.
  • 🔧 Refactoring / Code Cleanup – Improves readability or maintainability without changing functionality.
  • 📝 Documentation update – Changes to README, AppName.md, CONTRIBUTING.md, or other docs.
**Original Pull Request:** https://github.com/community-scripts/ProxmoxVE/pull/8871 **State:** closed **Merged:** Yes --- <!--🛑 New scripts must be submitted to [ProxmoxVED](https://github.com/community-scripts/ProxmoxVED) for testing. PRs without prior testing will be closed. --> ## ✍️ Description - **Retry logic** for APT operations (3 attempts, auto-cache refresh) - **OS-upgrade-safe** repository cleanup (all 3 keyring locations) - **Smart version detection** (already installed / upgrade / fresh install) - **Enhanced error handling** with validation and non-fatal fallbacks - **Legacy cleanup** (nvm, rbenv, rustup conflicts) **Fixes 10+ bugs:** Node.js Debian conflicts, MySQL 8.0 on Debian 13+, PHP-FPM service conflicts, PostgreSQL backup/restore, repository keyring cleanup, GitHub API timeouts, and more. ## AI-Summary <details> <summary>Click me</summary> #### 1. **Unified Retry Logic for APT Operations** - Added `install_packages_with_retry()` and `upgrade_packages_with_retry()` functions - Automatic retry (3 attempts) for transient APT/network failures - Automatic APT cache refresh between retries - Eliminates random installation failures in scripts #### 2. **OS-Upgrade-Safe Repository Management** - New `prepare_repository_setup()` function for unified repo cleanup - Cleanup from **all 3 keyring locations** (`/etc/apt/keyrings`, `/usr/share/keyrings`, `/etc/apt/trusted.gpg.d`) - Prevents "conflicting packages" errors after Debian/Ubuntu upgrades - `cleanup_tool_keyrings()` helper for multi-tool cleanup #### 3. **Smart Version Detection & Upgrade Scenarios** All `setup_*` functions now handle **3 scenarios**: 1. **Already at target version** → Quick update with `upgrade_packages_with_retry()` 2. **Different version installed** → Clean removal + fresh install 3. **Not installed** → Fresh install with retry logic This eliminates: - Unnecessary reinstalls when already at correct version - Version conflicts during upgrades - Repository mismatches #### 4. **Enhanced Error Handling & Validation** - Parameter validation in `setup_deb822_repo()` (fail-safe if GPG/repo URLs empty) - Version verification with `verify_tool_version()` - Improved error messages with context (e.g., "Failed to download from $URL") - Non-fatal fallbacks for optional operations (npm update, shell completions) #### 5. **Legacy Installation Cleanup** - New `cleanup_legacy_install()` function for nvm, rbenv, rustup, etc. - Prevents conflicts between system packages and user-space installers - Automatic cleanup of deprecated environment variables #### 6. **Service Management Improvements** - `stop_all_services()` with pattern matching (e.g., stops all `php*-fpm` services) - Prevents service conflicts during multi-version tool upgrades #### 7. **Comprehensive Documentation** - 32 lines of inline documentation at file header - Quick reference guide for core helpers - Usage examples for common patterns --- ## 🛠️ Functions Changed (Summary) ### 🆕 New Helper Functions - `cleanup_tool_keyrings()` - Remove keyrings from all 3 locations - `stop_all_services()` - Stop services by pattern - `verify_tool_version()` - Validate installed version - `cleanup_legacy_install()` - Remove nvm, rbenv, rustup, etc. - `prepare_repository_setup()` - Unified repo cleanup + APT validation - `install_packages_with_retry()` - Install with 3 retries - `upgrade_packages_with_retry()` - Upgrade with 3 retries - `is_tool_installed()` - Check if tool is installed (with version) - `remove_old_tool_version()` - Purge old tool version completely - `should_update_tool()` - Determine if update is needed - `manage_tool_repository()` - Unified repository management - `_install_uvx_wrapper()` - Helper for uvx setup ### 🔧 Refactored Setup Functions **Database Engines:** - `setup_mariadb()` - 3-scenario logic, retry, better version resolution - `setup_mysql()` - Debian 13+ libaio1t64 fix, retry logic, fallback to MariaDB - `setup_postgresql()` - Backup/restore on upgrade, retry logic, module support - `setup_mongodb()` - Unified repo management, retry logic - `setup_clickhouse()` - 3-scenario logic, retry, version detection **Programming Languages:** - `setup_nodejs()` - **Complete rewrite**: removes ALL Debian nodejs packages before NodeSource, prevents conflicts, retry logic - `setup_php()` - Stop all PHP-FPM services, cleanup, retry logic - `setup_ruby()` - Conditional rbenv/ruby-build download, retry logic - `setup_rust()` - 3-scenario logic, toolchain update - `setup_go()` - Version validation, clean upgrade, retry **Tools & Utilities:** - `setup_java()` - Temurin JDK upgrade logic, retry - `setup_uv()` - i686 arch support, uvx wrapper, shell completions, Python version install - `setup_yq()` - Better error handling, GitHub API timeout - `setup_composer()` - Self-update logic for existing installations - `setup_ffmpeg()` - Binary fallback if source build fails - `setup_gs()` (Ghostscript) - Fallback logic if GitHub API fails - `setup_hwaccel()` - Improved GPU detection, error handling - `setup_imagemagick()` - Version tracking, upgrade messages - `setup_adminer()` - No functional changes - `setup_local_ip_helper()` - Update detection --- ## 🐛 Bugfixes 1. **Node.js Debian Package Conflicts** (Critical Fix) - Removes ALL Debian nodejs packages (`libnode*`, `node-*`) **before** adding NodeSource - Prevents "trying to overwrite" errors on fresh installs - Fixes: `node-undici`, `node-cjs`, `node-acorn` conflicts 2. **MySQL 8.0 on Debian 13+** (Trixie/Sid) - Forces MySQL 8.4 LTS (libaio1t64 compatible) - Fallback to MariaDB if 8.4 fails - Prevents "libaio1 not found" errors 3. **PHP-FPM Service Conflicts** - Now stops **ALL** `php*-fpm` services before upgrade (not just old version) - Prevents socket conflicts during version changes 4. **PostgreSQL Backup/Restore** - Creates backup before major version upgrade - Attempts restore (with warning if fails) - Prevents data loss during upgrades 5. **Repository Keyring Cleanup** - Cleans keyrings from **all 3 locations** (not just `/etc/apt/keyrings`) - Fixes "conflicting keys" after OS upgrades 6. **APT Working State Validation** - `ensure_apt_working()` called before all operations - Fixes "dpkg interrupted" errors 7. **GitHub API Timeouts** - All `curl` calls now have `--max-time 15` timeout - Prevents indefinite hangs on slow networks - Fallback logic if API fails 8. **Empty Version Variables** - Parameter validation in `setup_deb822_repo()` - Prevents creating invalid repo files 9. **Module Installation Failures** - Non-fatal npm/gem/crate installs (warnings instead of errors) - Script continues even if optional modules fail 10. **Version Mismatch Warnings** - `verify_tool_version()` warns if installed version differs from expected - Helps debug upgrade issues --- ## 📊 Statistics - **File Changed:** `misc/tools.func` - **Lines Added:** `+1599` - **Lines Removed:** `-769` - **Net Change:** `+830 lines` - **Functions Added:** `12 new helper functions` - **Functions Refactored:** `20+ setup functions` --- ## 🧪 Testing Performed ### Test Environments - ✅ Debian 12 (Bookworm) - Fresh install - ✅ Debian 13 (Trixie) - Upgraded from Debian 12 - ✅ Ubuntu 24.04 (Noble) - Fresh install ### Tested Scenarios 1. **Fresh Installs:** MariaDB, MySQL, PostgreSQL, MongoDB, Node.js, PHP, Ruby, Go, Rust 2. **Version Upgrades:** PHP 8.2→8.3, Node 20→22, PostgreSQL 15→16, MariaDB 11.3→11.4 3. **OS Upgrades:** Debian 12→13 (repository cleanup validation) 4. **Network Failures:** Tested retry logic with `iptables` blocking 5. **Conflict Resolution:** Node.js Debian package conflicts, MySQL libaio1t64 </details> ## 🔗 Related PR / Issue Link: # ## ✅ Prerequisites (**X** in brackets) - [x] **Self-review completed** – Code follows project standards. - [x] **Tested thoroughly** – Changes work as expected. - [x] **No security risks** – No hardcoded secrets, unnecessary privilege escalations, or permission issues. --- ## 🛠️ Type of Change (**X** in brackets) - [ ] 🐞 **Bug fix** – Resolves an issue without breaking functionality. - [ ] ✨ **New feature** – Adds new, non-breaking functionality. - [ ] 💥 **Breaking change** – Alters existing functionality in a way that may require updates. - [ ] 🆕 **New script** – A fully functional and tested script or script set. - [ ] 🌍 **Website update** – Changes to website-related JSON files or metadata. - [x] 🔧 **Refactoring / Code Cleanup** – Improves readability or maintainability without changing functionality. - [ ] 📝 **Documentation update** – Changes to `README`, `AppName.md`, `CONTRIBUTING.md`, or other docs.
saavagebueno added the pull-request label 2025-11-20 08:06:27 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
🛑 Failure to comply with the guidelines
breaking change
bug
bug
bugfix
deferred
delete script
dependencies
enhancement
external
feature
github
help wanted
Implemented in VED waiting push to Main
in project pipeline
invalid
investigation
json
maintenance
new script
new script
nice to have
not a script issue
not planned
organization
pull-request
Mirrored from GitHub Pull Request
 question
refactor
rename script
security
update script
website
wontdo
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
saavagebueno
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: SVI/ProxmoxVE#7676
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?