Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add auto_delete and instance_redistribution_type to compute-vm and compute-mig modules. #890

Merged
merged 2 commits into from Oct 16, 2022

Conversation

giovannibaratta
Copy link
Contributor

While working with compute-vm and compute-mig modules, I encounted the following issues:

  • the default value for auto_delete used by google_compute_instance_template creates a conflict with READ_ONLY disks (ex. https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/tree/master/modules/compute-mig#stateful-migs---mig-config ) and it's not possible to customize the attribute in the module compute-vm, therefore it's not possible to create disk with read only disks with the current version of the module.
  • the default value for instance_redistribution_type used by google_compute_region_instance_group_manager is not compatible with stateful migs and it's not possible to customize the attribute in the module compute-mig, therefore it's not possible to create regional stateful mig with the current version of the module (if I did not miss anything).
  • (minor) I tried the code in the example Stateful MIGs - MIG Config in modules/compute-mig/README.md the block stateful_config and it's not working in my case. I guess there is an error.

This PR add the two attributes in the respective module and let the user to customize them in order to implement the mentioned use case.


Using a template with attached_disk of type read_only in compute-vm results in:

│ Error: Error creating instance template: googleapi: Error 400: Invalid value for field 'resource.properties.disks[1].autoDelete': 'true'. autoDelete can only be specified on read-write disks., invalid
│
│   with module.nginx-template.google_compute_instance_template.default[0],
│   on ../repos/cloud-foundation-fabric/modules/compute-vm/main.tf line 254, in resource "google_compute_instance_template" "default":
│  254: resource "google_compute_instance_template" "default" {

Using a regional stateful mig in compute-mig results in:

│ Error: Error creating RegionInstanceGroupManager: googleapi: Error 400: Invalid value for field 'resource.updatePolicy.instanceRedistributionType': 'UNSPECIFIED_TYPE'. Stateful features can be used only when instance redistribution is disabled (set to NONE)., invalid
│
│   with module.nginx-mig.google_compute_region_instance_group_manager.default[0],
│   on ../repos/cloud-foundation-fabric/modules/compute-mig/main.tf line 235, in resource "google_compute_region_instance_group_manager" "default":
│  235: resource "google_compute_region_instance_group_manager" "default" {

@google-cla
Copy link

google-cla bot commented Oct 16, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@ludoo
Copy link
Collaborator

ludoo commented Oct 16, 2022

Giovanni, thanks for the detailed explanation and code changes. Can you run the tfdoc tool to fix documentation linting errors? Our contributing guide has the details.

Copy link
Collaborator

@ludoo ludoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for this!

@ludoo ludoo merged commit ca1dc36 into GoogleCloudPlatform:master Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants