Renamed modules, fixed docs, renamed variables, and cleaned up logic.

11 files changed, 165 insertions, 126 deletions

diff --git a/roles/lib_openshift/src/ansible/oc_adm_registry.py b/roles/lib_openshift/src/ansible/oc_adm_registry.py
index 5fa9e1028..a49b84589 100644
--- a/roles/lib_openshift/src/ansible/oc_adm_registry.py
+++ b/roles/lib_openshift/src/ansible/oc_adm_registry.py
@@ -28,6 +28,9 @@ def main():
             edits=dict(default=None, type='list'),
             enforce_quota=dict(default=False, type='bool'),
             force=dict(default=False, type='bool'),
+            daemonset=dict(default=False, type='bool'),
+            tls_key=dict(default=None, type='str'),
+            tls_certificate=dict(default=None, type='str'),
         ),
 
         supports_check_mode=True,
diff --git a/roles/lib_openshift/src/class/oc_adm_registry.py b/roles/lib_openshift/src/class/oc_adm_registry.py
index bcb098663..f11737086 100644
--- a/roles/lib_openshift/src/class/oc_adm_registry.py
+++ b/roles/lib_openshift/src/class/oc_adm_registry.py
@@ -42,7 +42,7 @@ class Registry(OpenShiftCLI):
                                {'kind': 'svc', 'name': self.config.name},
                               ]
 
-        self.__registry_prep = None
+        self.__prepared_registry = None
         self.volume_mounts = []
         self.volumes = []
         if self.config.config_options['volume_mounts']['value']:
@@ -83,24 +83,24 @@ class Registry(OpenShiftCLI):
         self.svc = config
 
     @property
-    def registry_prep(self):
-        ''' registry_prep property '''
-        if not self.__registry_prep:
-            results = self.prep_registry()
+    def prepared_registry(self):
+        ''' prepared_registry property '''
+        if not self.__prepared_registry:
+            results = self._prepare_registry()
             if not results:
                 raise RegistryException('Could not perform registry preparation.')
-            self.__registry_prep = results
+            self.__prepared_registry = results
 
-        return self.__registry_prep
+        return self.__prepared_registry
 
-    @registry_prep.setter
-    def registry_prep(self, data):
-        ''' setter method for registry_prep attribute '''
-        self.__registry_prep = data
+    @prepared_registry.setter
+    def prepared_registry(self, data):
+        ''' setter method for prepared_registry attribute '''
+        self.__prepared_registry = data
 
-    def force_registry_prep(self):
+    def force_prepare_registry(self):
         '''force a registry prep'''
-        self.registry_prep = None
+        self._prepare_registry = None
 
     def get(self):
         ''' return the self.registry_parts '''
@@ -134,7 +134,7 @@ class Registry(OpenShiftCLI):
 
         return parts
 
-    def prep_registry(self):
+    def _prepare_registry(self):
         ''' prepare a registry for instantiation '''
         options = self.config.to_option_list()
 
@@ -175,14 +175,18 @@ class Registry(OpenShiftCLI):
         service_file = Utils.create_tmp_file_from_contents('service', service.yaml_dict)
         deployment_file = Utils.create_tmp_file_from_contents('deploymentconfig', deploymentconfig.yaml_dict)
 
-        return {"service": service, "service_file": service_file,
-                "deployment": deploymentconfig, "deployment_file": deployment_file}
+        return {"service": service,
+                "service_file": service_file,
+                "service_update": False,
+                "deployment": deploymentconfig,
+                "deployment_file": deployment_file,
+                "deployment_update": False}
 
     def create(self):
         '''Create a registry'''
         results = []
         for config_file in ['deployment_file', 'service_file']:
-            results.append(self._create(self.registry_prep[config_file]))
+            results.append(self._create(self.prepared_registry[config_file]))
 
         # Clean up returned results
         rval = 0
@@ -196,7 +200,7 @@ class Registry(OpenShiftCLI):
     def update(self):
         '''run update for the registry.  This performs a delete and then create '''
         # Store the current service IP
-        self.force_registry_prep()
+        self.force_prepare_registry()
 
         self.get()
         if self.service:
@@ -207,21 +211,23 @@ class Registry(OpenShiftCLI):
             if portip:
                 self.portal_ip = portip
 
-        parts = self.delete(complete=False)
-        for part in parts:
-            if part['returncode'] != 0:
-                if part.has_key('stderr') and 'not found' in part['stderr']:
-                    # the object is not there, continue
-                    continue
-                # something went wrong
-                return parts
+        #parts = self.delete(complete=False)
+        #for part in parts:
+        #    if part['returncode'] != 0:
+        #        if part.has_key('stderr') and 'not found' in part['stderr']:
+        #            # the object is not there, continue
+        #            continue
+        #        # something went wrong
+        #        return parts
 
         # Ugly built in sleep here.
         #time.sleep(10)
 
         results = []
-        results.append(self._create(self.registry_prep['deployment_file']))
-        results.append(self._replace(self.registry_prep['service_file']))
+        if self.prepared_registry['deployment_update']:
+            results.append(self._replace(self.prepared_registry['deployment_file']))
+        if self.prepared_registry['service_update']:
+            results.append(self._replace(self.prepared_registry['service_file']))
 
         # Clean up returned results
         rval = 0
@@ -277,11 +283,11 @@ class Registry(OpenShiftCLI):
             return True
 
         exclude_list = ['clusterIP', 'portalIP', 'type', 'protocol']
-        if not Utils.check_def_equal(self.registry_prep['service'].yaml_dict,
+        if not Utils.check_def_equal(self.prepared_registry['service'].yaml_dict,
                                      self.service.yaml_dict,
                                      exclude_list,
                                      verbose):
-            return True
+            self.prepared_registry['service_update'] = True
 
         exclude_list = ['dnsPolicy',
                         'terminationGracePeriodSeconds',
@@ -297,14 +303,13 @@ class Registry(OpenShiftCLI):
                         'activeDeadlineSeconds', # added in 1.5 for timeouts
                        ]
 
-        if not Utils.check_def_equal(self.registry_prep['deployment'].yaml_dict,
+        if not Utils.check_def_equal(self.prepared_registry['deployment'].yaml_dict,
                                      self.deploymentconfig.yaml_dict,
                                      exclude_list,
                                      verbose):
-            return True
-
-        return False
+            self.prepared_registry['deployment_update'] = True
 
+        return self.prepared_registry['deployment_update'] or self.prepared_registry['service_update'] or False
 
     @staticmethod
     def run_ansible(params, check_mode):
@@ -313,28 +318,37 @@ class Registry(OpenShiftCLI):
         rconfig = RegistryConfig(params['name'],
                                  params['namespace'],
                                  params['kubeconfig'],
-                                 {'default_cert': {'value': None, 'include': True},
-                                  'images': {'value': params['images'], 'include': True},
+                                 {'images': {'value': params['images'], 'include': True},
                                   'latest_images': {'value': params['latest_images'], 'include': True},
                                   'labels': {'value': params['labels'], 'include': True},
                                   'ports': {'value': ','.join(params['ports']), 'include': True},
                                   'replicas': {'value': params['replicas'], 'include': True},
                                   'selector': {'value': params['selector'], 'include': True},
                                   'service_account': {'value': params['service_account'], 'include': True},
-                                  'registry_type': {'value': params['registry_type'], 'include': False},
                                   'mount_host': {'value': params['mount_host'], 'include': True},
-                                  'volume': {'value': '/registry', 'include': True},
                                   'env_vars': {'value': params['env_vars'], 'include': False},
                                   'volume_mounts': {'value': params['volume_mounts'], 'include': False},
                                   'edits': {'value': params['edits'], 'include': False},
                                   'enforce_quota': {'value': params['enforce_quota'], 'include': True},
                                   'daemonset': {'value': params['daemonset'], 'include': True},
+                                  'tls_key': {'value': params['tls_key'], 'include': True},
+                                  'tls_certificate': {'value': params['tls_certificate'], 'include': True},
                                  })
 
 
         ocregistry = Registry(rconfig)
 
         state = params['state']
+        ########
+        # get
+        ########
+        if state == 'list':
+            api_rval = ocregistry.get()
+
+            if api_rval['returncode'] != 0:
+                return {'failed': True, 'msg': api_rval}
+
+            return {'changed': False, 'results': api_rval, 'state': state}
 
         ########
         # Delete
diff --git a/roles/lib_openshift/src/class/oc_adm_router.py b/roles/lib_openshift/src/class/oc_adm_router.py
index 8b4efcc3f..336232b0f 100644
--- a/roles/lib_openshift/src/class/oc_adm_router.py
+++ b/roles/lib_openshift/src/class/oc_adm_router.py
@@ -21,7 +21,9 @@ class Router(OpenShiftCLI):
            a router consists of 3 or more parts
            - dc/router
            - svc/router
-           - endpoint/router
+           - sa/router
+           - secret/router-certs
+           - clusterrolebinding/router-router-role
         '''
         super(Router, self).__init__('default', router_config.kubeconfig, verbose)
         self.config = router_config
@@ -31,32 +33,30 @@ class Router(OpenShiftCLI):
                              {'kind': 'sa', 'name': self.config.config_options['service_account']['value']},
                              {'kind': 'secret', 'name': self.config.name + '-certs'},
                              {'kind': 'clusterrolebinding', 'name': 'router-' + self.config.name + '-role'},
-                             #{'kind': 'endpoints', 'name': self.config.name},
                             ]
 
-        self.__router_prep = None
+        self.__prepared_router = None
         self.dconfig = None
         self.svc = None
         self._secret = None
         self._serviceaccount = None
         self._rolebinding = None
-        self.get()
 
     @property
-    def router_prep(self):
-        ''' property deploymentconfig'''
-        if self.__router_prep == None:
-            results = self.prepare_router()
+    def prepared_router(self):
+        ''' property for the prepared router'''
+        if self.__prepared_router == None:
+            results = self._prepare_router()
             if not results:
                 raise RouterException('Could not perform router preparation')
-            self.__router_prep = results
+            self.__prepared_router = results
 
-        return self.__router_prep
+        return self.__prepared_router
 
-    @router_prep.setter
-    def router_prep(self, obj):
-        '''set the router prep property'''
-        self.__router_prep = obj
+    @prepared_router.setter
+    def prepared_router(self, obj):
+        '''setter for the prepared_router'''
+        self.__prepared_router = obj
 
     @property
     def deploymentconfig(self):
@@ -70,7 +70,7 @@ class Router(OpenShiftCLI):
 
     @property
     def service(self):
-        ''' property service '''
+        ''' property for service '''
         return self.svc
 
     @service.setter
@@ -90,12 +90,12 @@ class Router(OpenShiftCLI):
 
     @property
     def serviceaccount(self):
-        ''' property secret '''
+        ''' property for serviceaccount '''
         return self._serviceaccount
 
     @serviceaccount.setter
     def serviceaccount(self, config):
-        ''' setter for property secret '''
+        ''' setter for property serviceaccount '''
         self._serviceaccount = config
 
     @property
@@ -173,7 +173,7 @@ class Router(OpenShiftCLI):
 
         return deploymentconfig
 
-    def prepare_router(self):
+    def _prepare_router(self):
         '''prepare router for instantiation'''
         # We need to create the pem file
         router_pem = '/tmp/router.pem'
@@ -199,11 +199,11 @@ class Router(OpenShiftCLI):
         if results['returncode'] != 0 and results['results'].has_key('items'):
             return results
 
-        oc_objects = {'DeploymentConfig': {'obj': None, 'path': None},
-                      'Secret': {'obj': None, 'path': None},
-                      'ServiceAccount': {'obj': None, 'path': None},
-                      'ClusterRoleBinding': {'obj': None, 'path': None},
-                      'Service': {'obj': None, 'path': None},
+        oc_objects = {'DeploymentConfig': {'obj': None, 'path': None, 'update': False},
+                      'Secret': {'obj': None, 'path': None, 'update': False},
+                      'ServiceAccount': {'obj': None, 'path': None, 'update': False},
+                      'ClusterRoleBinding': {'obj': None, 'path': None, 'update': False},
+                      'Service': {'obj': None, 'path': None, 'update': False},
                      }
         # pylint: disable=invalid-sequence-index
         for res in results['results']['items']:
@@ -223,7 +223,7 @@ class Router(OpenShiftCLI):
         if not oc_objects['DeploymentConfig']['obj']:
             return results
 
-        # results will need to get parsed here and modifications added
+        # add modifications added
         oc_objects['DeploymentConfig']['obj'] = self.add_modifications(oc_objects['DeploymentConfig']['obj'])
 
         for oc_type in oc_objects.keys():
@@ -233,11 +233,8 @@ class Router(OpenShiftCLI):
 
     def create(self):
         '''Create a deploymentconfig '''
-        # generate the objects and prepare for instantiation
-        self.prepare_router()
-
         results = []
-        for _, oc_data in self.router_prep.items():
+        for _, oc_data in self.prepared_router.items():
             results.append(self._create(oc_data['path']))
 
         rval = 0
@@ -248,21 +245,18 @@ class Router(OpenShiftCLI):
         return {'returncode': rval, 'results': results}
 
     def update(self):
-        '''run update for the router.  This performs a delete and then create '''
-        parts = self.delete()
-        for part in parts:
-            if part['returncode'] != 0:
-                if part.has_key('stderr') and 'not found' in part['stderr']:
-                    # the object is not there, continue
-                    continue
-
-                # something went wrong
-                return parts
+        '''run update for the router.  This performs a replace'''
+        results = []
+        for _, oc_data in self.prepared_router.items():
+            if oc_data['update']:
+                results.append(self._replace(oc_data['path']))
 
-        # Ugly built in sleep here.
-        time.sleep(15)
+        rval = 0
+        for result in results:
+            if result['returncode'] != 0:
+                rval = result['returncode']
 
-        return self.create()
+        return {'returncode': rval, 'results': results}
 
     # pylint: disable=too-many-return-statements,too-many-branches
     def needs_update(self):
@@ -270,64 +264,58 @@ class Router(OpenShiftCLI):
         if not self.deploymentconfig or not self.service or not self.serviceaccount or not self.secret:
             return True
 
-        oc_objects_prep = self.prepare_router()
-
-        # Since the output from oadm_router is returned as raw
-        # we need to parse it.  The first line is the stats_password in 3.1
-        # Inside of 3.2, it is just json
-
         # ServiceAccount:
-        #   Need to determine the pregenerated ones from the original
+        #   Need to determine changes from the pregenerated ones from the original
         #   Since these are auto generated, we can skip
         skip = ['secrets', 'imagePullSecrets']
-        if not Utils.check_def_equal(oc_objects_prep['ServiceAccount']['obj'].yaml_dict,
+        if not Utils.check_def_equal(self.prepared_router['ServiceAccount']['obj'].yaml_dict,
                                      self.serviceaccount.yaml_dict,
                                      skip_keys=skip,
                                      debug=self.verbose):
-            return True
+            self.prepared_router['ServiceAccount']['update'] = True
 
         # Secret:
-        #   In 3.2 oadm router generates a secret volume for certificates
         #   See if one was generated from our dry-run and verify it if needed
-        if oc_objects_prep['Secret']['obj']:
+        if self.prepared_router['Secret']['obj']:
             if not self.secret:
-                return True
-            if not Utils.check_def_equal(oc_objects_prep['Secret']['obj'].yaml_dict,
+                self.prepared_router['Secret']['update'] = True
+
+            if not Utils.check_def_equal(self.prepared_router['Secret']['obj'].yaml_dict,
                                          self.secret.yaml_dict,
                                          skip_keys=skip,
                                          debug=self.verbose):
-                return True
+                self.prepared_router['Secret']['update'] = True
 
         # Service:
         #   Fix the ports to have protocol=TCP
-        for port in oc_objects_prep['Service']['obj'].get('spec.ports'):
+        for port in self.prepared_router['Service']['obj'].get('spec.ports'):
             port['protocol'] = 'TCP'
 
         skip = ['portalIP', 'clusterIP', 'sessionAffinity', 'type']
-        if not Utils.check_def_equal(oc_objects_prep['Service']['obj'].yaml_dict,
+        if not Utils.check_def_equal(self.prepared_router['Service']['obj'].yaml_dict,
                                      self.service.yaml_dict,
                                      skip_keys=skip,
                                      debug=self.verbose):
-            return True
+            self.prepared_router['Service']['update'] = True
 
         # DeploymentConfig:
         #   Router needs some exceptions.
         #   We do not want to check the autogenerated password for stats admin
         if not self.config.config_options['stats_password']['value']:
-            for idx, env_var in enumerate(oc_objects_prep['DeploymentConfig']['obj'].get(\
+            for idx, env_var in enumerate(self.prepared_router['DeploymentConfig']['obj'].get(\
                         'spec.template.spec.containers[0].env') or []):
                 if env_var['name'] == 'STATS_PASSWORD':
                     env_var['value'] = \
                       self.deploymentconfig.get('spec.template.spec.containers[0].env[%s].value' % idx)
                     break
 
-        #   dry-run doesn't add the protocol to the ports section.  We will manually do that.
-        for idx, port in enumerate(oc_objects_prep['DeploymentConfig']['obj'].get(\
+        # dry-run doesn't add the protocol to the ports section.  We will manually do that.
+        for idx, port in enumerate(self.prepared_router['DeploymentConfig']['obj'].get(\
                         'spec.template.spec.containers[0].ports') or []):
             if not port.has_key('protocol'):
                 port['protocol'] = 'TCP'
 
-        #   These are different when generating
+        # These are different when generating
         skip = ['dnsPolicy',
                 'terminationGracePeriodSeconds',
                 'restartPolicy', 'timeoutSeconds',
@@ -336,11 +324,15 @@ class Router(OpenShiftCLI):
                 'defaultMode',
                ]
 
-        return not Utils.check_def_equal(oc_objects_prep['DeploymentConfig']['obj'].yaml_dict,
+        if not Utils.check_def_equal(self.prepared_router['DeploymentConfig']['obj'].yaml_dict,
                                          self.deploymentconfig.yaml_dict,
                                          skip_keys=skip,
-                                         debug=self.verbose)
+                                         debug=self.verbose):
+            self.prepared_router['DeploymentConfig']['update'] = True
 
+        # Check if any of the parts need updating, if so, return True
+        # else, no need to update
+        return any([self.prepared_router[oc_type]['update'] for oc_type in self.prepared_router.keys()])
 
     @staticmethod
     def run_ansible(params, check_mode):
@@ -386,10 +378,18 @@ class Router(OpenShiftCLI):
                                })
 
 
-        ocrouter = Router(rconfig)
-
         state = params['state']
 
+        ocrouter = Router(rconfig, verbose=params['debug'])
+
+        api_rval = ocrouter.get()
+
+        ########
+        # get
+        ########
+        if state == 'list':
+            return {'changed': False, 'results': api_rval, 'state': state}
+
         ########
         # Delete
         ########
diff --git a/roles/lib_openshift/src/doc/registry b/roles/lib_openshift/src/doc/registry
index 5ae969c73..232d30513 100644
--- a/roles/lib_openshift/src/doc/registry
+++ b/roles/lib_openshift/src/doc/registry
@@ -116,7 +116,7 @@ options:
     description:
     - Use a daemonset instead of a deployment config.
     required: false
-    default: None
+    default: False
     aliases: []
   edits:
     description:
@@ -144,7 +144,6 @@ extends_documentation_fragment: []
 EXAMPLES = '''
 - name: create a secure registry
   oadm_registry:
-    credentials: /etc/origin/master/openshift-registry.kubeconfig
     name: docker-registry
     service_account: registry
     replicas: 2
diff --git a/roles/lib_openshift/src/doc/router b/roles/lib_openshift/src/doc/router
index 3938d8415..6ff7e3f8d 100644
--- a/roles/lib_openshift/src/doc/router
+++ b/roles/lib_openshift/src/doc/router
@@ -43,12 +43,6 @@ options:
     required: false
     default: default
     aliases: []
-  credentials:
-    description:
-    - Path to a .kubeconfig file that will contain the credentials the registry should use to contact the master.
-    required: false
-    default: None
-    aliases: []
   images:
     description:
     - The image to base this router on - ${component} will be replaced with --type
@@ -159,7 +153,31 @@ options:
     aliases: []
 author:
 - "Kenny Woodson <kwoodson@redhat.com>"
-extends_documentation_fragment: []
+extends_documentation_fragment:
+- There are some exceptions to note when doing the idempotency in this module.
+- The strategy is to use the oc adm router command to generate a default
+- configuration when creating or updating a router.  Often times there
+- differences from the generated template and what is in memory in openshift.
+- We make exceptions to not check these specific values when comparing objects.
+- Here are a list of exceptions:
+- - DeploymentConfig:
+    - dnsPolicy
+    - terminationGracePeriodSeconds
+    - restartPolicy
+    - timeoutSeconds
+    - livenessProbe
+    - readinessProbe
+    - terminationMessagePath
+    - hostPort
+    - defaultMode
+  - Service:
+    - portalIP
+    - clusterIP
+    - sessionAffinity
+    - type
+  - ServiceAccount:
+    - secrets
+    - imagePullSecrets
 '''
 
 EXAMPLES = '''
@@ -184,10 +202,10 @@ EXAMPLES = '''
       action: put
     - key: spec.template.spec.containers[0].resources.limits.memory
       value: 2G
-      action: update
+      action: put
     - key: spec.template.spec.containers[0].resources.requests.memory
       value: 1G
-      action: update
+      action: put
     - key: spec.template.spec.containers[0].env
       value:
         name: EXTENDED_VALIDATION
diff --git a/roles/lib_openshift/src/lib/deploymentconfig.py b/roles/lib_openshift/src/lib/deploymentconfig.py
index e060d3707..f10c6bb8b 100644
--- a/roles/lib_openshift/src/lib/deploymentconfig.py
+++ b/roles/lib_openshift/src/lib/deploymentconfig.py
@@ -4,7 +4,7 @@
 
 # pylint: disable=too-many-public-methods
 class DeploymentConfig(Yedit):
-    ''' Class to wrap the oc command line tools '''
+    ''' Class to model an openshift DeploymentConfig'''
     default_deployment_config = '''
 apiVersion: v1
 kind: DeploymentConfig
diff --git a/roles/lib_openshift/src/lib/replicationcontroller.py b/roles/lib_openshift/src/lib/replicationcontroller.py
index ae585a986..8bcc1e3cc 100644
--- a/roles/lib_openshift/src/lib/replicationcontroller.py
+++ b/roles/lib_openshift/src/lib/replicationcontroller.py
@@ -4,7 +4,12 @@
 
 # pylint: disable=too-many-public-methods
 class ReplicationController(DeploymentConfig):
-    ''' Class to wrap the oc command line tools '''
+    ''' Class to model a replicationcontroller openshift object.
+
+        Currently we are modeled after a deployment config since they
+        are very similar.  In the future, when the need arises we
+        will add functionality to this class.
+    '''
     replicas_path = "spec.replicas"
     env_path = "spec.template.spec.containers[0].env"
     volumes_path = "spec.template.spec.volumes"
diff --git a/roles/lib_openshift/src/lib/rolebinding.py b/roles/lib_openshift/src/lib/rolebinding.py
index bbc1bb956..0835c9254 100644
--- a/roles/lib_openshift/src/lib/rolebinding.py
+++ b/roles/lib_openshift/src/lib/rolebinding.py
@@ -2,19 +2,19 @@
 
 # pylint: disable=too-many-instance-attributes
 class RoleBindingConfig(object):
-    ''' Handle route options '''
+    ''' Handle rolebinding config '''
     # pylint: disable=too-many-arguments
     def __init__(self,
-                 sname,
+                 name,
                  namespace,
                  kubeconfig,
                  group_names=None,
                  role_ref=None,
                  subjects=None,
                  usernames=None):
-        ''' constructor for handling route options '''
+        ''' constructor for handling rolebinding options '''
         self.kubeconfig = kubeconfig
-        self.name = sname
+        self.name = name
         self.namespace = namespace
         self.group_names = group_names
         self.role_ref = role_ref
@@ -25,7 +25,7 @@ class RoleBindingConfig(object):
         self.create_dict()
 
     def create_dict(self):
-        ''' return a service as a dict '''
+        ''' create a default rolebinding as a dict '''
         self.data['apiVersion'] = 'v1'
         self.data['kind'] = 'RoleBinding'
         self.data['groupNames'] = self.group_names
@@ -39,7 +39,7 @@ class RoleBindingConfig(object):
 
 # pylint: disable=too-many-instance-attributes,too-many-public-methods
 class RoleBinding(Yedit):
-    ''' Class to wrap the oc command line tools '''
+    ''' Class to model a rolebinding openshift object'''
     group_names_path = "groupNames"
     role_ref_path = "roleRef"
     subjects_path = "subjects"
diff --git a/roles/lib_openshift/src/lib/secret.py b/roles/lib_openshift/src/lib/secret.py
index 1ba78ddd5..32e67152d 100644
--- a/roles/lib_openshift/src/lib/secret.py
+++ b/roles/lib_openshift/src/lib/secret.py
@@ -20,7 +20,7 @@ class SecretConfig(object):
         self.create_dict()
 
     def create_dict(self):
-        ''' return a secret as a dict '''
+        ''' instantiate a secret as a dict '''
         self.data['apiVersion'] = 'v1'
         self.data['kind'] = 'Secret'
         self.data['metadata'] = {}
diff --git a/roles/lib_openshift/src/lib/serviceaccount.py b/roles/lib_openshift/src/lib/serviceaccount.py
index 47a55757e..50c104d44 100644
--- a/roles/lib_openshift/src/lib/serviceaccount.py
+++ b/roles/lib_openshift/src/lib/serviceaccount.py
@@ -18,7 +18,7 @@ class ServiceAccountConfig(object):
         self.create_dict()
 
     def create_dict(self):
-        ''' return a properly structured volume '''
+        ''' instantiate a properly structured volume '''
         self.data['apiVersion'] = 'v1'
         self.data['kind'] = 'ServiceAccount'
         self.data['metadata'] = {}
diff --git a/roles/lib_openshift/src/lib/volume.py b/roles/lib_openshift/src/lib/volume.py
index dc07d3ce1..fd47fa5c5 100644
--- a/roles/lib_openshift/src/lib/volume.py
+++ b/roles/lib_openshift/src/lib/volume.py
@@ -1,7 +1,7 @@
 # pylint: skip-file
 
 class Volume(object):
-    ''' Class to wrap the oc command line tools '''
+    ''' Class to model an openshift volume object'''
     volume_mounts_path = {"pod": "spec.containers[0].volumeMounts",
                           "dc":  "spec.template.spec.containers[0].volumeMounts",
                           "rc":  "spec.template.spec.containers[0].volumeMounts",