From 40e34aedeca875a215730d4660e4d284a0b08a13 Mon Sep 17 00:00:00 2001
From: Ian Denhardt <ian@zenhack.net>
Date: Tue, 9 Jan 2018 19:42:50 -0500
Subject: [PATCH 1/7] Start working on script for migrating ipmi info to obmd.

Mostly written, but untested and needs a bit of cleanup/more docs.
---
 hil/commands/admin.py             |   2 +
 hil/commands/migrate_ipmi_info.py | 103 ++++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+)
 create mode 100644 hil/commands/migrate_ipmi_info.py

diff --git a/hil/commands/admin.py b/hil/commands/admin.py
index f6ea4145..dd09ebfa 100644
--- a/hil/commands/admin.py
+++ b/hil/commands/admin.py
@@ -1,12 +1,14 @@
 """Implement the hil-admin command."""
 from hil import config, model
 from hil.commands import db
+from hil.commands.migrate_ipmi_info import MigrateIpmiInfo
 from hil.commands.util import ensure_not_root
 from hil.flaskapp import app
 from flask.ext.script import Manager
 
 manager = Manager(app)
 manager.add_command('db', db.command)
+manager.add_command('migrate-ipmi-info', MigrateIpmiInfo())
 
 
 def main():
diff --git a/hil/commands/migrate_ipmi_info.py b/hil/commands/migrate_ipmi_info.py
new file mode 100644
index 00000000..0ae01be2
--- /dev/null
+++ b/hil/commands/migrate_ipmi_info.py
@@ -0,0 +1,103 @@
+"""Helper command for extracting IPMI info from the database.
+
+This is part of our obmd migration strategy; the idea is that this
+information is then uploaded to obmd.
+"""
+import sys
+import json
+
+import requests
+
+from flask_script import Command, Option
+
+from hil import server
+from hil.flaskapp import app
+from hil import model
+
+
+class MigrateIpmiInfo(Command):
+    """Migrate ipmi info to obmd"""
+
+    option_list = (
+        Option('--obmd-base-url', dest='obmd_base_url',
+               help='Base url for the obmd api'),
+        Option('--obmd-admin-token', dest='obmd_admin_token',
+               help='Admin token for obmd'),
+    )
+
+    def run(self, obmd_base_url, obmd_admin_token):
+        server.init()
+        with app.app_context():
+            info = db_extract_ipmi_info()
+            obmd_upload_ipmi_info(obmd_base_url, obmd_admin_token, info)
+            db_add_obmd_info(obmd_base_url, obmd_admin_token)
+
+
+def db_extract_ipmi_info():
+    """Extract all nodes' ipmi connection info from the database.
+
+    This prints a json object of the form:
+
+        {
+            "node-23": {
+                "host": "172.16.32.23",
+                "user": "admin",
+                "password": "secret"
+            },
+            "node-46": {
+                "host": "172.16.32.46",
+                "user": "admin",
+                "password": "changeme"
+            }
+        }
+
+    It will fail if any of the
+    """
+    obms = model.Obm.query.all()
+
+    info = {}
+
+    for obm in obms:
+        if obm.type != 'ipmi':
+            sys.exit(("Node %s{label} has an obm of unspported "
+                      "type %s{type}").format({
+                          'label': obm.owner.label,
+                          'type': obm.type,
+                      }))
+        info[obm.owner.label] = {
+            'host': obm.host,
+            'user': obm.user,
+            'password': obm.password,
+        }
+
+    return info
+
+
+def obmd_upload_ipmi_info(obmd_base_url, obmd_admin_token, info):
+    """Upload nodes' info to obmd.
+
+    The info is loaded from stdin, and should be in the format output
+    by `extract_ipmi_info`.
+    """
+    info = json.load(sys.stdin)
+
+    sess = requests.Session(auth=('admin', obmd_admin_token))
+
+    # TODO: validate the input.
+
+    for key, val in info.items():
+        sess.put(obmd_base_url + '/node/' + key, data=json.dumps({
+            'type': 'ipmi',
+            'info': {
+                'addr': val['host'],
+                'user': val['user'],
+                'pass': val['password'],
+            },
+        }))
+
+
+def db_add_obmd_info(obmd_base_url, obmd_admin_token):
+    model.Node.query.update({'obmd_admin_token': obmd_admin_token})
+    for node in model.Node.query.all():
+        node.obmd_uri = obmd_base_url + '/node/' + node.label
+    model.db.commit()

From 21517f27ad61160187949ee6b8f548a96a8de2e1 Mon Sep 17 00:00:00 2001
From: Ian Denhardt <ian@zenhack.net>
Date: Wed, 7 Feb 2018 15:50:51 -0500
Subject: [PATCH 2/7] More or less finish obmd script.

Needs some more testing.
---
 hil/commands/migrate_ipmi_info.py | 39 +++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/hil/commands/migrate_ipmi_info.py b/hil/commands/migrate_ipmi_info.py
index 0ae01be2..06ea1abb 100644
--- a/hil/commands/migrate_ipmi_info.py
+++ b/hil/commands/migrate_ipmi_info.py
@@ -1,7 +1,8 @@
-"""Helper command for extracting IPMI info from the database.
+"""Helper commands for moving IPMI info from the database to obmd.
+
+This is part of our obmd migration strategy; see the discussion at:
+
 
-This is part of our obmd migration strategy; the idea is that this
-information is then uploaded to obmd.
 """
 import sys
 import json
@@ -36,7 +37,7 @@ def run(self, obmd_base_url, obmd_admin_token):
 def db_extract_ipmi_info():
     """Extract all nodes' ipmi connection info from the database.
 
-    This prints a json object of the form:
+    This returns an dictionary of the form:
 
         {
             "node-23": {
@@ -51,14 +52,19 @@ def db_extract_ipmi_info():
             }
         }
 
-    It will fail if any of the
+    It will fail if any of the nodes in the database have obms with a type
+    other than ipmi.
+
+    This must be run inside of an app context.
     """
     obms = model.Obm.query.all()
 
     info = {}
 
+    ipmi_api_name = 'http://schema.massopencloud.org/haas/v0/obm/ipmi'
+
     for obm in obms:
-        if obm.type != 'ipmi':
+        if obm.type != ipmi_api_name:
             sys.exit(("Node %s{label} has an obm of unspported "
                       "type %s{type}").format({
                           'label': obm.owner.label,
@@ -76,14 +82,16 @@ def db_extract_ipmi_info():
 def obmd_upload_ipmi_info(obmd_base_url, obmd_admin_token, info):
     """Upload nodes' info to obmd.
 
-    The info is loaded from stdin, and should be in the format output
-    by `extract_ipmi_info`.
-    """
-    info = json.load(sys.stdin)
+    `info` should be a dictionary of the form returned by
+    `db_extract_ipmi_info`.
 
-    sess = requests.Session(auth=('admin', obmd_admin_token))
+    `obmd_base_url` is the base URL for the obmd api. i.e. a node named
+    "example_node" would be at the URL `obmd_base_url + '/node/example_node'.`
 
-    # TODO: validate the input.
+    `obmd_admin_token` is the admin token to use when authenticating against
+    obmd.
+    """
+    sess = requests.Session(auth=('admin', obmd_admin_token))
 
     for key, val in info.items():
         sess.put(obmd_base_url + '/node/' + key, data=json.dumps({
@@ -97,6 +105,13 @@ def obmd_upload_ipmi_info(obmd_base_url, obmd_admin_token, info):
 
 
 def db_add_obmd_info(obmd_base_url, obmd_admin_token):
+    """Add obmd connection info to the HIL database.
+
+    This assumes each node is available from obmd at the URL
+    `obmd_base_url + '/node/' + node.label`.
+
+    This must be run inside of an app context.
+    """
     model.Node.query.update({'obmd_admin_token': obmd_admin_token})
     for node in model.Node.query.all():
         node.obmd_uri = obmd_base_url + '/node/' + node.label

From b73a5d37e0b8ad971a35d47cb2dd60e271e85530 Mon Sep 17 00:00:00 2001
From: Ian Denhardt <ian@zenhack.net>
Date: Wed, 7 Feb 2018 17:27:50 -0500
Subject: [PATCH 3/7] Write some tests, fix some bugs.

the script seems to be working now.
---
 hil/commands/migrate_ipmi_info.py      |  17 +++-
 tests/integration/migrate_ipmi_info.py | 126 +++++++++++++++++++++++++
 2 files changed, 139 insertions(+), 4 deletions(-)
 create mode 100644 tests/integration/migrate_ipmi_info.py

diff --git a/hil/commands/migrate_ipmi_info.py b/hil/commands/migrate_ipmi_info.py
index 06ea1abb..7c8bcb35 100644
--- a/hil/commands/migrate_ipmi_info.py
+++ b/hil/commands/migrate_ipmi_info.py
@@ -64,13 +64,21 @@ def db_extract_ipmi_info():
     ipmi_api_name = 'http://schema.massopencloud.org/haas/v0/obm/ipmi'
 
     for obm in obms:
+        # XXX: apparently we've incorrectly specified the relationship between
+        # Obms and nodes, such that the node attribute returns a list, even
+        # though there can only ever be one node. If we were keeping pluggable
+        # obm support for longer, We'd just fix this, but since we're removing
+        # the functionality soon it's easier to just do this and not have to
+        # worry about what other code we might disturb:
+        node = obm.node[0]
+
         if obm.type != ipmi_api_name:
             sys.exit(("Node %s{label} has an obm of unspported "
                       "type %s{type}").format({
-                          'label': obm.owner.label,
+                          'label': node.label,
                           'type': obm.type,
                       }))
-        info[obm.owner.label] = {
+        info[node.label] = {
             'host': obm.host,
             'user': obm.user,
             'password': obm.password,
@@ -91,7 +99,8 @@ def obmd_upload_ipmi_info(obmd_base_url, obmd_admin_token, info):
     `obmd_admin_token` is the admin token to use when authenticating against
     obmd.
     """
-    sess = requests.Session(auth=('admin', obmd_admin_token))
+    sess = requests.Session()
+    sess.auth = ('admin', obmd_admin_token)
 
     for key, val in info.items():
         sess.put(obmd_base_url + '/node/' + key, data=json.dumps({
@@ -115,4 +124,4 @@ def db_add_obmd_info(obmd_base_url, obmd_admin_token):
     model.Node.query.update({'obmd_admin_token': obmd_admin_token})
     for node in model.Node.query.all():
         node.obmd_uri = obmd_base_url + '/node/' + node.label
-    model.db.commit()
+    model.db.session.commit()
diff --git a/tests/integration/migrate_ipmi_info.py b/tests/integration/migrate_ipmi_info.py
new file mode 100644
index 00000000..d3586b5a
--- /dev/null
+++ b/tests/integration/migrate_ipmi_info.py
@@ -0,0 +1,126 @@
+from subprocess import check_call, Popen
+import shutil
+import tempfile
+import json
+import os
+
+import requests
+import pytest
+
+from hil.test_common import fresh_database
+from hil.flaskapp import app
+from hil import config, model
+
+ADMIN_TOKEN = '01234567890123456789012345678901'
+OBMD_BASE_URL = 'http://localhost:8080'
+
+
+@pytest.fixture()
+def tmpdir():
+    path = tempfile.mkdtemp()
+    cwd = os.getcwd()
+    os.chdir(path)
+    yield path
+    os.chdir(cwd)
+    shutil.rmtree(path)
+
+
+@pytest.fixture()
+def configure(tmpdir):
+    cfg = '\n'.join([
+        "[extensions]",
+        "hil.ext.network_allocators.null =",
+        "hil.ext.auth.null =",
+        "hil.ext.obm.ipmi = ",
+        "[devel]",
+        "dry_run = True",
+        "[headnode]",
+        "base_imgs = base-headnode, img1, img2, img3, img4",
+        "[database]",
+        "uri = sqlite:///" + tmpdir + "/hil.db",
+    ])
+    with open(tmpdir + '/hil.cfg', 'w') as f:
+        f.write(cfg)
+    config.load(tmpdir + '/hil.cfg')
+    config.configure_logging()
+    config.load_extensions()
+
+
+fresh_database = pytest.fixture(fresh_database)
+
+
+@pytest.fixture()
+def run_obmd(tmpdir):
+    check_call(['go', 'get', 'github.com/CCI-MOC/obmd'])
+
+    config_file_path = tmpdir + '/obmd-config.json'
+
+    with open(config_file_path, 'w') as f:
+        f.write(json.dumps({
+            'AdminToken': ADMIN_TOKEN,
+            'ListenAddr': ':8080',
+        }))
+
+    proc = Popen(['obmd', '-config', config_file_path])
+    try:
+        yield
+    finally:
+        proc.terminate()
+        proc.wait()
+
+
+pytestmark = pytest.mark.usefixtures('configure',
+                                     'fresh_database',
+                                     'run_obmd')
+
+
+def test_obmd_migrate(tmpdir):
+    from hil.ext.obm.ipmi import Ipmi
+
+    # Add some objects to the hil database:
+    with app.app_context():
+        for i in range(4):
+            node = model.Node(label='node-%d' % i,
+                              obm=Ipmi(user='admin',
+                                       host='10.0.0.%d' % (100 + i),
+                                       password='changeme'))
+            model.db.session.add(node)
+        model.db.session.commit()
+
+    check_call([
+        'hil-admin', 'migrate-ipmi-info',
+        '--obmd-base-url', OBMD_BASE_URL,
+        '--obmd-admin-token', ADMIN_TOKEN,
+    ])
+
+    with app.app_context():
+        for node in model.Node.query.all():
+
+            # Check that the db info was updated correctly:
+            assert node.obmd_admin_token == ADMIN_TOKEN, (
+                "Node %s{label}'s admin token was incorrect: %s{token}"
+                .format(
+                    label=node.label,
+                    token=node.obmd_admin_token,
+                )
+            )
+            assert node.obmd_uri == OBMD_BASE_URL + '/node/' + node.label, (
+                "Node %s{label}'s obmd_uri was incorrect: %s{uri}"
+                .format(
+                    label=node.label,
+                    uri=node.obmd_uri,
+                )
+            )
+
+            # Make sure obmd thinks the nodes are there; if so it should be
+            # possible to get a token:
+            sess = requests.Session()
+            sess.auth = ('admin', ADMIN_TOKEN)
+            resp = sess.post(node.obmd_uri + '/token')
+            assert resp.ok, (
+                "Failure getting token for node %s{label} from obmd; "
+                "response: %s{resp}".format(
+                    label=node.label,
+                    resp=resp,
+                )
+            )

From 194e701003993b23533b942e2c6889b38ebf892d Mon Sep 17 00:00:00 2001
From: Ian Denhardt <ian@zenhack.net>
Date: Wed, 7 Feb 2018 17:34:18 -0500
Subject: [PATCH 4/7] Install golang in CI.

This is necessary for the integration tests I've just written.
---
 .travis.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.travis.yml b/.travis.yml
index 38a798a9..b88ab135 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -17,6 +17,7 @@ addons:
     packages:
       - apache2
       - libapache2-mod-wsgi
+      - golang
 
 branches:
   except:

From c5d07d0dce9cfe5e769683efbf0cce2682e75997 Mon Sep 17 00:00:00 2001
From: Ian Denhardt <ian@zenhack.net>
Date: Wed, 7 Feb 2018 18:13:09 -0500
Subject: [PATCH 5/7] Actually run the new tests in CI.

---
 .travis.yml                      | 1 +
 ci/obmd/run_integration_tests.sh | 4 ++++
 2 files changed, 5 insertions(+)
 create mode 100755 ci/obmd/run_integration_tests.sh

diff --git a/.travis.yml b/.travis.yml
index b88ab135..12495816 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -40,4 +40,5 @@ script:
   - sh -e ci/run_unit_tests.sh
   - sh -e ci/apache/run_integration_tests.sh
   - sh -e ci/keystone/run_integration_tests.sh
+  - sh -e ci/obmd/run_integration_tests.sh
   - sh -e ci/deployment-mock-networks/run_integration_tests.sh
diff --git a/ci/obmd/run_integration_tests.sh b/ci/obmd/run_integration_tests.sh
new file mode 100755
index 00000000..11078b7f
--- /dev/null
+++ b/ci/obmd/run_integration_tests.sh
@@ -0,0 +1,4 @@
+#!/usr/bin/env sh
+set -ex
+
+py.test tests/integration/migrate_ipmi_info.py

From 0ca8b6435c7f5900788a7c23df8a7dc4a0f47a44 Mon Sep 17 00:00:00 2001
From: Ian Denhardt <ian@zenhack.net>
Date: Wed, 7 Feb 2018 18:39:07 -0500
Subject: [PATCH 6/7] Fix pylint errors

---
 hil/commands/migrate_ipmi_info.py      |  8 ++++++--
 tests/integration/migrate_ipmi_info.py | 15 +++++++++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/hil/commands/migrate_ipmi_info.py b/hil/commands/migrate_ipmi_info.py
index 7c8bcb35..8e6c1117 100644
--- a/hil/commands/migrate_ipmi_info.py
+++ b/hil/commands/migrate_ipmi_info.py
@@ -1,9 +1,8 @@
 """Helper commands for moving IPMI info from the database to obmd.
 
 This is part of our obmd migration strategy; see the discussion at:
-
-
 """
+
 import sys
 import json
 
@@ -26,6 +25,11 @@ class MigrateIpmiInfo(Command):
                help='Admin token for obmd'),
     )
 
+    # the correct arguments to this are a function of the available options;
+    # it's normal for subclasses to have implementations with different
+    # arguments.
+    #
+    # pylint: disable=arguments-differ
     def run(self, obmd_base_url, obmd_admin_token):
         server.init()
         with app.app_context():
diff --git a/tests/integration/migrate_ipmi_info.py b/tests/integration/migrate_ipmi_info.py
index d3586b5a..e9053d41 100644
--- a/tests/integration/migrate_ipmi_info.py
+++ b/tests/integration/migrate_ipmi_info.py
@@ -1,3 +1,5 @@
+"""Tests for the `hil-admin migrate-ipmi-info` command."""
+
 from subprocess import check_call, Popen
 import shutil
 import tempfile
@@ -17,6 +19,7 @@
 
 @pytest.fixture()
 def tmpdir():
+    """Create a temporary directory to store various files."""
     path = tempfile.mkdtemp()
     cwd = os.getcwd()
     os.chdir(path)
@@ -27,6 +30,12 @@ def tmpdir():
 
 @pytest.fixture()
 def configure(tmpdir):
+    """Set up HIL configuration.
+
+    This creates a hil.cfg in tmpdir, and loads it. The file needs to be
+    written out separately , since we invoke other commands that read it,
+    besides the test process.
+    """
     cfg = '\n'.join([
         "[extensions]",
         "hil.ext.network_allocators.null =",
@@ -51,6 +60,7 @@ def configure(tmpdir):
 
 @pytest.fixture()
 def run_obmd(tmpdir):
+    """Set up and start obmd."""
     check_call(['go', 'get', 'github.com/CCI-MOC/obmd'])
 
     config_file_path = tmpdir + '/obmd-config.json'
@@ -75,6 +85,11 @@ def run_obmd(tmpdir):
 
 
 def test_obmd_migrate(tmpdir):
+    """The test proper.
+
+    Create some nodes, run the script, and verify that it has done the right
+    thing.
+    """
     from hil.ext.obm.ipmi import Ipmi
 
     # Add some objects to the hil database:

From 520a4be493ce7c0d14d104768b8acc17d6ab79b1 Mon Sep 17 00:00:00 2001
From: Ian Denhardt <ian@zenhack.net>
Date: Thu, 8 Feb 2018 15:42:34 -0500
Subject: [PATCH 7/7] Fix missing issue reference.

Thanks to @naved001 for spotting this.
---
 hil/commands/migrate_ipmi_info.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hil/commands/migrate_ipmi_info.py b/hil/commands/migrate_ipmi_info.py
index 8e6c1117..e2f3f4dd 100644
--- a/hil/commands/migrate_ipmi_info.py
+++ b/hil/commands/migrate_ipmi_info.py
@@ -1,6 +1,6 @@
 """Helper commands for moving IPMI info from the database to obmd.
 
-This is part of our obmd migration strategy; see the discussion at:
+This is part of our obmd migration strategy; see issue #928.
 """
 
 import sys