[PATCH manager v2 0/2] ceph: osd: stop running pvremove during cleanup

[PATCH manager v2 0/2] ceph: osd: stop running pvremove during cleanup

[Maximiliano Sandoval]

See the individual commit descriptions.

Tested:

Ran the following commands in a 3-node ceph cluster:

- systemd reset-failed
- pveceph create osd $dev
- lsblk
- ceph osd out $id && ceph osd down $id && pveceph osd destroy $id --cleanup 1
- lsblk

a couple of times in a loop. And checked that each lsblk shows the right
state for $dev. Without the reset-failed the creation would fail after a
couple of iterations, but that seems unrelated.

Differences from v1:

- Fix a typo in the commit message
- s/Reef/Squid in commit message

Tested-by: Daniel Herzig <d.herzig@proxmox.com>
Reviewed-by: Kefu Chai <k.chai@proxmox.com>

Maximiliano Sandoval (2):
  ceph: osd: stop triggering udev
  ceph: osd: stop running pvremove during cleanup

 PVE/API2/Ceph/OSD.pm | 43 -------------------------------------------
 1 file changed, 43 deletions(-)

-- 
2.47.3

[PATCH manager v2 1/2] ceph: osd: stop triggering udev

[Maximiliano Sandoval]

As per the FIXME note, this was a workaround for #18525 which was fixed
in [#18904] and packaged in all versions since 249.1. We remove the
workaround for the createosd and destroyosd endpoints.

[#18904] https://github.com/systemd/systemd/pull/18904

Tested-by: Daniel Herzig <d.herzig@proxmox.com>
Reviewed-by: Kefu Chai <k.chai@proxmox.com>
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 PVE/API2/Ceph/OSD.pm | 29 -----------------------------
 1 file changed, 29 deletions(-)

diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
index 7c6046eef..eac913207 100644
--- a/PVE/API2/Ceph/OSD.pm
+++ b/PVE/API2/Ceph/OSD.pm
@@ -424,9 +424,6 @@ __PACKAGE__->register_method({
             file_set_contents($ceph_bootstrap_osd_keyring, $bindata);
         }
 
-        # See FIXME below
-        my @udev_trigger_devs = ();
-
         # $size is in kibibytes
         my $osd_lvcreate = sub {
             my ($vg, $lv, $size) = @_;
@@ -477,8 +474,6 @@ __PACKAGE__->register_method({
                     warn $@ if $@;
                 }
 
-                push @udev_trigger_devs, $dev->{devpath};
-
                 return "$vg/$lv";
 
             } elsif ($dev->{used} eq 'LVM') {
@@ -520,7 +515,6 @@ __PACKAGE__->register_method({
                     warn $@ if $@;
                 }
 
-                push @udev_trigger_devs, $part;
                 return $part;
             }
 
@@ -546,8 +540,6 @@ __PACKAGE__->register_method({
                 my $devpath = $disklist->{$devname}->{devpath};
                 print "create OSD on $devpath (bluestore)\n";
 
-                push @udev_trigger_devs, $devpath;
-
                 my $osd_size = $disklist->{$devname}->{size};
                 my $size_map = {
                     db => int($osd_size / 10), # 10% of OSD
@@ -593,12 +585,6 @@ __PACKAGE__->register_method({
                 }
 
                 run_command($cmd);
-
-                # FIXME: Remove once we depend on systemd >= v249.
-                # Work around udev bug https://github.com/systemd/systemd/issues/18525 to ensure the
-                # udev database is updated.
-                eval { run_command(['udevadm', 'trigger', @udev_trigger_devs]); };
-                warn $@ if $@;
             });
         };
 
@@ -1084,9 +1070,6 @@ __PACKAGE__->register_method({
             # try to unmount from standard mount point
             my $mountpoint = "/var/lib/ceph/osd/ceph-$osdid";
 
-            # See FIXME below
-            my $udev_trigger_devs = {};
-
             my $remove_partition = sub {
                 my ($part) = @_;
 
@@ -1094,8 +1077,6 @@ __PACKAGE__->register_method({
                 my $partnum = PVE::Diskmanage::get_partnum($part);
                 my $devpath = PVE::Diskmanage::get_blockdev($part);
 
-                $udev_trigger_devs->{$devpath} = 1;
-
                 PVE::Diskmanage::wipe_blockdev($part);
                 print "remove partition $part (disk '${devpath}', partnum $partnum)\n";
                 eval { run_command(['/sbin/sgdisk', '-d', $partnum, "${devpath}"]); };
@@ -1119,8 +1100,6 @@ __PACKAGE__->register_method({
                                 run_command(['/sbin/pvremove', $dev], errfunc => sub { });
                             };
                             warn $@ if $@;
-
-                            $udev_trigger_devs->{$dev} = 1;
                         }
                     }
                 }
@@ -1159,14 +1138,6 @@ __PACKAGE__->register_method({
                     }
                 }
             }
-
-            # FIXME: Remove once we depend on systemd >= v249.
-            # Work around udev bug https://github.com/systemd/systemd/issues/18525 to ensure the
-            # udev database is updated.
-            if ($cleanup) {
-                eval { run_command(['udevadm', 'trigger', keys $udev_trigger_devs->%*]); };
-                warn $@ if $@;
-            }
         };
 
         return $rpcenv->fork_worker('cephdestroyosd', $osdsection, $authuser, $worker);
-- 
2.47.3

[PATCH manager v2 2/2] ceph: osd: stop running pvremove during cleanup

[Maximiliano Sandoval]

Since [7f007e7fc] ceph will call pvremove while zapping the OSD. Thus we remove
our pvremove call in order to avoid failing to remove an already removed PV.

The aforementioned commit is included in all versions since Squid which is the
oldest supported version in Proxmox VE 9.

[7f007e7fc] https://github.com/ceph/ceph/commit/7f007e7fc75b4d6e7465c684f7e5b2458883dcc5

Suggested-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Daniel Herzig <d.herzig@proxmox.com>
Reviewed-by: Kefu Chai <k.chai@proxmox.com>
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 PVE/API2/Ceph/OSD.pm | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
index eac913207..f65815641 100644
--- a/PVE/API2/Ceph/OSD.pm
+++ b/PVE/API2/Ceph/OSD.pm
@@ -1089,20 +1089,6 @@ __PACKAGE__->register_method({
 
                 eval { PVE::Ceph::Tools::ceph_volume_zap($osdid, $cleanup) };
                 warn $@ if $@;
-
-                if ($cleanup) {
-                    # try to remove pvs, but do not fail if it does not work
-                    for my $osd_part (@{ $osd_list->{$osdid} }) {
-                        for my $dev (@{ $osd_part->{devices} }) {
-                            ($dev) = ($dev =~ m|^(/dev/[-_.a-zA-Z0-9\/]+)$|); #untaint
-
-                            eval {
-                                run_command(['/sbin/pvremove', $dev], errfunc => sub { });
-                            };
-                            warn $@ if $@;
-                        }
-                    }
-                }
             } else {
                 my $partitions_to_remove = [];
                 if ($cleanup) {
-- 
2.47.3