From f1e2163861cb4553a19feecefde99a32de0eead3 Mon Sep 17 00:00:00 2001 From: Rohit Jaini Date: Mon, 1 Jul 2019 16:46:00 -0700 Subject: [PATCH 1/2] Fix default case for EtcdDiskSizeGB on Azure Stack --- pkg/api/const.go | 3 + pkg/api/defaults.go | 14 ++++- pkg/api/defaults_test.go | 126 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 2 deletions(-) diff --git a/pkg/api/const.go b/pkg/api/const.go index e9e3cbd2cf..f5a19077f3 100644 --- a/pkg/api/const.go +++ b/pkg/api/const.go @@ -252,6 +252,9 @@ const ( // DefaultAzureStackFaultDomainCount set to 3 as Azure Stack today has minimum 4 node deployment. DefaultAzureStackFaultDomainCount = 3 + + // DefaultAzureStackEtcdDiskSizeGT10Nodes = size for Kubernetes master etcd disk volumes in GB if > 10 nodes + DefaultAzureStackEtcdDiskSizeGT10Nodes = "1023" ) // WindowsProfile defaults diff --git a/pkg/api/defaults.go b/pkg/api/defaults.go index 22d745051b..4296d2a4ad 100644 --- a/pkg/api/defaults.go +++ b/pkg/api/defaults.go @@ -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. + a.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB = DefaultAzureStackEtcdDiskSizeGT10Nodes + } else { + a.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB = DefaultEtcdDiskSizeGT20Nodes + } case a.TotalNodes() > 10: - a.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB = DefaultEtcdDiskSizeGT10Nodes + if a.IsAzureStackCloud() { + // Currently on Azure Stack max size of managed disk size is 1023GB. + a.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB = DefaultAzureStackEtcdDiskSizeGT10Nodes + } else { + a.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB = DefaultEtcdDiskSizeGT10Nodes + } case a.TotalNodes() > 3: a.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB = DefaultEtcdDiskSizeGT3Nodes default: diff --git a/pkg/api/defaults_test.go b/pkg/api/defaults_test.go index 196cbf8626..0df564fd39 100644 --- a/pkg/api/defaults_test.go +++ b/pkg/api/defaults_test.go @@ -2476,6 +2476,132 @@ func TestSetAgentProfileDefaultsOnAzureStack(t *testing.T) { } } +func TestEtcdDiskSizeOnAzureStack(t *testing.T) { + location := "testlocation" + mockCS := getMockBaseContainerService("1.11.6") + mockCS.Location = location + mockCS.Properties.MasterProfile.Count = 1 + mockCS.Properties.OrchestratorProfile.OrchestratorType = Kubernetes + mockCS.Properties.CustomCloudProfile = &CustomCloudProfile{ + PortalURL: "https://portal.testlocation.contoso.com", + } + + httpmock.Activate() + defer httpmock.DeactivateAndReset() + httpmock.RegisterResponder("GET", fmt.Sprintf("%smetadata/endpoints?api-version=1.0", fmt.Sprintf("https://management.%s.contoso.com/", location)), + func(req *http.Request) (*http.Response, error) { + resp := httpmock.NewStringResponse(200, `{"galleryEndpoint":"https://galleryartifacts.hosting.testlocation.contoso.com/galleryartifacts/","graphEndpoint":"https://graph.testlocation.contoso.com/","portalEndpoint":"https://portal.testlocation.contoso.com/","authentication":{"loginEndpoint":"https://adfs.testlocation.contoso.com/adfs","audiences":["https://management.adfs.azurestack.testlocation/ce080287-be51-42e5-b99e-9de760fecae7"]}}`) + return resp, nil + }, + ) + + mockCS.SetPropertiesDefaults(false, false) + if mockCS.Properties.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB != DefaultEtcdDiskSize { + t.Fatalf("EtcdDiskSizeGB did not have the expected size, got %s, expected %s", + mockCS.Properties.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB, DefaultEtcdDiskSize) + } + + // Case where total node count is 5. + mockCS = getMockBaseContainerService("1.11.6") + mockCS.Location = location + mockCS.Properties.OrchestratorProfile.OrchestratorType = Kubernetes + mockCS.Properties.MasterProfile.Count = 5 + mockCS.Properties.CustomCloudProfile = &CustomCloudProfile{ + PortalURL: "https://portal.testlocation.contoso.com", + } + + httpmock.DeactivateAndReset() + httpmock.Activate() + httpmock.RegisterResponder("GET", fmt.Sprintf("%smetadata/endpoints?api-version=1.0", fmt.Sprintf("https://management.%s.contoso.com/", location)), + func(req *http.Request) (*http.Response, error) { + resp := httpmock.NewStringResponse(200, `{"galleryEndpoint":"https://galleryartifacts.hosting.testlocation.contoso.com/galleryartifacts/","graphEndpoint":"https://graph.testlocation.contoso.com/","portalEndpoint":"https://portal.testlocation.contoso.com/","authentication":{"loginEndpoint":"https://adfs.testlocation.contoso.com/","audiences":["https://management.adfs.azurestack.testlocation/ce080287-be51-42e5-b99e-9de760fecae7"]}}`) + return resp, nil + }, + ) + + mockCS.SetPropertiesDefaults(false, false) + if mockCS.Properties.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB != DefaultEtcdDiskSizeGT3Nodes { + t.Fatalf("EtcdDiskSizeGB did not have the expected size, got %s, expected %s", + mockCS.Properties.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB, DefaultEtcdDiskSizeGT3Nodes) + } + + // Case where total node count is 11. + mockCS = getMockBaseContainerService("1.11.6") + mockCS.Location = location + mockCS.Properties.OrchestratorProfile.OrchestratorType = Kubernetes + mockCS.Properties.MasterProfile.Count = 5 + mockCS.Properties.AgentPoolProfiles[0].Count = 6 + mockCS.Properties.CustomCloudProfile = &CustomCloudProfile{ + PortalURL: "https://portal.testlocation.contoso.com", + } + + httpmock.DeactivateAndReset() + httpmock.Activate() + httpmock.RegisterResponder("GET", fmt.Sprintf("%smetadata/endpoints?api-version=1.0", fmt.Sprintf("https://management.%s.contoso.com/", location)), + func(req *http.Request) (*http.Response, error) { + resp := httpmock.NewStringResponse(200, `{"galleryEndpoint":"https://galleryartifacts.hosting.testlocation.contoso.com/galleryartifacts/","graphEndpoint":"https://graph.testlocation.contoso.com/","portalEndpoint":"https://portal.testlocation.contoso.com/","authentication":{"loginEndpoint":"https://adfs.testlocation.contoso.com/","audiences":["https://management.adfs.azurestack.testlocation/ce080287-be51-42e5-b99e-9de760fecae7"]}}`) + return resp, nil + }, + ) + + mockCS.SetPropertiesDefaults(false, false) + if mockCS.Properties.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB != DefaultAzureStackEtcdDiskSizeGT10Nodes { + t.Fatalf("EtcdDiskSizeGB did not have the expected size, got %s, expected %s", + mockCS.Properties.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB, DefaultAzureStackEtcdDiskSizeGT10Nodes) + } + + // Case where total node count is 21. + mockCS = getMockBaseContainerService("1.11.6") + mockCS.Location = location + mockCS.Properties.OrchestratorProfile.OrchestratorType = Kubernetes + mockCS.Properties.MasterProfile.Count = 5 + mockCS.Properties.AgentPoolProfiles[0].Count = 16 + mockCS.Properties.CustomCloudProfile = &CustomCloudProfile{ + PortalURL: "https://portal.testlocation.contoso.com", + } + + httpmock.DeactivateAndReset() + httpmock.Activate() + httpmock.RegisterResponder("GET", fmt.Sprintf("%smetadata/endpoints?api-version=1.0", fmt.Sprintf("https://management.%s.contoso.com/", location)), + func(req *http.Request) (*http.Response, error) { + resp := httpmock.NewStringResponse(200, `{"galleryEndpoint":"https://galleryartifacts.hosting.testlocation.contoso.com/galleryartifacts/","graphEndpoint":"https://graph.testlocation.contoso.com/","portalEndpoint":"https://portal.testlocation.contoso.com/","authentication":{"loginEndpoint":"https://adfs.testlocation.contoso.com/","audiences":["https://management.adfs.azurestack.testlocation/ce080287-be51-42e5-b99e-9de760fecae7"]}}`) + return resp, nil + }, + ) + + mockCS.SetPropertiesDefaults(false, false) + if mockCS.Properties.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB != DefaultAzureStackEtcdDiskSizeGT10Nodes { + t.Fatalf("EtcdDiskSizeGB did not have the expected size, got %s, expected %s", + mockCS.Properties.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB, DefaultAzureStackEtcdDiskSizeGT10Nodes) + } + + // Case where total node count is 55 but EtcdDiskSizeGB size is passed + mockCS = getMockBaseContainerService("1.11.6") + mockCS.Location = location + mockCS.Properties.OrchestratorProfile.OrchestratorType = Kubernetes + mockCS.Properties.MasterProfile.Count = 5 + mockCS.Properties.AgentPoolProfiles[0].Count = 50 + customEtcdDiskSize := "512" + mockCS.Properties.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB = customEtcdDiskSize + mockCS.Properties.CustomCloudProfile = &CustomCloudProfile{ + PortalURL: "https://portal.testlocation.contoso.com", + } + + httpmock.DeactivateAndReset() + httpmock.Activate() + httpmock.RegisterResponder("GET", fmt.Sprintf("%smetadata/endpoints?api-version=1.0", fmt.Sprintf("https://management.%s.contoso.com/", location)), + func(req *http.Request) (*http.Response, error) { + resp := httpmock.NewStringResponse(200, `{"galleryEndpoint":"https://galleryartifacts.hosting.testlocation.contoso.com/galleryartifacts/","graphEndpoint":"https://graph.testlocation.contoso.com/","portalEndpoint":"https://portal.testlocation.contoso.com/","authentication":{"loginEndpoint":"https://adfs.testlocation.contoso.com/","audiences":["https://management.adfs.azurestack.testlocation/ce080287-be51-42e5-b99e-9de760fecae7"]}}`) + return resp, nil + }, + ) + + mockCS.SetPropertiesDefaults(false, false) + if mockCS.Properties.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB != customEtcdDiskSize { + t.Fatalf("EtcdDiskSizeGB did not have the expected size, got %s, expected %s", + mockCS.Properties.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB, customEtcdDiskSize) + } +} func TestPreserveNodesProperties(t *testing.T) { mockCS := getMockBaseContainerService("1.10.8") mockCS.SetPropertiesDefaults(false, false) From 0f0797a19c8defd0ebbedabde4286360d591ff03 Mon Sep 17 00:00:00 2001 From: Rohit Jaini Date: Tue, 2 Jul 2019 15:22:21 -0700 Subject: [PATCH 2/2] Updating Disk default variable name to MaxAzureStackManagedDiskSize --- pkg/api/const.go | 4 ++-- pkg/api/defaults.go | 4 ++-- pkg/api/defaults_test.go | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/api/const.go b/pkg/api/const.go index f5a19077f3..185078e149 100644 --- a/pkg/api/const.go +++ b/pkg/api/const.go @@ -253,8 +253,8 @@ const ( // DefaultAzureStackFaultDomainCount set to 3 as Azure Stack today has minimum 4 node deployment. DefaultAzureStackFaultDomainCount = 3 - // DefaultAzureStackEtcdDiskSizeGT10Nodes = size for Kubernetes master etcd disk volumes in GB if > 10 nodes - DefaultAzureStackEtcdDiskSizeGT10Nodes = "1023" + // MaxAzureStackManagedDiskSize = size for Kubernetes master etcd disk volumes in GB if > 10 nodes as this is max what Azure Stack supports today. + MaxAzureStackManagedDiskSize = "1023" ) // WindowsProfile defaults diff --git a/pkg/api/defaults.go b/pkg/api/defaults.go index 4296d2a4ad..bd146ae264 100644 --- a/pkg/api/defaults.go +++ b/pkg/api/defaults.go @@ -235,14 +235,14 @@ func (cs *ContainerService) setOrchestratorDefaults(isUpgrade, isScale bool) { case a.TotalNodes() > 20: if a.IsAzureStackCloud() { // Currently on Azure Stack max size of managed disk size is 1023GB. - a.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB = DefaultAzureStackEtcdDiskSizeGT10Nodes + a.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB = MaxAzureStackManagedDiskSize } else { a.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB = DefaultEtcdDiskSizeGT20Nodes } case a.TotalNodes() > 10: if a.IsAzureStackCloud() { // Currently on Azure Stack max size of managed disk size is 1023GB. - a.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB = DefaultAzureStackEtcdDiskSizeGT10Nodes + a.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB = MaxAzureStackManagedDiskSize } else { a.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB = DefaultEtcdDiskSizeGT10Nodes } diff --git a/pkg/api/defaults_test.go b/pkg/api/defaults_test.go index 0df564fd39..3c69701a28 100644 --- a/pkg/api/defaults_test.go +++ b/pkg/api/defaults_test.go @@ -2545,9 +2545,9 @@ func TestEtcdDiskSizeOnAzureStack(t *testing.T) { ) mockCS.SetPropertiesDefaults(false, false) - if mockCS.Properties.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB != DefaultAzureStackEtcdDiskSizeGT10Nodes { + if mockCS.Properties.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB != MaxAzureStackManagedDiskSize { t.Fatalf("EtcdDiskSizeGB did not have the expected size, got %s, expected %s", - mockCS.Properties.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB, DefaultAzureStackEtcdDiskSizeGT10Nodes) + mockCS.Properties.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB, MaxAzureStackManagedDiskSize) } // Case where total node count is 21. @@ -2570,9 +2570,9 @@ func TestEtcdDiskSizeOnAzureStack(t *testing.T) { ) mockCS.SetPropertiesDefaults(false, false) - if mockCS.Properties.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB != DefaultAzureStackEtcdDiskSizeGT10Nodes { + if mockCS.Properties.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB != MaxAzureStackManagedDiskSize { t.Fatalf("EtcdDiskSizeGB did not have the expected size, got %s, expected %s", - mockCS.Properties.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB, DefaultAzureStackEtcdDiskSizeGT10Nodes) + mockCS.Properties.OrchestratorProfile.KubernetesConfig.EtcdDiskSizeGB, MaxAzureStackManagedDiskSize) } // Case where total node count is 55 but EtcdDiskSizeGB size is passed