Azure / azure-powershell

Microsoft Azure PowerShell
Other
4.27k stars 3.87k forks source link

Set-AzureRmNetworkInterfaceIpConfig parameter ApplicationGatewayBackendAddressPool causes clearing of Subnet and more #3493

Open mbsnl opened 7 years ago

mbsnl commented 7 years ago

Cmdlet(s)

Set-AzureRmNetworkInterfaceIpConfig

PowerShell Version

5.0 on W2012R2 5.1.15031.0 on W10

Module Version

AzureRM.Network 3.4.0

OS Version

W2012R2 & W10

Description

Using the parameter ApplicationGatewayBackendAddressPool of CmdLet Set-AzureRmNetworkInterfaceIpConfig is causing the clearing of several fields: "Subnet", "PublicIpAddress", "PrivateIpAddressVersion" Further, the field "Primary" changes from True to False

NOTE 1: There might be more issues, as I was limited in testing and unable to test the fields: "LoadBalancerBackendAddressPools" and "LoadBalancerInboundNatRules" NOTE 2: Same issue applies to a variant of the same CmdLet: "Set-AzureRmNetworkInterfaceIpConfig -ApplicationGatewayBackendAddressPoolId"

A consequence of this is that the command "Set-AzureRmNetworkInterface -NetworkInterface $nic" doesn't work anymore, and ARM giving an error stating e.g. the Subnet is not populated.

Script/Steps for Reproduction

$gateway=Get-AzureRmApplicationGateway $backendpool=Get-AzureRmApplicationGatewayBackendAddressPool -ApplicationGateway $gateway

$nic=Get-AzureRmNetworkInterface Set-AzureRmNetworkInterfaceIpConfig -Name $nic.IpConfigurations.Name -NetworkInterface $nic -ApplicationGatewayBackendAddressPool $backendpool

Set-AzureRmNetworkInterface -NetworkInterface $nic

vladimir-shcherbakov commented 6 years ago

@MikhailTryakhov Can you please assign it to a proper team?

MikhailTryakhov commented 6 years ago

@yushwang from VPN team

yushwang commented 6 years ago

@MikhailTryakhov - I have no clue but it's not VPN :-) It's either the Application Gateway team or the SDN team (for Nic). I can ask Amit (AppGw PM) tomorrow. Don't know his GitHub Id.

maddieclayton commented 6 years ago

@avijitgupta Can you take a look at this issue?

jtuliani commented 5 years ago

@MikhailTryakhov Please can we get some traction on this issue? It is very important. Adding existing VMs to either ALB or AppGw via PS is basically a broken scenario right now.

It's definitely an issue for the SDN team. The problem is with the cmdlets for the NIC. We need cmdlets that can incrementally change ANY of the settings on a NIC (or the ipconfig within a NIC), WITHOUT needing to re-specify all the existing settings.

EvgenyAgafonchikov commented 5 years ago

@jtuliani, in fact this is the way most Networking child resource Set cmdlets implemented (e.g. in this case IpConfiguration is child item of NIC). This allows you to remove some references when required, for example in this scenario:

  1. I decided to add PublicIp

    $nicEx = Get-AzNetworkInterface -ResourceGroupName test-evg -Name experiments
    Set-AzNetworkInterfaceIpConfig -Name $nicEx.IpConfigurations.Name -NetworkInterface $nicEx -PublicIpAddressId <ID> -SubnetId $nicEx.IpConfigurations[0].Subnet.Id
    Set-AzNetworkInterface -NetworkInterface $nicEx

    After this I got NicIpConfig with Subnet and PublicIp referenced

  2. Next I decided to remove it, so I need just to omit this parameter:

    Set-AzNetworkInterfaceIpConfig -Name $nicEx.IpConfigurations.Name -NetworkInterface $nicEx -SubnetId $nicEx.IpConfigurations[0].Subnet.Id
    Set-AzNetworkInterface -NetworkInterface $nicEx

    And I got IpConfig with subnet only.

Taking this usage into account, re-specifying parameters in Set cmdlet is expected flow.

However, there are some properties that couldn't be removed or should be kept to avoid changing to defaults: Subnet, PrivateIpAddressVersion and one that need to be processed in a custom way: Primary.

I opened PR with fixes, however I need someone from SDNNRP team to review and approve this change. @chandrasekarsrinivasan, please take a look or re-assign properly.

There is also a thing I'm not sure how to implement: in case we have 3 and more IpConfigs and remove Primary one, server cannot choose what is the next primary. Is there specification that describes this case?

jtuliani commented 5 years ago

Thanks for the update @EvgenyAgafonchikov .

Yes, I can see that the Set-.... cmdlets are useful for declaring the 'goal state' of the resource, which allows settings to be both added and removed. I don't think that should be changed.

Would it be possible to introduce new 'Add-...' and 'Remove-...' cmdlets to enable incremental changes?

To demonstrate the value these would bring, I have a challenge for you. Try to create a script for the following scenario, and I think you will convince yourself of the need.

SET UP (you can do this in a template, portal, or whatever) Create a VM with a public IP and a static-allocation private IP Associate two ASGs to the NIC Associate an NSG to the NIC

CHALLENGE Write a general-purpose PS script to add this VM (or any other VM) to either a Load Balancer backend pool or an App Gateway backend pool (your choice), without breaking any of the existing NIC settings

chandrasekarsrinivasan commented 5 years ago

We agree with your feedback. This will lead to customer pain every time the customer has to update an Ipconfig. Set-* is the place to support such incremental changes. I think we should add $ipconfig into Set-AzureRmNetworkInterfaceIpConfig. Will that solve your problem? Here is how you'll use it.

$ipconfig = Get-AzureRmNetworkInterfaceIpConfig -NetworkInterface $nic -IpconfigName $name $ipconfig.xyz = set values Set-AzureRmNetworkInterfaceIpConfig -NetworkInterface $nic -Ipconfig $ipconfig

It is also true that this way of handling is not available in other Network Resources. We may not be able to implement this immediately as this has to be changed for all child resources in Networking Stack.

Since we already have a mitigation to solve this problem by respecifying the parameters, we consider this as a 'good to have' request for future requests and cannot get to this request immediately.

jtuliani commented 5 years ago

That could work. To be completely sure, I'd like to understand exactly how '$ipconfig.xyz = set values' would work, for real-world examples.

For example, I can see how that would work for a simple property such as IP allocation method (static/dynamic).

But how would it work for properties which are nested objects? For example, how it work to create or update a load balancer backend pool array, App Gateway backend pool, or to define an array of ASGs?

This could just be my lack of PowerShell skill. Even so, some examples would be good.