-
Notifications
You must be signed in to change notification settings - Fork 4k
PR: LoadBalancerBackendAddressPool powershell Cmdlets #11734
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
Conversation
Can one of the admins verify this patch? |
|
||
[JsonProperty(Order = 1)] | ||
public string IpAddress { get; set; } | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to check the combination, as we dicussed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I don't think so, right now we don't populate NIC config if provided, so PsLoadBalancerBackendAddress will always have a IpAddress and VNET ID, other wise an error is returned by New-AzLoadBalancerBackendAddressConfig which creates this object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the fact that we cannot modify does not mean that we can see it as read only
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is read only if available now.
src/Network/Network/LoadBalancer/SetAzureLoadBalancerBackendPool.cs
Outdated
Show resolved
Hide resolved
src/Network/Network/LoadBalancer/SetAzureLoadBalancerBackendPool.cs
Outdated
Show resolved
Hide resolved
src/Network/Network/LoadBalancer/NewAzLoadBalancerBackendAddressConfig.cs
Show resolved
Hide resolved
src/Network/Network/LoadBalancer/NewAzLoadBalancerBackendAddressConfig.cs
Show resolved
Hide resolved
src/Network/Network/LoadBalancer/NewAzLoadBalancerBackendAddressConfig.cs
Show resolved
Hide resolved
// CNM to MNM | ||
cfg.CreateMap<CNM.PSNetworkInterfaceIPConfiguration, MNM.NetworkInterfaceIPConfiguration>(); | ||
|
||
// LoadBalancerBackendAddress |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move above nic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should actually remove NetworkInterfaceIPConfiguration conversion here as we are not allowing that in this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also when we do we'll do on the load balancer backend address, not on this networkInterfaces inside the backend pool
-updating help files -addressing feedback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address validation failures
@@ -1488,6 +1488,7 @@ | |||
"Set-AzVirtualNetworkGatewayConnectionSharedKey": "Set-AzureRmVirtualNetworkGatewayConnectionSharedKey", | |||
"Set-AzVirtualNetworkGatewayConnection": "Set-AzureRmVirtualNetworkGatewayConnection", | |||
"New-AzIpsecPolicy": "New-AzureRmIpsecPolicy", | |||
"Get-AzLoadBalancerBackendAddressPool": "Get-AzureRmLoadBalancerBackendAddressPool", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to add mappings AFAIK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesnt harm if I add it anyways? Wanna make sure I don't miss anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK AzureRm aliases aren't supported for new cmdlets as an incentive for users to move to Az
@VeryEarly could you please clarify?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the included nupkg needed as part of PR?
// CNM to MNM | ||
cfg.CreateMap<CNM.PSNetworkInterfaceIPConfiguration, MNM.NetworkInterfaceIPConfiguration>(); | ||
|
||
// LoadBalancerBackendAddress |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also when we do we'll do on the load balancer backend address, not on this networkInterfaces inside the backend pool
@@ -1488,6 +1488,7 @@ | |||
"Set-AzVirtualNetworkGatewayConnectionSharedKey": "Set-AzureRmVirtualNetworkGatewayConnectionSharedKey", | |||
"Set-AzVirtualNetworkGatewayConnection": "Set-AzureRmVirtualNetworkGatewayConnection", | |||
"New-AzIpsecPolicy": "New-AzureRmIpsecPolicy", | |||
"Get-AzLoadBalancerBackendAddressPool": "Get-AzureRmLoadBalancerBackendAddressPool", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why?
src/Network/Network/LoadBalancer/SetAzureLoadBalancerBackendPool.cs
Outdated
Show resolved
Hide resolved
|
||
[JsonProperty(Order = 1)] | ||
public string IpAddress { get; set; } | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the fact that we cannot modify does not mean that we can see it as read only
src/Network/Network.Test/ScenarioTests/LoadBalancerBackendPoolTests.ps1
Outdated
Show resolved
Hide resolved
src/Network/Network/LoadBalancer/GetAzureLoadBalancerBackendPool.cs
Outdated
Show resolved
Hide resolved
src/Network/Network/LoadBalancer/GetAzureLoadBalancerBackendPool.cs
Outdated
Show resolved
Hide resolved
src/Network/Network/LoadBalancer/NewAzureLoadBalancerBackendPool.cs
Outdated
Show resolved
Hide resolved
src/Network/Network/help/New-AzLoadBalancerBackendAddressConfig.md
Outdated
Show resolved
Hide resolved
src/Network/Network/help/Set-AzLoadBalancerBackendAddressPool.md
Outdated
Show resolved
Hide resolved
src/Network/Network/help/Remove-AzLoadBalancerBackendAddressPool.md
Outdated
Show resolved
Hide resolved
Adding @isra-fel to this, please take a look if you get a chance. |
Hi @aegal ,
|
* lb backend address pool create/update/delete operations * adding tests with draft * making some updates for design review PR to powershell team * adding .md files * -adding tests -updating help files -addressing feedback * pulling in sdk changes for network-may branch for validation errors * Addressing comments and updating output format for PsLoadBalancerBackendAddress * updating .md files and making final changes Co-authored-by: Ali Egal <[email protected]>
* PR: LoadBalancerBackendAddressPool powershell Cmdlets (#11734) * lb backend address pool create/update/delete operations * adding tests with draft * making some updates for design review PR to powershell team * adding .md files * -adding tests -updating help files -addressing feedback * pulling in sdk changes for network-may branch for validation errors * Addressing comments and updating output format for PsLoadBalancerBackendAddress * updating .md files and making final changes Co-authored-by: Ali Egal <[email protected]> * Adding new optional parameter to specify dns servers for Virtual Wan P2SVpnGateway and P2SClients (#12006) * Adding new optional parameter to specify dns servers for Virtual Wan-P2SVpnGateway and P2SClients * Incorporate code review comments and added test record * Suppress warnings * Suppress cred scan errors * updating help files examples (#12068) Co-authored-by: Ali Egal <[email protected]> * multipip changes * Do not pass new fields with default values unless configured explicitly (#12076) * multi pip changes * ps tests * generating help files * help files changes * help file * regenerating help files * updated markdown file * renaming the cmdlets * name change * tests * renaming a help file * updating the recordings * test * Used signed Network SDK with API version 2020-05-01 (#12100) * Network SDK for 2020-05-01 * Fixes for FirewallPolicy * Recs * Recs * SDK package * Ignore Network API version in Batch tests * CredScan suppression * clean up * updatid recordings * clean up * clean up * Vpn gateway commandlet update (#12108) * commit1 * commit2 * Fix1 Co-authored-by: Khushboo Baheti <[email protected]> * addressing comments * added exceptions * correcting the exceptions * Change RuleGroup, RuleCollectionGroup, and RuleType and add support for Multiple DNAT Rule Collections (#12095) * Change RuleGroup and RuleCollectionGroup based on swagger change. Also, add support for NAT Rule Collections containing multiple NAT Rule Collections * PR Comments * Updated the SDK * StaticAnalysis * Previous changes got removed in rebase * update to vnet reference id only * Use Network SDK from NuGet (#12114) * Use Network SDK from NuGet * Additional mappings * Firewall Policy ThreatIntelWhitelist (#12078) * ti whitelist changes * build fix * help files * added examples for help files * adding the online version * recordings done * changed to markdown files * fixed statis analysis * added exception for static analysis * addressing comments * updated recordings * new recordings * updating due to validation error on CICD pipeline * refreshing test files, updating to remove id from variable to fix casting error * DNS Proxy in Firewall Policy (#12120) * merge conflict * fixes * tejas review Co-authored-by: Ishani Gupta <[email protected]> * Add support for IPGroups in Firewall Policy Rules (#12118) * Add support for IPGroups in Firewall Policy Rules * Pr Comments * Resolved merge conflicts * Update ChangeLog.md * Update ChangeLog.md Co-authored-by: Yabo Hu <[email protected]> * Application Gateway Private Link Cmdlets (#12133) * Application Gateway Private Link Cmdlets * add change log * Add new cmdlet Reset-AzHubRouter (#12094) * add routing state and expose reset-hubrouter * update help * updated * Update ChangeLog.md * update help file name * export cmdlet * update test * fix * update md file * update tests * updated help * Update Reset-AzHubRouter.md * test recorded Co-authored-by: Yabo Hu <[email protected]> * Fixes after merge * Revert HPCCache.Test.csproj Co-authored-by: aegal <[email protected]> Co-authored-by: Ali Egal <[email protected]> Co-authored-by: Nilambari <[email protected]> Co-authored-by: Sai Mankala <[email protected]> Co-authored-by: tejasshah7 <[email protected]> Co-authored-by: Khushboo Baheti <[email protected]> Co-authored-by: Khushboo Baheti <[email protected]> Co-authored-by: Ishani Gupta <[email protected]> Co-authored-by: Ishani Gupta <[email protected]> Co-authored-by: Yabo Hu <[email protected]> Co-authored-by: jaishals <[email protected]> Co-authored-by: Ritvika Reddy Nagula <[email protected]>
Description
Checklist
CONTRIBUTING.md
ChangeLog.md
file(s) has been updated:ChangeLog.md
file can be found atsrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
## Upcoming Release
header -- no new version header should be added