fix: ASM module rewrite improvements (#1165)

* add guide for migrating from previous module

* add options for fleet registration and feature enablement

* fix test with membership name

Author: Sam Naser

docs/upgrading_to_v20.0.md

@@ -6,5 +6,24 @@ release for the Anthos Service Mesh (ASM) module.
 ### ASM module rewrite
 
 The [ASM submodule](https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/tree/master/modules/asm) has been rewritten to use the `ControlPlaneRevision` API to provision
-a managed control plane rather than using an installer script. Due to the drastic difference in implementation the module does not support an upgrade path
-from the previous version.
+a managed control plane rather than using an installer script. Due to implementation differences, there are migration steps required to safely move from
+an installation performed with the old module to using the new module. **NOTE:** these migration steps are best-effort and have not been tested against all possible ASM configurations.
+
+1. Run `terraform state rm module.asm`
+2. Update the module version to v20.0
+3. Import the system namespace into the new module with `terraform import module.asm.kubernetes_namespace.system istio-system`
+4. Run `terraform apply`
+
+There should be two ASM revisions present at this point (in-cluster or managed, depending on whether the previous installation was managed). Now,
+we must perform a canary upgrade to move workloads onto the new ASM revision. To do this:
+
+1. Relabel namespaces to use the revision label from the managed revision (`asm-managed`, `asm-managed-stable`, or `asm-managed-rapid`)
+2. Rollout workloads in those namespaces to get them onto the new ASM version
+3. [Optional] Remove the previous revision with `istioctl x uninstall --revision ...` (if the previous installation was in-cluster)
+
+
+#### Migrating options
+
+Another difference from the previous module is that the new ASM module does not provide variables for option configuration (e.g. `custom_overlay`, `options`). For the new version these should be managed separately
+outside the module. This is because those options were tightly coupled to pulling down an installer which the new module does not do. To use options specified in the previous module with the new module find the corresponding configuration [here](https://github.com/GoogleCloudPlatform/anthos-service-mesh-packages/tree/main/asm/istio/options) and move the
+config to the mesh configuration for the managed revision.

examples/simple_zonal_with_asm/main.tf

@@ -57,10 +57,12 @@ module "gke" {
 }
 
 module "asm" {
-  source            = "../../modules/asm"
-  project_id        = var.project_id
-  cluster_name      = module.gke.name
-  cluster_location  = module.gke.location
-  multicluster_mode = "connected"
-  enable_cni        = true
+  source                    = "../../modules/asm"
+  project_id                = var.project_id
+  cluster_name              = module.gke.name
+  cluster_location          = module.gke.location
+  multicluster_mode         = "connected"
+  enable_cni                = true
+  enable_fleet_registration = true
+  enable_mesh_feature       = true
 }

modules/asm/README.md

@@ -35,6 +35,8 @@ To deploy this config:
 | cluster\_location | The cluster location for this ASM installation. | `string` | n/a | yes |
 | cluster\_name | The unique name to identify the cluster in ASM. | `string` | n/a | yes |
 | enable\_cni | Determines whether to enable CNI for this ASM installation. Required to use Managed Data Plane (MDP). | `bool` | `false` | no |
+| enable\_fleet\_registration | Determines whether the module enables the mesh feature on the fleet. | `bool` | `false` | no |
+| enable\_mesh\_feature | Determines whether the module registers the cluster to the fleet. | `bool` | `false` | no |
 | enable\_vpc\_sc | Determines whether to enable VPC-SC for this ASM installation. For more information read https://cloud.google.com/service-mesh/docs/managed/vpc-sc | `bool` | `false` | no |
 | fleet\_id | The fleet to use for this ASM installation. | `string` | `""` | no |
 | multicluster\_mode | [Preview] Determines whether remote secrets should be autogenerated across fleet cluster. | `string` | `"manual"` | no |

examples/simple_zonal_with_asm/hub.tf renamed to modules/asm/hub.tf

@@ -1,5 +1,5 @@
 /**
- * Copyright 2018 Google LLC
+ * Copyright 2022 Google LLC
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -14,18 +14,20 @@
  * limitations under the License.
  */
 
-resource "google_gke_hub_membership" "cluster_membership" {
+resource "google_gke_hub_membership" "membership" {
+  count         = var.enable_fleet_registration ? 1 : 0
   provider      = google-beta
   project       = var.project_id
-  membership_id = "gke-asm-membership"
+  membership_id = "${data.google_container_cluster.asm.name}-membership"
   endpoint {
     gke_cluster {
-      resource_link = "//container.googleapis.com/${module.gke.cluster_id}"
+      resource_link = "//container.googleapis.com/${data.google_container_cluster.asm.id}"
     }
   }
 }
 
 resource "google_gke_hub_feature" "mesh" {
+  count    = var.enable_mesh_feature ? 1 : 0
   name     = "servicemesh"
   project  = var.project_id
   location = "global"

modules/asm/main.tf

@@ -46,6 +46,8 @@ resource "kubernetes_config_map" "asm_options" {
   data = {
     multicluster_mode = var.multicluster_mode
   }
+
+  depends_on = [google_gke_hub_membership.membership, google_gke_hub_feature.mesh]
 }
 
 module "cpr" {

modules/asm/variables.tf

@@ -74,3 +74,15 @@ variable "enable_vpc_sc" {
   type        = bool
   default     = false
 }
+
+variable "enable_fleet_registration" {
+  description = "Determines whether the module enables the mesh feature on the fleet."
+  type        = bool
+  default     = false
+}
+
+variable "enable_mesh_feature" {
+  description = "Determines whether the module registers the cluster to the fleet."
+  type        = bool
+  default     = false
+}

test/integration/simple_zonal_with_asm/controls/gcloud.rb

@@ -40,7 +40,7 @@
     end
   end
 
-  describe command("gcloud container hub memberships describe gke-asm-membership --project=#{project_id} --format=json") do
+  describe command("gcloud container hub memberships describe #{cluster_name}-membership --project=#{project_id} --format=json") do
     its(:exit_status) { should eq 0 }
     its(:stderr) { should eq '' }