Changes to KeypairSettings constructor to use kwargs.

And changed line lengths to 79 for pep8.

JIRA: SNAPS-98

Change-Id: I01f7f459bc46ed818d75d30f8bea7b1e09b5b376
Signed-off-by: spisarski <s.pisarski@cablelabs.com>

2 files changed, 84 insertions, 59 deletions

diff --git a/snaps/openstack/create_keypairs.py b/snaps/openstack/create_keypairs.py
index 9b7b350..cf5a93d 100644
--- a/snaps/openstack/create_keypairs.py
+++ b/snaps/openstack/create_keypairs.py
@@ -13,10 +13,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import logging
-import os
 
+import os
 from novaclient.exceptions import NotFound
-
 from snaps.openstack.utils import nova_utils
 
 __author__ = 'spisarski'
@@ -39,6 +38,7 @@ class OpenStackKeypair:
         self.__os_creds = os_creds
         self.keypair_settings = keypair_settings
         self.__nova = nova_utils.nova_client(os_creds)
+        self.__delete_keys_on_clean = True
 
         # Attributes instantiated on create()
         self.__keypair = None
@@ -46,27 +46,35 @@ class OpenStackKeypair:
     def create(self, cleanup=False):
         """
         Responsible for creating the keypair object.
-        :param cleanup: Denotes whether or not this is being called for cleanup or not
+        :param cleanup: Denotes whether or not this is being called for cleanup
+                        or not
         """
         self.__nova = nova_utils.nova_client(self.__os_creds)
 
         logger.info('Creating keypair %s...' % self.keypair_settings.name)
 
-        self.__keypair = nova_utils.get_keypair_by_name(self.__nova, self.keypair_settings.name)
+        self.__keypair = nova_utils.get_keypair_by_name(
+            self.__nova, self.keypair_settings.name)
 
         if not self.__keypair and not cleanup:
-            if self.keypair_settings.public_filepath and os.path.isfile(self.keypair_settings.public_filepath):
+            if self.keypair_settings.public_filepath and os.path.isfile(
+                    self.keypair_settings.public_filepath):
                 logger.info("Uploading existing keypair")
-                self.__keypair = nova_utils.upload_keypair_file(self.__nova, self.keypair_settings.name,
-                                                                self.keypair_settings.public_filepath)
+                self.__keypair = nova_utils.upload_keypair_file(
+                    self.__nova, self.keypair_settings.name,
+                    self.keypair_settings.public_filepath)
+                self.__delete_keys_on_clean = False
             else:
                 logger.info("Creating new keypair")
                 # TODO - Make this value configurable
                 keys = nova_utils.create_keys(1024)
-                self.__keypair = nova_utils.upload_keypair(self.__nova, self.keypair_settings.name,
-                                                           nova_utils.public_key_openssh(keys))
-                nova_utils.save_keys_to_files(keys, self.keypair_settings.public_filepath,
-                                              self.keypair_settings.private_filepath)
+                self.__keypair = nova_utils.upload_keypair(
+                    self.__nova, self.keypair_settings.name,
+                    nova_utils.public_key_openssh(keys))
+                nova_utils.save_keys_to_files(
+                    keys, self.keypair_settings.public_filepath,
+                    self.keypair_settings.private_filepath)
+                self.__delete_keys_on_clean = True
 
         return self.__keypair
 
@@ -81,12 +89,13 @@ class OpenStackKeypair:
                 pass
             self.__keypair = None
 
-        if self.keypair_settings.public_filepath:
-            os.chmod(self.keypair_settings.public_filepath, 0o777)
-            os.remove(self.keypair_settings.public_filepath)
-        if self.keypair_settings.private_filepath:
-            os.chmod(self.keypair_settings.private_filepath, 0o777)
-            os.remove(self.keypair_settings.private_filepath)
+        if self.__delete_keys_on_clean:
+            if self.keypair_settings.public_filepath:
+                os.chmod(self.keypair_settings.public_filepath, 0o777)
+                os.remove(self.keypair_settings.public_filepath)
+            if self.keypair_settings.private_filepath:
+                os.chmod(self.keypair_settings.private_filepath, 0o777)
+                os.remove(self.keypair_settings.private_filepath)
 
     def get_keypair(self):
         """
@@ -101,25 +110,20 @@ class KeypairSettings:
     Class representing a keypair configuration
     """
 
-    def __init__(self, config=None, name=None, public_filepath=None, private_filepath=None):
+    def __init__(self, **kwargs):
         """
         Constructor - all parameters are optional
-        :param config: Should be a dict object containing the configuration settings using the attribute names below
-                       as each member's the key and overrides any of the other parameters.
         :param name: The keypair name.
-        :param public_filepath: The path to/from the filesystem where the public key file is or will be stored
-        :param private_filepath: The path where the generated private key file will be stored
+        :param public_filepath: The path to/from the filesystem where the
+                                public key file is or will be stored
+        :param private_filepath: The path where the generated private key file
+                                 will be stored
         :return:
         """
 
-        if config:
-            self.name = config.get('name')
-            self.public_filepath = config.get('public_filepath')
-            self.private_filepath = config.get('private_filepath')
-        else:
-            self.name = name
-            self.public_filepath = public_filepath
-            self.private_filepath = private_filepath
+        self.name = kwargs.get('name')
+        self.public_filepath = kwargs.get('public_filepath')
+        self.private_filepath = kwargs.get('private_filepath')
 
         if not self.name:
-            raise Exception('The attributes name, public_filepath, and private_filepath are required')
+            raise Exception('Name is a required attribute')
diff --git a/snaps/openstack/tests/create_keypairs_tests.py b/snaps/openstack/tests/create_keypairs_tests.py
index b587a50..29fe843 100644
--- a/snaps/openstack/tests/create_keypairs_tests.py
+++ b/snaps/openstack/tests/create_keypairs_tests.py
@@ -12,13 +12,13 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-import os
-import uuid
 import unittest
+import uuid
 
+import os
 from snaps.openstack.create_keypairs import KeypairSettings, OpenStackKeypair
-from snaps.openstack.utils import nova_utils
 from snaps.openstack.tests.os_source_file_test import OSIntegrationTestCase
+from snaps.openstack.utils import nova_utils
 
 __author__ = 'spisarski'
 
@@ -34,7 +34,7 @@ class KeypairSettingsUnitTests(unittest.TestCase):
 
     def test_empty_config(self):
         with self.assertRaises(Exception):
-            KeypairSettings(config=dict())
+            KeypairSettings(**dict())
 
     def test_name_only(self):
         settings = KeypairSettings(name='foo')
@@ -43,7 +43,7 @@ class KeypairSettingsUnitTests(unittest.TestCase):
         self.assertIsNone(settings.private_filepath)
 
     def test_config_with_name_only(self):
-        settings = KeypairSettings(config={'name': 'foo'})
+        settings = KeypairSettings(**{'name': 'foo'})
         self.assertEqual('foo', settings.name)
         self.assertIsNone(settings.public_filepath)
         self.assertIsNone(settings.private_filepath)
@@ -55,7 +55,8 @@ class KeypairSettingsUnitTests(unittest.TestCase):
         self.assertIsNone(settings.private_filepath)
 
     def test_config_with_name_pub_only(self):
-        settings = KeypairSettings(config={'name': 'foo', 'public_filepath': '/foo/bar.pub'})
+        settings = KeypairSettings(
+            **{'name': 'foo', 'public_filepath': '/foo/bar.pub'})
         self.assertEqual('foo', settings.name)
         self.assertEqual('/foo/bar.pub', settings.public_filepath)
         self.assertIsNone(settings.private_filepath)
@@ -67,20 +68,23 @@ class KeypairSettingsUnitTests(unittest.TestCase):
         self.assertEqual('/foo/bar', settings.private_filepath)
 
     def test_config_with_name_priv_only(self):
-        settings = KeypairSettings(config={'name': 'foo', 'private_filepath': '/foo/bar'})
+        settings = KeypairSettings(
+            **{'name': 'foo', 'private_filepath': '/foo/bar'})
         self.assertEqual('foo', settings.name)
         self.assertIsNone(settings.public_filepath)
         self.assertEqual('/foo/bar', settings.private_filepath)
 
     def test_all(self):
-        settings = KeypairSettings(name='foo', public_filepath='/foo/bar.pub', private_filepath='/foo/bar')
+        settings = KeypairSettings(name='foo', public_filepath='/foo/bar.pub',
+                                   private_filepath='/foo/bar')
         self.assertEqual('foo', settings.name)
         self.assertEqual('/foo/bar.pub', settings.public_filepath)
         self.assertEqual('/foo/bar', settings.private_filepath)
 
     def test_config_all(self):
-        settings = KeypairSettings(config={'name': 'foo', 'public_filepath': '/foo/bar.pub',
-                                           'private_filepath': '/foo/bar'})
+        settings = KeypairSettings(
+            **{'name': 'foo', 'public_filepath': '/foo/bar.pub',
+                    'private_filepath': '/foo/bar'})
         self.assertEqual('foo', settings.name)
         self.assertEqual('/foo/bar.pub', settings.public_filepath)
         self.assertEqual('/foo/bar', settings.private_filepath)
@@ -126,27 +130,33 @@ class CreateKeypairsTests(OSIntegrationTestCase):
         Tests the creation of a generated keypair without saving to file
         :return:
         """
-        self.keypair_creator = OpenStackKeypair(self.os_creds, KeypairSettings(name=self.keypair_name))
+        self.keypair_creator = OpenStackKeypair(self.os_creds, KeypairSettings(
+            name=self.keypair_name))
         self.keypair_creator.create()
 
-        keypair = nova_utils.keypair_exists(self.nova, self.keypair_creator.get_keypair())
+        keypair = nova_utils.keypair_exists(self.nova,
+                                            self.keypair_creator.get_keypair())
         self.assertEqual(self.keypair_creator.get_keypair(), keypair)
 
     def test_create_delete_keypair(self):
         """
-        Tests the creation then deletion of an OpenStack keypair to ensure clean() does not raise an Exception.
+        Tests the creation then deletion of an OpenStack keypair to ensure
+        clean() does not raise an Exception.
         """
         # Create Image
-        self.keypair_creator = OpenStackKeypair(self.os_creds, KeypairSettings(name=self.keypair_name))
+        self.keypair_creator = OpenStackKeypair(self.os_creds, KeypairSettings(
+            name=self.keypair_name))
         created_keypair = self.keypair_creator.create()
         self.assertIsNotNone(created_keypair)
 
         # Delete Image manually
         nova_utils.delete_keypair(self.nova, created_keypair)
 
-        self.assertIsNone(nova_utils.get_keypair_by_name(self.nova, self.keypair_name))
+        self.assertIsNone(
+            nova_utils.get_keypair_by_name(self.nova, self.keypair_name))
 
-        # Must not throw an exception when attempting to cleanup non-existent image
+        # Must not throw an exception when attempting to cleanup non-existent
+        # image
         self.keypair_creator.clean()
         self.assertIsNone(self.keypair_creator.get_keypair())
 
@@ -156,30 +166,37 @@ class CreateKeypairsTests(OSIntegrationTestCase):
         :return:
         """
         self.keypair_creator = OpenStackKeypair(
-            self.os_creds, KeypairSettings(name=self.keypair_name, public_filepath=self.pub_file_path))
+            self.os_creds, KeypairSettings(name=self.keypair_name,
+                                           public_filepath=self.pub_file_path))
         self.keypair_creator.create()
 
-        keypair = nova_utils.keypair_exists(self.nova, self.keypair_creator.get_keypair())
+        keypair = nova_utils.keypair_exists(self.nova,
+                                            self.keypair_creator.get_keypair())
         self.assertEqual(self.keypair_creator.get_keypair(), keypair)
 
         file_key = open(os.path.expanduser(self.pub_file_path)).read()
-        self.assertEqual(self.keypair_creator.get_keypair().public_key, file_key)
+        self.assertEqual(self.keypair_creator.get_keypair().public_key,
+                         file_key)
 
     def test_create_keypair_save_both(self):
         """
-        Tests the creation of a generated keypair and saves both private and public key files[
+        Tests the creation of a generated keypair and saves both private and
+        public key files[
         :return:
         """
         self.keypair_creator = OpenStackKeypair(
-            self.os_creds, KeypairSettings(name=self.keypair_name, public_filepath=self.pub_file_path,
-                                           private_filepath=self.priv_file_path))
+            self.os_creds, KeypairSettings(
+                name=self.keypair_name, public_filepath=self.pub_file_path,
+                private_filepath=self.priv_file_path))
         self.keypair_creator.create()
 
-        keypair = nova_utils.keypair_exists(self.nova, self.keypair_creator.get_keypair())
+        keypair = nova_utils.keypair_exists(self.nova,
+                                            self.keypair_creator.get_keypair())
         self.assertEqual(self.keypair_creator.get_keypair(), keypair)
 
         file_key = open(os.path.expanduser(self.pub_file_path)).read()
-        self.assertEqual(self.keypair_creator.get_keypair().public_key, file_key)
+        self.assertEqual(self.keypair_creator.get_keypair().public_key,
+                         file_key)
 
         self.assertTrue(os.path.isfile(self.priv_file_path))
 
@@ -189,13 +206,17 @@ class CreateKeypairsTests(OSIntegrationTestCase):
         :return:
         """
         keys = nova_utils.create_keys()
-        nova_utils.save_keys_to_files(keys=keys, pub_file_path=self.pub_file_path)
+        nova_utils.save_keys_to_files(keys=keys,
+                                      pub_file_path=self.pub_file_path)
         self.keypair_creator = OpenStackKeypair(
-            self.os_creds, KeypairSettings(name=self.keypair_name, public_filepath=self.pub_file_path))
+            self.os_creds, KeypairSettings(name=self.keypair_name,
+                                           public_filepath=self.pub_file_path))
         self.keypair_creator.create()
 
-        keypair = nova_utils.keypair_exists(self.nova, self.keypair_creator.get_keypair())
+        keypair = nova_utils.keypair_exists(self.nova,
+                                            self.keypair_creator.get_keypair())
         self.assertEqual(self.keypair_creator.get_keypair(), keypair)
 
         file_key = open(os.path.expanduser(self.pub_file_path)).read()
-        self.assertEqual(self.keypair_creator.get_keypair().public_key, file_key)
+        self.assertEqual(self.keypair_creator.get_keypair().public_key,
+                         file_key)