Use cinderng module in controller.sls

Changed the way how volume types for backends are being installed.
Previously it was done by calling an external command grepping its
output. Now the cinderng module/state is used because using API
should be the most backwards compatible way to interract with Cinder.

Cinderng is switched to APIv2 from v3 because it's the most available
API version across releases.

While here, fixed the PEP8 compliance in the sources and added missing
indentity sections in some tests.

Change-Id: I6d1cd509f8e349ab15b698e3ebc1583b80065d4c
Fixes: PROD-14654
diff --git a/_modules/cinderng.py b/_modules/cinderng.py
index cd71348..a5c7d42 100644
--- a/_modules/cinderng.py
+++ b/_modules/cinderng.py
@@ -1,12 +1,12 @@
 # -*- coding: utf-8 -*-
 import logging
-from functools import wraps
+
 LOG = logging.getLogger(__name__)
 
 # Import third party libs
 HAS_CINDER = False
 try:
-    from cinderclient.v3 import client
+    from cinderclient.client import Client
     HAS_CINDER = True
 except ImportError:
     pass
@@ -15,58 +15,76 @@
 
 
 def __virtual__():
-    '''
+    """
     Only load this module if cinder
     is installed on this minion.
-    '''
+    """
     if HAS_CINDER:
         return 'cinderng'
     return False
 
+
 def _authng(profile=None):
-    '''
+    """
     Set up cinder credentials
-    '''
+    """
     credentials = {
         'username': profile['user'],
         'password': profile['password'],
         'project_id': profile['project_id'],
-        'auth_url': profile['protocol'] + "://" + profile['host'] + ":" + str(profile['port']) + "/v3",
+        'auth_url': "{}://{}:{}/v2.0".format(
+            profile['protocol'],
+            profile['host'],
+            profile['port']
+        ),
         'endpoint_type': profile['endpoint_type'],
         'certificate': profile['certificate'],
         'region_name': profile['region_name']
     }
     return credentials
 
+
 def create_conn(cred=None):
-    '''
+    """
     create connection
-    '''
-    nt = client.Client(username=cred['username'], api_key=cred['password'], project_id=cred['project_id'], auth_url=cred['auth_url'], endpoint_type=cred['endpoint_type'], cacert=cred['certificate'], region_name=cred['region_name'])
+    """
+    nt = Client(
+        '2',
+        username=cred['username'],
+        api_key=cred['password'],
+        project_id=cred['project_id'],
+        auth_url=cred['auth_url'],
+        endpoint_type=cred['endpoint_type'],
+        cacert=cred['certificate'],
+        region_name=cred['region_name']
+    )
     return nt
 
+
 def list_volumes(profile=None, **kwargs):
-    '''
+    """
     Return list of cinder volumes.
-    '''
+    """
     cred = _authng(profile)
     nt = create_conn(cred)
     return nt.volumes.list()
 
+
 def list_volume_type(profile=None, **kwargs):
-    '''
+    """
     Return list of volume types
-    '''
+    """
     cred = _authng(profile)
     nt = create_conn(cred)
     return nt.volume_types.list()
 
+
 def get_volume_type(type_name, profile=None, **kwargs):
-    '''
+    """
     Returns id of the specified volume type name
-    '''
+    """
     vt_id = None
-    vt_list = list_volume_type(profile);
+    vt_list = list_volume_type(profile)
     for vt in vt_list:
         if vt.name == type_name:
             vt_id = vt.id
@@ -82,10 +100,11 @@
     else:
         return
 
+
 def create_volume_type(type_name, profile=None, **kwargs):
-    '''
+    """
     Create cinder volume type
-    '''
+    """
     vt = get_volume_type(type_name, profile)
     if not vt:
         cred = _authng(profile)
@@ -100,9 +119,9 @@
 
 
 def get_keys_volume_type(type_name, profile=None, **kwargs):
-    '''
+    """
     Return extra specs of the specified volume type.
-    '''
+    """
 
     vt = get_volume_type(type_name, profile)
     if vt:
@@ -113,10 +132,11 @@
     else:
         return
 
+
 def set_keys_volume_type(type_name, keys={}, profile=None, **kwargs):
-    '''
+    """
     Set extra specs of the specified volume type.
-    '''
+    """
     set_keys = False
     vt = get_volume_type(type_name, profile)
     if vt:
diff --git a/_states/cinderng.py b/_states/cinderng.py
index e39a4d0..18a0979 100644
--- a/_states/cinderng.py
+++ b/_states/cinderng.py
@@ -1,29 +1,33 @@
 # -*- coding: utf-8 -*-
-'''
+"""
 Management of Cinder resources
 ===============================
 :depends:   - cinderclient Python module
-'''
+"""
+
 import ast
 import logging
-from functools import wraps
+
 LOG = logging.getLogger(__name__)
 
 
 def __virtual__():
-    '''
+    """
     Only load if python-cinderclient is present in __salt__
-    '''
+    """
     return 'cinderng'
 
+
 def volume_type_present(name=None, profile=None):
-    '''
+    """
     Ensures that the specified volume type is present.
-    '''
-    ret = {'name': name,
-           'changes': {},
-           'result': True,
-           'comment': 'Volume type "{0}" already exists'.format(name)}
+    """
+    ret = {
+        'name': name,
+        'changes': {},
+        'result': True,
+        'comment': 'Volume type "{0}" already exists'.format(name)
+    }
     signal = __salt__['cinderng.create_volume_type'](name, profile)
     if 'exists' in signal:
         pass
@@ -31,71 +35,93 @@
         ret['comment'] = 'Volume type {0} has been created'.format(name)
         ret['changes']['Volume type'] = 'Created'
     elif 'failed' in signal:
-        ret = {'name': name,
-               'changes': {},
-               'result': False,
-               'comment': 'Volume type "{0}" failed to create'.format(name)}
+        ret = {
+            'name': name,
+            'changes': {},
+            'result': False,
+            'comment': 'Volume type "{0}" failed to create'.format(name)
+        }
     return ret
 
+
 def volume_type_key_present(name=None, key=None, value=None, profile=None):
-    '''
+    """
     Ensures that the extra specs are present on a volume type.
-    '''
+    """
     keys = "{u'" + key + "': u'" + value + "'}"
     keys = ast.literal_eval(keys)
-    ret = {'name': name,
-           'changes': {},
-           'result': True,
-           'comment': 'Volume type keys "{0}" in volume type "{1}" already exist'.format(keys, name)}
+    ret = {
+        'name': name,
+        'changes': {},
+        'result': True,
+        'comment': 'Volume type keys "{0}" '
+                   'in volume type "{1}" already exist'.format(keys, name)
+    }
     signal = __salt__['cinderng.set_keys_volume_type'](name, keys, profile)
     if 'exist' in signal:
         pass
     elif 'updated' in signal:
-        ret['comment'] = 'Volume type keys "{0}" in volume type "{1}" have been updated'.format(keys, name)
+        ret['comment'] = 'Volume type keys "{0}" in volume type "{1}" ' \
+                         'have been updated'.format(keys, name)
         ret['changes']['Volume type keys'] = 'Updated'
     elif 'failed' in signal:
-        ret = {'name': name,
-               'changes': {},
-               'result': False,
-               'comment': 'Volume type keys "{0}" in volume type "{1}" failed to update'.format(keys, name)}
+        ret = {
+            'name': name,
+            'changes': {},
+            'result': False,
+            'comment': 'Volume type keys "{0}" in volume type "{1}" '
+                       'failed to update'.format(keys, name)
+        }
     elif 'not found' in signal:
-        ret = {'name': name,
-               'changes': {},
-               'result': False,
-               'comment': 'Volume type "{0}" was not found'.format(name)}
+        ret = {
+            'name': name,
+            'changes': {},
+            'result': False,
+            'comment': 'Volume type "{0}" was not found'.format(name)
+        }
     return ret
 
 
 def _already_exists(name, resource):
-    changes_dict = {'name': name,
-                    'changes': {},
-                    'result': True}
-    changes_dict['comment'] = \
-        '{0} {1} already exists'.format(resource, name)
+    changes_dict = {
+        'name': name,
+        'changes': {},
+        'result': True,
+        'comment': '{0} {1} already exists'.format(resource, name)
+    }
     return changes_dict
 
 
 def _created(name, resource, resource_definition):
-    changes_dict = {'name': name,
-                    'changes': resource_definition,
-                    'result': True,
-                    'comment': '{0} {1} created'.format(resource, name)}
+    changes_dict = {
+        'name': name,
+        'changes': resource_definition,
+        'result': True,
+        'comment': '{0} {1} created'.format(resource, name)
+    }
     return changes_dict
 
+
 def _updated(name, resource, resource_definition):
-    changes_dict = {'name': name,
-                    'changes': resource_definition,
-                    'result': True,
-                    'comment': '{0} {1} tenant was updated'.format(resource, name)}
+    changes_dict = {
+        'name': name,
+        'changes': resource_definition,
+        'result': True,
+        'comment': '{0} {1} tenant was updated'.format(resource, name)
+    }
     return changes_dict
 
+
 def _update_failed(name, resource):
-    changes_dict = {'name': name,
-                    'changes': {},
-                    'comment': '{0} {1} failed to update'.format(resource, name),
-                    'result': False}
+    changes_dict = {
+        'name': name,
+        'changes': {},
+        'comment': '{0} {1} failed to update'.format(resource, name),
+        'result': False
+    }
     return changes_dict
 
+
 def _no_change(name, resource, test=False):
     changes_dict = {'name': name,
                     'changes': {},
diff --git a/cinder/controller.sls b/cinder/controller.sls
index 05c3c99..ab189d9 100644
--- a/cinder/controller.sls
+++ b/cinder/controller.sls
@@ -125,6 +125,17 @@
 
 {%- if not grains.get('noservices', False) %}
 
+{%- set identity = controller.identity %}
+{%- set credentials = {'host': identity.host,
+                       'user': identity.user,
+                       'password': identity.password,
+                       'project_id': identity.tenant,
+                       'port': identity.get('port', 35357),
+                       'protocol': identity.get('protocol', 'http'),
+                       'region_name': identity.get('region_name', 'RegionOne'),
+                       'endpoint_type': identity.get('endpoint_type', 'internalURL'),
+                       'certificate': identity.get('certificate', 'None')} %}
+
 {%- for backend_name, backend in controller.get('backend', {}).iteritems() %}
 
 {%- if backend.engine is defined and backend.engine == 'nfs' or (backend.engine == 'netapp' and backend.storage_protocol == 'nfs') %}
@@ -154,20 +165,20 @@
 {%- endif %}
 
 cinder_type_create_{{ backend_name }}:
-  cmd.run:
-  - name: "source /root/keystonerc; cinder type-create {{ backend.type_name }}"
-  - unless: "source /root/keystonerc; cinder type-list | grep {{ backend.type_name }}"
-  - shell: /bin/bash
+  cinderng.volume_type_present:
+  - name: {{ backend.type_name }}
+  - profile: {{ credentials }}
   - require:
     - service: cinder_controller_services
 
 cinder_type_update_{{ backend_name }}:
-  cmd.run:
-  - name: "source /root/keystonerc; cinder type-key {{ backend.type_name }} set volume_backend_name={{ backend_name }}"
-  - unless: "source /root/keystonerc; cinder extra-specs-list | grep \"{u'volume_backend_name': u'{{ backend_name }}'}\""
-  - shell: /bin/bash
+  cinderng.volume_type_key_present:
+  - name: {{ backend.type_name }}
+  - key: volume_backend_name
+  - value: {{ backend_name }}
+  - profile: {{ credentials }}
   - require:
-    - cmd: cinder_type_create_{{ backend_name }}
+    - cinderng: cinder_type_create_{{ backend_name }}
 
 {%- endfor %}
 
diff --git a/tests/pillar/netapp.sls b/tests/pillar/netapp.sls
index 5390d66..5add497 100644
--- a/tests/pillar/netapp.sls
+++ b/tests/pillar/netapp.sls
@@ -9,6 +9,14 @@
       user: openstack
       password: pwd
       virtual_host: '/openstack'
+    identity:
+      engine: keystone
+      host: 127.0.0.1
+      port: 35357
+      tenant: service
+      user: cinder
+      password: pwd
+      region: regionOne
     backend:
       netapp:
         engine: netapp
diff --git a/tests/pillar/nfs.sls b/tests/pillar/nfs.sls
index f882c79..c53e486 100644
--- a/tests/pillar/nfs.sls
+++ b/tests/pillar/nfs.sls
@@ -10,6 +10,14 @@
       user: openstack
       password: pwd
       virtual_host: '/openstack'
+    identity:
+      engine: keystone
+      host: 127.0.0.1
+      port: 35357
+      tenant: service
+      user: cinder
+      password: pwd
+      region: regionOne
     backend:
       nfs-driver:
         engine: nfs