From 95e4410d61d7238cf405ca44aacb29a54593190e Mon Sep 17 00:00:00 2001 From: dkistner Date: Mon, 16 Nov 2020 09:04:18 +0100 Subject: [PATCH] Upgrade TF azurerm v2 and NatGateway pubIP migration --- charts/internal/azure-infra/templates/main.tf | 23 ++++++--- charts/internal/azure-infra/values.yaml | 9 ++-- hack/api-reference/api.md | 13 +++++ pkg/apis/azure/types_infrastructure.go | 3 ++ .../azure/v1alpha1/types_infrastructure.go | 4 ++ .../azure/v1alpha1/zz_generated.conversion.go | 2 + pkg/controller/infrastructure/actuator.go | 9 +--- pkg/internal/infrastructure/terraform.go | 48 +++++++++++++++++-- pkg/internal/infrastructure/terraform_test.go | 4 +- pkg/internal/terraform.go | 7 +-- 10 files changed, 94 insertions(+), 28 deletions(-) diff --git a/charts/internal/azure-infra/templates/main.tf b/charts/internal/azure-infra/templates/main.tf index feff588dc..119be7da8 100644 --- a/charts/internal/azure-infra/templates/main.tf +++ b/charts/internal/azure-infra/templates/main.tf @@ -3,6 +3,8 @@ provider "azurerm" { tenant_id = "{{ required "azure.tenantID is required" .Values.azure.tenantID }}" client_id = var.CLIENT_ID client_secret = var.CLIENT_SECRET + + features {} } {{ if .Values.create.resourceGroup -}} @@ -47,10 +49,8 @@ resource "azurerm_subnet" "workers" { virtual_network_name = data.azurerm_virtual_network.vnet.name resource_group_name = data.azurerm_virtual_network.vnet.resource_group_name {{- end }} - address_prefix = "{{ required "networks.worker is required" .Values.networks.worker }}" + address_prefixes = ["{{ required "networks.worker is required" .Values.networks.worker }}"] service_endpoints = [{{range $index, $serviceEndpoint := .Values.resourceGroup.subnet.serviceEndpoints}}{{if $index}},{{end}}"{{$serviceEndpoint}}"{{end}}] - route_table_id = azurerm_route_table.workers.id - network_security_group_id = azurerm_network_security_group.workers.id } resource "azurerm_route_table" "workers" { @@ -109,14 +109,23 @@ resource "azurerm_nat_gateway" "nat" { resource_group_name = data.azurerm_resource_group.rg.name {{- end }} sku_name = "Standard" - public_ip_address_ids = [azurerm_public_ip.natip.id] - {{- if .Values.natGateway }} - {{- if .Values.natGateway.idleConnectionTimeoutMinutes }} + {{ if .Values.natGateway -}} + {{ if .Values.natGateway.idleConnectionTimeoutMinutes -}} idle_timeout_in_minutes = {{ .Values.natGateway.idleConnectionTimeoutMinutes }} {{- end }} + + # TODO(natipmigration) This can be removed in future versions when the ip migration has been completed. + {{ if .Values.natGateway.migrateNatGatewayToIPAssociation -}} + public_ip_address_ids = [] + {{- end }} {{- end }} } +resource "azurerm_nat_gateway_public_ip_association" "natip-association" { + nat_gateway_id = azurerm_nat_gateway.nat.id + public_ip_address_id = azurerm_public_ip.natip.id +} + resource "azurerm_subnet_nat_gateway_association" "nat-worker-subnet-association" { subnet_id = azurerm_subnet.workers.id nat_gateway_id = azurerm_nat_gateway.nat.id @@ -216,4 +225,4 @@ output "{{ .Values.outputKeys.identityID }}" { output "{{ .Values.outputKeys.identityClientID }}" { value = data.azurerm_user_assigned_identity.identity.client_id } -{{- end }} \ No newline at end of file +{{- end }} diff --git a/charts/internal/azure-infra/values.yaml b/charts/internal/azure-infra/values.yaml index add75db10..aa23eebe9 100644 --- a/charts/internal/azure-infra/values.yaml +++ b/charts/internal/azure-infra/values.yaml @@ -15,6 +15,11 @@ create: # name: identity-name # resourceGroup: identity-resource-group +natGateway: + idleConnectionTimeoutMinutes: + # TODO(natipmigration) This can be removed in future versions when the ip migration has been completed. + migrateNatGatewayToIPAssociation: false + resourceGroup: name: my-resource-group vnet: @@ -42,7 +47,3 @@ outputKeys: securityGroupName: securityGroupName # identityID: managedIdentityID # identityClientID: managedIdentityClientID - -natGateway: - idleConnectionTimeoutMinutes: - diff --git a/hack/api-reference/api.md b/hack/api-reference/api.md index a3af773fc..7c4ce89c4 100644 --- a/hack/api-reference/api.md +++ b/hack/api-reference/api.md @@ -645,6 +645,19 @@ bool

Zoned indicates whether the cluster uses zones

+ + +natGatewayPublicIpMigrated
+ +bool + + + +(Optional) +

NatGatewayPublicIPMigrated is an indicator if the Gardener managed public ip address is already migrated. +TODO(natipmigration) This can be removed in future versions when the ip migration has been completed.

+ +

MachineImage diff --git a/pkg/apis/azure/types_infrastructure.go b/pkg/apis/azure/types_infrastructure.go index 2b409380a..4eecd2fc0 100644 --- a/pkg/apis/azure/types_infrastructure.go +++ b/pkg/apis/azure/types_infrastructure.go @@ -70,6 +70,9 @@ type InfrastructureStatus struct { Identity *IdentityStatus // Zoned indicates whether the cluster uses zones Zoned bool + // NatGatewayPublicIPMigrated is an indicator if the Gardener managed public ip address is already migrated. + // TODO(natipmigration) This can be removed in future versions when the ip migration has been completed. + NatGatewayPublicIPMigrated bool } // NetworkStatus is the current status of the infrastructure networks. diff --git a/pkg/apis/azure/v1alpha1/types_infrastructure.go b/pkg/apis/azure/v1alpha1/types_infrastructure.go index 2bc923485..069dce087 100644 --- a/pkg/apis/azure/v1alpha1/types_infrastructure.go +++ b/pkg/apis/azure/v1alpha1/types_infrastructure.go @@ -78,6 +78,10 @@ type InfrastructureStatus struct { // Zoned indicates whether the cluster uses zones // +optional Zoned bool `json:"zoned,omitempty"` + // NatGatewayPublicIPMigrated is an indicator if the Gardener managed public ip address is already migrated. + // TODO(natipmigration) This can be removed in future versions when the ip migration has been completed. + // +optional + NatGatewayPublicIPMigrated bool `json:"natGatewayPublicIpMigrated,omitempty"` } // NetworkStatus is the current status of the infrastructure networks. diff --git a/pkg/apis/azure/v1alpha1/zz_generated.conversion.go b/pkg/apis/azure/v1alpha1/zz_generated.conversion.go index 28f14f493..af5de4950 100644 --- a/pkg/apis/azure/v1alpha1/zz_generated.conversion.go +++ b/pkg/apis/azure/v1alpha1/zz_generated.conversion.go @@ -474,6 +474,7 @@ func autoConvert_v1alpha1_InfrastructureStatus_To_azure_InfrastructureStatus(in out.SecurityGroups = *(*[]azure.SecurityGroup)(unsafe.Pointer(&in.SecurityGroups)) out.Identity = (*azure.IdentityStatus)(unsafe.Pointer(in.Identity)) out.Zoned = in.Zoned + out.NatGatewayPublicIPMigrated = in.NatGatewayPublicIPMigrated return nil } @@ -494,6 +495,7 @@ func autoConvert_azure_InfrastructureStatus_To_v1alpha1_InfrastructureStatus(in out.SecurityGroups = *(*[]SecurityGroup)(unsafe.Pointer(&in.SecurityGroups)) out.Identity = (*IdentityStatus)(unsafe.Pointer(in.Identity)) out.Zoned = in.Zoned + out.NatGatewayPublicIPMigrated = in.NatGatewayPublicIPMigrated return nil } diff --git a/pkg/controller/infrastructure/actuator.go b/pkg/controller/infrastructure/actuator.go index 7ec7685c3..2337cba70 100644 --- a/pkg/controller/infrastructure/actuator.go +++ b/pkg/controller/infrastructure/actuator.go @@ -45,13 +45,8 @@ func NewActuator() infrastructure.Actuator { } } -func (a *actuator) updateProviderStatus( - ctx context.Context, - tf terraformer.Terraformer, - infra *extensionsv1alpha1.Infrastructure, - config *api.InfrastructureConfig, -) error { - status, err := infrainternal.ComputeStatus(tf, config) +func (a *actuator) updateProviderStatus(ctx context.Context, tf terraformer.Terraformer, infra *extensionsv1alpha1.Infrastructure, config *api.InfrastructureConfig) error { + status, err := infrainternal.ComputeStatus(tf, infra, config) if err != nil { return err } diff --git a/pkg/internal/infrastructure/terraform.go b/pkg/internal/infrastructure/terraform.go index 0e967a1e4..d62da6151 100644 --- a/pkg/internal/infrastructure/terraform.go +++ b/pkg/internal/infrastructure/terraform.go @@ -140,6 +140,14 @@ func ComputeTerraformerChartValues(infra *extensionsv1alpha1.Infrastructure, cli } } + // Checks if the Gardener managed NatGateway public ip needs to be migrated. + // TODO(natipmigration) This can be removed in future versions when the ip migration has been completed. + natGatewayIPMigrationRequired, err := requireNatGatewayIpMigration(infra, config) + if err != nil { + return nil, err + } + natGatewayConfig["migrateNatGatewayToIPAssociation"] = natGatewayIPMigrationRequired + if config.Identity != nil && config.Identity.Name != "" && config.Identity.ResourceGroup != "" { identityConfig = map[string]interface{}{ "name": config.Identity.Name, @@ -227,10 +235,13 @@ type TerraformState struct { IdentityID string // IdentityClientID is the client id of the identity. IdentityClientID string + // NatGatewayIPMigrated is the indicator if the nat gateway ip is migrated. + // TODO(natipmigration) This can be removed in future versions when the ip migration has been completed. + NatGatewayIPMigrated string } // ExtractTerraformState extracts the TerraformState from the given Terraformer. -func ExtractTerraformState(tf terraformer.Terraformer, config *api.InfrastructureConfig) (*TerraformState, error) { +func ExtractTerraformState(tf terraformer.Terraformer, infra *extensionsv1alpha1.Infrastructure, config *api.InfrastructureConfig) (*TerraformState, error) { var outputKeys = []string{ TerraformerOutputKeyResourceGroupName, TerraformerOutputKeyRouteTableName, @@ -289,6 +300,10 @@ func ExtractTerraformState(tf terraformer.Terraformer, config *api.Infrastructur tfState.IdentityClientID = vars[TerraformerOutputKeyIdentityClientID] } + if config.Networks.NatGateway != nil && config.Networks.NatGateway.Enabled { + tfState.NatGatewayIPMigrated = "true" + } + return &tfState, nil } @@ -344,12 +359,17 @@ func StatusFromTerraformState(state *TerraformState) *apiv1alpha1.Infrastructure }) } + // TODO(natipmigration) This can be removed in future versions when the ip migration has been completed. + if state.NatGatewayIPMigrated == "true" { + tfState.NatGatewayPublicIPMigrated = true + } + return &tfState } // ComputeStatus computes the status based on the Terraformer and the given InfrastructureConfig. -func ComputeStatus(tf terraformer.Terraformer, config *api.InfrastructureConfig) (*apiv1alpha1.InfrastructureStatus, error) { - state, err := ExtractTerraformState(tf, config) +func ComputeStatus(tf terraformer.Terraformer, infra *extensionsv1alpha1.Infrastructure, config *api.InfrastructureConfig) (*apiv1alpha1.InfrastructureStatus, error) { + state, err := ExtractTerraformState(tf, infra, config) if err != nil { return nil, err } @@ -418,3 +438,25 @@ func findDomainCounts(cluster *controller.Cluster, infra *extensionsv1alpha1.Inf updateDomains: *updateDomainCount, }, nil } + +// requireNatGatewayIpMigration checks if the Gardener managed NatGateway public ip needs to be migrated. +// TODO(natipmigration) This can be removed in future versions when the ip migration has been completed. +func requireNatGatewayIpMigration(infra *extensionsv1alpha1.Infrastructure, config *api.InfrastructureConfig) (bool, error) { + if config.Networks.NatGateway == nil || !config.Networks.NatGateway.Enabled { + return false, nil + } + + if infra.Status.ProviderStatus == nil { + return false, nil + } + + infrastructureStatus, err := helper.InfrastructureStatusFromInfrastructure(infra) + if err != nil { + return false, err + } + + if infrastructureStatus.NatGatewayPublicIPMigrated { + return false, nil + } + return true, nil +} diff --git a/pkg/internal/infrastructure/terraform_test.go b/pkg/internal/infrastructure/terraform_test.go index bef516481..6e66e6c4d 100644 --- a/pkg/internal/infrastructure/terraform_test.go +++ b/pkg/internal/infrastructure/terraform_test.go @@ -178,7 +178,9 @@ var _ = Describe("Terraform", func() { "securityGroupName": TerraformerOutputKeySecurityGroupName, } - expectedNatGatewayValues = map[string]interface{}{} + expectedNatGatewayValues = map[string]interface{}{ + "migrateNatGatewayToIPAssociation": false, + } expectedValues = map[string]interface{}{ "azure": expectedAzureValues, diff --git a/pkg/internal/terraform.go b/pkg/internal/terraform.go index eed5435ee..a7caf7123 100644 --- a/pkg/internal/terraform.go +++ b/pkg/internal/terraform.go @@ -41,12 +41,7 @@ func TerraformVariablesEnvironmentFromClientAuth(auth *ClientAuth) map[string]st } // NewTerraformer initializes a new Terraformer. -func NewTerraformer( - restConfig *rest.Config, - purpose, - namespace, - name string, -) (terraformer.Terraformer, error) { +func NewTerraformer(restConfig *rest.Config, purpose, namespace, name string) (terraformer.Terraformer, error) { tf, err := terraformer.NewForConfig(logger.NewLogger("info"), restConfig, purpose, namespace, name, imagevector.TerraformerImage()) if err != nil { return nil, err