Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

fix: Add ILB for vmss masters #319

Merged
merged 4 commits into from Jan 19, 2019
Merged

Conversation

ritazh
Copy link
Member

@ritazh ritazh commented Jan 15, 2019

Reason for Change:

  • Add internal loadbalancer for masters when availabilityProfile is VirtualMachineScaleSets and MasterProfile.Count > 1
  • New internal IP assigned to kubernetesAPIServerIP derived from "[concat(variables('masterFirstAddrOctets')[0],'.',variables('masterFirstAddrOctets')[1],'.255.', variables('masterInternalLbIPOffset'))]" e.g. 10.240.255.10
  • Add master node's primary IP to ILB's backend pool

Issue Fixed:

Fixes #304

Requirements:

Notes:

  • Also fixes few style issues in e2e

@acs-bot acs-bot added the size/M label Jan 15, 2019
@codecov
Copy link

codecov bot commented Jan 15, 2019

Codecov Report

Merging #319 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #319      +/-   ##
==========================================
+ Coverage   53.16%   53.17%   +<.01%     
==========================================
  Files          95       95              
  Lines       14244    14247       +3     
==========================================
+ Hits         7573     7576       +3     
  Misses       6006     6006              
  Partials      665      665

@CecileRobertMichon
Copy link
Contributor

CecileRobertMichon commented Jan 18, 2019

PR E2E failure is a flake on Windows test

(no need to rerun)

{{else}}
"kubernetesAPIServerIP": "[concat(variables('masterFirstAddrPrefix'), add(variables('masterInternalLbIPOffset'), int(variables('masterFirstAddrOctet4'))))]",
{{end}}
{{else}}
"kubernetesAPIServerIP": "[parameters('firstConsecutiveStaticIP')]",
{{end}}
"masterLbBackendPoolName": "[concat(parameters('orchestratorName'), '-master-pool-', parameters('nameSuffix'))]",
"masterFirstAddr": "[parameters('firstConsecutiveStaticIP')]",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we continue to re-use the parameter instead of re-assigning to an ARM variable?

@jackfrancis
Copy link
Member

Functional validation of vmss master cluster configuration checks out. This lgtm after @tariq1890's suggestions are incorporated. Thanks so much!

@jackfrancis
Copy link
Member

/lgtm

@acs-bot
Copy link

acs-bot commented Jan 19, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jackfrancis, ritazh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@acs-bot acs-bot merged commit e29d149 into Azure:master Jan 19, 2019
@jackfrancis jackfrancis added this to Done in backlog Jan 22, 2019
juhacket pushed a commit to juhacket/aks-engine that referenced this pull request Mar 14, 2019
* fix: Add ILB for vmss masters

* Add ut and e2e test

* Update parts/k8s/kubernetesmastercustomdata.yml

Co-Authored-By: ritazh <rita.z.zhang@gmail.com>

* Remove masterFirstAddr variable
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
backlog
  
Done
Development

Successfully merging this pull request may close these issues.

Missing internal load balancer when masterProfile.availabilityProfile is VirtualMachineScaleSets
5 participants