Clean up

- Adds docstrings
- Removes prefix sf- from charm options
- Charm options defaults to none

config.yaml

@@ -14,63 +14,46 @@ options:
   san-password:
     type: string
     default: sfpassword
-    description: Password for SAN controller
+    description: Password for SAN controller  
-  sf-account-prefix:
+  allow-template-caching:
-    type: string
-    default: !!null ""
-    description: |
-        Create SolidFire accounts with this prefix. Any string can be used
-        here, but the string “hostname” is special and will create a prefix
-        using the cinder node hostname (previous default behavior). 
-        The default is NO prefix.
-  sf-allow-template-caching:
     type: boolean
-    default: !!bool "true"
     description: |
         Create an internal cache of copy of images when a bootable volume is
         created to eliminate fetch from glance and qemu-conversion on
         subsequent calls.
-  sf-allow-tenant-qos:
+  allow-tenant-qos:
     type: boolean
-    default: !!bool "false"
     description: Allow tenants to specify QOS on create
-  sf-api-port:
+  api-port:
     type: int
-    default: 443
     description: |
         SolidFire API port. Useful if the device api is behind a proxy on
         a different port.
-  sf-emulate-512:
+  emulate-512:
     type: boolean
-    default: !!bool "true"
     description: Set 512 byte emulation on volume creation
-  sf-enable-vag:
+  enable-vag:
     type: boolean
-    default: !!bool "false"
     description: Utilize volume access groups on a per-tenant basis.
-  sf-enable-volume-mapping:
+  enable-volume-mapping:
     type: boolean
-    default: !!bool "true"
     description: |
         Create an internal mapping of volume IDs and account. Optimizes lookups
         and performance at the expense of memory, very large deployments may
         want to consider setting to False.
-  sf-svip:
+  svip:
     type: string
-    default: !!null ""
     description: |
         Overrides default cluster SVIP with the one specified. This is required
         or deployments that have implemented the use of VLANs for iSCSI
         networks in their cloud.
-  sf-template-account-name:
+  template-account-name:
     type: string
-    default: openstack-vtemplate
     description: |
         Account name on the SolidFire Cluster to use as owner of template/cache
         volumes (created if does not exist).
-  sf-volume-prefix:
+  volume-prefix:
     type: string
-    default: UUID
     description: |
         Create SolidFire volumes with this prefix. Volume names are of the form
         <sf_volume_prefix><cinder-volume-id>.

metadata.yaml

@@ -3,8 +3,7 @@ summary: Solidfire integration for OpenStack Block Storage
 maintainer: OpenStack Charmers <openstack-charmers@lists.ubuntu.com>
 description: |
  Cinder is the block storage service for the Openstack project.
- .
+ This charm provides a Solidfire backend for Cinder.
- This charm provides a Solidfire backend for Cinder
 tags:
   - openstack
   - storage

src/charm.py

@@ -37,6 +37,7 @@ class CinderSolidfireCharm(OSBaseCharm):
     PACKAGES = ['cinder-common']
 
     def __init__(self, *args, **kwargs):
+        """Set observables to watch"""
         super().__init__(*args, **kwargs)
         self.framework.observe(self.on.install, self._on_install)
         self.framework.observe(self.on.update_status, self._on_update_status)
@@ -45,55 +46,59 @@ class CinderSolidfireCharm(OSBaseCharm):
             self.on.storage_backend_relation_changed,
             self._on_storage_backend_changed)
 
-    # @observed
     def _on_install(self, event: InstallEvent):
+        """Install ubuntu packages"""
         self.install_pkgs()
         self.unit.status = ActiveStatus('Unit is ready')
 
-    # @observed
     def _on_update_status(self, event: UpdateStatusEvent):
+        """Mantain active status if everything is okay"""
         expected_status = WaitingStatus('Charm configuration in progress')
         if (self.unit.status == expected_status):
             self.unit.status = ActiveStatus('Unit is ready')
 
-    # @observed
     def _on_config_changed(self, event: ConfigChangedEvent):
+        """Update information to main charm on config change"""
         for relation in self.framework.model.relations.get('storage-backend'):
             self._set_relation_data(relation.data[self.unit])
         self.unit.status = ActiveStatus('Unit is ready')
 
-    # @observed
     def _on_storage_backend_changed(self, event: RelationChangedEvent):
+        """Send information to main charm on backend relation event"""
         self._set_relation_data(event.relation.data[self.unit])
 
     def _set_relation_data(self, data) -> None:
+        """Send information to main charm through subordinate relation"""
         backend_name = self.model.config['volume-backend-name']
         data['backend-name'] = backend_name
         data['subordinate_configuration'] = self._render_config(backend_name)
 
     def _render_config(self, backend_name) -> str:
+        """Generate backend configuration for cinder.conf"""
         cget = self.model.config.get
 
         sf_volume_prefix = str(uuid.uuid4()) if cget(
-            'sf-volume-prefix') == "UUID" else cget('sf-volume-prefix')
+            'volume-prefix') == "UUID" else cget('volume-prefix')
 
-        options = [
+        raw_options = [
             ('volume_driver', VOLUME_DRIVER),
             ('san_ip', cget('san-ip')),
             ('san_login', cget('san-login')),
             ('san_password', cget('san-password')),
-            ('sf_account_prefix', cget('sf-account-prefix')),
+            ('sf_account_prefix', cget('account-prefix')),
             ('sf_allow_template_caching',
-                cget('sf-allow-template-caching')),
+                cget('allow-template-caching')),
-            ('sf_allow_tenant_qos', cget('sf-allow-tenant-qos')),
+            ('sf_allow_tenant_qos', cget('allow-tenant-qos')),
-            ('sf_api_port', cget('sf-api-port')),
+            ('sf_api_port', cget('api-port')),
-            ('sf_emulate_512', cget('sf-emulate-512')),
+            ('sf_emulate_512', cget('emulate-512')),
-            ('sf_enable_vag', cget('sf-enable-vag')),
+            ('sf_enable_vag', cget('enable-vag')),
-            ('sf_enable_volume_mapping', cget('sf-enable-volume-mapping')),
+            ('sf_enable_volume_mapping', cget('enable-volume-mapping')),
-            ('sf_svip', cget('sf-svip')),
+            ('sf_svip', cget('svip')),
-            ('sf_template_account_name', cget('sf-template-account-name')),
+            ('sf_template_account_name', cget('template-account-name')),
             ('sf_volume_prefix', sf_volume_prefix)
         ]
+        options = [(x, y) for x, y in raw_options if y]
+
         return json.dumps({
             "cinder": {
                 "/etc/cinder/cinder.conf": {