Added scale check to ensure initial NSG is not for jumpbox #2781
Conversation
ghost
assigned 
Codecov Report
@@ Coverage Diff @@
## master #2781 +/- ##
==========================================
- Coverage 46.95% 46.82% -0.13%
==========================================
Files 86 86
Lines 12781 12775 -6
==========================================
- Hits 6001 5982 -19
- Misses 6226 6249 +23
+ Partials 554 544 -10
Continue to review full report at Codecov.
|
|
running scale test at https://jenkins.azure-containers.io/view/acs-engine%20ad-hoc/job/k8s-scale/26 |
pkg/acsengine/transform/transform.go
Outdated
| @@ -114,7 +114,8 @@ func (t *Transformer) NormalizeForK8sVMASScalingUp(logger *logrus.Entry, templat | |||
|
|
|||
| resourceType, ok := resourceMap[typeFieldName].(string) | |||
| if ok && resourceType == nsgResourceType { | |||
| if nsgIndex != -1 { | |||
| resourceName := resourceMap[nameFieldName].(string) | |||
| if nsgIndex != -1 && !strings.Contains(resourceName, "variables('jumpboxNetworkSecurityGroupName')") { | |||
There was a problem hiding this comment.
Thinking about this a bit more, I think the && !strings.Contains(resourceName, "variables('jumpboxNetworkSecurityGroupName')" should be on line 116. If the nsg belongs to the jumpbox, we don't want to set the nsgIndex to its index. Otherwise, if the non-jumpbox nsg comes later in the for loop, this will fail (the nsgIndex won't be -1 and it won't belong to the jumpbox).
There was a problem hiding this comment.
You are correct. I hadn't followed why nsgIndex was set, but get it now. Have made the change.
CecileRobertMichon
added
the
cse-sync-week
CecileRobertMichon
changed the title
What this PR does / why we need it:
Fixes issue where deployments with a jumpbox do not scale due to two NSGs being present.
Which issue this PR fixes *
fixes #2616
Notes*
Added check to ensure the NSG is not for a jumpbox. Tested scale up using repo details in issue.