Un-invert logic of upgrade_distro_manually

During resolution of RE-670, it was discovered that PR #554, which changed `should_run_distro_upgrade` to `upgrade_distro_manually`, failed to account for the fact that this change of name inverts the sense of the logic. Since none of the users of this method embed it in a logically complicated way, and since there aren't too many users, modify the method and its users to restore correct semantics.
cpanel · Dec 11, 2024 · 1c7553f · 1c7553f
1 parent 8832300
commit 1c7553f
Show file tree

Hide file tree

Showing 14 changed files with 32 additions and 32 deletions.
diff --git a/elevate-cpanel b/elevate-cpanel
@@ -700,7 +700,7 @@ BEGIN {    # Suppress load of all of these at earliest point.
 
     sub check ($self) {
 
-        return 1 unless $self->upgrade_distro_manually;    # skip when --upgrade-distro-manually is provided
+        return 1 if $self->upgrade_distro_manually;    # skip when --upgrade-distro-manually is provided
 
         my $ok = 0;
         try {
@@ -2312,7 +2312,7 @@ EOS
 
     sub verify_cmdline ($self) {
         return unless _check_command_exists();
-        if ( $self->cpev->upgrade_distro_manually() ) {
+        if ( !$self->cpev->upgrade_distro_manually() ) {
             my $arg = "elevate-" . _persistent_id;
             INFO("Checking for \"$arg\" in booted kernel's command line...");
 
@@ -2536,7 +2536,7 @@ EOS
     sub check ($self) {
 
         return 1 unless Elevate::OS::needs_leapp();
-        return 1 unless $self->upgrade_distro_manually;    # skip when --upgrade-distro-manually is provided
+        return 1 if $self->upgrade_distro_manually;    # skip when --upgrade-distro-manually is provided
 
         my $ok = 1;
         $ok = 0 unless $self->_blocker_grub2_workaround;
@@ -3094,7 +3094,7 @@ EOS
 
     sub check ($self) {    # $self is a cpev object here
 
-        return 0 unless $self->upgrade_distro_manually;
+        return 0 if $self->upgrade_distro_manually;
 
         if ( _is_container_envtype() ) {
             return $self->has_blocker( <<~'EOS');
@@ -3416,7 +3416,7 @@ EOS
 
         return unless Elevate::OS::needs_leapp();
 
-        return unless $self->upgrade_distro_manually;    # skip when --upgrade-distro-manually is provided
+        return if $self->upgrade_distro_manually;    # skip when --upgrade-distro-manually is provided
 
         return if ( $self->components->num_blockers_found() > 0 );    # skip if any blockers have already been found
 
@@ -4252,7 +4252,7 @@ EOS
     sub check ($self) {
 
         return 1 unless Elevate::OS::needs_leapp();
-        return 1 unless $self->upgrade_distro_manually;
+        return 1 if $self->upgrade_distro_manually;
 
         return $self->_blocker_bad_nics_naming;
     }
@@ -6418,7 +6418,7 @@ EOS
     use constant UFW => '/usr/sbin/ufw';
 
     sub pre_distro_upgrade ($self) {
-        return unless $self->upgrade_distro_manually();          # skip when --upgrade-distro-manually is provided
+        return if $self->upgrade_distro_manually();    # skip when --upgrade-distro-manually is provided
         return unless Elevate::OS::needs_do_release_upgrade();
 
         if ( !-x UFW ) {
@@ -8724,7 +8724,7 @@ deb https://wp-toolkit.plesk.com/cPanel/Ubuntu-22.04-x86_64/latest/thirdparty/ .
 
     sub upgrade ($self) {
 
-        return unless $self->cpev->upgrade_distro_manually();
+        return if $self->cpev->upgrade_distro_manually();
 
         $self->cpev->run_once(
             setup_answer_file => sub {
@@ -8853,7 +8853,7 @@ deb https://wp-toolkit.plesk.com/cPanel/Ubuntu-22.04-x86_64/latest/thirdparty/ .
     sub wait_for_leapp_completion ($self) {
         return 1 unless Elevate::OS::needs_leapp();
 
-        return 1 unless $self->cpev->upgrade_distro_manually();
+        return 1 if $self->cpev->upgrade_distro_manually();
 
         my $upgrade_log = LEAPP_UPGRADE_LOG;
 
@@ -10938,7 +10938,7 @@ sub run_stage_1 ($self) {
 
     Elevate::Motd->setup();
 
-    if ( !$self->upgrade_distro_manually() ) {
+    if ( $self->upgrade_distro_manually() ) {
         Elevate::Stages::bump_stage();
         return ACTION_CONTINUE;
     }
@@ -11021,7 +11021,7 @@ sub run_stage_3 ($self) {
     # and executing do-release-upgrade
     $self->run_component_once( 'Ufw' => 'pre_distro_upgrade' );
 
-    if ( !$self->upgrade_distro_manually() ) {
+    if ( $self->upgrade_distro_manually() ) {
         return $self->_request_to_upgrade_distro_manually();
     }
 
@@ -11475,7 +11475,7 @@ sub upgrade_distro_manually ($self) {
     # we need to check to see if the CLI option is passed here too in order
     # to allow users to run this script with the '--check --upgrade-distro-manually', etc. options
     my $manual_distro_upgrade = scalar grep { $self->getopt($_) || Elevate::StageFile::read_stage_file( tr/-/_/r, 0 ) } qw/upgrade-distro-manually no-leapp/;
-    return !$manual_distro_upgrade;
+    return $manual_distro_upgrade;
 }
 
 sub get_current_status ($self) {

diff --git a/lib/Elevate/Components/BootKernel.pm b/lib/Elevate/Components/BootKernel.pm
@@ -35,7 +35,7 @@ use Try::Tiny;
 
 sub check ($self) {
 
-    return 1 unless $self->upgrade_distro_manually;    # skip when --upgrade-distro-manually is provided
+    return 1 if $self->upgrade_distro_manually;    # skip when --upgrade-distro-manually is provided
 
     my $ok = 0;
     try {

diff --git a/lib/Elevate/Components/Grub2ChecksWorkarounds.pm b/lib/Elevate/Components/Grub2ChecksWorkarounds.pm
@@ -188,7 +188,7 @@ sub post_distro_upgrade ($self) {
 sub check ($self) {
 
     return 1 unless Elevate::OS::needs_leapp();
-    return 1 unless $self->upgrade_distro_manually;    # skip when --upgrade-distro-manually is provided
+    return 1 if $self->upgrade_distro_manually;    # skip when --upgrade-distro-manually is provided
 
     my $ok = 1;
     $ok = 0 unless $self->_blocker_grub2_workaround;

diff --git a/lib/Elevate/Components/Grub2ControlTest.pm b/lib/Elevate/Components/Grub2ControlTest.pm
@@ -140,7 +140,7 @@ sub _remove_kernel_arg ( $self, $arg ) {
 
 sub verify_cmdline ($self) {
     return unless _check_command_exists();
-    if ( $self->cpev->upgrade_distro_manually() ) {
+    if ( !$self->cpev->upgrade_distro_manually() ) {
         my $arg = "elevate-" . _persistent_id;
         INFO("Checking for \"$arg\" in booted kernel's command line...");
 

diff --git a/lib/Elevate/Components/IsContainer.pm b/lib/Elevate/Components/IsContainer.pm
@@ -27,7 +27,7 @@ use Log::Log4perl qw(:easy);
 
 sub check ($self) {    # $self is a cpev object here
 
-    return 0 unless $self->upgrade_distro_manually;
+    return 0 if $self->upgrade_distro_manually;
 
     if ( _is_container_envtype() ) {
         return $self->has_blocker( <<~'EOS');

diff --git a/lib/Elevate/Components/Leapp.pm b/lib/Elevate/Components/Leapp.pm
@@ -36,7 +36,7 @@ sub check ($self) {
 
     return unless Elevate::OS::needs_leapp();
 
-    return unless $self->upgrade_distro_manually;    # skip when --upgrade-distro-manually is provided
+    return if $self->upgrade_distro_manually;    # skip when --upgrade-distro-manually is provided
 
     return if ( $self->components->num_blockers_found() > 0 );    # skip if any blockers have already been found
 

diff --git a/lib/Elevate/Components/NICs.pm b/lib/Elevate/Components/NICs.pm
@@ -110,7 +110,7 @@ sub check ($self) {
 
     # This only matters for upgrades performed with the leapp utility
     return 1 unless Elevate::OS::needs_leapp();
-    return 1 unless $self->upgrade_distro_manually;
+    return 1 if $self->upgrade_distro_manually;
 
     return $self->_blocker_bad_nics_naming;
 }

diff --git a/lib/Elevate/Components/Ufw.pm b/lib/Elevate/Components/Ufw.pm
@@ -32,7 +32,7 @@ use parent qw{Elevate::Components::Base};
 use constant UFW => '/usr/sbin/ufw';
 
 sub pre_distro_upgrade ($self) {
-    return unless $self->upgrade_distro_manually();          # skip when --upgrade-distro-manually is provided
+    return if $self->upgrade_distro_manually();    # skip when --upgrade-distro-manually is provided
     return unless Elevate::OS::needs_do_release_upgrade();
 
     if ( !-x UFW ) {

diff --git a/lib/Elevate/Leapp.pm b/lib/Elevate/Leapp.pm
@@ -89,7 +89,7 @@ sub preupgrade ($self) {
 
 sub upgrade ($self) {
 
-    return unless $self->cpev->upgrade_distro_manually();
+    return if $self->cpev->upgrade_distro_manually();
 
     $self->cpev->run_once(
         setup_answer_file => sub {
@@ -240,7 +240,7 @@ sub wait_for_leapp_completion ($self) {
     return 1 unless Elevate::OS::needs_leapp();
 
     # No use waiting for leapp to complete if we did not run leapp
-    return 1 unless $self->cpev->upgrade_distro_manually();
+    return 1 if $self->cpev->upgrade_distro_manually();
 
     my $upgrade_log = LEAPP_UPGRADE_LOG;
 

diff --git a/script/elevate-cpanel.PL b/script/elevate-cpanel.PL
@@ -944,7 +944,7 @@ sub run_stage_1 ($self) {
 
     Elevate::Motd->setup();
 
-    if ( !$self->upgrade_distro_manually() ) {
+    if ( $self->upgrade_distro_manually() ) {
         Elevate::Stages::bump_stage();
         return ACTION_CONTINUE;
     }
@@ -1027,7 +1027,7 @@ sub run_stage_3 ($self) {
     # and executing do-release-upgrade
     $self->run_component_once( 'Ufw' => 'pre_distro_upgrade' );
 
-    if ( !$self->upgrade_distro_manually() ) {
+    if ( $self->upgrade_distro_manually() ) {
         return $self->_request_to_upgrade_distro_manually();
     }
 
@@ -1481,7 +1481,7 @@ sub upgrade_distro_manually ($self) {
     # we need to check to see if the CLI option is passed here too in order
     # to allow users to run this script with the '--check --upgrade-distro-manually', etc. options
     my $manual_distro_upgrade = scalar grep { $self->getopt($_) || Elevate::StageFile::read_stage_file( tr/-/_/r, 0 ) } qw/upgrade-distro-manually no-leapp/;
-    return !$manual_distro_upgrade;
+    return $manual_distro_upgrade;
 }
 
 sub get_current_status ($self) {

diff --git a/t/components-BootKernel.t b/t/components-BootKernel.t
@@ -34,14 +34,14 @@ my $comp = cpev->new->get_component('BootKernel');
         reboot_status => sub { die "should not be called here\n"; },
     );
 
-    $mock_cpev->redefine( upgrade_distro_manually => 0 );
+    $mock_cpev->redefine( upgrade_distro_manually => 1 );
     is( $comp->check(), 1, 'Should skip when upgrade-distro-manually is passed' );
 }
 
 {
     note 'Test BootKernel behavior';
 
-    $mock_cpev->redefine( upgrade_distro_manually => 1 );
+    $mock_cpev->redefine( upgrade_distro_manually => 0 );
 
     my $rv;
     my $bv;

diff --git a/t/components-Grub2ControlTest.t b/t/components-Grub2ControlTest.t
@@ -86,7 +86,7 @@ $mock_stage_file->redefine( _save_stage_file => sub { $stage_data = $_[0]; retur
     my $cmdline;
     $mock_slurp->redefine( read_binary => sub { return $cmdline } );
 
-    $mock_cpev->redefine( upgrade_distro_manually => 1 );
+    $mock_cpev->redefine( upgrade_distro_manually => 0 );
     $mock_cpev->redefine( do_cleanup              => sub { $stage_data = undef; return; } );
 
     my $mock_comp = Test::MockModule->new('Elevate::Components::Grub2ControlTest');
@@ -136,7 +136,7 @@ $mock_stage_file->redefine( _save_stage_file => sub { $stage_data = $_[0]; retur
     my $cmdline;
     $mock_slurp->redefine( read_binary => sub { return $cmdline } );
 
-    $mock_cpev->redefine( upgrade_distro_manually => 1 );
+    $mock_cpev->redefine( upgrade_distro_manually => 0 );
     $mock_cpev->redefine( do_cleanup              => sub { $stage_data = undef; return; } );
 
     my $mock_comp = Test::MockModule->new('Elevate::Components::Grub2ControlTest');

diff --git a/t/components-Ufw.t b/t/components-Ufw.t
@@ -46,15 +46,15 @@ $mock_stagefile->redefine(
     set_os_to('ubuntu');
 
     $mock_ufw->redefine(
-        upgrade_distro_manually => 0,
+        upgrade_distro_manually => 1,
     );
 
     is( $ufw->pre_distro_upgrade(), undef, 'Returns early if the user is updating the OS' );
     is( $ssystem_and_die_params,    [],    'No system commands were called' );
     no_messages_seen();
 
     $mock_ufw->redefine(
-        upgrade_distro_manually => 1,
+        upgrade_distro_manually => 0,
     );
 
     set_os_to('cent');

diff --git a/t/leapp_upgrade.t b/t/leapp_upgrade.t
@@ -314,13 +314,13 @@ note 'LEAPP upgrade log failure checks';
     $mock_leapp_upgrade_log->unlink;
 
     $mock_elevate->redefine(
-        upgrade_distro_manually => 0,
+        upgrade_distro_manually => 1,
     );
 
     is( cpev->leapp->wait_for_leapp_completion, 1, 'wait_for_leapp_completion returns early when it should NOT run leapp' );
 
     $mock_elevate->redefine(
-        upgrade_distro_manually => 1,
+        upgrade_distro_manually => 0,
     );
 
     is( cpev->leapp->wait_for_leapp_completion, 0, "wait_for_leapp_completion fails when the upgrade log is missing." );