Azure-cli: Specifying VM ID in "az vm extension set" generates an exception

[bmc-msft]

Describe the bug

When specifying a VM via the ID rather than the resource group & vm name, az cli generates an exception.

Command Name
az vm extension set

Errors:

The command failed with an unexpected error. Here is the traceback:

'child_name_1'
Traceback (most recent call last):
  File "/opt/az/lib/python3.6/site-packages/knack/cli.py", line 206, in invoke
    cmd_result = self.invocation.execute(args)
  File "/opt/az/lib/python3.6/site-packages/azure/cli/core/commands/__init__.py", line 274, in execute
    self.cli_ctx.raise_event(EVENT_INVOKER_POST_PARSE_ARGS, command=parsed_args.command, args=parsed_args)
  File "/opt/az/lib/python3.6/site-packages/knack/cli.py", line 162, in raise_event
    func(self, **kwargs)
  File "/opt/az/lib/python3.6/site-packages/azure/cli/core/commands/arm.py", line 310, in parse_ids_arguments
    getattr(namespace, arg.name).append(parts[arg.type.settings['id_part']])
KeyError: 'child_name_1'

To Reproduce:

Steps to reproduce the behavior. Note that argument values have been redacted, as they may contain sensitive information.

• _Put any pre-requisite steps here..._
• az vm extension set --ids {} --publisher {} -n {} --settings {}

Expected Behavior

Environment Summary

Linux-4.4.0-18908-Microsoft-x86_64-with-debian-buster-sid
Python 3.6.5
Shell: bash

azure-cli 2.0.62

Extensions:
interactive 0.4.1
resource-graph 0.1.8

[bmc-msft]

Verified this still fails on 2.0.66.

[bmc-msft]

To recreate:

az vm extension set --ids /subscriptions/A/resourceGroups/B/providers/Microsoft.Compute/virtualMachines/C  --publisher Microsoft.Azure.Extensions -n customScript --settings '{"commandToExecute": "touch /tmp/test"}'

[CraigWiand]

Issue still present in 2.0.74. What is missing from the description above is the logged line:
option '--name' will be ignored due to use of '--ids'.
We also see that in az vm extension set --help that it is treating --name as a parameter associated with the target resource, but it isn't. That is the vm-name parameter. If it is really ignoring the --name then it is ignoring the required name of the extension trying to be run.

[adewaleo]

@feiyushi would you be able to look into this?

@bmc-msft, @CraigWiand, I am no longer on the Azure CLI team.

[feiyushi]

@adewaleo I'm not the right person to solve this. I'm not familiar with the cli nor the vm extension service

[adewaleo]

@feiyushi, sorry, I meant to mention @qwordy (Feiyue Yu)

[yonzhan]

@qwordy We already planned in this sprint, right?

[qwordy]

@qwordy We already planned in this sprint, right?

Yes. But can't guarantee a fix in this sprint. First time for me to dive into extension.

[qwordy]

It's not bug.
--ids should be ids of extensions.
E.g.

/subscriptions/0000-0000-0000-0000/resourceGroups/fytest2/providers/Microsoft.Compute/virtualMachines/vm1/extensions/LinuxDiagnostic

However, the error message can be improved. I'm also very confused when I see the error message for the first time. It takes me a considerable time to find the root cause.

[CraigWiand]

--ids should be ids of extensions.

I disagree wit this approach. Having --ids of the extension paths serves little utility in my opinion. What I want to do is be able to do a az vm list --query and have that as input to this operation such that I can run an extension against all the VMs in my query (e.g. custom script). My expectation is that I could use --ids for this instead of the --vm-name and --resource-group.

I believe the problematic aspect here is that to invoke the operation one needs to provide the name of the extension. But if one uses the --ids as above, the --name parameter is still required.

[adewaleo]

I think --ids accepting a list of VM's makes sense. It would be good to discuss this with the team @qwordy after the holiday over there. To get clarity. If the point of this command is to carry an operation on a VM, then --ids should extend this operation to multiple vms. It would be good to compare this to other CLI commands.

cc: @yugangw-msft @yonzhan

[qwordy]

I'm on holiday from 1st Oct to 7th Oct . --ids is a general parameter in Azure CLI. It doesn't support the usage you expect. But your advice is great. We'll discuss if it's feasible later.

[adewaleo]

Hello @qwordy, no problem, let's continue this conversation when you are back.

I would say that even though --ids is a general parameter, its presence and behavior depends on code in params.py. If you don't specify id_part, then I don't believe --ids will be exposed. And how --ids behaves depends on which parameter you specify id_part on.

When setting an extensions you need to specify the extension type, which is different from the instance, which has a resource id (that includes the vm). I don't think the current code allows for specifying extension types with --ids, as --ids expect an already set extension instance (see code).

Finally, vm extension set takes --setting and --protected-setting for the extenson to be set (instantiated on the VM). If multiple extension types could even be specified with --ids it would not be possible to specify the different settings / protected-settings for each of them.

[yonzhan]

@qwordy we can sync up after holiday.

[qwordy]

--ids should be ids of extensions.

I disagree wit this approach. Having --ids of the extension paths serves little utility in my opinion. What I want to do is be able to do a az vm list --query and have that as input to this operation such that I can run an extension against all the VMs in my query (e.g. custom script). My expectation is that I could use --ids for this instead of the --vm-name and --resource-group.

I believe the problematic aspect here is that to invoke the operation one needs to provide the name of the extension. But if one uses the --ids as above, the --name parameter is still required.

Understand. You want to use --ids to specify VM(s) and -n to specify extension. It should be supported. Currently, --ids always require extension-level ids no matter whether -n is provided, which is not flexible. Add it to Sprint 76.

[qwordy]

Hello @qwordy, no problem, let's continue this conversation when you are back.

I would say that even though --ids is a general parameter, its presence and behavior depends on code in params.py. If you don't specify id_part, then I don't believe --ids will be exposed. And how --ids behaves depends on which parameter you specify id_part on.

When setting an extensions you need to specify the extension type, which is different from the instance, which has a resource id (that includes the vm). I don't think the current code allows for specifying extension types with --ids, as --ids expect an already set extension instance (see code).

Finally, vm extension set takes --setting and --protected-setting for the extenson to be set (instantiated on the VM). If multiple extension types could even be specified with --ids it would not be possible to specify the different settings / protected-settings for each of them.

Yes. A certain command can have its own behavior for --ids.
Do you think we should remove --vm-name and --extension-instance-name out of Resource Id Arguments group so that --ids can refer to VMs? But this command is about setting extension. The target is extension and extension has ID.

Do you think we should allow users to use an incomplete ID and provide other parameters separately?
But our docs write: One or more resource IDs (space-delimited). If provided, no other 'Resource Id' arguments should be specified.

[adewaleo]

Its okay for --vm-name to be associated with resource id arguments group, as the vm name should be extracted from the ids. But --extension-instance-name should not be extracted from the id as the id should refer only to vms.

This is similar to az vm delete --help where --name and --resource-group all fall under resource id arguments. A user can use either --ids or both --name and --resource-group. However if they use --ids the command parses the vm name and resource group for each id. This is why name and resource group fall under --ids.

Do you think we should allow users to use an incomplete ID and provide other parameters separately?
But our docs write: One or more resource IDs (space-delimited). If provided, no other 'Resource Id' arguments should be specified.

It is not an incomplete id, the behavior will be something like this

set an extension on a vm with regular arguments

    az vm extension set -n VMAccessForLinux --publisher Microsoft.OSTCExtensions --version 1.4 \
        --vm-name MyVm --resource-group MyResourceGroup \
        --protected-settings '{"username":"user1", "ssh_key":"ssh_rsa ..."}'

set an extension on multiple vms - this creates a separate extension instance from the ext type on each vm.

    az vm extension set -n VMAccessForLinux --publisher Microsoft.OSTCExtensions --version 1.4 \
        --protected-settings '{"username":"user1", "ssh_key":"ssh_rsa ..."}'
        --ids "/subscriptions/00000/resourceGroups/ova-test/providers/Microsoft.Compute/virtualMachines/test-vm"
                 "/subscriptions/.../virtualMachines/vm-2"
                 "/subscriptions/.../virtualMachines/linux-vm"

This behavior sets a vmaccess extension with those settings on test-vm, vm-2 and linux-vm.

The current implementation of vm extension set does not behave like this. It expects an extension instance resource id instead of a vm resource id for --ids when setting an extension.

[qwordy]

Tosin. Thank you so much! Your great insight of Azure CLI really impresses me.
I misunderstood the semantics of vm extension set. I thought it is like general set command of other command. But it’s more like a create command to create or add an extension to VM.
We can use id for existing resource rather then the resource we're going to create or install.

[adewaleo]

Thanks @qwordy for the compliment and kind words. It took months to get familiar with the CLI, and its code. To be honest there is still stuff in the CLI I don't know about or know well.

And yes you are totally right, vm extension add is more consistent / clear than set, at least according to these guidelines.

[qwordy]

The PR has been merged. If you still have questions, feel free to reopen the issue.