Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added possibility to assign RG name via env var ANSIBLE_AZURE_VMSS_RESOURCE_GROUPS for VMSS and Fix inventory fail when trying to parse instance from the flexible scale set #1440

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

Justwmz
Copy link

@Justwmz Justwmz commented Feb 9, 2024

SUMMARY

Added possibility to assign RG name via env var ANSIBLE_AZURE_VMSS_RESOURCE_GROUPS for VMSS
closes #1432

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

azure_rm

@Justwmz Justwmz changed the title Added possibility to assign RG name via env var ANSIBLE_AZURE_VMSS_RESOURCE_GROUPS for VMSS Added possibility to assign RG name via env var ANSIBLE_AZURE_VMSS_RESOURCE_GROUPS for VMSS and Fix inventory fail when trying to parse instance from the flexible scale set Feb 9, 2024
@Justwmz
Copy link
Author

Justwmz commented Feb 9, 2024

SUMMARY

Fix inventory failure when trying to parse instances from the flexible scale set
closes #1431

ISSUE TYPE
  • Bug Fix
COMPONENT NAME

azure_rm

@Fred-sun Fred-sun added medium_priority Medium priority work in In trying to solve, or in working with contributors labels Feb 17, 2024
# Since Flexible instance is a standalone VM, we are replacing ss provider to the vm one in order to allow inventory get instance view.
if vmss['properties']['orchestrationMode'] == 'Flexible':
newProvider = 'Microsoft.Compute/virtualMachineScaleSets/{0}/virtualMachines'.format(vmss['name'])
url = url.replace(newProvider, "Microsoft.Compute/virtualMachines")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Justwmz In this way, uniform VMSS instances can be obtained. However, because flexible VMSS instances are similar to regular VMS, the flexible VMSS instances will be obtained twice. Is there any way to avoid this? Thank you!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fred-sun Yes, there is the way to filter them out by the properties object, each flexible instance has a property which is indicated to which flexible scale set it is related.
I am just right now trying to test it, to see if that will works.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Justwmz Thanks for your feedback! Look forward a good results! Thank you very much!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fred-sun I tried multiple options to get it to work properly.
If you have VM and VMss RG's assigned to the same name where both types of scale sets are sitting, the call for pulling all VMs within RG will receive a list of those instances, since they are standalone VMs.

Maybe the better option will be to just obtain them as standalone VMs since they are pure VMs and need to be managed separately, not like in the Uniform instances which are managed by their parent?

P.S. I get one thing that works, which will parse the name of the instances from the Flexible scale set and then send a request per instance to obtain VM details. (But anyway, I still think that the ideal solution will be is ignore flexible parent since instances are managed separatly and just obtain them as VMs)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fred-sun To ignore flexible parent we just need to change this:

        for vmss in response['value']:
            url = '{0}/virtualMachines'.format(vmss['id'])

            # Since Flexible instance is a standalone VM, we are replacing ss provider to the vm one in order to allow inventory get instance view.
            if vmss['properties']['orchestrationMode'] == 'Flexible':
                newProvider = 'Microsoft.Compute/virtualMachineScaleSets/{0}/virtualMachines'.format(vmss['name'])
                url = url.replace(newProvider, "Microsoft.Compute/virtualMachines")

            # VMSS instances look close enough to regular VMs that we can share the handler impl...
            self._enqueue_get(url=url, api_version=self._compute_api_version, handler=self._on_vm_page_response, handler_args=dict(vmss=vmss))

To this:

        for vmss in response['value']:
            url = '{0}/virtualMachines'.format(vmss['id'])

            # Since Flexible instance is a standalone VM, we are ignoring parsing instances data from parent and just obtaining them as regular VMs.
            if vmss['properties']['orchestrationMode'] != 'Flexible':
            # VMSS instances look close enough to regular VMs that we can share the handler impl...
               self._enqueue_get(url=url, api_version=self._compute_api_version, handler=self._on_vm_page_response, handler_args=dict(vmss=vmss))

And it will work fine in all cases.
Maybe in the future MS will add support to get flexible ss instances in the same way as for the Uniform.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Justwmz That sounds great, but in my tests I found that I was returning regular VM information, not VMSS information! Thank you!

Return info:
vmss: {}

Actual VMSS info:
                "vmss": {
                    "id": "/subscriptions/xxx/resourceGroups/v-xisuRG-uniform/providers/Microsoft.Compute/virtualMachineScaleSets/flexible-vmss-name",
                    "name": "flexible-vmss"
                }

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fred-sun Yes you will be able to get info about scale set itself and list of the instances. But the key difference is that when you are obtaining a list of the instances in uniform scale set, it give you a full instance information.
In case of flexible scale set it will only present a limited information, like location, azure zone and 2-3 more parameters.

In order to get a full info about instance we need to use same api call as we are using for the standalone vms.

That’s why I basically propose ignore parent of the flexible scale set completely and only relay on the standalone vm api, since instances in the flexible scale set is standalone vms.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Justwmz Yes, I agree with the idea of returning only VM or only the flexible VMSS instance VMS. VM are all virtual machines with a large range, while flexible VMSS instances are more precise. So we can filter and return flexible VMSS instance information. It's more accurate. Thank you!

@Fred-sun Fred-sun added the question Further information is requested label Feb 20, 2024
@Fred-sun
Copy link
Collaborator

@Justwmz According to the return range, should we return VMSS instances and filter VMSS instances in VMS? right?

@Justwmz
Copy link
Author

Justwmz commented Mar 2, 2024

@Fred-sun We can do that, but there will be another problem.
If you supply the name of the rg for the include_vm_resource_groups and include_vmss_resource_groups to the same value.
It will cause a double fetch of instances within a scale set because they are regular VMS. After all, they are a part of both types...

We can add a check when we fetch vms if it is a part of the scale set, but you need to perform an API call to Azure to see from the response if this property is present or not.
Which will anyway cause a double API call.

So in our case, better to completely filter out flexible parents, and let inventory fetch flexible instances using --> include_vm_resource_groups.

This solves a problem with double API calls for the same instances and from the performance side it will be better.

@Fred-sun
Copy link
Collaborator

Fred-sun commented Mar 4, 2024

@Justwmz Your idea is really acceptable, but we still need to discuss it, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
medium_priority Medium priority question Further information is requested work in In trying to solve, or in working with contributors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add possibility to assign VMSS rg name via environment variable
2 participants