From 10e12cc5429f06ca48a7cc169acb9090fac79221 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/images.yaml | 2 +- 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 | 7 +-- pkg/internal/infrastructure/terraform.go | 42 ++++++++++++++++ pkg/internal/infrastructure/terraform_test.go | 49 ++++++++++++++++++- pkg/internal/terraform.go | 7 +-- 11 files changed, 135 insertions(+), 26 deletions(-) diff --git a/charts/images.yaml b/charts/images.yaml index 6703cd01a..fd16c3c8a 100644 --- a/charts/images.yaml +++ b/charts/images.yaml @@ -2,7 +2,7 @@ images: - name: terraformer sourceRepository: github.com/gardener/terraformer repository: eu.gcr.io/gardener-project/gardener/terraformer - tag: "v1.4.0" + tag: "v1.5.0" - name: cloud-controller-manager sourceRepository: github.com/kubernetes/kubernetes 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..e96f06d26 100644 --- a/pkg/controller/infrastructure/actuator.go +++ b/pkg/controller/infrastructure/actuator.go @@ -45,12 +45,7 @@ func NewActuator() infrastructure.Actuator { } } -func (a *actuator) updateProviderStatus( - ctx context.Context, - tf terraformer.Terraformer, - infra *extensionsv1alpha1.Infrastructure, - config *api.InfrastructureConfig, -) error { +func (a *actuator) updateProviderStatus(ctx context.Context, tf terraformer.Terraformer, infra *extensionsv1alpha1.Infrastructure, config *api.InfrastructureConfig) error { status, err := infrainternal.ComputeStatus(tf, config) if err != nil { return err diff --git a/pkg/internal/infrastructure/terraform.go b/pkg/internal/infrastructure/terraform.go index 0e967a1e4..b4aa19bfe 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 := isNatGatewayIPMigrationRequired(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,6 +235,9 @@ 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. @@ -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,6 +359,11 @@ 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 } @@ -418,3 +438,25 @@ func findDomainCounts(cluster *controller.Cluster, infra *extensionsv1alpha1.Inf updateDomains: *updateDomainCount, }, nil } + +// isNatGatewayIPMigrationRequired 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 isNatGatewayIPMigrationRequired(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..261ab38e0 100644 --- a/pkg/internal/infrastructure/terraform_test.go +++ b/pkg/internal/infrastructure/terraform_test.go @@ -21,7 +21,6 @@ import ( apiv1alpha1 "github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure/v1alpha1" "github.com/gardener/gardener-extension-provider-azure/pkg/internal" "github.com/gardener/gardener/extensions/pkg/controller" - gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" . "github.com/onsi/ginkgo" @@ -178,7 +177,9 @@ var _ = Describe("Terraform", func() { "securityGroupName": TerraformerOutputKeySecurityGroupName, } - expectedNatGatewayValues = map[string]interface{}{} + expectedNatGatewayValues = map[string]interface{}{ + "migrateNatGatewayToIPAssociation": false, + } expectedValues = map[string]interface{}{ "azure": expectedAzureValues, @@ -330,6 +331,50 @@ var _ = Describe("Terraform", func() { Expect(err).To(Not(HaveOccurred())) Expect(values).To(BeEquivalentTo(expectedValues)) }) + + // TODO(natipmigration) This can be removed in future versions when the ip migration has been completed. + Context("NatGateway Gardener managed IP migration", func() { + BeforeEach(func() { + config.Networks.NatGateway = &api.NatGatewayConfig{ + Enabled: true, + } + expectedCreateValues["natGateway"] = true + }) + + It("should migrate the NatGateway IP as it is not yet migrated", func() { + infrastructureStatus := api.InfrastructureStatus{ + NatGatewayPublicIPMigrated: false, + } + infrastructureStatusMarsheled, err := json.Marshal(infrastructureStatus) + Expect(err).NotTo(HaveOccurred()) + + infra.Status.ProviderStatus = &runtime.RawExtension{ + Raw: infrastructureStatusMarsheled, + } + + expectedNatGatewayValues["migrateNatGatewayToIPAssociation"] = true + values, err := ComputeTerraformerChartValues(infra, clientAuth, config, cluster) + Expect(err).To(Not(HaveOccurred())) + Expect(values).To(BeEquivalentTo(expectedValues)) + }) + + It("should not migrate the NatGateway IP as it is already migrated", func() { + infrastructureStatus := api.InfrastructureStatus{ + NatGatewayPublicIPMigrated: true, + } + infrastructureStatusMarsheled, err := json.Marshal(infrastructureStatus) + Expect(err).NotTo(HaveOccurred()) + + infra.Status.ProviderStatus = &runtime.RawExtension{ + Raw: infrastructureStatusMarsheled, + } + + expectedNatGatewayValues["migrateNatGatewayToIPAssociation"] = false + values, err := ComputeTerraformerChartValues(infra, clientAuth, config, cluster) + Expect(err).To(Not(HaveOccurred())) + Expect(values).To(BeEquivalentTo(expectedValues)) + }) + }) }) }) 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