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

feat: Set EtcdDiskSizeGB max default size to 1023 if target platform is Azure Stack #1558

Merged
merged 2 commits into from
Jul 3, 2019

Conversation

rjaini
Copy link
Contributor

@rjaini rjaini commented Jul 2, 2019

Reason for Change:
Azure Stack only supports max disk size to be 1023GB. So in case when the number of nodes increases
we cant increase the size to 1024 or 2048GB.

Issue Fixed:
Fixes #1557

Requirements:

Notes:

@@ -233,9 +233,19 @@ func (cs *ContainerService) setOrchestratorDefaults(isUpgrade, isScale bool) {
if "" == a.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB {
switch {
case a.TotalNodes() > 20:
a.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB = DefaultEtcdDiskSizeGT20Nodes
if a.IsAzureStackCloud() {
// Currently on Azure Stack max size of managed disk size is 1023GB.
Copy link
Member

Choose a reason for hiding this comment

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

@rjaini do we know how much disk is required for 20 + nodes? The cluster will be not usable if the disc is out of space.
Another alternative is to limit the total count of agent nodes.
And we should raise the request to Disk RP on Azure Stack to support larger size of disk.

Copy link
Contributor Author

@rjaini rjaini Jul 2, 2019

Choose a reason for hiding this comment

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

I am not sure about disk space requirements. But from #923 thread it looks like the 1024 size itself is very high.
I will check with Disk RP team to understand the reason of 1023 limit.

@mboersma : Do you know why we have such large disk size for higher number of nodes in AKS Engine ?.

Copy link
Contributor

Choose a reason for hiding this comment

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

Matt is out on vacation but see Azure/acs-engine#2435 for context.

pkg/api/const.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jul 2, 2019

Codecov Report

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

@@            Coverage Diff             @@
##           master    #1558      +/-   ##
==========================================
+ Coverage   75.86%   75.86%   +<.01%     
==========================================
  Files         128      128              
  Lines       18314    18320       +6     
==========================================
+ Hits        13893    13899       +6     
  Misses       3636     3636              
  Partials      785      785

1 similar comment
@codecov
Copy link

codecov bot commented Jul 2, 2019

Codecov Report

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

@@            Coverage Diff             @@
##           master    #1558      +/-   ##
==========================================
+ Coverage   75.86%   75.86%   +<.01%     
==========================================
  Files         128      128              
  Lines       18314    18320       +6     
==========================================
+ Hits        13893    13899       +6     
  Misses       3636     3636              
  Partials      785      785

Copy link
Contributor

@CecileRobertMichon CecileRobertMichon left a comment

Choose a reason for hiding this comment

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

/approve

@CecileRobertMichon
Copy link
Contributor

/lgtm

@acs-bot acs-bot added the lgtm label Jul 3, 2019
@acs-bot acs-bot merged commit a2d7988 into Azure:master Jul 3, 2019
@acs-bot
Copy link

acs-bot commented Jul 3, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon, honcao, rjaini

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

@rjaini rjaini deleted the etcddisksize branch October 11, 2019 18:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: Set EtcdDiskSizeGB max default size to 1023 if target platform is Azure Stack
4 participants