Multi az for azure #179

tei-k · 2020-08-11T08:43:43Z

Description

https://scalar-labs.atlassian.net/browse/DLT-6944

Done

Support multi az (add locations )
Add natgateway only for internal standard lb.
Delete unnecessary lifecycle in azure subnet.

Remark

Update ref after merge following pr.
scalar-labs/terraform-azurerm-compute#7

tei-k · 2020-08-17T08:56:25Z

modules/azure/scalardl/envoy.tf

+  domain_name_label = "envoy-${local.network_name}"
+  location          = local.location
+  sku               = length(local.locations) > 0 ? "Standard" : "Basic"
+  # zones               = length(local.locations) > 0 ? [local.locations[count.index]] : null


📝 Standard SKU Public IP Addresses that do not specify a zone are zone redundant by default.
https://www.terraform.io/docs/providers/azurerm/r/public_ip.html#zones

tei-k · 2020-08-17T09:02:20Z

modules/azure/scalardl/envoy.tf

+resource "azurerm_public_ip" "envoy_nat_ip" {
+  count = local.envoy.enable_nlb && local.envoy.nlb_internal && length(local.locations) > 0 ? 1 : 0
+
+  name                = "envoy-natip"
  location            = local.location
  resource_group_name = local.network_name
  allocation_method   = "Static"
+  sku                 = "Standard"
+}
+
+resource "azurerm_nat_gateway" "envoy_natgw" {
+  count = local.envoy.enable_nlb && local.envoy.nlb_internal && length(local.locations) > 0 ? 1 : 0
+
+  name                    = "envoy-natgw"
+  location                = local.location
+  resource_group_name     = local.network_name
+  public_ip_address_ids   = [azurerm_public_ip.envoy_nat_ip[count.index].id]
+  sku_name                = "Standard"
+  idle_timeout_in_minutes = 10
+}
+
+resource "azurerm_subnet_nat_gateway_association" "envoy_natgw_assoc" {
+  count = local.envoy.enable_nlb && local.envoy.nlb_internal && length(local.locations) > 0 ? 1 : 0
+
+  subnet_id      = local.envoy.subnet_id
+  nat_gateway_id = azurerm_nat_gateway.envoy_natgw[count.index].id


📝 Need outbound NAT when using internal standard lb. (NAT associated with private subnet that include monitoring server)
https://docs.microsoft.com/en-us/azure/load-balancer/load-balancer-outbound-connections

… multi-az-for-azure

penpyt · 2020-08-19T03:46:00Z

Kubernetes modules work well with a bit of modification => reimport master to this branch for fix region

I have 2 questions:

only one bastion has a public IP, so if the AZ who holds the bastion going down, we need to reattach the public IP to the available bastion, maybe better to have both have a public IP (each bastion should have a public IP)?
should we chain the locations to kubernetes_cluster_availability_zones like Cassandra or should we do it another PR? see below

locals.tf in Kubernetes examples

locals {
  network = {
    name     = data.terraform_remote_state.network.outputs.network_name
    dns      = data.terraform_remote_state.network.outputs.dns_zone_id
    id       = data.terraform_remote_state.network.outputs.network_id
    region   = data.terraform_remote_state.network.outputs.region
    locations = join(",", data.terraform_remote_state.network.outputs.locations)

main.tf in Kubernetes examples

module "kubernetes" {
  source = "../../../modules/azure/kubernetes"

  # Required variables (use network remote state)
  network = local.network

  # Required kubernetes variable for AZs
  kubernetes_cluster_availability_zones = local.network.locations

… multi-az-for-azure

tei-k · 2020-08-19T05:14:20Z

each bastion should have a public IP

Good point! 👍 Agree ! fixed in e8562ce.

should we chain the locations to kubernetes_cluster_availability_zones like Cassandra or should we do it another PR?

It's a minor correction, so I fixed it directly.
(As with the other modules, we can use the network map variable, don't need specify kubernetes_cluster_availability_zones in main.tf )

tei-k · 2020-08-19T05:42:59Z

modules/azure/network/main.tf

-
-  lifecycle {
-    ignore_changes = [network_security_group_id]
-  }


📝 Removed lifecycle because it didn't seem to be necessary.

tei-k · 2020-08-19T05:49:13Z

(As with the other modules, we can use the network map variable, don't need specify kubernetes_cluster_availability_zones in main.tf )

Refactored the k8s part to use local.locations like the other modules in 5125f06.

penpyt · 2020-08-19T06:01:15Z

(As with the other modules, we can use the network map variable, don't need specify kubernetes_cluster_availability_zones in main.tf )

Refactored the k8s part to use local.locations like the other modules in 5125f06.

Great thank you

penpyt · 2020-08-19T06:52:09Z

modules/azure/network/bastion/main.tf

@@ -9,7 +9,7 @@ module "bastion_cluster" {
  vm_hostname                   = "bastion"
  vm_os_simple                  = var.image_id
  vnet_subnet_id                = var.subnet_id
-  nb_public_ip                  = "1"
+  nb_public_ip                  = var.resource_count


@tei-k I try to start 3 bastions and got the following error, we should add a random to the dns for public IP

Error: Error Creating/Updating Public IP "bastion-2-publicIP" (Resource Group "paul-k8s-azure-s9rt0ek"): network.PublicIPAddressesClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="DnsRecordInUse" Message="DNS record bastion-paul-k8s-azure-s9rt0ek.westus2.cloudapp.azure.com is already used by another public IP." Details=[] on .terraform/modules/network.bastion.bastion_cluster/main.tf line 251, in resource "azurerm_public_ip" "vm": 251: resource "azurerm_public_ip" "vm" { Error: Error Creating/Updating Public IP "bastion-3-publicIP" (Resource Group "paul-k8s-azure-s9rt0ek"): network.PublicIPAddressesClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="DnsRecordInUse" Message="DNS record bastion-paul-k8s-azure-s9rt0ek.westus2.cloudapp.azure.com is already used by another public IP." Details=[] on .terraform/modules/network.bastion.bastion_cluster/main.tf line 251, in resource "azurerm_public_ip" "vm": 251: resource "azurerm_public_ip" "vm" {

@penpyt

Excellent point 👍 👍 Fixed in d02cc64.

e.g. Create different public dns for each bastion.

bastion-1-tei-multi-kclkyfk.japaneast.cloudapp.azure.com bastion-2-tei-multi-kclkyfk.japaneast.cloudapp.azure.com bastion-3-tei-multi-kclkyfk.japaneast.cloudapp.azure.com

Great, Look good to me !

@tei-k one last thing, should we modify the outputs.tf (network modules) as well to include the x number of bastion servers to give the user the public dns in output terraform command.

I say that because Kubernetes must use only one bastion but the tools (helm, kubectl) need to be available on everyone on them.

I propose to keep bastion_ip and create bastions_ip as follow, let's me know what do you think?

output "bastion_ip" { value = module.network.bastion_ip } output "bastions_ip" { [OMIT] }

@penpyt

I think it's OK to add it to the output, but also need to fix aws as well, So let me do it in other pr. (may not need it now.)

feeblefakie

LGTM! Great work!
Since it is an important PR, I would like to wait for 2 more approvals from Paul and Yusuke.

penpyt

LGTM!

ymorimo

Looking good! Left one minor comment.

modules/azure/scalardl/envoy.tf

ymorimo

LGTM!

penpyt

LGTM

tei-k added 7 commits August 11, 2020 12:09

Support multi-az for azure

0bd4e8e

Fix to optional for locations

2880843

Fix cassandra module

f415ebd

Revert to use version1 for azure provider

f0f43dc

Fix scalardl module

ba092d2

Fix monitor module

0f542c0

Fix public ip for envoy

ea0faae

tei-k self-assigned this Aug 11, 2020

tei-k added 3 commits August 12, 2020 12:55

Fix default to use availability set

0c03a30

Fix ci failure

0d7781a

Fix fmt

fcde56d

tei-k force-pushed the multi-az-for-azure branch from 5e4685d to fcde56d Compare August 12, 2020 06:54

tei-k added 2 commits August 12, 2020 16:56

Fix readme

f258583

Add natgateway for envoy

8b6d146

tei-k commented Aug 17, 2020

View reviewed changes

tei-k added 3 commits August 18, 2020 10:47

Merge branch 'master' of github.com:scalar-labs/scalar-terraform into…

a0e9921

… multi-az-for-azure

Fix conflict

a9f6f76

Fix fmt

5d97c84

tei-k force-pushed the multi-az-for-azure branch from 6fa9766 to 5d97c84 Compare August 18, 2020 02:04

tei-k changed the title ~~[WIP] Multi az for azure~~ Multi az for azure Aug 18, 2020

tei-k requested review from ymorimo and feeblefakie August 18, 2020 02:44

Fix bug in avset

df700b0

feeblefakie requested a review from penpyt August 18, 2020 08:48

tei-k added 3 commits August 19, 2020 13:49

Fix to create public ip per bastion

e8562ce

Fix k8s

aa1d87a

Merge branch 'master' of github.com:scalar-labs/scalar-terraform into…

5b9bd70

… multi-az-for-azure

tei-k added 2 commits August 19, 2020 14:26

Refactoring k8s to using locations

5125f06

Fix locations in test

e9977d6

tei-k commented Aug 19, 2020

View reviewed changes

penpyt reviewed Aug 19, 2020

View reviewed changes

Fix to use different public dns for bastion

d02cc64

feeblefakie previously approved these changes Aug 20, 2020

View reviewed changes

penpyt previously approved these changes Aug 20, 2020

View reviewed changes

Update ref

d202477

tei-k dismissed stale reviews from penpyt and feeblefakie via d202477 August 20, 2020 03:12

Fix index error

7d8df8c

ymorimo reviewed Aug 21, 2020

View reviewed changes

modules/azure/scalardl/envoy.tf Outdated Show resolved Hide resolved

Delete comment out

74722ec

ymorimo approved these changes Aug 21, 2020

View reviewed changes

penpyt approved these changes Aug 21, 2020

View reviewed changes

feeblefakie merged commit 7f9456b into master Aug 24, 2020

feeblefakie deleted the multi-az-for-azure branch August 24, 2020 00:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multi az for azure #179

Multi az for azure #179

tei-k commented Aug 11, 2020 •

edited

Loading

tei-k Aug 17, 2020

tei-k Aug 17, 2020

penpyt commented Aug 19, 2020

tei-k commented Aug 19, 2020

tei-k Aug 19, 2020

tei-k commented Aug 19, 2020

penpyt commented Aug 19, 2020

penpyt Aug 19, 2020

tei-k Aug 19, 2020 •

edited

Loading

penpyt Aug 19, 2020

penpyt Aug 19, 2020

tei-k Aug 20, 2020

feeblefakie left a comment

penpyt left a comment

ymorimo left a comment

ymorimo left a comment

penpyt left a comment

Multi az for azure #179

Multi az for azure #179

Conversation

tei-k commented Aug 11, 2020 • edited Loading

Description

Done

Remark

tei-k Aug 17, 2020

Choose a reason for hiding this comment

tei-k Aug 17, 2020

Choose a reason for hiding this comment

penpyt commented Aug 19, 2020

tei-k commented Aug 19, 2020

tei-k Aug 19, 2020

Choose a reason for hiding this comment

tei-k commented Aug 19, 2020

penpyt commented Aug 19, 2020

penpyt Aug 19, 2020

Choose a reason for hiding this comment

tei-k Aug 19, 2020 • edited Loading

Choose a reason for hiding this comment

penpyt Aug 19, 2020

Choose a reason for hiding this comment

penpyt Aug 19, 2020

Choose a reason for hiding this comment

tei-k Aug 20, 2020

Choose a reason for hiding this comment

feeblefakie left a comment

Choose a reason for hiding this comment

penpyt left a comment

Choose a reason for hiding this comment

ymorimo left a comment

Choose a reason for hiding this comment

ymorimo left a comment

Choose a reason for hiding this comment

penpyt left a comment

Choose a reason for hiding this comment

tei-k commented Aug 11, 2020 •

edited

Loading

tei-k Aug 19, 2020 •

edited

Loading