ceph: fix rbd CLI handling to more reliably use environment variables

Re PL-133145
flyingcircusio · Nov 7, 2024 · 91fe054 · 91fe054
1 parent 6596da7
commit 91fe054
Show file tree

Hide file tree

Showing 6 changed files with 21 additions and 22 deletions.
diff --git a/CHANGES.txt b/CHANGES.txt
@@ -4,6 +4,10 @@ Release notes
 1.7 (unreleased)
 ----------------
 
+- rbd: fix command line calls that only ran interactively due to missing
+  config file and client id settings. switch to environment variables
+  so they can't be forgotten when writing new rbd CLI calls.  (PL-133145)
+
 - snapshots: fix a race condition where a snapshot was created by
   impatient clients and we ended up not thawing correctly when
   creating the snapshot resulted in an ImageExists error. (PL-133149)

diff --git a/src/fc/qemu/hazmat/ceph.py b/src/fc/qemu/hazmat/ceph.py
@@ -5,7 +5,8 @@
 
 import hashlib
 import json
-from typing import Dict, List, Optional
+import os
+from typing import Dict, Optional
 
 import rados
 import rbd
@@ -387,6 +388,7 @@ def __enter__(self):
         # without 'client.': qemu doesn't want to see this, whereas the
         # Rados binding does ... :/
         self.log.debug("connect-rados")
+        os.environ["CEPH_ARGS"] = f"--id {self.CEPH_CLIENT} -c {self.CEPH_CONF}"
         self.rados = rados.Rados(
             conffile=self.CEPH_CONF,
             name="client." + self.CEPH_CLIENT,

diff --git a/src/fc/qemu/hazmat/qemu.py b/src/fc/qemu/hazmat/qemu.py
@@ -407,18 +407,17 @@ def thaw(self):
     def write_file(self, path, content: bytes):
         if not isinstance(content, bytes):
             raise TypeError("Expected bytes, got string.")
+        handle = self.guestagent.cmd("guest-file-open", path=path, mode="w")
         try:
-            handle = self.guestagent.cmd("guest-file-open", path=path, mode="w")
             self.guestagent.cmd(
                 "guest-file-write",
                 handle=handle,
                 # The ASCII armour needs to be turned into text again, because the
                 # JSON encoder doesn't handle bytes-like objects.
                 **{"buf-b64": encode(content, "base64").decode("ascii")},
             )
+        finally:
             self.guestagent.cmd("guest-file-close", handle=handle)
-        except ClientError:
-            self.log.error("guest-write-file", exc_info=True)
 
     def inmigrate(self):
         self._start([f"-incoming {self.migration_address}"])

diff --git a/src/fc/qemu/hazmat/volume.py b/src/fc/qemu/hazmat/volume.py
@@ -57,10 +57,7 @@ def wait_for_part1dev(self):
     def map(self):
         if self.device is not None:
             return
-        self.cmd(
-            f'rbd -c "{self.ceph.CEPH_CONF}" --id "{self.ceph.CEPH_CLIENT}" '
-            f'map "{self.fullname}"'
-        )
+        self.cmd(f'rbd map "{self.fullname}"')
         device = Path("/dev/rbd") / self.fullname
         while not device.exists():
             time.sleep(0.1)
@@ -69,10 +66,7 @@ def map(self):
     def unmap(self):
         if self.device is None:
             return
-        self.cmd(
-            f'rbd -c "{self.ceph.CEPH_CONF}" --id "{self.ceph.CEPH_CLIENT}" '
-            f'unmap "{self.device}"'
-        )
+        self.cmd(f'rbd unmap "{self.device}"')
         self.device = None
 
     @contextlib.contextmanager

diff --git a/tests/hazmat/test_ceph.py b/tests/hazmat/test_ceph.py
@@ -112,14 +112,14 @@ def test_rbd_pool_migration(ceph_inst, patterns):
 ensure-size machine=simplevm subsystem=ceph volume_spec=swap
 start machine=simplevm subsystem=ceph volume_spec=swap
 start-swap machine=simplevm subsystem=ceph volume=rbd.hdd/simplevm.swap
-rbd args=-c "/etc/ceph/ceph.conf" --id "host1" map "rbd.hdd/simplevm.swap" machine=simplevm subsystem=ceph volume=rbd.hdd/simplevm.swap
+rbd args=map "rbd.hdd/simplevm.swap" machine=simplevm subsystem=ceph volume=rbd.hdd/simplevm.swap
 rbd>    /dev/rbd0
 rbd machine=simplevm returncode=0 subsystem=ceph volume=rbd.hdd/simplevm.swap
 mkswap args=-f -L "swap" /dev/rbd/rbd.hdd/simplevm.swap machine=simplevm subsystem=ceph volume=rbd.hdd/simplevm.swap
 mkswap> Setting up swapspace version 1, size = 50 MiB (52424704 bytes)
 mkswap> LABEL=swap, UUID=...-...-...-...-...
 mkswap machine=simplevm returncode=0 subsystem=ceph volume=rbd.hdd/simplevm.swap
-rbd args=-c "/etc/ceph/ceph.conf" --id "host1" unmap "/dev/rbd/rbd.hdd/simplevm.swap" machine=simplevm subsystem=ceph volume=rbd.hdd/simplevm.swap
+rbd args=unmap "/dev/rbd/rbd.hdd/simplevm.swap" machine=simplevm subsystem=ceph volume=rbd.hdd/simplevm.swap
 rbd machine=simplevm returncode=0 subsystem=ceph volume=rbd.hdd/simplevm.swap
 
 pre-start machine=simplevm subsystem=ceph volume_spec=tmp
@@ -129,7 +129,7 @@ def test_rbd_pool_migration(ceph_inst, patterns):
 ensure-size machine=simplevm subsystem=ceph volume_spec=tmp
 start machine=simplevm subsystem=ceph volume_spec=tmp
 start-tmp machine=simplevm subsystem=ceph volume=rbd.hdd/simplevm.tmp
-rbd args=-c "/etc/ceph/ceph.conf" --id "host1" map "rbd.hdd/simplevm.tmp" machine=simplevm subsystem=ceph volume=rbd.hdd/simplevm.tmp
+rbd args=map "rbd.hdd/simplevm.tmp" machine=simplevm subsystem=ceph volume=rbd.hdd/simplevm.tmp
 rbd>    /dev/rbd0
 rbd machine=simplevm returncode=0 subsystem=ceph volume=rbd.hdd/simplevm.tmp
 create-fs machine=simplevm subsystem=ceph volume=rbd.hdd/simplevm.tmp
@@ -153,7 +153,7 @@ def test_rbd_pool_migration(ceph_inst, patterns):
 binary-generation generation=2 machine=simplevm subsystem=ceph volume=rbd.hdd/simplevm.tmp
 umount args="/mnt/rbd/rbd.hdd/simplevm.tmp" machine=simplevm subsystem=ceph volume=rbd.hdd/simplevm.tmp
 umount machine=simplevm returncode=0 subsystem=ceph volume=rbd.hdd/simplevm.tmp
-rbd args=-c "/etc/ceph/ceph.conf" --id "host1" unmap "/dev/rbd/rbd.hdd/simplevm.tmp" machine=simplevm subsystem=ceph volume=rbd.hdd/simplevm.tmp
+rbd args=unmap "/dev/rbd/rbd.hdd/simplevm.tmp" machine=simplevm subsystem=ceph volume=rbd.hdd/simplevm.tmp
 rbd machine=simplevm returncode=0 subsystem=ceph volume=rbd.hdd/simplevm.tmp
 
 rbd-status locker=None machine=simplevm subsystem=ceph volume=rbd.hdd/simplevm.root

diff --git a/tests/test_vm.py b/tests/test_vm.py
@@ -72,14 +72,14 @@ def test_simple_vm_lifecycle_start_stop(vm, patterns):
 ensure-size machine=simplevm subsystem=ceph volume_spec=swap
 start machine=simplevm subsystem=ceph volume_spec=swap
 start-swap machine=simplevm subsystem=ceph volume=rbd.ssd/simplevm.swap
-rbd args=-c "/etc/ceph/ceph.conf" --id "host1" map "rbd.ssd/simplevm.swap" machine=simplevm subsystem=ceph volume=rbd.ssd/simplevm.swap
+rbd args=map "rbd.ssd/simplevm.swap" machine=simplevm subsystem=ceph volume=rbd.ssd/simplevm.swap
 rbd> /dev/rbd0
 rbd machine=simplevm returncode=0 subsystem=ceph volume=rbd.ssd/simplevm.swap
 mkswap args=-f -L "swap" /dev/rbd/rbd.ssd/simplevm.swap machine=simplevm subsystem=ceph volume=rbd.ssd/simplevm.swap
 mkswap> Setting up swapspace version 1, size = 1024 MiB (1073737728 bytes)
 mkswap> LABEL=swap, UUID=...-...-...-...-...
 mkswap machine=simplevm returncode=0 subsystem=ceph volume=rbd.ssd/simplevm.swap
-rbd args=-c "/etc/ceph/ceph.conf" --id "host1" unmap "/dev/rbd/rbd.ssd/simplevm.swap" machine=simplevm subsystem=ceph volume=rbd.ssd/simplevm.swap
+rbd args=unmap "/dev/rbd/rbd.ssd/simplevm.swap" machine=simplevm subsystem=ceph volume=rbd.ssd/simplevm.swap
 rbd machine=simplevm returncode=0 subsystem=ceph volume=rbd.ssd/simplevm.swap
 
 pre-start machine=simplevm subsystem=ceph volume_spec=tmp
@@ -88,7 +88,7 @@ def test_simple_vm_lifecycle_start_stop(vm, patterns):
 ensure-size machine=simplevm subsystem=ceph volume_spec=tmp
 start machine=simplevm subsystem=ceph volume_spec=tmp
 start-tmp machine=simplevm subsystem=ceph volume=rbd.ssd/simplevm.tmp
-rbd args=-c "/etc/ceph/ceph.conf" --id "host1" map "rbd.ssd/simplevm.tmp" machine=simplevm subsystem=ceph volume=rbd.ssd/simplevm.tmp
+rbd args=map "rbd.ssd/simplevm.tmp" machine=simplevm subsystem=ceph volume=rbd.ssd/simplevm.tmp
 rbd> /dev/rbd0
 rbd machine=simplevm returncode=0 subsystem=ceph volume=rbd.ssd/simplevm.tmp
 create-fs machine=simplevm subsystem=ceph volume=rbd.ssd/simplevm.tmp
@@ -115,7 +115,7 @@ def test_simple_vm_lifecycle_start_stop(vm, patterns):
 binary-generation generation=2 machine=simplevm subsystem=ceph volume=rbd.ssd/simplevm.tmp
 umount args="/mnt/rbd/rbd.ssd/simplevm.tmp" machine=simplevm subsystem=ceph volume=rbd.ssd/simplevm.tmp
 umount machine=simplevm returncode=0 subsystem=ceph volume=rbd.ssd/simplevm.tmp
-rbd args=-c "/etc/ceph/ceph.conf" --id "host1" unmap "/dev/rbd/rbd.ssd/simplevm.tmp" machine=simplevm subsystem=ceph volume=rbd.ssd/simplevm.tmp
+rbd args=unmap "/dev/rbd/rbd.ssd/simplevm.tmp" machine=simplevm subsystem=ceph volume=rbd.ssd/simplevm.tmp
 rbd machine=simplevm returncode=0 subsystem=ceph volume=rbd.ssd/simplevm.tmp
 generate-config machine=simplevm
 acquire-global-lock machine=simplevm subsystem=qemu target=/run/fc-qemu.lock
@@ -169,7 +169,7 @@ def test_simple_vm_lifecycle_start_stop(vm, patterns):
 fc-create-vm> Finished
 fc-create-vm> --------
 /nix/store/.../bin/fc-create-vm machine=simplevm returncode=0 subsystem=ceph volume=simplevm.root
-rbd args=-c "/etc/ceph/ceph.conf" --id "host1" map "rbd.ssd/simplevm.root" machine=simplevm subsystem=ceph volume=rbd.ssd/simplevm.root
+rbd args=map "rbd.ssd/simplevm.root" machine=simplevm subsystem=ceph volume=rbd.ssd/simplevm.root
 rbd> /dev/rbd0
 rbd machine=simplevm returncode=0 subsystem=ceph volume=rbd.ssd/simplevm.root
 waiting interval=0 machine=simplevm remaining=4 subsystem=ceph volume=rbd.ssd/simplevm.root
@@ -191,7 +191,7 @@ def test_simple_vm_lifecycle_start_stop(vm, patterns):
 xfs_admin> writing all SBs
 xfs_admin> new UUID = ...-...-...-...-...
 xfs_admin machine=simplevm returncode=0 subsystem=ceph volume=rbd.ssd/simplevm.root
-rbd args=-c "/etc/ceph/ceph.conf" --id "host1" unmap "/dev/rbd/rbd.ssd/simplevm.root" machine=simplevm subsystem=ceph volume=rbd.ssd/simplevm.root
+rbd args=unmap "/dev/rbd/rbd.ssd/simplevm.root" machine=simplevm subsystem=ceph volume=rbd.ssd/simplevm.root
 rbd machine=simplevm returncode=0 subsystem=ceph volume=rbd.ssd/simplevm.root
 """