From b1679ad21aebd6de752526c19d8ba1ff2579bc98 Mon Sep 17 00:00:00 2001 From: Sam Bentley Date: Sat, 12 Aug 2023 01:25:17 +1000 Subject: [PATCH] Fix: Instance level stateful disk config (#1578) * update doco * fix bug in TF code * change instance name in README to fix test * revert disk name * Update stateful.yaml * fix examples and tests --------- Co-authored-by: Julio Castillo Co-authored-by: Ludovico Magnocavallo --- modules/compute-mig/README.md | 121 +++++++----------- modules/compute-mig/stateful-config.tf | 4 +- modules/compute-vm/README.md | 4 +- modules/compute-vm/variables.tf | 6 +- .../compute_mig/examples/stateful.yaml | 34 ++++- 5 files changed, 84 insertions(+), 85 deletions(-) diff --git a/modules/compute-mig/README.md b/modules/compute-mig/README.md index ef87d085..b281c2e3 100644 --- a/modules/compute-mig/README.md +++ b/modules/compute-mig/README.md @@ -6,8 +6,23 @@ This module can be coupled with the [`compute-vm`](../compute-vm) module which c Stateful disks can be created directly, as shown in the last example below. + +- [Examples](#examples) + - [Simple Example](#simple-example) + - [Multiple Versions](#multiple-versions) + - [Health Check and Autohealing Policies](#health-check-and-autohealing-policies) + - [Autoscaling](#autoscaling) + - [Update Policy](#update-policy) + - [Stateful MIGs - MIG Config](#stateful-migs-mig-config) + - [Stateful MIGs - Instance Config](#stateful-migs-instance-config) +- [Variables](#variables) +- [Outputs](#outputs) + + ## Examples +### Simple Example + This example shows how to manage a simple MIG that leverages the `compute-vm` module to manage the underlying instance template. The following sub-examples will only show how to enable specific features of this module, and won't replicate the combined setup. ```hcl @@ -49,7 +64,7 @@ module "nginx-mig" { # tftest modules=2 resources=2 inventory=simple.yaml ``` -### Multiple versions +### Multiple Versions If multiple versions are desired, use more `compute-vm` instances for the additional templates used in each version (not shown here), and reference them like this: @@ -100,7 +115,7 @@ module "nginx-mig" { # tftest modules=2 resources=2 ``` -### Health check and autohealing policies +### Health Check and Autohealing Policies Autohealing policies can use an externally defined health check, or have this module auto-create one: @@ -205,7 +220,7 @@ module "nginx-mig" { # tftest modules=2 resources=3 inventory=autoscaling.yaml ``` -### Update policy +### Update Policy ```hcl module "cos-nginx" { @@ -262,7 +277,7 @@ You can configure a disk defined in the instance template to be stateful for al An example using only the configuration at the MIG level can be seen below. -Note that when referencing the stateful disk, you use `device_name` and not `disk_name`. +Note that when referencing the stateful disk, you use `device_name` and not `disk_name`. Specifying an existing disk in the template (and stateful config) only allows a single instance to be managed by the MIG, typically coupled with an autohealing policy (shown in the examples above). ```hcl module "cos-nginx" { @@ -270,16 +285,15 @@ module "cos-nginx" { } module "nginx-template" { - source = "./fabric/modules/compute-vm" - project_id = var.project_id - name = "nginx-template" - zone = "europe-west1-b" - tags = ["http-server", "ssh"] + source = "./fabric/modules/compute-vm" + project_id = "my-prj" + name = "nginx-template" + zone = "europe-west8-b" + tags = ["http-server", "ssh"] + instance_type = "e2-small" network_interfaces = [{ network = var.vpc.self_link subnetwork = var.subnet.self_link - nat = false - addresses = null }] boot_disk = { initialize_params = { @@ -287,15 +301,10 @@ module "nginx-template" { } } attached_disks = [{ - name = "repd-1" - size = null source_type = "attach" - source = "regions/${var.region}/disks/repd-test-1" - options = { - mode = "READ_ONLY" - replica_zone = "${var.region}-c" - type = "PERSISTENT" - } + name = "data-1" + size = 10 + source = "test-data-1" }] create_template = true metadata = { @@ -305,34 +314,21 @@ module "nginx-template" { module "nginx-mig" { source = "./fabric/modules/compute-mig" - project_id = "my-project" - location = "europe-west1-b" - name = "mig-test" - target_size = 3 + project_id = "my-prj" + location = "europe-west8-b" + name = "mig-test-2" + target_size = 1 instance_template = module.nginx-template.template.self_link - autoscaler_config = { - max_replicas = 3 - min_replicas = 1 - cooldown_period = 30 - scaling_signals = { - cpu_utilization = { - target = 0.65 - } - } - } stateful_disks = { - repd-1 = false + data-1 = false } } -# tftest modules=2 resources=3 - +# tftest modules=2 resources=2 ``` ### Stateful MIGs - Instance Config -Here is an example defining the stateful config at the instance level. - -Note that you will need to know the instance name in order to use this configuration. +Here is an example defining the stateful config at the instance level. As in the example above, specifying an existing disk in the template (and stateful config) only allows a single instance to be managed by the MIG, typically coupled with an autohealing policy (shown in the examples above). ```hcl module "cos-nginx" { @@ -340,16 +336,15 @@ module "cos-nginx" { } module "nginx-template" { - source = "./fabric/modules/compute-vm" - project_id = var.project_id - name = "nginx-template" - zone = "europe-west1-b" - tags = ["http-server", "ssh"] + source = "./fabric/modules/compute-vm" + project_id = "my-prj" + name = "nginx-template" + zone = "europe-west8-b" + tags = ["http-server", "ssh"] + instance_type = "e2-small" network_interfaces = [{ network = var.vpc.self_link subnetwork = var.subnet.self_link - nat = false - addresses = null }] boot_disk = { initialize_params = { @@ -357,15 +352,10 @@ module "nginx-template" { } } attached_disks = [{ - name = "repd-1" - size = null source_type = "attach" - source = "regions/${var.region}/disks/repd-test-1" - options = { - mode = "READ_ONLY" - replica_zone = "${var.region}-c" - type = "PERSISTENT" - } + name = "data-1" + size = 10 + source = "test-data-1" }] create_template = true metadata = { @@ -375,30 +365,18 @@ module "nginx-template" { module "nginx-mig" { source = "./fabric/modules/compute-mig" - project_id = "my-project" - location = "europe-west1-b" + project_id = "my-prj" + location = "europe-west8-b" name = "mig-test" - target_size = 3 instance_template = module.nginx-template.template.self_link - autoscaler_config = { - max_replicas = 3 - min_replicas = 1 - cooldown_period = 30 - scaling_signals = { - cpu_utilization = { - target = 0.65 - } - } - } stateful_config = { - # name needs to match a MIG instance name instance-1 = { minimal_action = "NONE", most_disruptive_allowed_action = "REPLACE" preserved_state = { disks = { - persistent-disk-1 = { - source = "test-disk", + data-1 = { + source = "projects/my-prj/zones/europe-west8-b/disks/test-data-1" } } metadata = { @@ -408,8 +386,7 @@ module "nginx-mig" { } } } -# tftest modules=2 resources=4 inventory=stateful.yaml - +# tftest modules=2 resources=3 inventory=stateful.yaml ``` diff --git a/modules/compute-mig/stateful-config.tf b/modules/compute-mig/stateful-config.tf index 1e9e056e..dc0329d1 100644 --- a/modules/compute-mig/stateful-config.tf +++ b/modules/compute-mig/stateful-config.tf @@ -22,7 +22,7 @@ resource "google_compute_per_instance_config" "default" { zone = var.location name = each.key instance_group_manager = try( - google_compute_instance_group_manager.default.0.id, null + google_compute_instance_group_manager.default.0.name, null ) minimal_action = each.value.minimal_action most_disruptive_allowed_action = each.value.most_disruptive_action @@ -59,7 +59,7 @@ resource "google_compute_region_per_instance_config" "default" { region = var.location name = each.key region_instance_group_manager = try( - google_compute_region_instance_group_manager.default.0.id, null + google_compute_region_instance_group_manager.default.0.name, null ) minimal_action = each.value.minimal_action most_disruptive_allowed_action = each.value.most_disruptive_action diff --git a/modules/compute-vm/README.md b/modules/compute-vm/README.md index e47d9530..92a5e3c3 100644 --- a/modules/compute-vm/README.md +++ b/modules/compute-vm/README.md @@ -626,7 +626,6 @@ module "instance" { # tftest modules=1 resources=5 inventory=snapshot-schedule-create.yaml ``` - ## Variables | name | description | type | required | default | @@ -636,7 +635,7 @@ module "instance" { | [project_id](variables.tf#L277) | Project id. | string | ✓ | | | [zone](variables.tf#L379) | Compute zone. | string | ✓ | | | [attached_disk_defaults](variables.tf#L17) | Defaults for attached disks options. | object({…}) | | {…} | -| [attached_disks](variables.tf#L38) | Additional disks, if options is null defaults will be used in its place. Source type is one of 'image' (zonal disks in vms and template), 'snapshot' (vm), 'existing', and null. | list(object({…})) | | [] | +| [attached_disks](variables.tf#L37) | Additional disks, if options is null defaults will be used in its place. Source type is one of 'image' (zonal disks in vms and template), 'snapshot' (vm), 'existing', and null. | list(object({…})) | | [] | | [boot_disk](variables.tf#L83) | Boot disk properties. | object({…}) | | {…} | | [can_ip_forward](variables.tf#L117) | Enable IP forwarding. | bool | | false | | [confidential_compute](variables.tf#L123) | Enable Confidential Compute for these instances. | bool | | false | @@ -678,7 +677,6 @@ module "instance" { | [service_account_iam_email](outputs.tf#L74) | Service account email. | | | [template](outputs.tf#L82) | Template resource. | | | [template_name](outputs.tf#L87) | Template name. | | - ## TODO diff --git a/modules/compute-vm/variables.tf b/modules/compute-vm/variables.tf index efdf7d97..bf33e517 100644 --- a/modules/compute-vm/variables.tf +++ b/modules/compute-vm/variables.tf @@ -28,7 +28,6 @@ variable "attached_disk_defaults" { replica_zone = null type = "pd-balanced" } - validation { condition = var.attached_disk_defaults.mode == "READ_WRITE" || !var.attached_disk_defaults.auto_delete error_message = "auto_delete can only be specified on READ_WRITE disks." @@ -38,8 +37,9 @@ variable "attached_disk_defaults" { variable "attached_disks" { description = "Additional disks, if options is null defaults will be used in its place. Source type is one of 'image' (zonal disks in vms and template), 'snapshot' (vm), 'existing', and null." type = list(object({ - name = string - device_name = optional(string) + name = string + device_name = optional(string) + # TODO: size can be null when source_type is attach size = string snapshot_schedule = optional(string) source = optional(string) diff --git a/tests/modules/compute_mig/examples/stateful.yaml b/tests/modules/compute_mig/examples/stateful.yaml index 5fcd7ff4..4783e23e 100644 --- a/tests/modules/compute_mig/examples/stateful.yaml +++ b/tests/modules/compute_mig/examples/stateful.yaml @@ -13,25 +13,49 @@ # limitations under the License. values: + module.nginx-mig.google_compute_instance_group_manager.default[0]: + all_instances_config: [] + auto_healing_policies: [] + base_instance_name: mig-test + description: Terraform managed. + list_managed_instances_results: PAGELESS + name: mig-test + named_port: [] + project: my-prj + stateful_disk: [] + stateful_external_ip: [] + stateful_internal_ip: [] + target_pools: null + timeouts: null + version: + - name: default + target_size: [] + wait_for_instances: false + wait_for_instances_status: STABLE + zone: europe-west8-b module.nginx-mig.google_compute_per_instance_config.default["instance-1"]: + instance_group_manager: mig-test minimal_action: NONE most_disruptive_allowed_action: REPLACE name: instance-1 preserved_state: - disk: - delete_rule: NEVER - device_name: persistent-disk-1 + device_name: data-1 mode: READ_WRITE - source: test-disk + source: projects/my-prj/zones/europe-west8-b/disks/test-data-1 metadata: foo: bar - project: my-project + project: my-prj remove_instance_state_on_destroy: false timeouts: null - zone: europe-west1-b + zone: europe-west8-b counts: - google_compute_autoscaler: 1 google_compute_instance_group_manager: 1 google_compute_instance_template: 1 google_compute_per_instance_config: 1 + modules: 2 + resources: 3 + +outputs: {}