From 1c7553fd8ef8abc99893eaeb4826292377eb45a5 Mon Sep 17 00:00:00 2001 From: Sloane Bernstein Date: Wed, 11 Dec 2024 12:35:13 -0600 Subject: [PATCH] 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. --- elevate-cpanel | 24 +++++++++---------- lib/Elevate/Components/BootKernel.pm | 2 +- .../Components/Grub2ChecksWorkarounds.pm | 2 +- lib/Elevate/Components/Grub2ControlTest.pm | 2 +- lib/Elevate/Components/IsContainer.pm | 2 +- lib/Elevate/Components/Leapp.pm | 2 +- lib/Elevate/Components/NICs.pm | 2 +- lib/Elevate/Components/Ufw.pm | 2 +- lib/Elevate/Leapp.pm | 4 ++-- script/elevate-cpanel.PL | 6 ++--- t/components-BootKernel.t | 4 ++-- t/components-Grub2ControlTest.t | 4 ++-- t/components-Ufw.t | 4 ++-- t/leapp_upgrade.t | 4 ++-- 14 files changed, 32 insertions(+), 32 deletions(-) diff --git a/elevate-cpanel b/elevate-cpanel index 2343bd3f..bf5c4a48 100755 --- 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 index d079491c..67cb800f 100644 --- 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 index 3a0310b9..f4378e19 100644 --- 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 index d1ab591e..95c2261f 100644 --- 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 index 1808e3fd..500a51e7 100644 --- 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 index 1270b7ae..0a965956 100644 --- 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 index a381c9f4..c06c4bf0 100644 --- 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 index 2bf51128..99b55379 100644 --- 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 index cecc8ed9..465cf859 100644 --- 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 index 6cdb936b..60945e27 100755 --- 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 index 75c3b775..1570a2b3 100644 --- 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 index b5aef58f..b49d5eda 100644 --- 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 index 362cd1de..253a22fb 100644 --- a/t/components-Ufw.t +++ b/t/components-Ufw.t @@ -46,7 +46,7 @@ $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' ); @@ -54,7 +54,7 @@ $mock_stagefile->redefine( 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 index aa24e05e..f6ade0cb 100644 --- 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." );