refactor: implement dynamic storage discovery and add unit tests

infra_cli/proxmox.py

@@ -2,11 +2,12 @@ from .ssh import SSHClient
 
 class ProxmoxManager:
     def __init__(self, config, node_name=None):
-        node = config.get_node(node_name)
+        self.config = config
+        self.node_name = node_name or 'la-vmh-11'
+        node = config.get_node(self.node_name)
         if not node:
-            raise ValueError(f"Node '{node_name}' not found in config")
+            raise ValueError(f"Node '{self.node_name}' not found in configuration")
         
-        self.node_name = node_name
         self.host = node['host']
         self.password = node.get('pass')
         self.user = config.get('proxmox.user', 'root')
@@ -17,10 +18,6 @@ class ProxmoxManager:
         res = self.client.run("pct list")
         return res.stdout
 
-    def list_vms(self):
-        res = self.client.run("qm list")
-        return res.stdout
-
     def status_lxc(self, vmid):
         res = self.client.run(f"pct status {vmid}")
         return res.stdout
@@ -28,39 +25,43 @@ class ProxmoxManager:
     def resolve_template(self, alias):
         """Resolves a template alias (e.g. 'debian-13') to the latest full path on the node"""
         if alias == "debian-13":
-            # Search for debian-13 templates in all storages
+            # Search for debian-13 templates in all active storages
             res = self.client.run("pvesm list --content vztmpl | grep 'debian-13-standard' | sort -V | tail -n 1")
             if res.returncode == 0 and res.stdout.strip():
-                # Extract first column (Volid)
                 return res.stdout.split()[0]
-        return alias # Fallback to original if no resolution
+        return alias
 
-    def create_lxc(self, vmid, template, hostname, ip, gateway, bridge="vmbr0", storage="local-zfs", password=None):
+    def discover_storage(self):
-        # Resolve template if it's an alias
+        """Finds the best storage for container rootfs (prefers ZFS or LVM-thin)"""
+        res = self.client.run("pvesm status | grep -E 'zfspool|lvmthin' | grep ' active' | head -n 1")
+        if res.returncode == 0 and res.stdout.strip():
+            return res.stdout.split()[0]
+        return "local" # Absolute fallback
+
+    def create_lxc(self, vmid, template, hostname, ip, gateway, bridge="vmbr0", storage=None, password=None):
         full_template = self.resolve_template(template)
+        target_storage = storage or self.discover_storage()
         
         # Professional creation command with sane defaults
         cmd = f"pct create {vmid} {full_template} --hostname {hostname} " \
               f"--net0 name=eth0,bridge={bridge},ip={ip},gw={gateway} " \
-              f"--storage {storage} --onboot 1"
+              f"--storage {target_storage} --onboot 1"
-
         
         if password:
             cmd += f" --password {password}"
 
         res = self.client.run(cmd)
         if res.returncode != 0:
-            raise RuntimeError(f"Failed to create LXC {vmid}: {res.stderr}")
+            raise RuntimeError(f"Failed to create LXC {vmid} on {self.node_name}: {res.stderr}")
+        
+        # Start the container
+        self.client.run(f"pct start {vmid}")
         return res.stdout
 
-    def start_lxc(self, vmid):
-        return self.client.run(f"pct start {vmid}")
-
-    def stop_lxc(self, vmid):
-        return self.client.run(f"pct stop {vmid}")
-
     def delete_lxc(self, vmid):
-        res = self.client.run(f"pct destroy {vmid}")
+        # Stop first
+        self.client.run(f"pct stop {vmid}")
+        res = self.client.run(f"pct destroy {vmid} --purge")
         if res.returncode != 0:
             raise RuntimeError(f"Failed to destroy LXC {vmid}: {res.stderr}")
         return res.stdout

tests/unit/test_proxmox_logic.py

@@ -0,0 +1,33 @@
+import pytest
+from unittest.mock import MagicMock, patch
+from infra_cli.proxmox import ProxmoxManager
+
+class MockConfig:
+    def get_node(self, name):
+        return {"host": "1.2.3.4", "pass": "secret"}
+    def get(self, key, default=None):
+        return default
+
+@patch('infra_cli.proxmox.SSHClient')
+def test_template_resolution(mock_ssh):
+    # Setup mock SSH response
+    mock_instance = mock_ssh.return_value
+    mock_instance.run.return_value.returncode = 0
+    mock_instance.run.return_value.stdout = "local:vztmpl/debian-13-standard_13.1-2_amd64.tar.zst tzst vztmpl 129710398"
+    
+    mgr = ProxmoxManager(MockConfig())
+    resolved = mgr.resolve_template("debian-13")
+    
+    assert resolved == "local:vztmpl/debian-13-standard_13.1-2_amd64.tar.zst"
+    mock_instance.run.assert_called_with("pvesm list --content vztmpl | grep 'debian-13-standard' | sort -V | tail -n 1")
+
+@patch('infra_cli.proxmox.SSHClient')
+def test_storage_discovery(mock_ssh):
+    mock_instance = mock_ssh.return_value
+    mock_instance.run.return_value.returncode = 0
+    mock_instance.run.return_value.stdout = "local-zfs zfspool active 1000 500 500 50%"
+    
+    mgr = ProxmoxManager(MockConfig())
+    storage = mgr.discover_storage()
+    
+    assert storage == "local-zfs"