Update compute flavor module for 2.0.0

Switch sdk calls to use the proxy layer where sensible.

Ensure that returned resource objects are converted to dicts.

Removes undocumented id return value.

Rename flavorid to id. Keep flavorid as an alias for backward
compatibility.

Rename the test role from nova_flavor to compute_flavor to keep naming
consistent.

Fold tests from compute_flavor_info into the compute_flavor role.

Add additional tests to improve coverage.

Update return docs

Change-Id: I5419d1c02b9b50625beb3bff88c8e4a4f1c14667

.zuul.yaml

@@ -67,6 +67,7 @@
         auth
         catalog_service
         client_config
+        compute_flavor
         dns
         dns_zone_info
         endpoint
@@ -89,7 +90,6 @@
         logging
         loadbalancer
         network
-        nova_flavor
         nova_services
         object
         object_container

ci/roles/compute_flavor/defaults/main.yml

@@ -0,0 +1,14 @@
+expected_fields:
+  - description
+  - disk
+  - ephemeral
+  - extra_specs
+  - id
+  - is_disabled
+  - is_public
+  - name
+  - original_name
+  - ram
+  - rxtx_factor
+  - swap
+  - vcpus

ci/roles/compute_flavor/tasks/main.yml

@@ -0,0 +1,254 @@
+---
+- name: Delete resources before tests
+  openstack.cloud.compute_flavor:
+    cloud: "{{ cloud }}"
+    state: absent
+    name: "{{ item }}"
+  loop:
+    - ansible_public_flavor
+    - ansible_private_flavor
+    - ansible_extra_specs_flavor
+    - ansible_defaults_flavor
+
+- name: Create public flavor
+  openstack.cloud.compute_flavor:
+    cloud: "{{ cloud }}"
+    state: present
+    name: ansible_public_flavor
+    is_public: True
+    ram: 1024
+    vcpus: 1
+    disk: 10
+    ephemeral: 10
+    swap: 1
+    id: 12345
+  register: result
+
+- assert:
+    that: item in result.flavor
+  loop: "{{ expected_fields }}"
+
+- name: Assert changed
+  assert:
+    that: result is changed
+
+- name: Create public flavor again
+  openstack.cloud.compute_flavor:
+    cloud: "{{ cloud }}"
+    state: present
+    name: ansible_public_flavor
+    is_public: True
+    ram: 1024
+    vcpus: 1
+    disk: 10
+    ephemeral: 10
+    swap: 1
+    id: 12345
+  register: result
+
+- name: Assert not changed
+  assert:
+    that: result is not changed
+
+- name: Delete public flavor
+  openstack.cloud.compute_flavor:
+    cloud: "{{ cloud }}"
+    state: absent
+    name: ansible_public_flavor
+  register: result
+
+- name: Assert changed
+  assert:
+    that: result is changed
+
+- name: Delete public flavor again
+  openstack.cloud.compute_flavor:
+    cloud: "{{ cloud }}"
+    state: absent
+    name: ansible_public_flavor
+  register: result
+
+- name: Assert not changed
+  assert:
+    that: result is not changed
+
+- name: Create private flavor
+  openstack.cloud.compute_flavor:
+    cloud: "{{ cloud }}"
+    state: present
+    name: ansible_private_flavor
+    is_public: False
+    ram: 1024
+    vcpus: 1
+    disk: 10
+    ephemeral: 10
+    swap: 1
+    id: 12345
+
+- name: Delete private flavor
+  openstack.cloud.compute_flavor:
+    cloud: "{{ cloud }}"
+    state: absent
+    name: ansible_private_flavor
+
+- name: Create flavor (defaults)
+  openstack.cloud.compute_flavor:
+    cloud: "{{ cloud }}"
+    state: present
+    name: ansible_defaults_flavor
+    ram: 1024
+    vcpus: 1
+    disk: 10
+  register: result
+
+- name: Assert changed
+  assert:
+    that: result is changed
+
+- name: Create flavor (defaults) again
+  openstack.cloud.compute_flavor:
+    cloud: "{{ cloud }}"
+    state: present
+    name: ansible_defaults_flavor
+    ram: 1024
+    vcpus: 1
+    disk: 10
+  register: result
+
+- name: Assert not changed
+  assert:
+    that: result is not changed
+
+- name: Delete flavor (defaults)
+  openstack.cloud.compute_flavor:
+    cloud: "{{ cloud }}"
+    state: absent
+    name: ansible_defaults_flavor
+
+- name: Create flavor (extra_specs)
+  openstack.cloud.compute_flavor:
+    cloud: "{{ cloud }}"
+    state: present
+    name: ansible_extra_specs_flavor
+    ram: 1024
+    vcpus: 1
+    disk: 10
+    extra_specs:
+      "os:secure_boot": "required"
+  register: result
+
+- name: Assert returned value
+  assert:
+    that:
+      - result is changed
+      - result.flavor.extra_specs['os:secure_boot'] == 'required'
+
+- name: Create flavor (extra_specs) again
+  openstack.cloud.compute_flavor:
+    cloud: "{{ cloud }}"
+    state: present
+    name: ansible_extra_specs_flavor
+    ram: 1024
+    vcpus: 1
+    disk: 10
+    extra_specs:
+      "os:secure_boot": "required"
+  register: result
+
+- name: Assert not changed
+  assert:
+    that: result is not changed
+
+- name: Change extra_specs value
+  openstack.cloud.compute_flavor:
+    cloud: "{{ cloud }}"
+    state: present
+    name: ansible_extra_specs_flavor
+    ram: 1024
+    vcpus: 1
+    disk: 10
+    extra_specs:
+      "os:secure_boot": "disabled"
+  register: result
+
+- name: Assert returned value
+  assert:
+    that:
+      - result is changed
+      - result.flavor.extra_specs['os:secure_boot'] == 'disabled'
+
+- name: Append extra_specs value
+  openstack.cloud.compute_flavor:
+    cloud: "{{ cloud }}"
+    state: present
+    name: ansible_extra_specs_flavor
+    ram: 1024
+    vcpus: 1
+    disk: 10
+    extra_specs:
+      "os:secure_boot": "disabled"
+      "hw_video:ram_max_mb": 200
+  register: result
+
+- name: Assert returned value
+  assert:
+    that:
+      - result is changed
+      - result.flavor.extra_specs | length == 2
+      - "'os:secure_boot' in result.flavor.extra_specs"
+      - "'hw_video:ram_max_mb' in result.flavor.extra_specs"
+
+- name: Drop extra_specs value
+  openstack.cloud.compute_flavor:
+    cloud: "{{ cloud }}"
+    state: present
+    name: ansible_extra_specs_flavor
+    ram: 1024
+    vcpus: 1
+    disk: 10
+    extra_specs:
+      "hw_video:ram_max_mb": 200
+  register: result
+
+- name: Assert returned value
+  assert:
+    that:
+      - result is changed
+      - result.flavor.extra_specs | length == 1
+      - "'hw_video:ram_max_mb' in result.flavor.extra_specs"
+
+- name: Assert changed
+  assert:
+    that: result is changed
+
+- name: Clean up
+  openstack.cloud.compute_flavor:
+    cloud: "{{ cloud }}"
+    state: absent
+    name: "{{ item }}"
+  loop:
+    - ansible_public_flavor
+    - ansible_private_flavor
+    - ansible_extra_specs_flavor
+    - ansible_defaults_flavor
+
+- name: List flavors
+  openstack.cloud.compute_flavor_info:
+    cloud: "{{ cloud }}"
+  register: flavor_info
+
+- assert:
+    that: item in flavor_info.openstack_flavors[0]
+  loop: "{{ expected_fields }}"
+
+- name: List flavors with filter
+  openstack.cloud.compute_flavor_info:
+    cloud: "{{ cloud }}"
+    name: "m1.tiny"
+  register: flavor_name
+
+- name: Check output of list flavors with filter
+  assert:
+    that:
+      - flavor_name.openstack_flavors | length == 1
+

ci/roles/compute_flavor_info/tasks/main.yml

@@ -1,33 +0,0 @@
----
-- name: List flavors
-  openstack.cloud.compute_flavor_info:
-    cloud: "{{ cloud }}"
-
-- name: List flavors with filter
-  openstack.cloud.compute_flavor_info:
-    cloud: "{{ cloud }}"
-    name: "m1.tiny"
-  register: flavor_name
-
-- name: Check output of list flavors with filter
-  assert:
-    that:
-      - flavor_name.openstack_flavors | length == 1
-
-- name: Assert fields on SDK 0.*
-  when: sdk_version is version(1.0, '<')
-  assert:
-    that:
-      - '["name", "description", "disk", "is_public", "ram",
-          "vcpus", "swap", "ephemeral", "is_disabled", "rxtx_factor", "id",
-          "extra_specs"] | difference(flavor_name.openstack_flavors.0.keys())
-          | length == 0'
-
-- name: Assert fields on SDK 1.*
-  when: sdk_version is version(1.0, '>=')
-  assert:
-    that:
-      - '["name", "original_name", "description", "disk", "is_public", "ram",
-          "vcpus", "swap", "ephemeral", "is_disabled", "rxtx_factor", "id",
-          "extra_specs"] | difference(flavor_name.openstack_flavors.0.keys())
-          | length == 0'

ci/roles/nova_flavor/tasks/main.yml

@@ -1,53 +0,0 @@
----
-- name: Create public flavor
-  openstack.cloud.compute_flavor:
-     cloud: "{{ cloud }}"
-     state: present
-     name: ansible_public_flavor
-     is_public: True
-     ram: 1024
-     vcpus: 1
-     disk: 10
-     ephemeral: 10
-     swap: 1
-     flavorid: 12345
-
-- name: Delete public flavor
-  openstack.cloud.compute_flavor:
-     cloud: "{{ cloud }}"
-     state: absent
-     name: ansible_public_flavor
-
-- name: Create private flavor
-  openstack.cloud.compute_flavor:
-     cloud: "{{ cloud }}"
-     state: present
-     name: ansible_private_flavor
-     is_public: False
-     ram: 1024
-     vcpus: 1
-     disk: 10
-     ephemeral: 10
-     swap: 1
-     flavorid: 12345
-
-- name: Delete private flavor
-  openstack.cloud.compute_flavor:
-     cloud: "{{ cloud }}"
-     state: absent
-     name: ansible_private_flavor
-
-- name: Create flavor (defaults)
-  openstack.cloud.compute_flavor:
-     cloud: "{{ cloud }}"
-     state: present
-     name: ansible_defaults_flavor
-     ram: 1024
-     vcpus: 1
-     disk: 10
-
-- name: Delete flavor (defaults)
-  openstack.cloud.compute_flavor:
-     cloud: "{{ cloud }}"
-     state: absent
-     name: ansible_defaults_flavor

ci/run-collection.yml

@@ -8,6 +8,7 @@
     - { role: auth, tags: auth }
     - { role: catalog_service, tags: catalog_service }
     - { role: client_config, tags: client_config }
+    - { role: compute_flavor, tags: compute_flavor }
     - { role: dns_zone_info, tags: dns_zone_info }
     - role: object_container
       tags: object_container
@@ -44,9 +45,6 @@
       tags:
         - rbac
         - neutron_rbac
-    - { role: nova_flavor, tags: nova_flavor }
-    - role: compute_flavor_info
-      tags: nova_flavor
     - role: nova_services
       tags: nova_services
       when: sdk_version is version(0.44, '>=')

plugins/modules/compute_flavor.py

@@ -12,72 +12,74 @@ author: OpenStack Ansible SIG
 description:
    - Add or remove flavors from OpenStack.
 options:
-   state:
+  state:
-     description:
+    description:
-        - Indicate desired state of the resource. When I(state) is 'present',
+       - Indicate desired state of the resource. When I(state) is 'present',
-          then I(ram), I(vcpus), and I(disk) are all required. There are no
+         then I(ram), I(vcpus), and I(disk) are all required. There are no
-          default values for those parameters.
+         default values for those parameters.
-     choices: ['present', 'absent']
+    choices: ['present', 'absent']
-     default: present
+    default: present
-     type: str
+    type: str
-   name:
+  name:
-     description:
+    description:
-        - Flavor name.
+       - Flavor name.
-     required: true
+    required: true
-     type: str
+    type: str
-   ram:
+  ram:
-     description:
+    description:
-        - Amount of memory, in MB.
+       - Amount of memory, in MB.
-     type: int
+    type: int
-   vcpus:
+  vcpus:
-     description:
+    description:
-        - Number of virtual CPUs.
+       - Number of virtual CPUs.
-     type: int
+    type: int
-   disk:
+  disk:
-     description:
+    description:
-        - Size of local disk, in GB.
+       - Size of local disk, in GB.
-     default: 0
+    default: 0
-     type: int
+    type: int
-   ephemeral:
+  ephemeral:
-     description:
+    description:
-        - Ephemeral space size, in GB.
+       - Ephemeral space size, in GB.
-     default: 0
+    default: 0
-     type: int
+    type: int
-   swap:
+  swap:
-     description:
+    description:
-        - Swap space size, in MB.
+       - Swap space size, in MB.
-     default: 0
+    default: 0
-     type: int
+    type: int
-   rxtx_factor:
+  rxtx_factor:
-     description:
+    description:
-        - RX/TX factor.
+       - RX/TX factor.
-     default: 1.0
+    default: 1.0
-     type: float
+    type: float
-   is_public:
+  is_public:
-     description:
+    description:
-        - Make flavor accessible to the public.
+       - Make flavor accessible to the public.
-     type: bool
+    type: bool
-     default: 'yes'
+    default: 'yes'
-   flavorid:
+  id:
-     description:
+    description:
-        - ID for the flavor. This is optional as a unique UUID will be
+       - ID for the flavor. This is optional as a unique UUID will be
-          assigned if a value is not specified.
+         assigned if a value is not specified.
-     default: "auto"
+       - Note that this ID will only be used when first creating the flavor.
-     type: str
+    default: "auto"
-   extra_specs:
+    type: str
-     description:
+    aliases: ['flavorid']
-        - Metadata dictionary
+  extra_specs:
-     type: dict
+    description:
+       - Metadata dictionary
+    type: dict
 requirements:
-    - "python >= 3.6"
+   - "python >= 3.6"
-    - "openstacksdk"
+   - "openstacksdk"
 
 extends_documentation_fragment:
 - openstack.cloud.openstack
 '''
 
 EXAMPLES = '''
-- name: "Create 'tiny' flavor with 1024MB of RAM, 1 virtual CPU, and 10GB of local disk, and 10GB of ephemeral."
+- name: Create tiny flavor with 1024MB RAM, 1 vCPU, 10GB disk, 10GB ephemeral
   openstack.cloud.compute_flavor:
     cloud: mycloud
     state: present
@@ -87,7 +89,7 @@ EXAMPLES = '''
     disk: 10
     ephemeral: 10
 
-- name: "Delete 'tiny' flavor"
+- name: Delete tiny flavor
   openstack.cloud.compute_flavor:
     cloud: mycloud
     state: absent
@@ -108,57 +110,77 @@ EXAMPLES = '''
 
 RETURN = '''
 flavor:
-    description: Dictionary describing the flavor.
+  description: Dictionary describing the flavor.
-    returned: On success when I(state) is 'present'
+  returned: On success when I(state) is 'present'
-    type: complex
+  type: dict
-    contains:
+  contains:
-        id:
+    description:
-            description: Flavor ID.
+      description: Description attached to flavor
-            returned: success
+      returned: success
-            type: str
+      type: str
-            sample: "515256b8-7027-4d73-aa54-4e30a4a4a339"
+      sample: Example description
-        name:
+    disk:
-            description: Flavor name.
+      description: Size of local disk, in GB.
-            returned: success
+      returned: success
-            type: str
+      type: int
-            sample: "tiny"
+      sample: 10
-        disk:
+    ephemeral:
-            description: Size of local disk, in GB.
+      description: Ephemeral space size, in GB.
-            returned: success
+      returned: success
-            type: int
+      type: int
-            sample: 10
+      sample: 10
-        ephemeral:
+    extra_specs:
-            description: Ephemeral space size, in GB.
+      description: Flavor metadata
-            returned: success
+      returned: success
-            type: int
+      type: dict
-            sample: 10
+      sample:
-        ram:
+        "quota:disk_read_iops_sec": 5000
-            description: Amount of memory, in MB.
+        "aggregate_instance_extra_specs:pinned": false
-            returned: success
+    id:
-            type: int
+      description: Flavor ID.
-            sample: 1024
+      returned: success
-        swap:
+      type: str
-            description: Swap space size, in MB.
+      sample: "515256b8-7027-4d73-aa54-4e30a4a4a339"
-            returned: success
+    is_disabled:
-            type: int
+      description: Whether the flavor is disabled
-            sample: 100
+      returned: success
-        vcpus:
+      type: bool
-            description: Number of virtual CPUs.
+      sample: true
-            returned: success
+    is_public:
-            type: int
+      description: Make flavor accessible to the public.
-            sample: 2
+      returned: success
-        is_public:
+      type: bool
-            description: Make flavor accessible to the public.
+      sample: true
-            returned: success
+    name:
-            type: bool
+      description: Flavor name.
-            sample: true
+      returned: success
-        extra_specs:
+      type: str
-            description: Flavor metadata
+      sample: "tiny"
-            returned: success
+    original_name:
-            type: dict
+      description: The name of this flavor when returned by server list/show
-            sample:
+      type: str
-                "quota:disk_read_iops_sec": 5000
+      returned: success
-                "aggregate_instance_extra_specs:pinned": false
+    ram:
+      description: Amount of memory, in MB.
+      returned: success
+      type: int
+      sample: 1024
+    rxtx_factor:
+      description: |
+        The bandwidth scaling factor this flavor receives on the network
+      returned: success
+      type: int
+      sample: 100
+    swap:
+      description: Swap space size, in MB.
+      returned: success
+      type: int
+      sample: 100
+    vcpus:
+      description: Number of virtual CPUs.
+      returned: success
+      type: int
+      sample: 2
 '''
 
 from ansible_collections.openstack.cloud.plugins.module_utils.openstack import OpenStackModule
@@ -179,7 +201,7 @@ class ComputeFlavorModule(OpenStackModule):
         swap=dict(default=0, type='int'),
         rxtx_factor=dict(default=1.0, type='float'),
         is_public=dict(default=True, type='bool'),
-        flavorid=dict(default="auto"),
+        id=dict(default='auto', aliases=['flavorid']),
         extra_specs=dict(type='dict'),
     )
 
@@ -190,45 +212,57 @@ class ComputeFlavorModule(OpenStackModule):
         supports_check_mode=True
     )
 
-    def _system_state_change(self, flavor):
+    def _system_state_change(self, flavor, extra_specs, old_extra_specs):
         state = self.params['state']
-        if state == 'present' and not flavor:
+        if state == 'present':
-            return True
+            if not flavor:
+                return True
+            return self._needs_update(flavor) or extra_specs != old_extra_specs
         if state == 'absent' and flavor:
             return True
         return False
 
+    def _needs_update(self, flavor):
+        fields = ['ram', 'vcpus', 'disk', 'ephemeral', 'swap', 'rxtx_factor',
+                  'is_public']
+        for k in fields:
+            if self.params[k] is not None and self.params[k] != flavor[k]:
+                self.debug['diff'] = (k, self.params[k], flavor[k])
+                return True
+
+    def _build_flavor_specs_diff(self, extra_specs, old_extra_specs):
+        new_extra_specs = dict([(k, str(v)) for k, v in extra_specs.items()])
+        unset_keys = set(old_extra_specs.keys()) - set(extra_specs.keys())
+        return new_extra_specs, unset_keys
+
     def run(self):
         state = self.params['state']
         name = self.params['name']
         extra_specs = self.params['extra_specs'] or {}
 
-        flavor = self.conn.get_flavor(name)
+        flavor = self.conn.compute.find_flavor(name, get_extra_specs=True)
+        old_extra_specs = {}
+        if flavor:
+            old_extra_specs = flavor['extra_specs']
+            if flavor['swap'] == '':
+                flavor['swap'] = 0
 
         if self.ansible.check_mode:
-            self.exit_json(changed=self._system_state_change(flavor))
+            self.exit_json(changed=self._system_state_change(
+                flavor, extra_specs, old_extra_specs))
 
         if state == 'present':
-            old_extra_specs = {}
+            flavorid = self.params['id']
-            require_update = False
+            if flavor and self._needs_update(flavor):
-
+                # Because only flavor descriptions are updateable, we have to
-            if flavor:
+                # delete and recreate a flavor to "update" it
-                old_extra_specs = flavor['extra_specs']
+                self.conn.compute.delete_flavor(flavor)
-                if flavor['swap'] == "":
-                    flavor['swap'] = 0
-                for param_key in ['ram', 'vcpus', 'disk', 'ephemeral',
-                                  'swap', 'rxtx_factor', 'is_public']:
-                    if self.params[param_key] != flavor[param_key]:
-                        require_update = True
-                        break
-            flavorid = self.params['flavorid']
-            if flavor and require_update:
-                self.conn.delete_flavor(name)
                 old_extra_specs = {}
                 if flavorid == 'auto':
                     flavorid = flavor['id']
                 flavor = None
 
+            changed = False
             if not flavor:
                 flavor = self.conn.create_flavor(
                     name=name,
@@ -242,26 +276,27 @@ class ComputeFlavorModule(OpenStackModule):
                     is_public=self.params['is_public']
                 )
                 changed = True
-            else:
-                changed = False
 
-            new_extra_specs = dict([(k, str(v)) for k, v in extra_specs.items()])
+            new_extra_specs, unset_keys = self._build_flavor_specs_diff(
-            unset_keys = set(old_extra_specs.keys()) - set(extra_specs.keys())
+                extra_specs, old_extra_specs)
 
-            if unset_keys and not require_update:
+            if unset_keys:
                 self.conn.unset_flavor_specs(flavor['id'], unset_keys)
 
             if old_extra_specs != new_extra_specs:
-                self.conn.set_flavor_specs(flavor['id'], extra_specs)
+                self.conn.compute.create_flavor_extra_specs(
+                    flavor['id'], extra_specs)
+                changed = True
 
-            changed = (changed or old_extra_specs != new_extra_specs)
+            # Have to refetch updated extra_specs
+            flavor = self.conn.compute.fetch_flavor_extra_specs(flavor)
 
             self.exit_json(
-                changed=changed, flavor=flavor, id=flavor['id'])
+                changed=changed, flavor=flavor.to_dict(computed=False))
 
         elif state == 'absent':
             if flavor:
-                self.conn.delete_flavor(name)
+                self.conn.compute.delete_flavor(flavor)
                 self.exit_json(changed=True)
             self.exit_json(changed=False)