metalsmith_deployment role switch to metalsmith_instances

Instead of wrapping the metalsmith CLI, the metalsmith_deployment role
now uses the metalsmith_instances module. There are differences between
the instances formats of the role and the module which are partially
resolved with a simple transformation module called
metalsmith_deployment_defaults.

A 'candidates' attribute is added to metalsmith_instances, but keeping
the single 'name' attribute to remain compatible with the TripleO
usage.

Unresolved differences between the 2 are described below:

metalsmith_instances doesn't have a per-instance state attribute,
instead it has one state attribute for all instances. I propose that
support for per-instance state is dropped. This was never documented
in the README.rst anyway.

extra_args only applies to a CLI. Apart from --dry-run these arguments
are either for output formatting or Ironic API authentication. I propose
that this option is dropped. A metalsmith_debug arg is added to make
the ouput more verbose.

Change-Id: Ia30620821182c58050813e807cdde50a27d03c15

metalsmith/test/test_metalsmith_instances.py

@@ -56,6 +56,7 @@ class TestMetalsmithInstances(unittest.TestCase):
         provisioner = mock.Mock()
         instances = [{
             'name': 'node',
+            'candidates': ['other_node'],
             'resource_class': 'boxen',
             'capabilities': {'foo': 'bar'},
             'traits': ['this', 'that'],
@@ -72,7 +73,7 @@ class TestMetalsmithInstances(unittest.TestCase):
         result = mi.reserve(provisioner, instances, True)
         provisioner.reserve_node.assert_has_calls([
             mock.call(
-                candidates=['node'],
+                candidates=['other_node', 'node'],
                 capabilities={'foo': 'bar'},
                 conductor_group='group',
                 resource_class='boxen',

metalsmith_ansible/ansible_plugins/modules/metalsmith_deployment_defaults.py

@@ -0,0 +1,137 @@
+# Copyright 2020 Red Hat, Inc.
+# All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import os
+
+from ansible.module_utils.basic import AnsibleModule
+
+import yaml
+
+
+DOCUMENTATION = '''
+---
+module: metalsmith_deployment_defaults
+short_description: Transform instances list data for metalsmith_instances
+author: "Steve Baker (@stevebaker)"
+description:
+  - Takes a list of instances from the metalsmith_deployment role
+    and a dict of defaults and transforms that to the format required
+    by the metalsmith_instances module.
+options:
+  instances:
+    description:
+      - List of node description dicts to perform operations on (in the
+        metalsmith_deployment instances format)
+    type: list
+    default: []
+    elements: dict
+  defaults:
+    description:
+      - Dict of defaults to use for missing values. Keys correspond to the
+        metalsmith_deployment instances format.
+    type: dict
+'''
+
+
+def transform(module, instances, defaults):
+    mi = []
+
+    def value(src, key, dest, to_key=None):
+        if not to_key:
+            to_key = key
+        value = src.get(key, defaults.get(key))
+        if value:
+            dest[to_key] = value
+
+    for src in instances:
+        dest = {'image': {}}
+        value(src, 'hostname', dest)
+        value(src, 'candidates', dest)
+        value(src, 'nics', dest)
+        value(src, 'netboot', dest)
+        value(src, 'root_size', dest, 'root_size_gb')
+        value(src, 'swap_size', dest, 'swap_size_mb')
+        value(src, 'capabilities', dest)
+        value(src, 'traits', dest)
+        value(src, 'resource_class', dest)
+        value(src, 'conductor_group', dest)
+        value(src, 'user_name', dest)
+        image = dest['image']
+        value(src, 'image', image, 'href')
+        value(src, 'image_checksum', image, 'checksum')
+        value(src, 'image_kernel', image, 'kernel')
+        value(src, 'image_ramdisk', image, 'ramdisk')
+
+        # keys in metalsmith_instances not currently in metalsmith_deployment:
+        # passwordless_sudo
+
+        # keys in metalsmith_deployment not currently in metalsmith_instances:
+        # extra_args (CLI args cannot translate to the python lib,
+        #             but they are mainly for auth and output formatting apart
+        #             from --dry-run)
+        if 'extra_args' in src:
+            module.fail_json(
+                changed=False,
+                msg="extra_args is no longer supported"
+            )
+
+        # state (metalsmith_instances has a single state attribute for every
+        #        instance)
+        if 'state' in src:
+            module.fail_json(
+                changed=False,
+                msg="Per-instance state is no longer supported, "
+                    "use variable metalsmith_state"
+            )
+
+        # source keys could be a string or a list of strings
+        # and the strings could be a path to a public key or the key contents.
+        # Normalize this to a list of key contents
+        keys = []
+        source_keys = src.get('ssh_public_keys')
+        if source_keys:
+            if isinstance(source_keys, str):
+                source_keys = [source_keys]
+            for source_key in source_keys:
+                if os.path.isfile(source_key):
+                    with open(source_key) as f:
+                        source_key = f.read()
+                keys.append(source_key)
+        if keys:
+            dest['ssh_public_keys'] = keys
+
+        mi.append(dest)
+
+    module.exit_json(
+        changed=False,
+        msg="{} instances transformed".format(len(mi)),
+        instances=mi
+    )
+    return mi
+
+
+def main():
+    module = AnsibleModule(
+        argument_spec=yaml.safe_load(DOCUMENTATION)['options'],
+        supports_check_mode=False,
+    )
+
+    instances = module.params['instances']
+    defaults = module.params['defaults']
+    transform(module, instances, defaults)
+
+
+if __name__ == '__main__':
+    main()

metalsmith_ansible/ansible_plugins/modules/metalsmith_instances.py

@@ -58,8 +58,14 @@ options:
         type: str
       name:
         description:
-          - The name of an existing node to provision
+          - The name of an existing node to provision, this name is appended
+            to the candidates list
         type: str
+      candidates:
+        description:
+          - List of nodes (UUIDs or names) to be considered for deployment
+        type: list
+        elements: str
       image:
         description:
           - Details of the image you want to provision onto the node
@@ -127,7 +133,6 @@ options:
       ssh_public_keys:
         description:
           - SSH public keys to load
-        type: str
       resource_class:
         description:
           - Node resource class to provision
@@ -179,6 +184,7 @@ options:
       - Maximum number of instances to provision at once. Set to 0 to have no
         concurrency limit
     type: int
+    default: 0
   log_level:
     description:
       - Set the logging level for the log which is available in the
@@ -238,12 +244,10 @@ def _get_source(instance):
 def reserve(provisioner, instances, clean_up):
     nodes = []
     for instance in instances:
+        candidates = instance.get('candidates', [])
         if instance.get('name') is not None:
-            # NOTE(dtantsur): metalsmith accepts list of instances to pick
+            candidates.append(instance['name'])
-            # from. We implement a simplest case when a user can pick a
+        if not candidates:
-            # node by its name (actually, UUID will also work).
-            candidates = [instance['name']]
-        else:
             candidates = None
         try:
             node = provisioner.reserve_node(

metalsmith_ansible/roles/metalsmith_deployment/README.rst

@@ -19,6 +19,8 @@ The following optional variables provide the defaults for Instance_ attributes:
     the default for ``capabilities``.
 ``metalsmith_conductor_group``
     the default for ``conductor_group``.
+``metalsmith_debug``
+    Show extra debug information, defaults to ``false``.
 ``metalsmith_extra_args``
     the default for ``extra_args``.
 ``metalsmith_image``
@@ -39,6 +41,9 @@ The following optional variables provide the defaults for Instance_ attributes:
     the default for ``root_size``.
 ``metalsmith_ssh_public_keys``
     the default for ``ssh_public_keys``.
+``metalsmith_state``
+    the default state for instances, valid values are ``reserved``, ``absent``
+    or the default value ``present``.
 ``metalsmith_swap_size``
     the default for ``swap_size``.
 ``metalsmith_traits``
@@ -62,6 +67,7 @@ Each instances has the following attributes:
 
 ``extra_args`` (defaults to ``metalsmith_extra_args``)
     additional arguments to pass to the ``metalsmith`` CLI on all calls.
+    (No longer supported, will raise an error if used)
 ``image`` (defaults to ``metalsmith_image``)
     UUID, name or HTTP(s) URL of the image to use for deployment. Mandatory.
 ``image_checksum`` (defaults to ``metalsmith_image_checksum``)

metalsmith_ansible/roles/metalsmith_deployment/defaults/main.yml

@@ -2,6 +2,7 @@
 metalsmith_candidates: []
 metalsmith_capabilities: {}
 metalsmith_conductor_group:
+metalsmith_debug: false
 metalsmith_extra_args:
 metalsmith_image_checksum:
 metalsmith_image_kernel:
@@ -11,6 +12,7 @@ metalsmith_nics: []
 metalsmith_resource_class:
 metalsmith_root_size:
 metalsmith_ssh_public_keys: []
+metalsmith_state: present
 metalsmith_swap_size:
 metalsmith_traits: []
 metalsmith_user_name: metalsmith

metalsmith_ansible/roles/metalsmith_deployment/tasks/main.yml

@@ -1,86 +1,42 @@
 ---
 
-- name: Start provisioning of instances
+- name: Build instance defaults
-  command: >
+  metalsmith_deployment_defaults:
-    metalsmith {{ extra_args }} deploy --no-wait
+    instances: "{{ metalsmith_instances }}"
-    {% for cap_name, cap_value in capabilities.items() %}
+    defaults:
-      --capability {{ cap_name }}={{ cap_value }}
+      candidates: "{{ metalsmith_candidates }}"
-    {% endfor %}
+      capabilities: "{{ metalsmith_capabilities }}"
-    {% for trait in traits %}
+      conductor_group: "{{ metalsmith_conductor_group }}"
-      --trait {{ trait }}
+      extra_args: "{{ metalsmith_extra_args }}"
-    {% endfor %}
+      image: "{{ metalsmith_image }}"
-    {% for nic in nics %}
+      image_checksum: "{{ metalsmith_image_checksum }}"
-      {% for nic_type, nic_value in nic.items() %}
+      image_kernel: "{{ metalsmith_image_kernel }}"
-        --{{ nic_type }} {{ nic_value }}
+      image_ramdisk: "{{ metalsmith_image_ramdisk }}"
-      {% endfor %}
+      netboot: "{{ metalsmith_netboot }}"
-    {% endfor %}
+      nics: "{{ metalsmith_nics }}"
-    {% if root_size %}
+      resource_class: "{{ metalsmith_resource_class }}"
-      --root-size {{ root_size }}
+      root_size: "{{ metalsmith_root_size }}"
-    {% endif %}
+      ssh_public_keys: "{{ metalsmith_ssh_public_keys }}"
-    {% if swap_size %}
+      swap_size: "{{ metalsmith_swap_size }}"
-      --swap-size {{ swap_size }}
+      traits: "{{ metalsmith_traits }}"
-    {% endif %}
+      user_name: "{{ metalsmith_user_name }}"
-    {% for ssh_key in ssh_public_keys %}
+  register: instances
-      --ssh-public-key {{ ssh_key }}
-    {% endfor %}
-    --image {{ image }}
-    {% if image_checksum %}
-      --image-checksum {{ image_checksum }}
-    {% endif %}
-    {% if image_kernel %}
-      --image-kernel {{ image_kernel }}
-    {% endif %}
-    {% if image_ramdisk %}
-      --image-ramdisk {{ image_ramdisk }}
-    {% endif %}
-    --hostname {{ instance.hostname }}
-    {% if netboot %}
-    --netboot
-    {% endif %}
-    {% if user_name %}
-    --user-name {{ user_name }}
-    {% endif %}
-    {% if resource_class %}
-    --resource-class {{ resource_class }}
-    {% endif %}
-    {% if conductor_group %}
-    --conductor-group {{ conductor_group }}
-    {% endif %}
-    {% for node in candidates %}
-      --candidate {{ node }}
-    {% endfor %}
-  when: state == 'present'
-  vars:
-    candidates: "{{ instance.candidates | default(metalsmith_candidates) }}"
-    capabilities: "{{ instance.capabilities | default(metalsmith_capabilities) }}"
-    conductor_group: "{{ instance.conductor_group | default(metalsmith_conductor_group) }}"
-    extra_args: "{{ instance.extra_args | default(metalsmith_extra_args) }}"
-    image: "{{ instance.image | default(metalsmith_image) }}"
-    image_checksum: "{{ instance.image_checksum | default(metalsmith_image_checksum) }}"
-    image_kernel: "{{ instance.image_kernel | default(metalsmith_image_kernel) }}"
-    image_ramdisk: "{{ instance.image_ramdisk | default(metalsmith_image_ramdisk) }}"
-    netboot: "{{ instance.netboot | default(metalsmith_netboot) }}"
-    nics: "{{ instance.nics | default(metalsmith_nics) }}"
-    resource_class: "{{ instance.resource_class | default(metalsmith_resource_class) }}"
-    root_size: "{{ instance.root_size | default(metalsmith_root_size) }}"
-    ssh_public_keys: "{{ instance.ssh_public_keys | default(metalsmith_ssh_public_keys) }}"
-    state: "{{ instance.state | default('present') }}"
-    swap_size: "{{ instance.swap_size | default(metalsmith_swap_size) }}"
-    traits: "{{ instance.traits | default(metalsmith_traits) }}"
-    user_name: "{{ instance.user_name | default(metalsmith_user_name) }}"
-  with_items: "{{ metalsmith_instances }}"
-  loop_control:
-    label: "{{ instance.hostname or instance }}"
-    loop_var: instance
 
-- name: Wait for provisioning of instances
+- name: Show instances data
-  command: >
+  debug:
-    metalsmith {{ metalsmith_extra_args }} wait
+    msg: "{{ instances.instances | to_yaml }}"
-    {% if metalsmith_provisioning_timeout %}
+  when: metalsmith_debug|bool
-    --timeout {{ metalsmith_provisioning_timeout }}
+
-    {% endif %}
+- name: Provision instances
-    {% for instance in metalsmith_instances %}
+  metalsmith_instances:
-      {% if (instance.state | default('present')) == 'present' %}
+    instances: "{{ instances.instances }}"
-        {{ instance.hostname }}
+    state: "{{ metalsmith_state }}"
-      {% endif %}
+    wait: true
-    {% endfor %}
+    timeout: "{{ metalsmith_provisioning_timeout }}"
+    log_level: "{{ 'debug' if metalsmith_debug|bool else 'info' }}"
+  register: baremetal_reserved
+
+- name: Metalsmith log for reserve instances
+  debug:
+    var: baremetal_reserved.logging
+  when: metalsmith_debug|bool

playbooks/integration/exercise.yaml

@@ -19,7 +19,7 @@
   include_role:
     name: metalsmith_deployment
   vars:
-    metalsmith_extra_args: --debug
+    metalsmith_debug: true
     metalsmith_resource_class: baremetal
     metalsmith_instances:
       - hostname: test

playbooks/integration/library

@@ -0,0 +1 @@
+../../metalsmith_ansible/ansible_plugins/modules