Ensure vm_path() is used to put files into a VM

igvm/hypervisor.py

@@ -4,6 +4,7 @@
 """
 
 import logging
+import traceback
 import math
 
 from adminapi.dataset import DatasetError
@@ -506,6 +507,10 @@ def mount_vm_storage(self, vm, transaction=None):
             )
 
         vm.mounted = True
+        log.warning(
+            'FSOP mount_vm_storage %s mounted=True path=%s',
+            vm.fqdn, self._mount_path[vm],
+        )
         return self._mount_path[vm]
 
     def umount_vm_storage(self, vm):
@@ -516,6 +521,10 @@ def umount_vm_storage(self, vm):
         self.remove_temp(self._mount_path[vm])
         del self._mount_path[vm]
         vm.mounted = False
+        log.warning(
+            'FSOP umount_vm_storage %s mounted=False\n%s',
+            vm.fqdn, ''.join(traceback.format_stack()[:-1]),
+        )
 
     def vm_sync_from_hypervisor(self, vm):
         """Synchronizes serveradmin information from the actual data on
@@ -880,6 +889,12 @@ def mount_temp(self, device, suffix=''):
         """Mounts given device into temporary path"""
         mount_dir = self.run('mktemp -d --suffix {}'.format(suffix))
         self.run('mount {0} {1}'.format(device, mount_dir))
+        # Guard: a silently-unmounted dir would make later chroot/writes land
+        # on the hypervisor's own root filesystem (NDCO-6147).
+        if self.run('mountpoint -q {}'.format(mount_dir), warn_only=True).failed:
+            raise HypervisorError(
+                'Device {} is not mounted at {}'.format(device, mount_dir)
+            )
         return mount_dir
 
     def umount_temp(self, device_or_path):

igvm/vm.py

@@ -10,6 +10,7 @@
 import re
 import stat
 import time
+import traceback
 
 import botocore.exceptions
 import tqdm
@@ -94,6 +95,21 @@ def vm_host(self):
             return self.hypervisor.fabric_settings()
         return self.fabric_settings()
 
+    def _debug_fs_op(self, op, target):
+        """DEBUG: trace writes that must land inside the VM, not the HV.
+
+        ponytail: temporary; grep 'FSOP' and remove once NDCO-6147 is closed.
+        """
+        try:
+            resolved = self.vm_path(target)
+        except Exception as e:
+            resolved = 'vm_path() FAILED: {!r}'.format(e)
+        log.warning(
+            'FSOP %s class=%s mounted=%s target=%r resolved=%r\n%s',
+            op, type(self).__name__, self.mounted, target, resolved,
+            ''.join(traceback.format_stack()[:-1]),
+        )
+
     def vm_path(self, path=''):
         """ Append correct prefix to reach VM's / directory """
 
@@ -113,6 +129,11 @@ def run(self, command, silent=False, with_sudo=True):
         """
         with self.vm_host():
             if self.mounted:
+                log.warning(
+                    'FSOP run(chroot) class=%s mounted=%s root=%r cmd=%r',
+                    type(self).__name__, self.mounted, self.vm_path(''),
+                    command,
+                )
                 return self.hypervisor.run(
                     'chroot {} /bin/sh -c \'{}\''.format(
                         self.vm_path(''), command,
@@ -121,6 +142,11 @@ def run(self, command, silent=False, with_sudo=True):
                     silent=silent,
                     with_sudo=with_sudo,
                 )
+            log.warning(
+                'FSOP run(no-chroot) class=%s mounted=%s cmd=%r\n%s',
+                type(self).__name__, self.mounted, command,
+                ''.join(traceback.format_stack()[:-1]),
+            )
             return super(VM, self).run(command, silent=silent)
 
     def read_file(self, path):
@@ -130,6 +156,7 @@ def read_file(self, path):
 
     def upload_template(self, filename, destination, context=None):
         """" Same as Fabric's template() but works on mounted or running vm """
+        self._debug_fs_op('upload_template', destination)
         template_dir = os.path.join(os.path.dirname(__file__), 'templates')
         with self.vm_host():
             return upload_template(
@@ -154,11 +181,20 @@ def put(self, remote_path, local_path, mode='0644'):
             seems broken, at least for mounted VM. This is why we run
             extra commands here.
         """
+        # with `_remote_path = remote_path`
+        # sudo: chroot /tmp/tmp.leAAGb5wRG-igvm-00-0.test.ig.local/ /bin/sh -c 'mv /tmp/b0021aa1-d5e1-4ecc-ab6c-070ded9b93d9 /etc/hostname ; chmod 0644 /etc/hostname'
+
+        # with `_remote_path = self.vm_path(remote_path)`
+        # sudo: chroot /tmp/tmp.rdY4ZXUVTv-igvm-00-0.test.ig.local/ /bin/sh -c 'mv /tmp/cf4ecdff-e3c9-4a2d-a3b1-ea824ef03d46 /tmp/tmp.rdY4ZXUVTv-igvm-00-0.test.ig.local//etc/hostname ; chmod 0644 /tmp/tmp.rdY4ZXUVTv-igvm-00-0.test.ig.local//etc/hostname'
+
+        self._debug_fs_op('put', remote_path)
         with self.vm_host():
             tempfile = '/tmp/' + str(uuid4())
             put(local_path, self.vm_path(tempfile))
+            _remote_path = remote_path
+            # _remote_path = self.vm_path(remote_path)
             self.run('mv {0} {1} ; chmod {2} {1}'.format(
-                tempfile, remote_path, mode
+                tempfile, _remote_path, mode
             ))
 
     def set_state(self, new_state, transaction=None):

requirements.txt

@@ -4,7 +4,7 @@ netaddr~=0.8
 cffi~=1.14
 paramiko~=2.7
 Fabric3~=1.14
-libvirt-python>=7,<=9
+libvirt-python>=11,<12
 jinja2~=2.11
 markupsafe~=1.1
 boto3~=1.26