Merge lp:~cprov/uci-engine/1335753-glance-creds into lp:uci-engine

[Celso Providelo]

Independent set of credentials for ImageBuilder and TestRunner in unit_config.

Resuming Vincent's work for creating an independent set of credentials for using in ImageBuilder and TestRunner, since they may operate in an different environment (cloud) than they are deployed.

Original MP -> https://code.launchpad.net/~vila/uci-engine/1335753-glance-creds/+merge/225191

[lp:~cprov/uci-engine/1335753-glance-creds]

634. By Celso Providelo on 2014-07-02

adding missing test.

635. By Celso Providelo on 2014-07-02

Adding missing docstring.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:635
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/990/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/990/rebuild

review: Needs Fixing (continuous-integration)

[lp:~cprov/uci-engine/1335753-glance-creds]

636. By Celso Providelo on 2014-07-02

Fixing test failures.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:636
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/992/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/992/rebuild

review: Approve (continuous-integration)

[Vincent Ladeuil (vila)]

Typos aside, there is an issue with tests creating and removing unit_config at the root of the branch.

With a full deployment available on hp and GLANCE_* vars defined, I get:

$ ./run-tests
<...>

======================================================================
ERROR: tests.test_test_runner.TestTestRunner.test_process_ticket
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/test_test_runner.py", line 90, in test_process_ticket
    raise_errors=True)
  File "/home/vila/ci/uci-engine/reviews/1335753-glance-creds/ci-utils/ci_utils/amqp_utils.py", line 99, in send
    conn, channel = declare_queue(queue_name, config)
  File "/home/vila/ci/uci-engine/reviews/1335753-glance-creds/ci-utils/ci_utils/amqp_utils.py", line 83, in declare_queue
    raise e
error: [Errno 110] Connection timed out
======================================================================
FAIL: juju-deployer.test_deploy.TestCheckEnvironment.test_check_environment_defaults
----------------------------------------------------------------------
Traceback (most recent call last):
  File "build/bdist.linux-x86_64/egg/mock.py", line 1201, in patched
    return func(*args, **keywargs)
  File "juju-deployer/test_deploy.py", line 77, in test_check_environment_defaults
    self.assertEqual('', os.environ['GLANCE_OS_USERNAME'])
  File "/usr/lib/python2.7/unittest/case.py", line 515, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/usr/lib/python2.7/unittest/case.py", line 508, in _baseAssertEqual
    raise self.failureException(msg)
AssertionError: '' != '<email address hidden>'

Ran 594 tests in 501.909s
FAILED (failures=2)

test_process_tickey is now understood as missing an exposed rabbit.

But re-running:
$ ./run-tests
<...>
======================================================================
ERROR: test_runner.tstrun.tests.test_data_store.TestDataStore.test_put_file_stores_content
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_runner/tstrun/tests/test_data_store.py", line 62, in setUp
    tstrun.get_auth_config(), public=True)
  File "test_runner/tstrun/__init__.py", line 28, in get_auth_config
    with open(path) as f:
IOError: [Errno 2] No such file or directory: '/home/vila/ci/uci-engine/reviews/1335753-glance-creds/unit_config'
======================================================================
ERROR: test_runner.tstrun.tests.test_data_store.TestDataStore.test_store_created_empty
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_runner/tstrun/tests/test_data_store.py", line 62, in setUp
    tstrun.get_auth_config(), public=True)
  File "test_runner/tstrun/__init__.py", line 28, in get_auth_config
    with open(path) as f:
IOError: [Errno 2] No such file or directory: '/home/vila/ci/uci-engine/reviews/1335753-glance-creds/unit_config'
======================================================================
ERROR: test_runner.tstrun.tests.test_testbed.TestTestbed.test_create_new_ssh_key
-----------------------------------------------------...

review: Needs Fixing

[Vincent Ladeuil (vila)]

I'll dig further and try to fix those.

[lp:~cprov/uci-engine/1335753-glance-creds]

637. By Celso Providelo on 2014-07-02

merge trunk

638. By Celso Providelo on 2014-07-02

FIxing typos and test env var isolation.

[Celso Providelo (cprov)]

The fact that we have tests that depend on 'unit_config' on the project root is wrong, in fact. If it's not committed in the VCS it should be handled ad-hoc by a testing fixture.

Also, the only test that needs to exercise disk operations with 'unit_config' is the one I've added in this branch, all the rest of the system should mock get_auth_config() and use whatever result they expect.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:638
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/996/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/996/rebuild

review: Approve (continuous-integration)

[Vincent Ladeuil (vila)]

> The fact that we have tests that depend on 'unit_config' on the project root
> is wrong, in fact.
> If it's not committed in the VCS it should be handled ad-
> hoc by a testing fixture.

There is more than one way to do it :)

The tests that rely on unit_config do use fixtures but still let the user decide whether or not he want to run those tests by creating that file or not.

If you have a better way that doesn't break the existing tests, I'm fine with that.

>
> Also, the only test that needs to exercise disk operations with 'unit_config'
> is the one I've added in this branch, all the rest of the system should mock
> get_auth_config() and use whatever result they expect.

Yes, that's the point I raised when you introduced deploy.install_unit_config()... I don't know which tests required that.

[Andy Doan (doanac)]

I mostly like it. I noticed a bug a though. I think any test failing because it assumes the location of the unit-config should be considered broken. I've got MP's on the way:

 https://code.launchpad.net/~doanac/uci-engine/lander-code-layout/+merge/225046

that are placing unit_config *outside* our code directory, so moving forward this assumption will definitely not work.

[Celso Providelo (cprov)]

Thanks for the review Andy, comments addressed inline.

[lp:~cprov/uci-engine/1335753-glance-creds]

639. By Celso Providelo on 2014-07-02

Re-implement unit_config module get() for not breaking possibly untested code.

[Andy Doan (doanac)]

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:639
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1001/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1001/rebuild

review: Approve (continuous-integration)

[lp:~cprov/uci-engine/1335753-glance-creds]

640. By Celso Providelo on 2014-07-03

unit_config.get() not deprecated anymore, it's useful.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:640
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1006/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1006/rebuild

review: Approve (continuous-integration)

[lp:~cprov/uci-engine/1335753-glance-creds]

641. By Celso Providelo on 2014-07-04

merge trunk

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:641
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1017/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1017/rebuild

review: Needs Fixing (continuous-integration)

Unmerged revisions

641. By Celso Providelo on 2014-07-04

merge trunk

640. By Celso Providelo on 2014-07-03

unit_config.get() not deprecated anymore, it's useful.

639. By Celso Providelo on 2014-07-02

Re-implement unit_config module get() for not breaking possibly untested code.

638. By Celso Providelo on 2014-07-02

FIxing typos and test env var isolation.

637. By Celso Providelo on 2014-07-02

merge trunk

636. By Celso Providelo on 2014-07-02

Fixing test failures.

635. By Celso Providelo on 2014-07-02

Adding missing docstring.

634. By Celso Providelo on 2014-07-02

adding missing test.

633. By Celso Providelo on 2014-07-02

Making GLANCE_* variables optional and adding tests for ci_utils.unit_config.

=== added file 'ci-utils/ci_utils/tests/test_unit_config.py'
--- ci-utils/ci_utils/tests/test_unit_config.py	1970-01-01 00:00:00 +0000
+++ ci-utils/ci_utils/tests/test_unit_config.py	2014-07-04 21:48:29 +0000
@@ -0,0 +1,125 @@
+#!/usr/bin/env python
+# Ubuntu CI Engine
+# Copyright 2014 Canonical Ltd.
+
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU Affero General Public License version 3, as
+# published by the Free Software Foundation.
+
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU Affero General Public License for more details.
+
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+"""Tests for the 'unit_config' loader."""
+import os
+import unittest
+
+from ci_utils import unit_config
+
+
+class TestUnitConfig(unittest.TestCase):
+
+    sample_content = {
+        'auth_user': 'a_user',
+    }
+
+    def setUp(self):
+        """Reset configuration cache."""
+        unit_config._reset_config()
+        self.config_path = unit_config._get_config_path()
+
+    def test_asserts_exists(self):
+        # `get_auth_config` asserts the 'unit_config' path exists.
+        with self.assertRaises(AssertionError) as cm:
+            unit_config.get_auth_config()
+        msg = 'unit_config does not exist! ({})'.format(self.config_path)
+        self.assertEqual(msg, cm.exception.message)
+
+    def test_asserts_not_empty(self):
+        # `get_auth_config` asserts the 'unit_config' is not an empty file.
+        with open(self.config_path, 'w') as fd:
+            fd.write('')
+        with self.assertRaises(AssertionError) as cm:
+            unit_config.get_auth_config()
+        msg = 'unit_config is empty! ({})'.format(self.config_path)
+        self.assertEqual(msg, cm.exception.message)
+
+    def create_config(self, content=None):
+        """Creates a 'unit_config' pseudo-YAML for tests."""
+        options = self.sample_content.copy()
+        if content is not None:
+            options.update(content)
+        with open(self.config_path, 'w') as fd:
+            for k, v in options.iteritems():
+                fd.write('{}: {}\n'.format(k, v))
+
+        def drop_if_exists():
+            if os.path.exists(self.config_path):
+                os.remove(self.config_path)
+        # Delete testing config if it exists.
+        self.addCleanup(drop_if_exists)
+
+    def test_loading(self):
+        # `get_auth_config` returns a dictionary corresponding to the
+        # YAML content. 'auth_*' attributes are mirrored to 'glance_auth_*'
+        self.create_config()
+        config = unit_config.get_auth_config()
+        self.assertEqual(
+            {'auth_user': 'a_user', 'glance_auth_user': 'a_user'}, config)
+
+    def test_get(self):
+        # `get` returns specific configuration options/attributes.
+        self.create_config()
+        self.assertEqual('a_user', unit_config.get('auth_user'))
+        self.assertEqual('a_user', unit_config.get('glance_auth_user'))
+
+    def test_loading_glance(self):
+        # `get_auth_config` preserves original 'glance_auth_*'
+        # attributes if 'glance_auth_user' is not empty.
+        glance_spec = {
+            'auth_url': 'a_url',
+            'glance_auth_user': 'another_user',
+            'glance_auth_url': 'another_url',
+        }
+        self.create_config(content=glance_spec)
+        config = unit_config.get_auth_config()
+        self.assertEqual(
+            {'auth_url': 'a_url',
+             'auth_user': 'a_user',
+             'glance_auth_url': 'another_url',
+             'glance_auth_user': 'another_user'},
+            config)
+
+    def test_cache(self):
+        # `get_auth_config` loads configuration file contents only once
+        # and caches it's (parsed) results. `_reset_config` is a testing
+        # artifact for cleaning its cache (use with care!).
+        self.create_config()
+        config = unit_config.get_auth_config()
+        self.create_config(content={'auth_user': 'fresh'})
+        cached = unit_config.get_auth_config()
+        self.assertEqual(cached, config)
+        unit_config._reset_config()
+        fresh = unit_config.get_auth_config()
+        self.assertEqual(
+            {'auth_user': 'fresh', 'glance_auth_user': 'fresh'},
+            fresh)
+
+    def test_no_cache_poisoning(self):
+        # `get_auth_config` returns a copy of the configuration cache,
+        # so its cache cannot be *poisoned*.
+        self.create_config()
+        config = unit_config.get_auth_config()
+        config['auth_user'] = 'someone-else'
+        config['auth_poison'] = 'bad-data'
+        new_config = unit_config.get_auth_config()
+        self.assertEqual(
+            {'auth_user': 'a_user', 'glance_auth_user': 'a_user'},
+            new_config)
+
+
+if __name__ == "__main__":
+    unittest.main()

=== modified file 'ci-utils/ci_utils/unit_config.py'
--- ci-utils/ci_utils/unit_config.py	2014-05-14 09:11:00 +0000
+++ ci-utils/ci_utils/unit_config.py	2014-07-04 21:48:29 +0000
@@ -12,35 +12,76 @@
 
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
+"""Application 'unit_config' loader."""
 import os
-
 import yaml
 
 
 HERE = os.path.abspath(os.path.dirname(__file__))
-
-
 _unit_config = None
 
 
-def _load():
-    path = os.path.join(HERE, '../../unit_config')
+def _reset_config():
+    """Testing utility for reseting unit_config cache."""
+    global _unit_config
+    _unit_config = None
+
+
+def _get_config_path():
+    """Testing utility for retrieving the configuration path."""
+    return os.path.join(HERE, '../../unit_config')
+
+
+def _load_config():
+    """Load the configuration file as YAML and cache it.
+
+    Asserts the expected 'unit_config' exists and contains valid YAML.
+    """
+    global _unit_config
+    path = _get_config_path()
+    assert os.path.exists(path), (
+        'unit_config does not exist! ({})'.format(path))
     with open(path) as f:
-        global _unit_config
         _unit_config = yaml.safe_load(f)
+    assert _unit_config is not None, (
+        'unit_config is empty! ({})'.format(path))
+
+
+def get_auth_config():
+    """Return a configuration dictionary for applications.
+
+    Loads the deployment 'unit_config' YAML file, if it wasn't loaded
+    before, and return the corresponding python dictionary.
+
+    If 'glance_auth_user' is undefined (empty), 'glance_*' parameters
+    are filled with the corresponding 'auth_*' values, i.e., GLANCE_*
+    variables only have to be defined when the ci-airline 'testbed'
+    environment is different than where the system is deployed e.g.:
+    - stagingstack + scalingstack
+    - stagingstack + hpcloud
+    """
+    global _unit_config
+    if _unit_config is None:
+        _load_config()
+
+    # Apply configuration defaults/overrides in a copy of the cached
+    # configuration dictionary. This way applications will not be able
+    # to poisoning the cached value.
+    cfg = _unit_config.copy()
+
+    # 'glance_*' defaults.
+    if cfg.get('glance_auth_user') is None:
+        auth_keys = [k for k in cfg.keys() if k.startswith('auth_')]
+        for auth_key in auth_keys:
+            glance_key = 'glance_' + auth_key
+            cfg[glance_key] = cfg[auth_key]
+
+    return cfg
 
 
 def get(key):
-    if not _unit_config:
-        _load()
-    return _unit_config[key]
-
-
-def get_auth_config():
-    if not _unit_config:
-        _load()
-    return _unit_config
+    """Get specific 'unit_config' options."""
+    return get_auth_config()[key]
 
 
 def is_hpcloud(string=None):

=== modified file 'juju-deployer/configs/unit_config.yaml.tmpl'
--- juju-deployer/configs/unit_config.yaml.tmpl	2014-07-01 15:58:10 +0000
+++ juju-deployer/configs/unit_config.yaml.tmpl	2014-07-04 21:48:29 +0000
@@ -1,26 +1,34 @@
-# for nova direct access (swift, image builder, test runner among others)
+#--------------------------------------------------------------
+# This file is managed by Juju; ANY CHANGES WILL BE OVERWRITTEN
+#--------------------------------------------------------------
+# CI-Airline unit configuration file.
+
+# Options for system deployment and common Swift access.
 auth_url: $OS_AUTH_URL
 auth_user: $OS_USERNAME
 auth_password: $OS_PASSWORD
 auth_tenant_name: $OS_TENANT_NAME
 auth_region: $OS_REGION_NAME
 
+# Options for Gatekeeper Swift tempurl generation and upload
+# signature validation.
 auth_access_key_id: $HP_ACCESS_KEY_ID
 auth_tenant_id: $HP_TENANT_ID
 tempurl_signing_key: $CI_TEMPURL_SIGNING_KEY
 tempurl_expires: $CI_TEMPURL_EXPIRES
 tempurl_public_host: $CI_TEMPURL_PUBLIC_HOST
+keyring_uri: $CI_KEYRING_URI
+keyserver_url: $CI_KEYSERVER_URL
 
+# Options for acessing a isolated environment for
+# Image-builder (Glance) and Test-runner (Nova).
+# If not provided (empty), the system environment is used.
 glance_auth_url: $GLANCE_OS_AUTH_URL
 glance_auth_user: $GLANCE_OS_USERNAME
 glance_auth_password: $GLANCE_OS_PASSWORD
 glance_auth_tenant_name: $GLANCE_OS_TENANT_NAME
 glance_auth_region: $GLANCE_OS_REGION_NAME
 
-
-keyring_uri: $CI_KEYRING_URI
-keyserver_url: $CI_KEYSERVER_URL
-
 # for launchpad apis
 # and see: https://help.launchpad.net/API/SigningRequests
 # and see ../../create_lp_creds.py

=== modified file 'juju-deployer/deploy.py'
--- juju-deployer/deploy.py	2014-07-03 17:25:58 +0000
+++ juju-deployer/deploy.py	2014-07-04 21:48:29 +0000
@@ -94,11 +94,11 @@
     '''
     cheetah_vars = {
         'OS_USERNAME': None,
-        'GLANCE_OS_USERNAME': None,
-        'GLANCE_OS_AUTH_URL': None,
-        'GLANCE_OS_PASSWORD': None,
-        'GLANCE_OS_TENANT_NAME': None,
-        'GLANCE_OS_REGION_NAME': None,
+        'GLANCE_OS_USERNAME': '',
+        'GLANCE_OS_AUTH_URL': '',
+        'GLANCE_OS_PASSWORD': '',
+        'GLANCE_OS_TENANT_NAME': '',
+        'GLANCE_OS_REGION_NAME': '',
         'CI_CODE_SOURCE': 'branch',
         'CI_BRANCH': 'lp:uci-engine',
         'CI_PAYLOAD_URL': 'n/a',
@@ -154,7 +154,7 @@
         ERROR: Missing required environment variables:
           {}
         Please ensure the novarc file has been sourced.''')
-        sys.exit(template.format('\n  '.join(missing_required)))
+        sys.exit(template.format('\n  '.join(sorted(missing_required))))
 
 
 def bootstrap():

=== modified file 'juju-deployer/test_deploy.py'
--- juju-deployer/test_deploy.py	2014-07-03 17:25:58 +0000
+++ juju-deployer/test_deploy.py	2014-07-04 21:48:29 +0000
@@ -36,11 +36,6 @@
 
     DEFAULT_ENV = {
         'OS_USERNAME': 'os-user',
-        'GLANCE_OS_USERNAME': 'glance-os-user',
-        'GLANCE_OS_AUTH_URL': 'glance-url',
-        'GLANCE_OS_REGION_NAME': 'glance-region',
-        'GLANCE_OS_TENANT_NAME': 'glance-tenant',
-        'GLANCE_OS_PASSWORD': 'glance-password',
         'OS_AUTH_URL': 'something.stack',
         'CI_OAUTH_CONSUMER_KEY': 'key',
         'CI_OAUTH_TOKEN': 'token',
@@ -61,6 +56,7 @@
             'CI_TEMPURL_SIGNING_KEY': None,
             'CI_KEYRING_URI': None,
             'CI_KEYSERVER_URL': None,
+            'GLANCE_OS_USERNAME': None,
             'HP_ACCESS_KEY_ID': None,
             'HP_TENANT_ID': None,
         }
@@ -79,6 +75,7 @@
         self.assertEqual(os.environ['CI_KEYRING_URI'], '')
         self.assertEqual(
             os.environ['CI_KEYSERVER_URL'], 'keyserver.ubuntu.com')
+        self.assertEqual('', os.environ['GLANCE_OS_USERNAME'])
         # CI_TEMPURL_SIGNING_KEY used for canonistack/prodstack TempURL
         # generation and `deploy.py` generated a new random HMAC-SHA1
         # (40 chars) key for each deployment.