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

fix: use standard API for Azure VMs in VMSS with flexible orchestration mode #829

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

pdecat
Copy link
Contributor

@pdecat pdecat commented Aug 26, 2024

If VMs are managed by VMSS in flexible orchestration mode, the standard VM API must be used: https://learn.microsoft.com/en-us/answers/questions/1418158/rest-api-request-returns-badrequest-for-vmss

Without this change, the following error occurs when trying to read power_state of such VMs:

# steampipe query 'select power_state from azure_compute_virtual_machine_scale_set_vm'                                                 11:24:09

Error: azure: compute.VirtualMachineScaleSetVMsClient#GetInstanceView: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned an error. Status=400 Code="InvalidParameter" Message="Virtual Machine Scale Set VM instanceId must be a number." Target="instanceId" (SQLSTATE HV000)

+-------------+
| power_state |
+-------------+
+-------------+

With this change, this returns the power state properly:

# steampipe query 'select power_state from azure_compute_virtual_machine_scale_set_vm'
+-------------+
| power_state |
+-------------+
| running     |
+-------------+

@pdecat pdecat force-pushed the fix/vmss_flexible_power_state branch from 13e6373 to 6cbb647 Compare August 26, 2024 15:04
@@ -47,6 +47,12 @@ func tableAzureComputeVirtualMachine(_ context.Context) *plugin.Table {
Description: "The friendly name that identifies the virtual machine.",
Type: proto.ColumnType_STRING,
},
{
Name: "virtual_machine_scale_set",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is necessary to distinguish standard VMs from VMs managed by VMSS in flexible orchestration mode.

@misraved misraved requested review from misraved and ParthaI and removed request for misraved August 27, 2024 07:05
Copy link
Contributor

@ParthaI ParthaI left a comment

Choose a reason for hiding this comment

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

Hi @pdecat,

Thank you so much for raising the PR! The changes make perfect sense to me.

I have a few suggestions:

Thank you again!


return op, nil
op, err := client.InstanceView(ctx, resourceGroupName, *virtualMachine.Name)
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please include the error log statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

client.Authorizer = session.Authorizer

op, err := client.GetInstanceView(ctx, resourceGroupName, virtualMachine.ScaleSetName, instanceId)
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please include the error log statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@pdecat pdecat force-pushed the fix/vmss_flexible_power_state branch from 4a15eeb to e3dee99 Compare August 29, 2024 08:29
@pdecat pdecat force-pushed the fix/vmss_flexible_power_state branch from e3dee99 to c69ade6 Compare August 29, 2024 08:32
@pdecat pdecat requested a review from ParthaI August 29, 2024 08:32
@ParthaI ParthaI requested a review from misraved September 3, 2024 06:09
@misraved misraved merged commit d039004 into turbot:main Sep 19, 2024
1 check passed
@pdecat pdecat deleted the fix/vmss_flexible_power_state branch September 19, 2024 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants