Simplify code for Azure fence agent in qesap (#20320)

Remove qesapdeployment unused API to get the fencing type: most of the code just get that from settings. Move more setting access in caller for the lib function creating the list of ansible -playbook commands.
os-autoinst · Oct 1, 2024 · 0adba77 · 0adba77
1 parent 31dd5c1
commit 0adba77
Show file tree

Hide file tree

Showing 7 changed files with 59 additions and 64 deletions.
diff --git a/lib/qesapdeployment.pm b/lib/qesapdeployment.pm
@@ -99,7 +99,6 @@ our @EXPORT = qw(
   qesap_az_get_active_peerings
   qesap_az_clean_old_peerings
   qesap_az_setup_native_fencing_permissions
-  qesap_az_get_native_fencing_type
   qesap_az_enable_system_assigned_identity
   qesap_az_assign_role
   qesap_az_get_tenant_id
@@ -2207,22 +2206,6 @@ sub qesap_az_clean_old_peerings {
     }
 }
 
-=head2 qesap_az_get_native_fencing_type
-
-    Gets the native fencing type (spn/msi)
-
-=cut
-
-sub qesap_az_get_native_fencing_type {
-    my $type = get_var('AZURE_FENCE_AGENT_CONFIGURATION',
-        get_var('QESAP_AZURE_FENCE_AGENT_CONFIGURATION', 'msi'));
-
-    unless ($type eq 'msi' || $type eq 'spn') {
-        die "Invalid type: $type. Must be 'msi' or 'spn'.";
-    }
-    return $type;
-}
-
 =head2 qesap_az_setup_native_fencing_permissions
 
     qesap_az_setup_native_fencing_permissions(vmname=>$vm_name,

diff --git a/lib/sles4sap_publiccloud.pm b/lib/sles4sap_publiccloud.pm
@@ -860,7 +860,7 @@ sub delete_network_peering {
     Detects HANA/HA scenario from function arguments and returns a list of ansible playbooks to include
     in the "ansible: create:" section of config.yaml file.
 
-=over 7
+=over 10
 
 =item B<ha_enabled> - Enable the installation of HANA and the cluster configuration
 
@@ -871,13 +871,19 @@ sub delete_network_peering {
 
 =item B<fencing> - select fencing mechanism
 
+=item B<fence_type> - select Azure native fencing mechanism. Only two accepted values 'spn' or 'msi'. This argument is only applicable to Azure. (optional)
+
+=item B<spn_application_id> - Application ID for the SPN Azure native fencing agent.This argument is only applicable to Azure configured with native fencing of type SPN. (optional)
+
+=item B<spn_application_password> - password for the SPN Azure native fencing agent.This argument is only applicable to Azure configured with native fencing of type SPN. (optional)
+
 =item B<ptf_files> - list of PTF files (optional)
 
 =item B<ptf_token> - SAS token to access the PTF files (optional)
 
-=item B<ptf_account> - name of the account for the ptf container
+=item B<ptf_account> - name of the account for the ptf container (optional)
 
-=item B<ptf_container> - name of the container for PTF files
+=item B<ptf_container> - name of the container for PTF files (optional)
 
 =back
 =cut
@@ -887,6 +893,14 @@ sub create_playbook_section_list {
     $args{ha_enabled} //= 1;
     $args{registration} //= 'registercloudguest';
     $args{fencing} //= 'sbd';
+    if ($args{fencing} eq 'native' and is_azure) {
+        croak "Argument <fence_type> missing" unless $args{fence_type};
+    }
+
+    if ($args{fencing} eq 'native' and is_azure and $args{fence_type} eq 'spn') {
+        croak "Argument <spn_application_id> missing" unless $args{spn_application_id};
+        croak "Argument <spn_application_password> missing" unless $args{spn_application_password};
+    }
 
     my @playbook_list;
 
@@ -920,9 +934,9 @@ sub create_playbook_section_list {
     if ($args{fencing} eq 'native' and is_azure) {
         # Prepares Azure native fencing related arguments for 'sap-hana-cluster.yaml' playbook
         my $azure_native_fencing_args = azure_fencing_agents_playbook_args(
-            fence_type => get_var('AZURE_FENCE_AGENT_CONFIGURATION', 'msi'),
-            spn_application_id => get_var('_SECRET_AZURE_SPN_APPLICATION_ID'),
-            spn_application_password => get_var('_SECRET_AZURE_SPN_APP_PASSWORD')
+            fence_type => $args{fence_type},
+            spn_application_id => $args{spn_application_id},
+            spn_application_password => $args{spn_application_password}
         );
         $hana_cluster_playbook = join(' ', $hana_cluster_playbook, $azure_native_fencing_args);
     }

diff --git a/t/12_sles4sap_publicccloud.t b/t/12_sles4sap_publicccloud.t
@@ -619,12 +619,26 @@ subtest '[create_playbook_section_list] ha_enabled => 0' => sub {
 };
 
 
-subtest '[create_playbook_section_list] fencing => native in azure' => sub {
+subtest '[create_playbook_section_list] fencing => azure native msi' => sub {
     my $sles4sap_publiccloud = Test::MockModule->new('sles4sap_publiccloud', no_auto => 1);
     $sles4sap_publiccloud->redefine(is_azure => sub { return 1 });
     set_var('SCC_REGCODE_SLES4SAP', 'Magellano');
     set_var('USE_SAPCONF', 'Colombo');
-    my $ansible_playbooks = create_playbook_section_list(fencing => 'native');
+    my $ansible_playbooks = create_playbook_section_list(fencing => 'native', fence_type => 'msi');
+    set_var('SCC_REGCODE_SLES4SAP', undef);
+    set_var('USE_SAPCONF', undef);
+    note("\n  -->  " . join("\n  -->  ", @$ansible_playbooks));
+    ok((none { /.*cluster_sbd_prep\.yaml.*/ } @$ansible_playbooks), 'cluster_sbd_prep playbook is not called when fencing => native');
+    ok((any { /.*sap-hana-cluster\.yaml.*azure_identity_management=.*/ } @$ansible_playbooks), 'registration playbook is called when ha_enabled => 0');
+};
+
+
+subtest '[create_playbook_section_list] fencing => azure native spn' => sub {
+    my $sles4sap_publiccloud = Test::MockModule->new('sles4sap_publiccloud', no_auto => 1);
+    $sles4sap_publiccloud->redefine(is_azure => sub { return 1 });
+    set_var('SCC_REGCODE_SLES4SAP', 'Magellano');
+    set_var('USE_SAPCONF', 'Colombo');
+    my $ansible_playbooks = create_playbook_section_list(fencing => 'native', fence_type => 'spn', spn_application_id => '123', spn_application_password => 'abc');
     set_var('SCC_REGCODE_SLES4SAP', undef);
     set_var('USE_SAPCONF', undef);
     note("\n  -->  " . join("\n  -->  ", @$ansible_playbooks));

diff --git a/t/15_qesap_azure.t b/t/15_qesap_azure.t
@@ -404,27 +404,6 @@ subtest '[qesap_az_list_container_files] command composition' => sub {
     ok((any { /.*--prefix GURGLE*/ } @calls), 'prefix argument used for --prefix');
 };
 
-subtest '[qesap_az_get_native_fencing_type]' => sub {
-    my $res_empty = qesap_az_get_native_fencing_type();
-    ok($res_empty eq 'msi', "Return 'msi' if openqa var is empty");
-};
-
-subtest '[qesap_az_get_native_fencing_type] wrong value for openqa variable' => sub {
-    set_var('QESAP_AZURE_FENCE_AGENT_CONFIGURATION', 'AEGEAN'),
-      dies_ok { qesap_az_get_native_fencing_type(); } 'Expected die if value is unexpected';
-    set_var('QESAP_AZURE_FENCE_AGENT_CONFIGURATION', undef),;
-};
-
-subtest '[qesap_az_get_native_fencing_type] correct variable' => sub {
-    set_var('QESAP_AZURE_FENCE_AGENT_CONFIGURATION', 'msi'),
-      my $res_msi = qesap_az_get_native_fencing_type();
-    set_var('QESAP_AZURE_FENCE_AGENT_CONFIGURATION', 'spn'),
-      my $res_spn = qesap_az_get_native_fencing_type();
-    set_var('QESAP_AZURE_FENCE_AGENT_CONFIGURATION', undef),
-      ok($res_msi eq 'msi', "Return 'msi' if openqa var is 'msi'");
-    ok($res_spn eq 'spn', "Return 'spn' if openqa var is 'spn'");
-};
-
 subtest '[qesap_az_diagnostic_log] no VMs' => sub {
     my $qesap = Test::MockModule->new('qesapdeployment', no_auto => 1);
     my @calls;

diff --git a/tests/sles4sap/publiccloud/azure_fence_agents_test.pm b/tests/sles4sap/publiccloud/azure_fence_agents_test.pm
@@ -64,13 +64,13 @@ sub run {
         '--tenantId=$TENANT_ID') if $fence_agent_configuration eq 'spn';
 
     select_serial_terminal;
-    # prepare bashrc file - this way credentials ar enot presented in outputs
+    # prepare bashrc file - this way credentials are not presented in outputs
     save_tmp_file('bashrc', $bashrc_vars);
     assert_script_run('curl ' . autoinst_url . '/files/bashrc -o /tmp/bashrc');
 
     foreach my $instance (@$instances) {
         $self->{my_instance} = $instance;
-        # do not probe VMs that are nto a part of cluster
+        # do not probe VMs that are not part of the cluster
         next unless grep(/^$instance->{instance_id}$/, @cluster_nodes);
         my $scp_cmd = join('', 'scp /tmp/bashrc ',
             $instance->{username},

diff --git a/tests/sles4sap/publiccloud/qesap_terraform.pm b/tests/sles4sap/publiccloud/qesap_terraform.pm
@@ -157,21 +157,26 @@ sub run {
         $reg_mode = 'suseconnect';
     }
     my $ansible_playbooks;
+    my %playbook_configs = (
+        ha_enabled => $ha_enabled,
+        registration => $reg_mode,
+        fencing => get_var('FENCING_MECHANISM'));
+
     if (get_var('PTF_ACCOUNT') && get_var('PTF_CONTAINER') && get_var('PTF_KEYNAME')) {
-        $ansible_playbooks = create_playbook_section_list(
-            ha_enabled => $ha_enabled,
-            registration => $reg_mode,
-            fencing => get_var('FENCING_MECHANISM'),
-            ptf_files => $ptf_files,
-            ptf_token => $ptf_token,
-            ptf_container => (split("/", get_required_var('PTF_CONTAINER')))[0],
-            ptf_account => get_required_var('PTF_ACCOUNT'));
-    } else {
-        $ansible_playbooks = create_playbook_section_list(
-            ha_enabled => $ha_enabled,
-            registration => $reg_mode,
-            fencing => get_var('FENCING_MECHANISM'));
+        $playbook_configs{ptf_files} = $ptf_files;
+        $playbook_configs{ptf_token} = $ptf_token;
+        $playbook_configs{ptf_container} = (split("/", get_required_var('PTF_CONTAINER')))[0];
+        $playbook_configs{ptf_account} = get_required_var('PTF_ACCOUNT');
     }
+    if ($playbook_configs{fencing} eq 'native' and is_azure) {
+        $playbook_configs{fence_type} = get_var('AZURE_FENCE_AGENT_CONFIGURATION', 'msi');
+        if ($playbook_configs{fence_type} eq 'spn') {
+            $playbook_configs{spn_application_id} = get_var('AZURE_SPN_APPLICATION_ID', get_required_var('_SECRET_AZURE_SPN_APPLICATION_ID'));
+            $playbook_configs{spn_application_password} = get_var('AZURE_SPN_APP_PASSWORD', get_required_var('_SECRET_AZURE_SPN_APP_PASSWORD'));
+        }
+    }
+    $ansible_playbooks = create_playbook_section_list(%playbook_configs);
+
     my $ansible_hana_vars = create_hana_vars_section($ha_enabled);
 
     # Prepare QESAP deployment

diff --git a/tests/sles4sap/qesapdeployment/configure.pm b/tests/sles4sap/qesapdeployment/configure.pm
@@ -72,10 +72,10 @@ sub run {
         $variables{VNET_ADDRESS_RANGE} = $peering_settings{vnet_address_range};
         $variables{SUBNET_ADDRESS_RANGE} = $peering_settings{subnet_address_range};
         if ($variables{FENCING} eq 'native') {
-            $variables{AZURE_NATIVE_FENCING_AIM} = qesap_az_get_native_fencing_type();
+            $variables{AZURE_NATIVE_FENCING_AIM} = get_var('QESAPDEPLOY_AZURE_FENCE_AGENT_CONFIGURATION', 'msi');
             if ($variables{AZURE_NATIVE_FENCING_AIM} eq 'spn') {
-                $variables{AZURE_NATIVE_FENCING_APP_ID} = get_required_var('_SECRET_AZURE_SPN_APPLICATION_ID');
-                $variables{AZURE_NATIVE_FENCING_APP_PASSWORD} = get_required_var('_SECRET_AZURE_SPN_APP_PASSWORD');
+                $variables{AZURE_NATIVE_FENCING_APP_ID} = get_var('QESAPDEPLOY_AZURE_SPN_APPLICATION_ID', get_required_var('_SECRET_AZURE_SPN_APPLICATION_ID'));
+                $variables{AZURE_NATIVE_FENCING_APP_PASSWORD} = get_var('QESAPDEPLOY_AZURE_SPN_APP_PASSWORD', get_required_var('_SECRET_AZURE_SPN_APP_PASSWORD'));
             }
         }
     }