Fix regression in Hybrid clusters using custom Vnets#4038
Fix regression in Hybrid clusters using custom Vnets#4038tariq1890 wants to merge 2 commits intoAzure:masterfrom
Conversation
Signed-off-by: tariqibrahim <tariq.ibrahim@microsoft.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tariq1890 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ghost
assigned
Codecov Report
@@ Coverage Diff @@
## master #4038 +/- ##
=======================================
Coverage 50.42% 50.42%
=======================================
Files 109 109
Lines 16895 16895
=======================================
Hits 8519 8519
Misses 7584 7584
Partials 792 792 |
|
Added param-->var replacement for vmss Approach makes sense @tariq1890! |
| "vnetSubnetID": "[concat(variables('vnetID'),'/subnets/',variables('subnetName'))]", | ||
| "virtualNetworkName": "[concat(parameters('orchestratorName'), '-vnet-', parameters('nameSuffix'))]", | ||
| "virtualNetworkResourceGroupName": "''", | ||
| "masterSubnet": "[parameters('masterSubnet')]", |
There was a problem hiding this comment.
Do we need to translate this to a variable? Why not add a default value of "" for the parameter?
There was a problem hiding this comment.
I like the idea. I think it's much better and cleaner. I followed this because of my limited ARM knowledge and variables to params delegation is a pattern that has already being followed. Thanks for the suggestion!
@jackfrancis FYI
|
@CecileRobertMichon @tariq1890 Is this what we want to do instead? #4058 |
|
Yes! |
4 participants
Fixes issue #4027