Skip to content

Commit

Permalink
Avoid die in check_takeover for uninitialized variables
Browse files Browse the repository at this point in the history
Avoid an exception if get_hana_topology return an empty result. It could
happens if SAPHanaSR-showAttr is not succesful and output is not
parsable.
This new behavior is obtained avoiding the usage of local variables in
check_takeover.
Sleep 30 moved in a different position only to be used once for each
get_hana_topology and not to be called when there's no more retry to
run.
Add some unit testing for these two functions.
mpagot committed Dec 18, 2023
1 parent 9a73222 commit 18da85a
Showing 2 changed files with 221 additions and 11 deletions.
22 changes: 11 additions & 11 deletions lib/sles4sap_publiccloud.pm
Original file line number Diff line number Diff line change
@@ -199,18 +199,19 @@ sub sles4sap_cleanup {
sub get_hana_topology {
my ($self, %args) = @_;
my @topology;
my $hostname = $args{hostname};
my $cmd_out = $self->run_cmd(cmd => "SAPHanaSR-showAttr --format=script", quiet => 1);
my $cmd_out = $self->run_cmd(cmd => 'SAPHanaSR-showAttr --format=script', quiet => 1);
record_info("cmd_out", $cmd_out);
my @all_parameters = map { if (/^Hosts/) { s,Hosts/,,; s,",,g; $_ } else { () } } split("\n", $cmd_out);
my @all_hosts = uniq map { (split("/", $_))[0] } @all_parameters;
for my $host (@all_hosts) {
# Only takes parameter and value for lines about one specific host at time
my %host_parameters = map { my ($node, $parameter, $value) = split(/[\/=]/, $_);
if ($host eq $node) { ($parameter, $value) } else { () } } @all_parameters;
if ($host eq $node) { ($parameter, $value) } else { () } }
@all_parameters;
push(@topology, \%host_parameters);
if (defined($hostname) && $hostname eq $host) {
if (defined($args{hostname}) && $args{hostname} eq $host) {
return \%host_parameters;
}
}
@@ -352,25 +353,24 @@ sub cleanup_resource {
sub check_takeover {
my ($self) = @_;
my $hostname = $self->{my_instance}->{instance_id};
my $retry_count = 0;
die("Database on the fenced node '$hostname' is not offline") if ($self->is_hana_database_online);
die("System replication '$hostname' is not offline") if ($self->is_primary_node_online);
my $retry_count = 0;
TAKEOVER_LOOP: while (1) {
my $topology = $self->get_hana_topology();
$retry_count++;
for my $entry (@$topology) {
my %host_entry = %$entry;
my $sync_state = $host_entry{sync_state};
my $takeover_host = $host_entry{vhost};
if ($takeover_host ne $hostname && $sync_state eq "PRIM") {
record_info("Takeover status:", "Takeover complete to node '$takeover_host'");
die "Missing 'vhost' field in topology output" unless defined($host_entry{vhost});
die "Missing 'sync_state' field in topology output" unless defined($host_entry{sync_state});
if ($host_entry{vhost} ne $hostname && $host_entry{sync_state} eq "PRIM") {
record_info("Takeover status:", "Takeover complete to node '$host_entry{vhost}'");
last TAKEOVER_LOOP;
}
sleep 30;
}
die "Test failed: takeover failed to complete." if ($retry_count > 40);
sleep 30;
}
return 1;
210 changes: 210 additions & 0 deletions t/12_sles4sap_publicccloud.t
Original file line number Diff line number Diff line change
@@ -175,4 +175,214 @@ subtest '[is_primary_node_online]' => sub {
is $res, 1, "System replication is online on primary node";
};

subtest '[get_hana_topology]' => sub {
my $self = sles4sap_publiccloud->new();
my $sles4sap_publiccloud = Test::MockModule->new('sles4sap_publiccloud', no_auto => 1);
my @calls;
$sles4sap_publiccloud->redefine(run_cmd => sub {
my ($self, %args) = @_;
push @calls, $args{cmd};
my $res = <<END;
Global/global/cib-time="Fri Dec 15 05:54:20 2023"
Global/global/maintenance="false"
Hosts/vmhana01/clone_state="PROMOTED"
Hosts/vmhana01/lpa_ha0_lpt="1702619602"
Hosts/vmhana01/node_state="online"
Hosts/vmhana01/op_mode="logreplay"
Hosts/vmhana01/remoteHost="vmhana02"
Hosts/vmhana01/roles="2:P:master1:master:worker:master"
Hosts/vmhana01/site="site_a"
Hosts/vmhana01/srah="-"
Hosts/vmhana01/srmode="sync"
Hosts/vmhana01/sync_state="PRIM"
Hosts/vmhana01/version="2.00.073.00"
Hosts/vmhana01/vhost="vmhana01"
Hosts/vmhana02/clone_state="DEMOTED"
Hosts/vmhana02/lpa_ha0_lpt="30"
Hosts/vmhana02/node_state="online"
Hosts/vmhana02/op_mode="logreplay"
Hosts/vmhana02/remoteHost="vmhana01"
Hosts/vmhana02/roles="4:S:master1:master:worker:master"
Hosts/vmhana02/site="site_b"
Hosts/vmhana02/srah="-"
Hosts/vmhana02/srmode="sync"
Hosts/vmhana02/sync_state="SOK"
Hosts/vmhana02/version="2.00.073.00"
Hosts/vmhana02/vhost="vmhana02"
END
return $res;
});
$sles4sap_publiccloud->redefine(record_info => sub { note(join(' ', 'RECORD_INFO -->', @_)); });

my $topology = $self->get_hana_topology();

note("\n --> " . join("\n --> ", @calls));
my $num_of_hosts = 0;
for my $entry (@$topology) {
$num_of_hosts++;
my %host_entry = %$entry;
note("vhost: $host_entry{vhost}");
like $host_entry{vhost}, qr/vmhana/, "Parsing is ok for field vhost";
}

ok $num_of_hosts eq 2;
};


subtest '[get_hana_topology] for a specific node' => sub {
my $self = sles4sap_publiccloud->new();
my $sles4sap_publiccloud = Test::MockModule->new('sles4sap_publiccloud', no_auto => 1);
my @calls;
$sles4sap_publiccloud->redefine(run_cmd => sub {
my ($self, %args) = @_;
push @calls, $args{cmd};
my $res = <<END;
Hosts/vmhana01/vhost="vmhana01"
Hosts/vmhana02/vhost="vmhana02"
END
return $res;
});
$sles4sap_publiccloud->redefine(record_info => sub { note(join(' ', 'RECORD_INFO -->', @_)); });

my $entry = $self->get_hana_topology(hostname => 'vmhana02');

note("\n --> " . join("\n --> ", @calls));
my %host_entry = %$entry;
note("vhost: $host_entry{vhost}");
like $host_entry{vhost}, qr/vmhana02/, "Parsing is ok for field vhost";
};


subtest '[get_hana_topology] bad output' => sub {
my $self = sles4sap_publiccloud->new();
my $sles4sap_publiccloud = Test::MockModule->new('sles4sap_publiccloud', no_auto => 1);
my @calls;
$sles4sap_publiccloud->redefine(run_cmd => sub {
my ($self, %args) = @_;
push @calls, $args{cmd};
my $res = <<END;
Signon to CIB failed: Transport endpoint is not connected
Init failed, could not perform requested operations
No attributes found for SID=ha0
END
return $res;
});
$sles4sap_publiccloud->redefine(record_info => sub { note(join(' ', 'RECORD_INFO -->', @_)); });

my $topology = $self->get_hana_topology();

note("\n --> " . join("\n --> ", @calls));
ok scalar @$topology eq 0;
};


subtest '[check_takeover]' => sub {
my $self = sles4sap_publiccloud->new();
$self->{my_instance}->{instance_id} = 'Yondu';
my $sles4sap_publiccloud = Test::MockModule->new('sles4sap_publiccloud', no_auto => 1);
my @calls;
$sles4sap_publiccloud->redefine(is_hana_database_online => sub { return 0 });
$sles4sap_publiccloud->redefine(is_primary_node_online => sub { return 0 });
$sles4sap_publiccloud->redefine(run_cmd => sub {
my ($self, %args) = @_;
push @calls, $args{cmd};
my $res = <<END;
Hosts/vmhana01/sync_state="PRIM"
Hosts/vmhana01/vhost="vmhana01"
Hosts/vmhana02/sync_state="SOK"
Hosts/vmhana02/vhost="vmhana02"
END
return $res;
});
$sles4sap_publiccloud->redefine(record_info => sub { note(join(' ', 'RECORD_INFO -->', @_)); });

# Note how it pass at the first iteration because:
# - two nodes in the output are named vmhana01 and vmhana02
# - none has the name of "current node" that is Yondu
# - at least one of them with name different from Yondu is in state PRIM
ok $self->check_takeover();
note("\n --> " . join("\n --> ", @calls));
};


subtest '[check_takeover] fail in showAttr' => sub {
my $self = sles4sap_publiccloud->new();
$self->{my_instance}->{instance_id} = 'Yondu';
my $sles4sap_publiccloud = Test::MockModule->new('sles4sap_publiccloud', no_auto => 1);
my @calls;
$sles4sap_publiccloud->redefine(is_hana_database_online => sub { return 0 });
$sles4sap_publiccloud->redefine(is_primary_node_online => sub { return 0 });
$sles4sap_publiccloud->redefine(run_cmd => sub {
my ($self, %args) = @_;
push @calls, $args{cmd};
my $res = <<END;
Signon to CIB failed: Transport endpoint is not connected
Init failed, could not perform requested operations
No attributes found for SID=ha0
END
return $res;
});
$sles4sap_publiccloud->redefine(record_info => sub { note(join(' ', 'RECORD_INFO -->', @_)); });
dies_ok { $self->check_takeover() } "check_takeover fails if SAPHanaSR-showAttr keep give bad respose";
note("\n --> " . join("\n --> ", @calls));
};


subtest '[check_takeover] missing fields in SAPHanaSR-showAttr' => sub {
my $self = sles4sap_publiccloud->new();
$self->{my_instance}->{instance_id} = 'vmhana01';
my $sles4sap_publiccloud = Test::MockModule->new('sles4sap_publiccloud', no_auto => 1);
my @calls;
$sles4sap_publiccloud->redefine(is_hana_database_online => sub { return 0 });
$sles4sap_publiccloud->redefine(is_primary_node_online => sub { return 0 });
my $showAttr;
$sles4sap_publiccloud->redefine(run_cmd => sub {
my ($self, %args) = @_;
push @calls, $args{cmd};
return $showAttr;
});
$sles4sap_publiccloud->redefine(record_info => sub { note(join(' ', 'RECORD_INFO -->', @_)); });

$showAttr = <<END;
Hosts/vmhana01/vhost="vmhana01"
Hosts/vmhana01/sync_state="SOK"
Hosts/vmhana02/vhost="vmhana02"
END
dies_ok { $self->check_takeover() } "check_takeover fails if sync_state is missing in SAPHanaSR-showAttr output";
note("\n --> " . join("\n --> ", @calls));
@calls = ();

$showAttr = <<END;
Hosts/vmhana01/vhost="vmhana01"
Hosts/vmhana01/sync_state="SOK"
Hosts/vmhana02/sync_state="SOK"
END
dies_ok { $self->check_takeover() } "check_takeover fails if vhost is missing in SAPHanaSR-showAttr output";
note("\n --> " . join("\n --> ", @calls));
};


subtest '[check_takeover] fail if DB online' => sub {
my $self = sles4sap_publiccloud->new();
$self->{my_instance}->{instance_id} = 'Yondu';
my $sles4sap_publiccloud = Test::MockModule->new('sles4sap_publiccloud', no_auto => 1);
my @calls;
$sles4sap_publiccloud->redefine(is_hana_database_online => sub { return 1 });

dies_ok { $self->check_takeover() } "Takeover failed if sles4sap_publiccloud return 1";
};


subtest '[check_takeover] fail if primary online' => sub {
my $self = sles4sap_publiccloud->new();
$self->{my_instance}->{instance_id} = 'Yondu';
my $sles4sap_publiccloud = Test::MockModule->new('sles4sap_publiccloud', no_auto => 1);
my @calls;
$sles4sap_publiccloud->redefine(is_hana_database_online => sub { return 0 });
$sles4sap_publiccloud->redefine(is_primary_node_online => sub { return 1 });

dies_ok { $self->check_takeover() } "Takeover failed if is_primary_node_online return 1";
};


done_testing;

0 comments on commit 18da85a

Please sign in to comment.