Merge pull request #434 from cPholloway/RE-134

Have script block if mount -a fails to exit cleanly
cpanel · May 13, 2024 · bb21c0d · bb21c0d
2 parents 369fee7 + 5fbc2a8
commit bb21c0d
Show file tree

Hide file tree

Showing 5 changed files with 187 additions and 34 deletions.
diff --git a/docs-website-src/content/blockers.md b/docs-website-src/content/blockers.md
@@ -74,6 +74,10 @@ You can discover many of these issues by downloading `elevate-cpanel` and runnin
 
 The following is a list of other known issues that could prevent your server's successful elevation.
 
+## Filesystem
+
+Since elevate needs to reboot your system multiple times as part of the upgrade process, we ensure that the command 'mount -a' succeeds successfully before allowing the elevation to proceed.  The reason for this is that we need to be able to trust that the filesystem remains the same between each reboot.
+
 ## PostgreSQL
 
 If you are using the PostgreSQL software provided by your distribution (which includes PostgreSQL as installed by cPanel), ELevate will upgrade the software packages. However, your PostgreSQL service is unlikely to start properly. The reason for this is that ELevate will **not** attempt to update the data directory being used by your PostgreSQL instance to store settings and databases; and PostgreSQL will detect this condition and refuse to start, to protect your data from corruption, until you have performed this update.

diff --git a/elevate-cpanel b/elevate-cpanel
@@ -1143,12 +1143,11 @@ EOS
     # use Log::Log4perl qw(:easy);
     INIT { Log::Log4perl->import(qw{:easy}); }
 
-    use constant {
-        GRUB2_WORKAROUND_NONE      => 0,
-        GRUB2_WORKAROUND_OLD       => 1,
-        GRUB2_WORKAROUND_NEW       => 2,
-        GRUB2_WORKAROUND_UNCERTAIN => -1,
-    };
+    use constant GRUB2_WORKAROUND_NONE => 0;
+    use constant GRUB2_WORKAROUND_OLD  => 1;
+    use constant GRUB2_WORKAROUND_NEW  => 2;
+
+    use constant GRUB2_WORKAROUND_UNCERTAIN => -1;
 
     sub GRUB2_PREFIX_DEBIAN { return '/boot/grub' }
     sub GRUB2_PREFIX_RHEL   { return '/boot/grub2' }
@@ -1437,11 +1436,52 @@ EOS
     BEGIN { push @ISA, qw(Elevate::Blockers::Base); }
 
     use constant FINDMNT_BIN => '/usr/bin/findmnt';
+    use constant MOUNT_BIN   => '/usr/bin/mount';
 
     # use Log::Log4perl qw(:easy);
     INIT { Log::Log4perl->import(qw{:easy}); }
 
     sub check ($self) {
+        $self->_check_for_rhel_23449();
+        $self->_ensure_mount_dash_a_succeeds();
+        return;
+    }
+
+    sub _ensure_mount_dash_a_succeeds ($self) {
+
+        return if $self->is_check_mode();
+
+        my $ret    = $self->ssystem_capture_output( MOUNT_BIN, '-a' );
+        my $stderr = join "\n", @{ $ret->{stderr} };
+        if ( $ret->{status} != 0 ) {
+
+            $self->blockers->abort_on_first_blocker(1);
+
+            my $bin = MOUNT_BIN();
+            return $self->has_blocker( <<~"EOS");
+        The following command failed to execute successfully on your server:
+
+        $bin -a
+
+        The following message was given as the reason for the failure:
+
+        $stderr
+
+        Since this script will need to reboot your server, we need to ensure a
+        consistent file system in between in each reboot.  Please review the
+        entries in '/etc/fstab' and ensure that each entry is valid and that
+        '$bin -a' returns exit code 0 before continuing.
+
+        If your '/etc/fstab' file has not been customized, you may want to
+        consider reaching out to cPanel Support for assistance:
+        https://docs.cpanel.net/knowledge-base/technical-support-services/how-to-open-a-technical-support-ticket/
+        EOS
+        }
+
+        return;
+    }
+
+    sub _check_for_rhel_23449 ($self) {
 
         my $out = $self->ssystem_capture_output( FINDMNT_BIN, '-no', 'PROPAGATION', '/usr' );
 

diff --git a/lib/Elevate/Blockers/Grub2.pm b/lib/Elevate/Blockers/Grub2.pm
@@ -22,12 +22,11 @@ use parent qw{Elevate::Blockers::Base};
 use Cwd           ();
 use Log::Log4perl qw(:easy);
 
-use constant {
-    GRUB2_WORKAROUND_NONE      => 0,
-    GRUB2_WORKAROUND_OLD       => 1,
-    GRUB2_WORKAROUND_NEW       => 2,
-    GRUB2_WORKAROUND_UNCERTAIN => -1,
-};
+use constant GRUB2_WORKAROUND_NONE => 0;
+use constant GRUB2_WORKAROUND_OLD  => 1;
+use constant GRUB2_WORKAROUND_NEW  => 2;
+
+use constant GRUB2_WORKAROUND_UNCERTAIN => -1;
 
 sub GRUB2_PREFIX_DEBIAN { return '/boot/grub' }
 sub GRUB2_PREFIX_RHEL   { return '/boot/grub2' }

diff --git a/lib/Elevate/Blockers/MountPoints.pm b/lib/Elevate/Blockers/MountPoints.pm
@@ -15,10 +15,53 @@ use cPstrict;
 use parent qw{Elevate::Blockers::Base};
 
 use constant FINDMNT_BIN => '/usr/bin/findmnt';
+use constant MOUNT_BIN   => '/usr/bin/mount';
 
 use Log::Log4perl qw(:easy);
 
 sub check ($self) {
+    $self->_check_for_rhel_23449();
+    $self->_ensure_mount_dash_a_succeeds();
+    return;
+}
+
+sub _ensure_mount_dash_a_succeeds ($self) {
+
+    # Only do this in start mode because it can change the file system mounts
+    return if $self->is_check_mode();
+
+    my $ret    = $self->ssystem_capture_output( MOUNT_BIN, '-a' );
+    my $stderr = join "\n", @{ $ret->{stderr} };
+    if ( $ret->{status} != 0 ) {
+
+        # No use in letting leapp preupgrade execute if this fails
+        $self->blockers->abort_on_first_blocker(1);
+
+        my $bin = MOUNT_BIN();
+        return $self->has_blocker( <<~"EOS");
+        The following command failed to execute successfully on your server:
+
+        $bin -a
+
+        The following message was given as the reason for the failure:
+
+        $stderr
+
+        Since this script will need to reboot your server, we need to ensure a
+        consistent file system in between in each reboot.  Please review the
+        entries in '/etc/fstab' and ensure that each entry is valid and that
+        '$bin -a' returns exit code 0 before continuing.
+
+        If your '/etc/fstab' file has not been customized, you may want to
+        consider reaching out to cPanel Support for assistance:
+        https://docs.cpanel.net/knowledge-base/technical-support-services/how-to-open-a-technical-support-ticket/
+        EOS
+    }
+
+    return;
+}
+
+sub _check_for_rhel_23449 ($self) {
 
     my $out = $self->ssystem_capture_output( FINDMNT_BIN, '-no', 'PROPAGATION', '/usr' );
 

diff --git a/t/blocker-MountPoints.t b/t/blocker-MountPoints.t
@@ -29,34 +29,101 @@ my $mp   = $cpev->get_blocker('MountPoints');
 
 my @cmds;
 my @stdout;
+my @stderr;
 my $capture_output_status;
 $cpev_mock->redefine(
     ssystem_capture_output => sub ( $, @args ) {
         push @cmds, [@args];
-        return { status => $capture_output_status, stdout => \@stdout, stderr => [] };
+        return { status => $capture_output_status, stdout => \@stdout, stderr => \@stderr };
     },
 );
 
-$capture_output_status = 1;
-is( $mp->check(), undef, 'No blockers are returned when /usr is NOT a separate mount point' );
-
-$capture_output_status = 0;
-$stdout[0] = 'shared';
-is( $mp->check(), undef, 'No blockers are returned when /usr is a separate shared mount point' );
-
-$stdout[0] = 'private';
-my $blocker = $mp->check();
-like(
-    $blocker,
-    {
-        id  => 'Elevate::Blockers::MountPoints::check',
-        msg => qr/The current filesystem setup on your server will prevent/,
-    },
-    'A blocker is returned when /usr is a separate private mount point',
-);
-
-message_seen( WARN => qr/The current filesystem setup on your server will prevent/ );
-
-no_messages_seen();
+{
+    note 'Test _check_for_rhel_23449';
+
+    $capture_output_status = 1;
+    is( $mp->_check_for_rhel_23449(), undef, 'No blockers are returned when /usr is NOT a separate mount point' );
+    is(
+        \@cmds,
+        [
+            [
+                '/usr/bin/findmnt',
+                '-no',
+                'PROPAGATION',
+                '/usr',
+            ],
+        ],
+        'The expected command is called',
+    ) or diag explain \@cmds;
+
+    $capture_output_status = 0;
+    $stdout[0] = 'shared';
+    is( $mp->_check_for_rhel_23449(), undef, 'No blockers are returned when /usr is a separate shared mount point' );
+
+    $stdout[0] = 'private';
+    my $blocker = $mp->_check_for_rhel_23449();
+    like(
+        $blocker,
+        {
+            id  => 'Elevate::Blockers::MountPoints::_check_for_rhel_23449',
+            msg => qr/The current filesystem setup on your server will prevent/,
+        },
+        'A blocker is returned when /usr is a separate private mount point',
+    );
+
+    message_seen( WARN => qr/The current filesystem setup on your server will prevent/ );
+
+    no_messages_seen();
+}
+
+{
+    note 'Test _ensure_mount_dash_a_succeeds';
+
+    undef @cmds;
+    undef @stdout;
+    undef @stderr;
+    $capture_output_status = undef;
+
+    my $mp_mock = Test::MockModule->new('Elevate::Blockers::MountPoints');
+    $mp_mock->redefine(
+        is_check_mode => 1,
+    );
+
+    is( $mp->_ensure_mount_dash_a_succeeds, undef, 'Returns undef when in check mode' );
+    is( \@cmds,                             [],    'No commands are executed when in check mode' );
+
+    $mp_mock->redefine(
+        is_check_mode => 0,
+    );
+
+    $capture_output_status = 0;
+
+    is( $mp->_ensure_mount_dash_a_succeeds, undef, 'Returns undef when mount -a succeeds' );
+    is(
+        \@cmds,
+        [
+            [
+                '/usr/bin/mount',
+                '-a',
+            ],
+        ],
+        'The expected command is called',
+    );
+
+    $capture_output_status = 42;
+    $stderr[0] = 'mount: mount point does not exist';
+
+    my $blocker = dies { $mp->_ensure_mount_dash_a_succeeds() };
+    like(
+        $blocker,
+        {
+            id  => 'Elevate::Blockers::MountPoints::_ensure_mount_dash_a_succeeds',
+            msg => qr/The following command failed to execute successfully on your server/,
+        },
+        'A blocker is returned when mount -a does not execute successfully',
+    );
+
+    no_messages_seen();
+}
 
 done_testing();