add missing hostvars properties in azure_rm.py inventory

[yungezz]

SUMMARY

fixing issue #51864.

adding missed hostvars from azure_rm.py inventory. properties added:

• image
• mac_address
• network_interface id and name
• os_disk
• resource_group
• vm_size
• public_ip id and name

sample hostvars after fixing

{
    'powerstate': u 'running',
    'resource_group': u 'asb-it-deployment',
    'image': {
        'sku': u '16.04.0-LTS',
        'publisher': u 'Canonical',
        'version': u 'latest',
        'offer': u 'UbuntuServer'
    },
    'vmss': {},
    'public_ip_name': u 'myPublicIP',
    'public_ip_id': u '/subscriptions/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx/resourceGroups/asb-it-deployment/providers/Microsoft.Network/publicIPAddresses/myPublicIP',
    'private_ipv4_addresses': [u '10.0.0.4'],
    'public_ipv4_addresses': [u '40.112.59.26'],
    'id': u '/subscriptions/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx/resourceGroups/ASB-IT-DEPLOYMENT/providers/Microsoft.Compute/virtualMachines/MyUbuntuVM',
    'public_dns_hostnames': [u 'xxxxxx.eastus.cloudapp.azure.com'],
    'os_disk': {
        'operating_system_type': u 'linux',
        'name': u 'osdisk'
    },
    'virtual_machine_size': u 'Standard_D1',
    'mac_address': u '00-0D-3A-1C-xx-xx',
    'network_interface': u 'myVMNic',
    'tags': {},
    'provisioning_state': u 'succeeded',
    'plan': None,
    'name': u 'MyUbuntuVM',
    'vmid': u 'xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx',
    'location': u 'eastus',
    'network_interface_id': u '/subscriptions/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx/resourceGroups/asb-it-deployment/providers/Microsoft.Network/networkInterfaces/myVMNic',
    'resource_type': u 'Microsoft.Compute/virtualMachines'
}

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

ADDITIONAL INFORMATION

[yungezz]

HI @AlanCoding , could you pls help to look at if all those information added? I've compared output with old inventory output, except for public_ip_alloc_method and private_ip_alloc_method, all info there. old fqdn was mapped to public_dns_hostnames at beginning. and security_group info will be added later maybe in another PR.

[yungezz]

ready_for_review

[AlanCoding]

Testing the reported keys:

• image
  • confirmed, this was restored and has same content as script
• mac_address
  • confirmed restored and same
• network_interface id and name
  • confirmed restored and same
• os_disk
  • confirmed restored and same
• resource_group
  • confirmed restored and same
• vm_size
  • confirmed restored and same
• public_ip id and name
  • confirmed restored and same for public_ip_id and public_ip_name, the plugin already had public_ipv4_addresses and public_dns_hostnames, which is different information (all 4 may be important). The only piece of information that the script had which the plugin still appears to not have after this patch is public_ip_alloc_method, example value: "Static". I don't know if this is important, I'm just documenting

The only other things that the script had which are not covered by this PR are:

• security_group and security_group_id
• fqdn

The fqdn key from the script might have been garbage, I don't know. The only value I have seen for it is just "null". I do not think anyone should seek to replicate that behavior.

However, security group looks important, that's the 1 thing that's probably important to get in, but this PR doesn't yet cover.

With that said, I give a big 👍 to this PR for adding the above enumerated fields. This is a big improvement.

[AlanCoding]

@zikalino @yungezz what is the status of this PR now? Are you going to expand it to cover the security groups or handle that as a separate patch? On a fairly basic level I can poke at it and see if I can discover this data anywhere.

The change here is a clear improvement. I would support merging as-is, if nobody has any code quality concerns.

[AlanCoding]

@yungezz this obtains the security group hostvars as desired

diff --git a/lib/ansible/plugins/inventory/azure_rm.py b/lib/ansible/plugins/inventory/azure_rm.py
index 226bd7e68c..dfffa30cc4 100644
--- a/lib/ansible/plugins/inventory/azure_rm.py
+++ b/lib/ansible/plugins/inventory/azure_rm.py
@@ -527,6 +527,8 @@ class AzureHost(object):
             new_hostvars['mac_address'] = nic._nic_model['properties'].get('macAddress')
             new_hostvars['network_interface'] = nic._nic_model['name']
             new_hostvars['network_interface_id'] = nic._nic_model['id']
+            new_hostvars['security_group_id'] = nic._nic_model['properties'].get('networkSecurityGroup')['id']
+            new_hostvars['security_group'] = parse_resource_id(new_hostvars['security_group_id'])['resource_name'].lower()
 
         # set image and os_disk
         new_hostvars['image'] = {}

Could you please consider if this diff can be added to your current PR? Let me know if there is anything I can do to help move this forward.

[AlanCoding]

a branch in case that helps:

https://github.com/ansible/ansible/compare/devel...AlanCoding:azure_security_group?expand=1

[kdelee]

🎉 Yes if there is anything I can do to help get this in, ping me

[yungezz]

Thanks @AlanCoding for the code snippet. I'll take the suggestion. @nitzmahone mentioned he'd like to review and test before merging. some concern on multiple network interfaces.

[yungezz]

security_group id and name was added.

@nitzmahone for multiple nic, currently both contrib/inventory/azure_rm.py and lib/ansible/plugin/inventory/azure_rm.py shows primary nic. are you planning to change to show multiple nics? if yes, output in new inventory will be out of sync with old one.

[AlanCoding]

Ran my script again, confirmed keys and values match for list in prior comment, plus security group.

shipit

[yungezz]

HI @AlanCoding , I have question on Ansible Tower, not related to this PR. Would like to take this opportunity to ask the expert. Could you pls help to have a look? Thanks in advance. I want to configure github webhook hook to ansible tower job template launch, like https://xxxxx/api/v2/job_templates/8/launch/, how to pass the ansible tower authentication token then? I always get 401. Thanks.

[AlanCoding]

https://github.com/ansible/awx/blob/devel/docs/auth/oauth.md

I think that you want the pattern like

curl -H "Authorization: Bearer kqHqxfpHGRRBXLNCOXxT5Zt3tpJogn" http://<awx>/api/v2/credentials/

tower-cli built its login command to be a role model for using this, so you can login and then make requests and see what headers are used.

This assumes that you will use token auth for webhooks. That's the only good option, and might be the only workable option for your situation.

[AlanCoding]

@nitzmahone there was one unresolved question for you, which AFAIK is the only merge blocker right now. For what it's worth, I am only concerned with parity with the old script for the 2.8 release. Listing multiple nics (aside from the primary) may be an okay feature at some later time, but would complicate current efforts.

[yungezz]

thank you @AlanCoding for sharing the knowledge! I've tried and it works well.