-
Notifications
You must be signed in to change notification settings - Fork 0
Add ProxMox multi-instance installation support #16
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.
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
Add ProxMox multi-instance installation support #16
Conversation
Add comprehensive support for deploying TailOpsMCP from a Proxmox host to multiple LXC containers simultaneously. New features: - install-proxmox-multi.sh: Main installation script for multi-container deployment - proxmox-api.sh: Helper library for Proxmox LXC operations (container management, feature configuration) - proxmox-multi.conf: Configuration template with extensive options - Sequential and parallel deployment strategies - Auto-configure container features (nesting, TUN device, AppArmor) - Per-container configuration overrides - Comprehensive validation and health checks - Deployment status tracking and reporting Documentation: - PROXMOX_MULTI_CONTAINER_INSTALL.md: Complete installation guide with examples - PROXMOX_MULTI_CONTAINER_QUICKSTART.md: Quick reference cheat sheet - Updated README.md with new installation method Use cases: - Development environments: Multiple isolated test instances - Production deployments: High availability across containers - Multi-tenant setups: Separate instances per team/user - Configuration testing: Different auth modes and settings This enables efficient management of multiple TailOpsMCP instances in home lab and production Proxmox environments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds comprehensive multi-container deployment capabilities for TailOpsMCP in Proxmox environments, enabling deployment to multiple LXC containers from a single Proxmox host.
Key Changes:
- New multi-container installation script with support for sequential and parallel deployment strategies
- Proxmox API helper library for LXC container operations (creation, feature configuration, validation)
- Configuration template system with per-container overrides and multiple authentication modes
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/install/templates/proxmox-multi.conf | Configuration template with deployment options, container settings, and authentication modes |
| scripts/install/lib/proxmox-api.sh | Helper library providing Proxmox LXC operations (container management, feature checks, resource validation) |
| scripts/install/install-proxmox-multi.sh | Main installation orchestrator handling multi-container deployment with status tracking |
| docs/PROXMOX_MULTI_CONTAINER_QUICKSTART.md | Quick reference guide with common commands and configuration examples |
| docs/PROXMOX_MULTI_CONTAINER_INSTALL.md | Comprehensive installation guide with detailed configuration options and troubleshooting |
| README.md | Updated with new Proxmox multi-container installation method |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Update existing features line | ||
| sed -i 's/^features:.*/&,nesting=1,keyctl=1/' "$config_file" | ||
| # Remove duplicate features | ||
| sed -i 's/nesting=1,nesting=1/nesting=1/g' "$config_file" | ||
| sed -i 's/keyctl=1,keyctl=1/keyctl=1/g' "$config_file" |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sed command on line 136 appends 'nesting=1,keyctl=1' to existing features without checking if they already exist, potentially creating entries like 'features: nesting=1,keyctl=1,nesting=1,keyctl=1'. While lines 138-139 attempt to remove duplicates, this approach won't work if the features already exist in different order (e.g., 'keyctl=1,nesting=1'). Use grep to check for each feature individually before adding, or use a more robust parsing approach.
| # Update existing features line | |
| sed -i 's/^features:.*/&,nesting=1,keyctl=1/' "$config_file" | |
| # Remove duplicate features | |
| sed -i 's/nesting=1,nesting=1/nesting=1/g' "$config_file" | |
| sed -i 's/keyctl=1,keyctl=1/keyctl=1/g' "$config_file" | |
| # Update existing features line robustly (avoid duplicates) | |
| current_features=$(grep '^features:' "$config_file" | head -n1 | cut -d':' -f2- | tr -d ' ') | |
| new_features="$current_features" | |
| # Add nesting=1 if missing | |
| if ! echo ",$new_features," | grep -q ",nesting=1,"; then | |
| if [ -n "$new_features" ]; then | |
| new_features="$new_features,nesting=1" | |
| else | |
| new_features="nesting=1" | |
| fi | |
| fi | |
| # Add keyctl=1 if missing | |
| if ! echo ",$new_features," | grep -q ",keyctl=1,"; then | |
| if [ -n "$new_features" ]; then | |
| new_features="$new_features,keyctl=1" | |
| else | |
| new_features="keyctl=1" | |
| fi | |
| fi | |
| # Replace the features line with the updated one | |
| sed -i "s/^features:.*/features: $new_features/" "$config_file" |
| --exclude='.git' \ | ||
| --exclude='*.pyc' \ | ||
| --exclude='__pycache__' \ | ||
| --exclude='venv' \ | ||
| --exclude='.env' \ |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The exclusion list for creating the tarball is duplicated logic that should be centralized. If the project structure or exclusion requirements change, this needs to be updated in multiple places. Consider defining the exclusions in a shared variable or configuration file.
| --exclude='.git' \ | |
| --exclude='*.pyc' \ | |
| --exclude='__pycache__' \ | |
| --exclude='venv' \ | |
| --exclude='.env' \ | |
| "${TARBALL_EXCLUDES[@]}" \ |
|
|
||
| # Merge with CONTAINERS if specified | ||
| if [ -n "$CONTAINERS" ]; then | ||
| CONTAINERS="$CONTAINERS,${resolved_ids[*]}" |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using ${resolved_ids[]} without IFS=',' will concatenate array elements with spaces instead of commas, creating an invalid CONTAINERS value like '101,102 103 104'. Should use '$(IFS=,; echo "${resolved_ids[]}")' to properly join with commas.
| CONTAINERS="$CONTAINERS,${resolved_ids[*]}" | |
| CONTAINERS="$CONTAINERS,$(IFS=,; echo "${resolved_ids[*]}")" |
| # Try to get IP from container's network interface | ||
| ip=$(pct exec "$ctid" -- ip -4 addr show eth0 2>/dev/null | grep -oP 'inet \K[\d.]+' | head -1) |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function assumes the network interface is named 'eth0', which may not be true for all container configurations. Consider detecting the primary network interface dynamically or making it configurable.
| # Try to get IP from container's network interface | |
| ip=$(pct exec "$ctid" -- ip -4 addr show eth0 2>/dev/null | grep -oP 'inet \K[\d.]+' | head -1) | |
| # Dynamically detect the primary network interface (excluding 'lo') | |
| local iface | |
| iface=$(pct exec "$ctid" -- ip -o -4 addr show | awk '!/ lo / {print $2; exit}') | |
| if [ -n "$iface" ]; then | |
| ip=$(pct exec "$ctid" -- ip -4 addr show "$iface" 2>/dev/null | grep -oP 'inet \K[\d.]+' | head -1) | |
| fi |
| local disk_size | ||
| disk_size=$(echo "$rootfs" | grep -oP 'size=\K\d+' || echo "0") | ||
| if [ "$disk_size" -gt 0 ] && [ "$disk_size" -lt "$min_disk" ]; then | ||
| issues+=("Disk space ($disk_size GB) is less than minimum ($min_disk GB)") | ||
| fi |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The grep pattern extracts only the numeric value but doesn't account for the unit suffix (e.g., 'G' in '8G'). The comparison on line 359 treats this as GB, but if the actual value is in TB or MB, the comparison will be incorrect. Parse and normalize the unit or extract it properly.
| local disk_size | |
| disk_size=$(echo "$rootfs" | grep -oP 'size=\K\d+' || echo "0") | |
| if [ "$disk_size" -gt 0 ] && [ "$disk_size" -lt "$min_disk" ]; then | |
| issues+=("Disk space ($disk_size GB) is less than minimum ($min_disk GB)") | |
| fi | |
| local disk_size_raw disk_size_num disk_size_unit disk_size_gb | |
| disk_size_raw=$(echo "$rootfs" | grep -oP 'size=\K[0-9]+[KMGTP]?' || echo "0") | |
| disk_size_num=$(echo "$disk_size_raw" | grep -oP '^\d+') | |
| disk_size_unit=$(echo "$disk_size_raw" | grep -oP '[KMGTP]$') | |
| # Default to G if no unit is specified | |
| if [ -z "$disk_size_unit" ]; then | |
| disk_size_unit="G" | |
| fi | |
| case "$disk_size_unit" in | |
| K) disk_size_gb=$((disk_size_num / 1024 / 1024));; | |
| M) disk_size_gb=$((disk_size_num / 1024));; | |
| G) disk_size_gb=$((disk_size_num));; | |
| T) disk_size_gb=$((disk_size_num * 1024));; | |
| P) disk_size_gb=$((disk_size_num * 1024 * 1024));; | |
| *) disk_size_gb=$((disk_size_num));; # fallback, treat as GB | |
| esac | |
| if [ "$disk_size_gb" -gt 0 ] && [ "$disk_size_gb" -lt "$min_disk" ]; then | |
| issues+=("Disk space ($disk_size_gb GB) is less than minimum ($min_disk GB)") | |
| fi |
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@mdlmarkham I've opened a new pull request, #17, to work on those changes. Once the pull request is ready, I'll request review from you. |
Add comprehensive support for deploying TailOpsMCP from a Proxmox host to multiple LXC containers simultaneously.
New features:
Documentation:
Use cases:
This enables efficient management of multiple TailOpsMCP instances in home lab and production Proxmox environments.