Use JSON baremetal data in testing

To move bifrost testing to *_ipmitool drivers + virtualbmc, we need
the baremetal data file to support ipmi port setting, as in the case of
virtualbmc, all test VM nodes use the same local IPMI address with
different port per-node.

Unfortunately, the CSV baremetal inventory format that is used in our
testing procedures does not support setting IPMI ports.

As the CSV format is considered a legacy format, let's move testing to use
JSON-formatted baremetal data instead of fixing the legacy format parser.

Changes are mostly to 'bifrost-create-vm-nodes' role and it's callers.
Some level of backward compatibility is provided:
- baremetal_csv_file var is still accepted, and its value is used as
  path to write data, although the data will nevertheless be in JSON format.

An extra helper script is added to reduce the number of nodes in
inventory when testing DHCP.
Also the script in 'bifrost-test-dhcp' role is changed to support
loading data from JSON.

This change officially deprecates using CSV formatted baremetal inventory
files.
Handling CSV baremetal inventory files will be removed in the Queens
release.

Change-Id: If2dcf43857195611ef342fe602330878378b021b
Partial-Bug: #1659876

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=$?