Merge "Use JSON baremetal data in testing"

doc/source/howto.rst

@@ -230,7 +230,7 @@ pre-requisite software packages, Ansible, and then execute the
 to provide a single step testing mechanism.
 
 ``playbooks/test-bifrost-create-vm.yaml`` creates one or more VMs for
-testing and saves out a baremetal.csv file which is used by
+testing and saves out a baremetal.json file which is used by
 ``playbooks/test-bifrost.yaml`` to execute the remaining roles.  Two
 additional roles are invoked by this playbook which enables Ansible to
 connect to the new nodes by adding them to the inventory, and then
@@ -270,8 +270,8 @@ run virsh commands.
    test-bifrost-create-vm.yaml`` command to create a test virtual
    machine.
 #. Set the environment variable of ``BIFROST_INVENTORY_SOURCE`` to the
-   path to the csv file, which by default has been written to
-   /tmp/baremetal.csv.
+   path to the JSON file, which by default has been written to
+   /tmp/baremetal.json.
 #. Run the enrollment step, as documented above, using the CSV file
    you created in the previous step.
 #. Run the deployment step, as documented above.

playbooks/roles/bifrost-create-vm-nodes/README.md

@@ -28,7 +28,17 @@ as extra-vars instead.
 Role Variables
 --------------
 
-baremetal_csv_file: "/tmp/baremetal.csv"
+baremetal_csv_file: Deprecated. CSV file format is deprecated, and
+                    this variable will be removed in the Queens release.
+                    Use 'baremetal_json_file' variable instead.
+                    Default is undefined. If defined, its value will be
+                    used for 'baremetal_json_file' variable (see below),
+                    although file created will still be in JSON format.
+                    The driver assigned to nodes will be 'agent_ssh'
+
+baremetal_json_file: Defaults to '/tmp/baremetal.json' but will be overridden
+                     by 'baremetal_csv_file' if that is defined.
+                     The driver assigned to nodes will be 'agent_ssh'
 
 test_vm_memory_size: Tunable setting to allow a user to define a specific
                      amount of RAM in MB to allocate to guest/test VMs.

playbooks/roles/bifrost-create-vm-nodes/defaults/main.yml

@@ -1,6 +1,6 @@
 ---
 # defaults file for bifrost-create-vm-nodes
-baremetal_csv_file: "/tmp/baremetal.csv"
+baremetal_json_file: '/tmp/baremetal.json'
 test_vm_memory_size: "3072"
 test_vm_num_nodes: 1
 test_vm_domain_type: "qemu"

playbooks/roles/bifrost-create-vm-nodes/tasks/create_vm.yml

@@ -98,22 +98,29 @@
   set_fact:
     vm_mac: "{{ (testvm_xml.get_xml | regex_findall(\"<mac address='.*'/>\") | first).split('=') | last | regex_replace(\"['/>]\", '') }}"
 
-- name: set the csv entry for vm
+- name: set the json entry for vm
   set_fact:
-    vm_csv_items:
-      - "{{ vm_mac }}"
-      - "root"
-      - "undefined"
-      - "192.168.122.1"
-      - "{{ test_vm_cpu_count }}"
-      - "{{ test_vm_memory_size }}"
-      - "{{ test_vm_disk_gib }}"
-      - "flavor"
-      - "type"
-      - "a8cb6624-0d9f-c882-affc-046ebb96ec0{{ testvm_csv_data | length + 1 }}"
-      - "{{ vm_name }}"
-      - "192.168.122.{{ testvm_csv_data | length + 2 }}"
+    testvm_data:
+      name: "{{ vm_name }}"
+      uuid: "{{ vm_name | to_uuid }}"
+      driver: "agent_ssh"
+      driver_info:
+        power:
+          ssh_address: "192.168.122.1"
+          ssh_port: "22"
+          ssh_username: "ironic"
+          ssh_key_filename: "/home/ironic/.ssh/id_rsa"
+          ssh_virt_type: "virsh"
+      nics:
+        - mac: "{{ vm_mac }}"
+      ansible_ssh_host: "192.168.122.{{ testvm_json_data | length + 2 }}"
+      ipv4_address: "192.168.122.{{ testvm_json_data | length + 2 }}"
+      properties:
+        cpu_arch: "{{ test_vm_arch }}"
+        ram: "{{ test_vm_memory_size }}"
+        cpus: "{{ test_vm_cpu_count }}"
+        disk_size: "{{ test_vm_disk_gib }}"
 
 - name: add created vm info
   set_fact:
-    testvm_csv_data: "{{ testvm_csv_data + [vm_csv_items] }}"
+    testvm_json_data: "{{ testvm_json_data | combine({vm_name: testvm_data}) }}"

playbooks/roles/bifrost-create-vm-nodes/tasks/main.yml

@@ -12,6 +12,20 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 ---
+- name: produce warning when csv file is defined
+  debug:
+    msg: >
+      "WARNING - Variable 'baremetal_csv_file' is deprecated.
+      For backward compatibility, its value will be used as path for
+      file to write data for created 'virtual' baremetal nodes,
+      but the file will be JSON formatted."
+  when: baremetal_csv_file is defined
+
+- name: override baremetal_json_file with csv file path
+  set_fact:
+    baremetal_json_file: "{{ baremetal_csv_file }}"
+  when: baremetal_csv_file is defined
+
 # NOTE(cinerama) openSUSE Tumbleweed & Leap have different distribution
 # IDs which are not currently accounted for in Ansible, so adjust facts
 # so we can have shared defaults for the whole SuSE family.
@@ -81,37 +95,27 @@
 
 - name: create placeholder var for vm entries in CSV format
   set_fact:
-    testvm_csv_data: []
+    testvm_json_data: {}
 
 - include: create_vm.yml
   with_items: "{{ test_vm_node_names }}"
 
-- name: remove previous baremetal csv file
+- name: remove previous baremetal data file
   file:
     state: absent
-    path: "{{ baremetal_csv_file }}"
+    path: "{{ baremetal_json_file }}"
 
-- name: create empty baremetal csv file
-  file:
-    state: touch
-    path: "{{ baremetal_csv_file }}"
-
-# NOTE(pas-ha) this is a weird Ansible way to not flatten list of lists
-- name: write to baremetal csv file
-  lineinfile:
-    state: present
-    name: "{{ baremetal_csv_file }}"
-    line: "{{ item | join(',') }}"
-  with_nested:
-    - "{{ testvm_csv_data }}"
+- name: write to baremetal json file
+  copy:
+    dest: "{{ baremetal_json_file }}"
+    content: "{{ testvm_json_data | to_nice_json }}"
 
 - name: >
-    "Set file permissions such that the baremetal csv file at /tmp/baremetal.csv
+    "Set file permissions such that the baremetal data file
     can be read by the user executing Ansible"
   file:
-    path: "{{ baremetal_csv_file }}"
+    path: "{{ baremetal_json_file }}"
     owner: "{{ ansible_env.SUDO_USER }}"
   when: >
     ansible_env.SUDO_USER is defined and
-    baremetal_csv_file is defined and
-    baremetal_csv_file != ""
+    baremetal_json_file != ""

playbooks/roles/bifrost-test-dhcp/files/test-dhcp.py

@@ -17,10 +17,34 @@
 
 from __future__ import print_function
 import csv
+import json
 import os
 import sys
 
 
+def _load_data_from_csv(path):
+    with open(path) as csvfile:
+        csvdata = [row for row in csv.reader(csvfile)]
+    inventory = {}
+    # NOTE(pas-ha) convert to structure similar to JSON inventory
+    for entry in csvdata:
+        mac = entry[0]
+        hostname = entry[10]
+        ip = entry[11]
+        inventory[hostname] = {
+            'nics': [{'mac': mac}],
+            'name': hostname,
+            'ipv4_address': ip
+        }
+    return inventory
+
+
+def _load_data_from_json(path):
+    with open(path) as jsonfile:
+        inventory = json.load(jsonfile)
+    return inventory
+
+
 def main(argv):
     # first item is the inventory_dhcp setting
     # second item is the inventory_dhcp_static_ip setting
@@ -31,17 +55,20 @@ def main(argv):
         # nothing to validate
         sys.exit(0)
 
-    # extract data from csv file
-    inventory = []
-    if not os.path.exists('/tmp/baremetal.csv'):
+    # load data from json file
+    if os.path.exists('/tmp/baremetal.json'):
+        inventory = _load_data_from_json('/tmp/baremetal.json')
+    # load data from csv file
+    elif os.path.exists('/tmp/baremetal.csv'):
+        try:
+            inventory = _load_data_from_csv('/tmp/baremetal.csv')
+        except Exception:
+            # try load *.csv as json for backward compatibility
+            inventory = _load_data_from_json('/tmp/baremetal.csv')
+    else:
         print('ERROR: Inventory file has not been generated')
         sys.exit(1)
 
-    with open('/tmp/baremetal.csv') as csvfile:
-        inventory_reader = csv.reader(csvfile)
-        for row in inventory_reader:
-            inventory.append(row)
-
     # now check that we only have these entries in leases file
     leases = []
     if not os.path.exists('/var/lib/misc/dnsmasq.leases'):
@@ -59,27 +86,23 @@ def main(argv):
         sys.exit(1)
 
     # then we check that all macs and hostnames are present
-    for entry in inventory:
-        mac = entry[0]
-        hostname = entry[10]
-        ip = entry[11]
+    for value in inventory.values():
+        # NOTE(pas-ha) supporting only single nic
+        mac = value['nics'][0]['mac']
+        hostname = value['name']
+        ip = value['ipv4_address']
 
         # mac check
-        found = False
         for lease_entry in leases:
             if lease_entry[1] == mac:
-                found = True
                 break
-        if not found:
+        else:
             print('ERROR: No mac found in leases')
             sys.exit(1)
 
         # hostname check
-        found = False
         for lease_entry in leases:
             if lease_entry[3] == hostname:
-                found = True
-
                 # if we use static ip, we need to check that ip matches
                 # with hostname in leases
                 if inventory_dhcp_static_ip:
@@ -87,7 +110,7 @@ def main(argv):
                         print('ERROR: IP does not match with inventory')
                         sys.exit(1)
                 break
-        if not found:
+        else:
             print('ERROR: No hostname found in leases')
             sys.exit(1)
 

playbooks/test-bifrost-create-vm.yaml

@@ -6,10 +6,21 @@
   become: yes
   gather_facts: yes
   pre_tasks:
-    - name: "Set default baremetal.csv file if not already defined"
+    - name: "Warn if baremetal_csv_file is defined"
+      debug:
+        msg: >
+          "WARNING - 'baremetal_csv_file' variable is defined.
+          Its use is deprecated. The file created will be in JSON format.
+          Use 'baremetal_json_file' variable instead."
+      when: baremetal_csv_file is defined
+    - name: "Re-set baremetal json to csv file if defined"
       set_fact:
-         baremetal_csv_file: "/tmp/baremetal.csv"
-      when: baremetal_csv_file is not defined
+        baremetal_json_file: "{{ baremetal_csv_file }}"
+      when: baremetal_csv_file is defined
+    - name: "Set default baremetal.json file if not already defined"
+      set_fact:
+         baremetal_json_file: "/tmp/baremetal.json"
+      when: baremetal_json_file is not defined
     - name: "Set ci_testing flag if a list of changes are found in the environment variables"
       set_fact:
          ci_testing: true

playbooks/test-bifrost.yaml

@@ -2,7 +2,7 @@
 # Create a VM:
 # ansible-playbook -vvvv -i inventory/localhost test-bifrost-create-vm.yaml
 # Set BIFROST_INVENTORY_SOURCE
-# export BIFROST_INVENTORY_SOURCE=/tmp/baremetal.csv
+# export BIFROST_INVENTORY_SOURCE=/tmp/baremetal.json
 # Execute the installation and VM startup test.
 # ansible-playbook -vvvv -i inventory/bifrost_inventory.py test-bifrost.yaml -e use_cirros=true -e testing_user=cirros
 ---
@@ -143,16 +143,20 @@
 # The following tasks are intended to test DHCP functionality
 - hosts: localhost
   connection: local
-  name: "Executes DHCP test script"
+  name: "Start VMs that were not enrolled to ironic"
   become: yes
+  vars:
+    not_enrolled_data_file: /tmp/baremetal.json.rest
   tasks:
     # NOTE(TheJulia): Moved the power ON of the excess VMs until after
     # the other test VMs have been shutdown, in order to explicitly
     # validate that the dhcp config is working as expected and not
     # serving these requests.
   - name: Power on remaining test VMs
-    command: virsh start testvm{{item}}
-    with_sequence: start=4 end={{ test_vm_num_nodes | default('5') }}
+    virt:
+      name: "{{item.key}}"
+      state: running
+    with_dict: "{{ lookup('file', not_enrolled_data_file) | from_json }}"
     ignore_errors: yes
     when: inventory_dhcp | bool == true
   - name: Wait 30 seconds

releasenotes/notes/test-with-json-inventory-b05204009f880431.yaml

@@ -0,0 +1,30 @@
+---
+features:
+  - |
+    Bifrost's testing has been moved to use JSON-formatted baremetal inventory
+    file instead of deprecated CSV-formatted one.
+    The ``bifrost-create-vm-nodes`` role still accepts ``baremetal_csv_file``
+    variable as path to where to write inventory, but the file content will
+    always be in JSON format.
+    A new variable ``baremetal_json_file`` should instead be used
+    as a location to where to write the test baremetal inventory file.
+upgrade:
+  - |
+    The ``baremetal_csv_file`` variable in ``bifrost-create-vm-nodes`` role
+    has been deprecated and will be removed in the Queens release.
+    The inventory file written to this location by this role is now always
+    in JSON format.
+    The variable ``baremetal_json_file`` should be used instead of
+    ``baremetal_csv_file``.
+    This concerns only those operators who run tests for bifrost on
+    virtual hardware using ``bifrost-create-vm-nodes`` role and out-of-tree
+    scripts to process the baremetal inventory file produced by this role.
+    If such scripts do rely on this file being in CSV format,
+    they must be updated to use JSON format instead.
+deprecations:
+  - |
+    The CSV format for baremetal inventory file is deprecated and using
+    it will be impossible in the Queens release.
+    During deprecation period it's handling is still supported by bifrost's
+    dynamic inventory, but this functionality will be removed in the Queens
+    release.

scripts/split_json.py

@@ -0,0 +1,99 @@
+#!/usr/bin/env python
+#
+# Copyright (c) 2017 Mirantis Inc
+#
+# 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.
+
+"""Helper script to cut down number of etries in JSON baremetal data file.
+
+Splits the JSON file containing a top-level dict structure into two files,
+where first has at most N entries, and the second has the rest of them.
+
+Uses OrderedDict to preserve ordering of dict elements in input JSON file.
+"""
+
+from __future__ import print_function
+
+import collections
+import json
+import os
+import sys
+
+HELP_MSG = """
+Usage:
+    python %s <N> <input> <first> <second>
+
+where:
+    N - max number of dict elements to be defined in the <first> JSON file
+    input - path to input JSON file
+    <first> - path to first output JSON file containig at most N entries
+    <second> - path to the second output JSON file containig the rest
+""" % sys.argv[0]
+
+
+def fail(msg=None):
+    if msg:
+        print("Error: %s" % msg)
+        print()
+    print(HELP_MSG)
+    sys.exit(1)
+
+
+def parse_args(args):
+
+    if len(args) != 4:
+        fail("Wrong number of arguments")
+    num, infile, out1, out2 = args
+
+    try:
+        num = int(num)
+    except ValueError:
+        fail("First argument is not integer")
+
+    if not os.path.isfile(infile):
+        fail("Input file %s does not exist or can not be accessed." % infile)
+    return num, infile, out1, out2
+
+
+def write_to_json(fname, data):
+    with open(fname, 'w') as of:
+        try:
+            json.dump(data, of, indent=4)
+        except Exception as ex:
+            fail("Failed to save data to %s file - Error %s" % (fname, ex))
+
+
+def split_json_dict(args):
+    num, infile, out1, out2 = parse_args(args)
+    data = {}
+    with open(infile) as f:
+        data = json.load(f, object_pairs_hook=collections.OrderedDict)
+    if not data:
+        fail("Baremetal data file %s is empty or non-valid JSON." % infile)
+
+    first_data = collections.OrderedDict()
+    second_data = collections.OrderedDict()
+    for i, k in enumerate(data):
+        if i < num:
+            first_data[k] = data[k]
+        else:
+            second_data[k] = data[k]
+
+    write_to_json(out1, first_data)
+    write_to_json(out2, second_data)
+
+
+if __name__ == '__main__':
+    split_json_dict(sys.argv[1:])
+    sys.exit(0)

scripts/test-bifrost.sh

@@ -10,6 +10,7 @@ ENABLE_VENV="false"
 USE_DHCP="false"
 USE_VENV="false"
 BUILD_IMAGE="false"
+BAREMETAL_DATA_FILE=${BAREMETAL_DATA_FILE:-'/tmp/baremetal.json'}
 
 # Set defaults for ansible command-line options to drive the different
 # tests.
@@ -157,17 +158,23 @@ ${ANSIBLE} -vvvv \
        -e test_vm_num_nodes=${TEST_VM_NUM_NODES} \
        -e test_vm_memory_size=${VM_MEMORY_SIZE} \
        -e test_vm_domain_type=${VM_DOMAIN_TYPE} \
+       -e baremetal_json_file=${BAREMETAL_DATA_FILE} \
        -e enable_venv=${ENABLE_VENV}
 
 if [ ${USE_DHCP} = "true" ]; then
-    # cut file to limit number of nodes to enroll for testing purposes
-    head -n -2 /tmp/baremetal.csv > /tmp/baremetal.csv.new && mv /tmp/baremetal.csv.new /tmp/baremetal.csv
+    # reduce the number of nodes in JSON file
+    # to limit number of nodes to enroll for testing purposes
+    python $BIFROST_HOME/scripts/split_json.py 3 \
+        ${BAREMETAL_DATA_FILE} \
+        ${BAREMETAL_DATA_FILE}.new \
+        ${BAREMETAL_DATA_FILE}.rest \
+        && mv ${BAREMETAL_DATA_FILE}.new ${BAREMETAL_DATA_FILE}
 fi
 
 set +e
 
 # Set BIFROST_INVENTORY_SOURCE
-export BIFROST_INVENTORY_SOURCE=/tmp/baremetal.csv
+export BIFROST_INVENTORY_SOURCE=${BAREMETAL_DATA_FILE}
 
 # Execute the installation and VM startup test.
 
@@ -189,6 +196,7 @@ ${ANSIBLE} -vvvv \
     -e noauth_mode=${NOAUTH_MODE} \
     -e enable_keystone=${ENABLE_KEYSTONE} \
     -e wait_for_node_deploy=${WAIT_FOR_DEPLOY} \
+    -e not_enrolled_data_file=${BAREMETAL_DATA_FILE}.rest \
     ${CLOUD_CONFIG}
 EXITCODE=$?