sap_vm_provision: Rework role with best practices to align with project

[marcelmamula]

Scope

This is improvement iteration on amazing role that was created by @sean-freeman and all credit goes to him for great job.
Scope of this PR is to:

• Replace and simplify code where possible, to improve readability as well as execution speed.
• Remove linting ignores and fix all reported issues
  • Remove all lookups and keep them in vars/main.yml which improves code readability and future maintainability.
  • Variable naming
  • Jinja2 formatting
  • Spacing and comments
• Update usage of injected ansible vars to remove deprecation warning in ansible-core 2.20
• SSH Arguments override hinted at in sap_vm_provision - Move ansible_ssh_common_args Configuration to Inventory #102 @geetikakay

Variable Naming

• All internal variables had to be updated to follow project wide syntax:

  • __<role name>_fact_<fact name> for facts set during runtime
  • __<role name>_register_<register name> for registers set during runtime
  • <role name>_variable for all user customizable variables in defaults/main.yml
  • __<role name>_variable for all internal overrides in vars/main.yml

• Some short variable names are still kept, but they are strictly limited to vars: sections, where they are not set as facts, but used as task/block specific variable.

Tests

NOTE: Terraform is not part of testing scope, but improvements will be applied there as well.

Platform	Tester	Results
aws_ec2_vs	@marcelmamula	OK - AP4S: sap_hana_ha, sap_nwas_abap_ascs_ers_cluster
gcp_ce_vm	@marcelmamula	OK - AP4S: sap_hana_ha, sap_nwas_abap_ascs_ers_cluster
ibmcloud_vs	@ehaefele
ibmcloud_powervs	@ehaefele
ibmpowervm_vm	@ehaefele
msazure_vm	@marcelmamula	OK - AP4S: sap_nwas_abap_ascs_ers_cluster
kubevirt_vm	@newkit
ovirt_vm	N/A	N/A
vmware_vm	N/A	N/A

All changes are being tested using AP4S playbooks.

Linting Results

Before:

# Rule Violation Summary

102 jinja profile:basic tags:formatting
259 var-naming profile:basic tags:idiom
 20 no-changed-when profile:basic tags:command-shell,idempotency
  5 no-handler profile:basic tags:idiom

Failed: 284 failure(s), 102 warning(s) in 58 files processed of 80 encountered. Last profile that met the validation criteria was 'min'.

After

Passed: 0 failure(s), 0 warning(s) in 72 files processed of 94 encountered. Last profile that met the validation criteria was 'production'.

[marcelmamula]

@newkit I have updated all platforms including kubevirt that you are maintaining. I found some issues in existing code that were fixed so I need you to test it with your environment.

Make sure to check that variables were updated to remove dependency on wrong variable names and __sap_vm_provision_kubevirt_vm_register_execution_host_user that should have never been exposed to users in playbook sample.

[geetikakay]

lgtm

[ehaefele]

LGTM.
Verified provisioning of VMs by using sap_vm_provision on ibmcloud_vs and ibmcloud_powervs.
(Still need to establish an environment on ibmpowervm_vm)

[sean-freeman]

LGTM - tested on msazure_vm and verified flow/logic/outcomes match everything previously but with a significantly improved code refactoring and cleanse👍 Very happy to see this upgrade!

Awesome effort @marcelmamula 🙂