fix: added missing zone parameter in gcp_compute_instance_template

[simonkey007]

SUMMARY

gcp_compute_instance_template is missing zone parameter

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

module gcp_compute_instance_template

ADDITIONAL INFORMATION

I was trying to create an instance template by providing pd_ssd disk type

---
  - name: gcp vm instance
    hosts: localhost
    gather_facts: true
    vars_files:
      - vars.yaml
    tasks:
      - name: create an instance template
        google.cloud.gcp_compute_instance_template:
          name: "{{ instance_template_name }}"
          properties:
            disks:
            - auto_delete: 'true'
              boot: 'true'
              initialize_params:
                disk_size_gb: "200"
                disk_type: "pd_ssd"
                source_image: "projects/centos-cloud/global/images/centos-7-v20221102"
            machine_type: "{{ machine_type }}"
            metadata:
              startup-script: |
                sudo yum update
                sudo yum install -y mysql-server
            network_interfaces:
              - network:
                  selfLink: "projects/{{ project_id }}/global/networks/default"
          project: "{{ project_id }}"
          auth_kind: serviceaccount
          service_account_file: "{{ service_account_file }}"
          state: present

It turned out that function inside module requires zone parameter

def disk_type_selflink(name, params):
    if name is None:
        return
    url = r"https://compute.googleapis.com/compute/v1/projects/.*/zones/.*/diskTypes/.*"
    if not re.match(url, name):
        name = "https://compute.googleapis.com/compute/v1/projects/{project}/zones/{zone}/diskTypes/%s".format(**params) % name
    return name

When I run ansible-playbook playbook.yaml I get this error:

  File "/var/folders/07/b8b1dky11434yyql0k2451_m0000gn/T/ansible_google.cloud.gcp_compute_instance_template_payload_1bcy5hhm/ansible_google.cloud.gcp_compute_instance_template_payload.zip/ansible_collections/google/cloud/plugins/modules/gcp_compute_instance_template.py", line 1208, in disk_type_selflink
KeyError: 'zone'

[toumorokoshi]

Thanks for the PR!

Ideally we would have added this field to the disk type itself (since templates are not regional), but I could imagine that'll constitute a backwards-incompatible change (For those that figured this out and fixed it already).

I'll get it merged in.

[toumorokoshi]

Since compute_instance_template is now a resource that has integration testing, can you update the integration test and try it out?

Adding that zone-specific config in the test would help prevent regressions.

see https://github.com/ansible-collections/google.cloud/blob/master/CONTRIBUTING.md#running-integration-tests.

The test would be:

ansible-test integration gcp_compute_instance_template -vvv

[toumorokoshi]

You can ignore the failing integration test - that's a config fix that I can merge in (integration tests should not run on external contributor PRs)

[simonkey007]

I added additional zone-specific tasks to gcp_compute_instance_template integration tests.

[toumorokoshi]

Thanks for adding those! I ran the tests and it looks like it's failing?

The first error was around zone not being a supported parameter: that can be fixed by ensuring that zone is a valid argument put into the GcpModule (starts ~line 1006).

Even after that, I get:

fatal: [testhost]: FAILED! => {"changed": false, "msg": "GCP returned error: {'error': {'code': 400, 'message': \"Invalid value for field 'resource.properties.disks[0].initializeParams.diskType': 'https://compute.googleapis.com/compute/v1/projects/<redacted>/zones/us-central1-a/diskTypes/pd-ssd'. DiskType 'https://compute.googleapis.com/compute/v1/projects/<redacted>/zones/us-central1-a/diskTypes/pd-ssd' must be a valid resource name (not an url).\", 'errors': [{'message': \"Invalid value for field 'resource.properties.disks[0].initializeParams.diskType': 'https://compute.googleapis.com/compute/v1/projects/<redacted>/zones/us-central1-a/diskTypes/pd-ssd'. DiskType 'https://compute.googleapis.com/compute/v1/projects/<redacted>/zones/us-central1-a/diskTypes/pd-ssd' must be a valid resource name (not an url).\", 'domain': 'global', 'reason': 'invalid', 'debugInfo': 'java.lang.Exception\\n\\tat com.google.cloud.control.common.publicerrors.PublicErrorProtoUtils.newErrorBuilder(PublicErrorProtoUtils.java:1960)\\n\\tat com.google.cloud.control.common.publicerrors.PublicErrorProtoUtils.createInvalidFieldValue(PublicErrorProtoUtils.java:891)\\n\\tat com.google.cloud.control.common.publicerrors.PublicErrorProtoUtils.createInvalidFieldValue(PublicErrorProtoUtils.java:873)\\n\\tat com.google.cloud.cluster.manager.compute.services.instancetemplate.common.InstancePropertiesMixerToProtoConverter.convertAttachedDisksExternalToInternal(InstancePropertiesMixerToProtoConverter.java:1113)\\n\\tat com.google.cloud.cluster.manager.compute.services.instancetemplate.common.InstancePropertiesMixerToProtoConverter.validateAndConvertInstancePropertiesExternalToInternal(InstancePropertiesMixerToProtoConverter.java:511)\\n\\tat com.google.cloud.cluster.manager.compute.services.instancetemplate.global.insert.InstanceTemplatesInsertMutationHandler.prepareInsertFlow(InstanceTemplatesInsertMutationHandler.java:152)\\n\\tat com.google.cloud.cluster.manager.compute.services.instancetemplate.global.insert.InstanceTemplatesInsertMutationHandler.prepareInsertFlow(InstanceTemplatesInsertMutationHandler.java:48)\\n\\tat com.google.cloud.control.frontend.action.MixerInsertActionExecutor$InnerHandler.prepareMutationFlow(MixerInsertActionExecutor.java:856)\\n\\tat com.google.cloud.control.frontend.action.MixerInsertActionExecutor$InnerHandler.prepareMutationFlow(MixerInsertActionExecutor.java:701)\\n\\tat com.google.cloud.control.frontend.action.MixerMutationActionExecutor$InnerHandler.runAttempt(MixerMutationActionExecutor.java:1073)\\n\\tat com.google.cloud.control.frontend.action.MixerMutationActionExecutor$InnerHandler.runAttempt(MixerMutationActionExecutor.java:731)\\n\\tat com.google.cloud.cluster.metastore.RetryingMetastoreTransactionExecutor$1.runAttempt(RetryingMetastoreTransactionExecutor.java:79)\\n\\tat com.google.cloud.cluster.metastore.MetastoreRetryLoop.runHandler(MetastoreRetryLoop.java:499)\\n\\t...Stack trace is shortened.\\n'}]}}"}

Which makes me wonder if there's still a bug in the disk type resolution?

If the test can be fixed to work, I'm happy to merge.

[toumorokoshi]

Hi! it looks like the test fails today. As noted in the other comment, if the test (and likely the code it exercises) can be fixed, happy to merge the feature.

[toumorokoshi]

See #516 (comment).