ngine-io / ansible-collection-cloudstack

CloudStack Ansible Collections
https://galaxy.ansible.com/ngine_io/cloudstack
GNU General Public License v3.0
21 stars 28 forks source link

Handle template with no display text(can be optional) #49

Closed jespada closed 3 years ago

jespada commented 3 years ago

The filed displaytext is optional, so for templates without it, you'll get:

KeyError: 'displaytext'

This PR fix that issue.

codecov[bot] commented 3 years ago

Codecov Report

Merging #49 (0834d34) into master (461bfb4) will decrease coverage by 6.72%. The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #49      +/-   ##
==========================================
- Coverage   84.37%   77.64%   -6.73%     
==========================================
  Files          53       32      -21     
  Lines        5510     3642    -1868     
  Branches     1247      847     -400     
==========================================
- Hits         4649     2828    -1821     
- Misses        432      495      +63     
+ Partials      429      319     -110     
Impacted Files Coverage Δ
plugins/modules/cs_instance.py 71.52% <0.00%> (ø)
plugins/modules/cs_project.py 45.34% <0.00%> (-38.38%) :arrow_down:
plugins/modules/cs_sshkeypair.py 50.43% <0.00%> (-36.53%) :arrow_down:
plugins/modules/cs_vpc.py 68.57% <0.00%> (-22.86%) :arrow_down:
plugins/modules/cs_template.py 39.08% <0.00%> (-20.69%) :arrow_down:
plugins/module_utils/cloudstack.py 63.14% <0.00%> (-9.44%) :arrow_down:
plugins/modules/cs_securitygroup.py 87.50% <0.00%> (-8.34%) :arrow_down:
plugins/modules/cs_zone.py 86.58% <0.00%> (-4.88%) :arrow_down:
plugins/modules/cs_account.py 81.81% <0.00%> (-0.76%) :arrow_down:
plugins/modules/cs_portforward.py
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 461bfb4...0834d34. Read the comment docs.

resmo commented 3 years ago

please ignore sanity test failure.

resmo commented 3 years ago

According to the docs, displaytext should be returned by the API, even as empty string if not set. May I ask you which version of cloudstack you see this issue or which on cloud provider?

jespada commented 3 years ago

According to the docs, displaytext should be returned by the API, even as empty string if not set. May I ask you which version of cloudstack you see this issue or which on cloud provider?

This happens when used with Exoscale, which treat that parameter as optional, is not documented as such, but you can see in packer provider src/doc : https://github.com/exoscale/packer-builder-exoscale#optional-parameters https://github.com/exoscale/packer-builder-exoscale/blob/master/config.go#L46 https://github.com/exoscale/packer-builder-exoscale/blob/master/step_register_template.go#L39

But if you think this is a bit too far from CS, feel free to close this PR. Sorry for the noise.

resmo commented 3 years ago

@jespada I understand the param is optional for registering a new template but this is unrelated to what the api returns. the api for listTemplates returns this key even being optional as an empty string. Could you give more information, which api of exoscale was used? I would agree to fix this but what you provided didn't look like to fix the bug you run into.

https://community.exoscale.com/api/compute/#listtemplates_GET

resmo commented 3 years ago

please rebase, also feel free to give more details so I can reproduce on exoscale

jespada commented 3 years ago

@jespada I understand the param is optional for registering a new template but this is unrelated to what the api returns. the api for listTemplates returns this key even being optional as an empty string. Could you give more information, which api of exoscale was used? I would agree to fix this but what you provided didn't look like to fix the bug you run into.

https://community.exoscale.com/api/compute/#listtemplates_GET

@resmo If you register a custom template without a description and then try use the custom template(template_filter: "self") to create an instance, you'll be able to reproduce the issue.

In order to reproduce it , you can use the following packer definition to build a custom template without template_description:

packer-file

## Source
#  REF: https://www.packer.io/docs/from-1.5/blocks/source

# Exoscale builder
# REF: https://github.com/exoscale/packer-builder-exoscale/
source "exoscale" "custom" {
  # Output template
  template_zone     = "ch-gva-2"
  template_name     = "ubuntu.custom"
  template_username = "ubuntu"

  #this is optional but can lead to https://github.com/ngine-io/ansible-collection-cloudstack/pull/49
  #template_description = "Ubuntu Custom Template"

  # Exoscale API
  api_endpoint = "https://api.exoscale.com/v1"
  api_key      = "EXOxxxx"
  api_secret   = "sxxxx"

  # Builder instance
  instance_template         = "Linux Ubuntu 20.04 LTS 64-bit"
  instance_disk_size        = 10
  instance_security_groups  = ["default"]

  # Communicator
  ssh_username = "ubuntu"
}

## Build
#  REF: https://www.packer.io/docs/from-1.5/blocks/build
build {
  sources = ["source.exoscale.custom"]

  # REF: https://www.packer.io/docs/provisioners/shell
  provisioner "shell" {
    inline = ["echo foo"]
  }
}

Build

➜  PACKER_PLUGIN_PATH=~/.packer.d packer build .
exoscale.custom: output will be in this color.

==> exoscale.custom: Build ID: bvdp1flnf4qf7763j4ug
==> exoscale.custom: Creating SSH key
==> exoscale.custom: Creating Compute instance
==> exoscale.custom: Using ssh communicator to connect: 159.XXXXX
==> exoscale.custom: Waiting for SSH to become available...
.
.
.
    exoscale.custom: foo
==> exoscale.custom: Stopping Compute instance
==> exoscale.custom: Creating Compute instance snapshot
==> exoscale.custom: Exporting Compute instance snapshot
==> exoscale.custom: Registering Compute instance template
==> exoscale.custom: Cleanup: destroying Compute instance
==> exoscale.custom: Cleanup: deleting SSH key
Build 'exoscale.custom' finished after 2 minutes 31 seconds.

==> Wait completed after 2 minutes 31 seconds

==> Builds finished. The artifacts of successful builds are:
--> exoscale.custom: ubuntu.custom @ ch-gva-2 (d3b0d0d4-cfbf-459c-995c-95c767a59cca)

List template

➜ cs --region=jorge listTemplates templatefilter=self
{
  "count": 1,
  "template": [
    {
      "bootmode": "legacy",
      "checksum": "4d159d8d504e9c7f813e5cd5a500ff9f",
      "created": "2020-12-17T17:11:43+0000",
      "crossZones": false,
      "details": {
        "username": "ubuntu"
      },
      "directdownload": true,
      "format": "QCOW2",
      "hypervisor": "KVM",
      "id": "d3b0d0d4-cfbfxxx",
      "isdynamicallyscalable": false,
      "isextractable": true,
      "isfeatured": false,
      "ispublic": false,
      "isready": true,
      "name": "ubuntu.custom",
      "oscategoryid": "981265e8-f3xxx",
      "oscategoryname": "Other",
      "ostypeid": "8fec0e5f-345a-4xxxx",
      "ostypename": "Other (64-bit)",
      "passwordenabled": true,
      "size": 10737418240,
      "sshkeyenabled": true,
      "tags": [],
      "templatetype": "USER",
      "zoneid": "1128bd56-b4xxx",
      "zonename": "ch-gva-2"
    }
  ]
}

As you can see on the listTemplates call there is not displaytext

resmo commented 3 years ago

Perfect, thanks for providing these infos, merging...