diff --git a/lib/ansible/modules/storage/netapp/na_ontap_snapshot.py b/lib/ansible/modules/storage/netapp/na_ontap_snapshot.py
index cedb3f1359..534937f6a1 100644
--- a/lib/ansible/modules/storage/netapp/na_ontap_snapshot.py
+++ b/lib/ansible/modules/storage/netapp/na_ontap_snapshot.py
@@ -1,6 +1,6 @@
 #!/usr/bin/python
 
-# (c) 2018, NetApp, Inc
+# (c) 2018-2019, NetApp, Inc
 # GNU General Public License v3.0+
 # (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
@@ -32,6 +32,10 @@ options:
       Name of the snapshot to be managed.
       The maximum string length is 256 characters.
     required: true
+  from_name:
+    description:
+    - Name of the existing snapshot to be renamed to.
+    version_added: '2.8'
   volume:
     description:
     - Name of the volume on which the snapshot is to be created.
@@ -65,37 +69,37 @@ EXAMPLES = """
       tags:
         - create
       na_ontap_snapshot:
-        state=present
-        snapshot={{ snapshot name }}
-        volume={{ vol name }}
-        comment="i am a comment"
-        vserver={{ vserver name }}
-        username={{ netapp username }}
-        password={{ netapp password }}
-        hostname={{ netapp hostname }}
+        state: present
+        snapshot: "{{ snapshot name }}"
+        volume: "{{ vol name }}"
+        comment: "i am a comment"
+        vserver: "{{ vserver name }}"
+        username: "{{ netapp username }}"
+        password: "{{ netapp password }}"
+        hostname: "{{ netapp hostname }}"
     - name: delete SnapShot
       tags:
         - delete
       na_ontap_snapshot:
-        state=absent
-        snapshot={{ snapshot name }}
-        volume={{ vol name }}
-        vserver={{ vserver name }}
-        username={{ netapp username }}
-        password={{ netapp password }}
-        hostname={{ netapp hostname }}
+        state: absent
+        snapshot: "{{ snapshot name }}"
+        volume: "{{ vol name }}"
+        vserver: "{{ vserver name }}"
+        username: "{{ netapp username }}"
+        password: "{{ netapp password }}"
+        hostname: "{{ netapp hostname }}"
     - name: modify SnapShot
       tags:
         - modify
       na_ontap_snapshot:
-        state=present
-        snapshot={{ snapshot name }}
-        comment="New comments are great"
-        volume={{ vol name }}
-        vserver={{ vserver name }}
-        username={{ netapp username }}
-        password={{ netapp password }}
-        hostname={{ netapp hostname }}
+        state: present
+        snapshot: "{{ snapshot name }}"
+        comment: "New comments are great"
+        volume: "{{ vol name }}"
+        vserver: "{{ vserver name }}"
+        username: "{{ netapp username }}"
+        password: "{{ netapp password }}"
+        hostname: "{{ netapp hostname }}"
 """
 
 RETURN = """
@@ -103,6 +107,7 @@ RETURN = """
 import traceback
 
 from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.netapp_module import NetAppModule
 from ansible.module_utils._text import to_native
 import ansible.module_utils.netapp as netapp_utils
 
@@ -119,6 +124,7 @@ class NetAppOntapSnapshot(object):
         self.argument_spec.update(dict(
             state=dict(required=False, choices=[
                        'present', 'absent'], default='present'),
+            from_name=dict(required=False, type='str'),
             snapshot=dict(required=True, type="str"),
             volume=dict(required=True, type="str"),
             async_bool=dict(required=False, type="bool", default=False),
@@ -134,30 +140,54 @@ class NetAppOntapSnapshot(object):
             supports_check_mode=True
         )
 
-        parameters = self.module.params
-
-        # set up state variables
-        # These are the required variables
-        self.state = parameters['state']
-        self.snapshot = parameters['snapshot']
-        self.vserver = parameters['vserver']
-        # these are the optional variables for creating a snapshot
-        self.volume = parameters['volume']
-        self.async_bool = parameters['async_bool']
-        self.comment = parameters['comment']
-        self.snapmirror_label = parameters['snapmirror_label']
-        # these are the optional variables for deleting a snapshot\
-        self.ignore_owners = parameters['ignore_owners']
-        self.snapshot_instance_uuid = parameters['snapshot_instance_uuid']
+        self.na_helper = NetAppModule()
+        self.parameters = self.na_helper.set_parameters(self.module.params)
 
         if HAS_NETAPP_LIB is False:
             self.module.fail_json(
                 msg="the python NetApp-Lib module is required")
         else:
             self.server = netapp_utils.setup_na_ontap_zapi(
-                module=self.module, vserver=self.vserver)
+                module=self.module, vserver=self.parameters['vserver'])
         return
 
+    def get_snapshot(self, snapshot_name=None):
+        """
+        Checks to see if a snapshot exists or not
+        :return: Return True if a snapshot exists, False if it doesn't
+        """
+        if snapshot_name is None:
+            snapshot_name = self.parameters['snapshot']
+        snapshot_obj = netapp_utils.zapi.NaElement("snapshot-get-iter")
+        desired_attr = netapp_utils.zapi.NaElement("desired-attributes")
+        snapshot_info = netapp_utils.zapi.NaElement('snapshot-info')
+        comment = netapp_utils.zapi.NaElement('comment')
+        snapmirror_label = netapp_utils.zapi.NaElement('snapmirror-label')
+        # add more desired attributes that are allowed to be modified
+        snapshot_info.add_child_elem(comment)
+        snapshot_info.add_child_elem(snapmirror_label)
+        desired_attr.add_child_elem(snapshot_info)
+        snapshot_obj.add_child_elem(desired_attr)
+        # compose query
+        query = netapp_utils.zapi.NaElement("query")
+        snapshot_info_obj = netapp_utils.zapi.NaElement("snapshot-info")
+        snapshot_info_obj.add_new_child("name", snapshot_name)
+        snapshot_info_obj.add_new_child("volume", self.parameters['volume'])
+        query.add_child_elem(snapshot_info_obj)
+        snapshot_obj.add_child_elem(query)
+        result = self.server.invoke_successfully(snapshot_obj, True)
+        return_value = None
+        if result.get_child_by_name('num-records') and \
+                int(result.get_child_content('num-records')) == 1:
+            attributes_list = result.get_child_by_name('attributes-list')
+            snap_info = attributes_list.get_child_by_name('snapshot-info')
+            return_value = {'comment': snap_info.get_child_content('comment')}
+            if snap_info.get_child_by_name('snapmirror-label'):
+                return_value['snapmirror_label'] = snap_info.get_child_content('snapmirror-label')
+            else:
+                return_value['snapmirror_label'] = None
+        return return_value
+
     def create_snapshot(self):
         """
         Creates a new snapshot
@@ -165,21 +195,21 @@ class NetAppOntapSnapshot(object):
         snapshot_obj = netapp_utils.zapi.NaElement("snapshot-create")
 
         # set up required variables to create a snapshot
-        snapshot_obj.add_new_child("snapshot", self.snapshot)
-        snapshot_obj.add_new_child("volume", self.volume)
+        snapshot_obj.add_new_child("snapshot", self.parameters['snapshot'])
+        snapshot_obj.add_new_child("volume", self.parameters['volume'])
         # Set up optional variables to create a snapshot
-        if self.async_bool:
-            snapshot_obj.add_new_child("async", self.async_bool)
-        if self.comment:
-            snapshot_obj.add_new_child("comment", self.comment)
-        if self.snapmirror_label:
+        if self.parameters.get('async_bool'):
+            snapshot_obj.add_new_child("async", self.parameters['async_bool'])
+        if self.parameters.get('comment'):
+            snapshot_obj.add_new_child("comment", self.parameters['comment'])
+        if self.parameters.get('snapmirror_label'):
             snapshot_obj.add_new_child(
-                "snapmirror-label", self.snapmirror_label)
+                "snapmirror-label", self.parameters['snapmirror_label'])
         try:
             self.server.invoke_successfully(snapshot_obj, True)
         except netapp_utils.zapi.NaApiError as error:
             self.module.fail_json(msg='Error creating snapshot %s: %s' %
-                                  (self.snapshot, to_native(error)),
+                                  (self.parameters['snapshot'], to_native(error)),
                                   exception=traceback.format_exc())
 
     def delete_snapshot(self):
@@ -189,19 +219,18 @@ class NetAppOntapSnapshot(object):
         snapshot_obj = netapp_utils.zapi.NaElement("snapshot-delete")
 
         # Set up required variables to delete a snapshot
-        snapshot_obj.add_new_child("snapshot", self.snapshot)
-        snapshot_obj.add_new_child("volume", self.volume)
+        snapshot_obj.add_new_child("snapshot", self.parameters['snapshot'])
+        snapshot_obj.add_new_child("volume", self.parameters['volume'])
         # set up optional variables to delete a snapshot
-        if self.ignore_owners:
-            snapshot_obj.add_new_child("ignore-owners", self.ignore_owners)
-        if self.snapshot_instance_uuid:
-            snapshot_obj.add_new_child(
-                "snapshot-instance-uuid", self.snapshot_instance_uuid)
+        if self.parameters.get('ignore_owners'):
+            snapshot_obj.add_new_child("ignore-owners", self.parameters['ignore_owners'])
+        if self.parameters.get('snapshot_instance_uuid'):
+            snapshot_obj.add_new_child("snapshot-instance-uuid", self.parameters['snapshot_instance_uuid'])
         try:
             self.server.invoke_successfully(snapshot_obj, True)
         except netapp_utils.zapi.NaApiError as error:
             self.module.fail_json(msg='Error deleting snapshot %s: %s' %
-                                  (self.snapshot, to_native(error)),
+                                  (self.parameters['snapshot'], to_native(error)),
                                   exception=traceback.format_exc())
 
     def modify_snapshot(self):
@@ -213,95 +242,79 @@ class NetAppOntapSnapshot(object):
         # Create query object, this is the existing object
         query = netapp_utils.zapi.NaElement("query")
         snapshot_info_obj = netapp_utils.zapi.NaElement("snapshot-info")
-        snapshot_info_obj.add_new_child("name", self.snapshot)
+        snapshot_info_obj.add_new_child("name", self.parameters['snapshot'])
         query.add_child_elem(snapshot_info_obj)
         snapshot_obj.add_child_elem(query)
 
         # this is what we want to modify in the snapshot object
         attributes = netapp_utils.zapi.NaElement("attributes")
         snapshot_info_obj = netapp_utils.zapi.NaElement("snapshot-info")
-        snapshot_info_obj.add_new_child("name", self.snapshot)
-        if self.comment is not None:
-            snapshot_info_obj.add_new_child("comment", self.comment)
+        snapshot_info_obj.add_new_child("name", self.parameters['snapshot'])
+        if self.parameters.get('comment'):
+            snapshot_info_obj.add_new_child("comment", self.parameters['comment'])
+        if self.parameters.get('snapmirror_label'):
+            snapshot_info_obj.add_new_child("snapmirror-label", self.parameters['snapmirror_label'])
         attributes.add_child_elem(snapshot_info_obj)
         snapshot_obj.add_child_elem(attributes)
         try:
             self.server.invoke_successfully(snapshot_obj, True)
         except netapp_utils.zapi.NaApiError as error:
             self.module.fail_json(msg='Error modifying snapshot %s: %s' %
-                                  (self.snapshot, to_native(error)),
+                                  (self.parameters['snapshot'], to_native(error)),
                                   exception=traceback.format_exc())
 
-    def does_snapshot_exist(self):
+    def rename_snapshot(self):
         """
-        Checks to see if a snapshot exists or not
-        :return: Return True if a snapshot exists, false if it dosn't
+        Rename the sanpshot
         """
-        snapshot_obj = netapp_utils.zapi.NaElement("snapshot-get-iter")
-        desired_attr = netapp_utils.zapi.NaElement("desired-attributes")
-        snapshot_info = netapp_utils.zapi.NaElement('snapshot-info')
-        comment = netapp_utils.zapi.NaElement('comment')
-        # add more desired attributes that are allowed to be modified
-        snapshot_info.add_child_elem(comment)
-        desired_attr.add_child_elem(snapshot_info)
-        snapshot_obj.add_child_elem(desired_attr)
-        # compose query
-        query = netapp_utils.zapi.NaElement("query")
-        snapshot_info_obj = netapp_utils.zapi.NaElement("snapshot-info")
-        snapshot_info_obj.add_new_child("name", self.snapshot)
-        snapshot_info_obj.add_new_child("volume", self.volume)
-        query.add_child_elem(snapshot_info_obj)
-        snapshot_obj.add_child_elem(query)
-        result = self.server.invoke_successfully(snapshot_obj, True)
-        return_value = None
-        # TODO: Snapshot with the same name will mess this up,
-        # need to fix that later
-        if result.get_child_by_name('num-records') and \
-                int(result.get_child_content('num-records')) == 1:
-            attributes_list = result.get_child_by_name('attributes-list')
-            snap_info = attributes_list.get_child_by_name('snapshot-info')
-            return_value = {'comment': snap_info.get_child_content('comment')}
-        return return_value
+        snapshot_obj = netapp_utils.zapi.NaElement("snapshot-rename")
+
+        # set up required variables to rename a snapshot
+        snapshot_obj.add_new_child("current-name", self.parameters['from_name'])
+        snapshot_obj.add_new_child("new-name", self.parameters['snapshot'])
+        snapshot_obj.add_new_child("volume", self.parameters['volume'])
+        try:
+            self.server.invoke_successfully(snapshot_obj, True)
+        except netapp_utils.zapi.NaApiError as error:
+            self.module.fail_json(msg='Error renaming snapshot %s to %s: %s' %
+                                  (self.parameters['from_name'], self.parameters['snapshot'], to_native(error)),
+                                  exception=traceback.format_exc())
 
     def apply(self):
         """
         Check to see which play we should run
         """
-        changed = False
-        comment_changed = False
+        current = self.get_snapshot()
         netapp_utils.ems_log_event("na_ontap_snapshot", self.server)
-        existing_snapshot = self.does_snapshot_exist()
-        if existing_snapshot is not None:
-            if self.state == 'absent':
-                changed = True
-            elif self.state == 'present' and self.comment is not None:
-                if existing_snapshot['comment'] != self.comment:
-                    comment_changed = True
-                    changed = True
+        rename, cd_action = None, None
+        modify = {}
+        if self.parameters.get('from_name'):
+            current_old_name = self.get_snapshot(self.parameters['from_name'])
+            rename = self.na_helper.is_rename_action(current_old_name, current)
+            modify = self.na_helper.get_modified_attributes(current_old_name, self.parameters)
         else:
-            if self.state == 'present':
-                changed = True
-        if changed:
+            cd_action = self.na_helper.get_cd_action(current, self.parameters)
+            if cd_action is None:
+                modify = self.na_helper.get_modified_attributes(current, self.parameters)
+        if self.na_helper.changed:
             if self.module.check_mode:
                 pass
             else:
-                if self.state == 'present':
-                    if not existing_snapshot:
-                        self.create_snapshot()
-                    elif comment_changed:
-                        self.modify_snapshot()
-                elif self.state == 'absent':
-                    if existing_snapshot:
-                        self.delete_snapshot()
-
-        self.module.exit_json(changed=changed)
+                if rename:
+                    self.rename_snapshot()
+                if cd_action == 'create':
+                    self.create_snapshot()
+                elif cd_action == 'delete':
+                    self.delete_snapshot()
+                elif modify:
+                    self.modify_snapshot()
+        self.module.exit_json(changed=self.na_helper.changed)
 
 
 def main():
     """
     Creates, modifies, and deletes a Snapshot
     """
-
     obj = NetAppOntapSnapshot()
     obj.apply()
 
diff --git a/test/units/modules/storage/netapp/test_na_ontap_snapshot.py b/test/units/modules/storage/netapp/test_na_ontap_snapshot.py
new file mode 100644
index 0000000000..d4e140b396
--- /dev/null
+++ b/test/units/modules/storage/netapp/test_na_ontap_snapshot.py
@@ -0,0 +1,226 @@
+# (c) 2018, NetApp, Inc
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+''' unit tests ONTAP Ansible module: na_ontap_nvme_snapshot'''
+
+from __future__ import print_function
+import json
+import pytest
+
+from units.compat import unittest
+from units.compat.mock import patch
+from ansible.module_utils import basic
+from ansible.module_utils._text import to_bytes
+import ansible.module_utils.netapp as netapp_utils
+
+from ansible.modules.storage.netapp.na_ontap_snapshot \
+    import NetAppOntapSnapshot as my_module
+
+if not netapp_utils.has_netapp_lib():
+    pytestmark = pytest.mark.skip('skipping as missing required netapp_lib')
+
+
+def set_module_args(args):
+    """prepare arguments so that they will be picked up during module creation"""
+    args = json.dumps({'ANSIBLE_MODULE_ARGS': args})
+    basic._ANSIBLE_ARGS = to_bytes(args)  # pylint: disable=protected-access
+
+
+class AnsibleExitJson(Exception):
+    """Exception class to be raised by module.exit_json and caught by the test case"""
+    pass
+
+
+class AnsibleFailJson(Exception):
+    """Exception class to be raised by module.fail_json and caught by the test case"""
+    pass
+
+
+def exit_json(*args, **kwargs):  # pylint: disable=unused-argument
+    """function to patch over exit_json; package return data into an exception"""
+    if 'changed' not in kwargs:
+        kwargs['changed'] = False
+    raise AnsibleExitJson(kwargs)
+
+
+def fail_json(*args, **kwargs):  # pylint: disable=unused-argument
+    """function to patch over fail_json; package return data into an exception"""
+    kwargs['failed'] = True
+    raise AnsibleFailJson(kwargs)
+
+
+class MockONTAPConnection(object):
+    ''' mock server connection to ONTAP host '''
+
+    def __init__(self, kind=None):
+        ''' save arguments '''
+        self.type = kind
+        self.xml_in = None
+        self.xml_out = None
+
+    def invoke_successfully(self, xml, enable_tunneling):  # pylint: disable=unused-argument
+        ''' mock invoke_successfully returning xml data '''
+        self.xml_in = xml
+        if self.type == 'snapshot':
+            xml = self.build_snapshot_info()
+        elif self.type == 'snapshot_fail':
+            raise netapp_utils.zapi.NaApiError(code='TEST', message="This exception is from the unit test")
+        self.xml_out = xml
+        return xml
+
+    @staticmethod
+    def build_snapshot_info():
+        ''' build xml data for snapshot-info '''
+        xml = netapp_utils.zapi.NaElement('xml')
+        data = {'num-records': 1,
+                'attributes-list': {'snapshot-info': {'comment': 'new comment',
+                                                      'name': 'ansible',
+                                                      'snapmirror-label': 'label12'}}}
+        xml.translate_struct(data)
+        return xml
+
+
+class TestMyModule(unittest.TestCase):
+    ''' a group of related Unit Tests '''
+
+    def setUp(self):
+        self.mock_module_helper = patch.multiple(basic.AnsibleModule,
+                                                 exit_json=exit_json,
+                                                 fail_json=fail_json)
+        self.mock_module_helper.start()
+        self.addCleanup(self.mock_module_helper.stop)
+        self.server = MockONTAPConnection()
+        self.onbox = False
+
+    def set_default_args(self):
+        if self.onbox:
+            hostname = '10.193.75.3'
+            username = 'admin'
+            password = 'netapp1!'
+            vserver = 'ansible'
+            volume = 'ansible'
+            snapshot = 'ansible'
+            comment = 'new comment'
+            snapmirror_label = 'label12'
+        else:
+            hostname = 'hostname'
+            username = 'username'
+            password = 'password'
+            vserver = 'vserver'
+            volume = 'ansible'
+            snapshot = 'ansible'
+            comment = 'new comment'
+            snapmirror_label = 'label12'
+        return dict({
+            'hostname': hostname,
+            'username': username,
+            'password': password,
+            'vserver': vserver,
+            'volume': volume,
+            'snapshot': snapshot,
+            'comment': comment,
+            'snapmirror_label': snapmirror_label
+        })
+
+    def test_module_fail_when_required_args_missing(self):
+        ''' required arguments are reported as errors '''
+        with pytest.raises(AnsibleFailJson) as exc:
+            set_module_args({})
+            my_module()
+        print('Info: %s' % exc.value.args[0]['msg'])
+
+    def test_ensure_get_called(self):
+        ''' test get_snapshot()  for non-existent snapshot'''
+        set_module_args(self.set_default_args())
+        my_obj = my_module()
+        my_obj.server = self.server
+        assert my_obj.get_snapshot() is None
+
+    def test_ensure_get_called_existing(self):
+        ''' test get_snapshot()  for existing snapshot'''
+        set_module_args(self.set_default_args())
+        my_obj = my_module()
+        my_obj.server = MockONTAPConnection(kind='snapshot')
+        assert my_obj.get_snapshot()
+
+    @patch('ansible.modules.storage.netapp.na_ontap_snapshot.NetAppOntapSnapshot.create_snapshot')
+    def test_successful_create(self, create_snapshot):
+        ''' creating snapshot and testing idempotency '''
+        set_module_args(self.set_default_args())
+        my_obj = my_module()
+        if not self.onbox:
+            my_obj.server = self.server
+        with pytest.raises(AnsibleExitJson) as exc:
+            my_obj.apply()
+        assert exc.value.args[0]['changed']
+        create_snapshot.assert_called_with()
+        # to reset na_helper from remembering the previous 'changed' value
+        my_obj = my_module()
+        if not self.onbox:
+            my_obj.server = MockONTAPConnection('snapshot')
+        with pytest.raises(AnsibleExitJson) as exc:
+            my_obj.apply()
+        assert not exc.value.args[0]['changed']
+
+    @patch('ansible.modules.storage.netapp.na_ontap_snapshot.NetAppOntapSnapshot.modify_snapshot')
+    def test_successful_modify(self, modify_snapshot):
+        ''' modifying snapshot and testing idempotency '''
+        data = self.set_default_args()
+        data['comment'] = 'adding comment'
+        data['snapmirror_label'] = 'label22'
+        set_module_args(data)
+        my_obj = my_module()
+        if not self.onbox:
+            my_obj.server = MockONTAPConnection('snapshot')
+        with pytest.raises(AnsibleExitJson) as exc:
+            my_obj.apply()
+        assert exc.value.args[0]['changed']
+        modify_snapshot.assert_called_with()
+        # to reset na_helper from remembering the previous 'changed' value
+        data['comment'] = 'new comment'
+        data['snapmirror_label'] = 'label12'
+        set_module_args(data)
+        my_obj = my_module()
+        if not self.onbox:
+            my_obj.server = MockONTAPConnection('snapshot')
+        with pytest.raises(AnsibleExitJson) as exc:
+            my_obj.apply()
+        assert not exc.value.args[0]['changed']
+
+    @patch('ansible.modules.storage.netapp.na_ontap_snapshot.NetAppOntapSnapshot.delete_snapshot')
+    def test_successful_delete(self, delete_snapshot):
+        ''' deleting snapshot and testing idempotency '''
+        data = self.set_default_args()
+        data['state'] = 'absent'
+        set_module_args(data)
+        my_obj = my_module()
+        if not self.onbox:
+            my_obj.server = MockONTAPConnection('snapshot')
+        with pytest.raises(AnsibleExitJson) as exc:
+            my_obj.apply()
+        assert exc.value.args[0]['changed']
+        delete_snapshot.assert_called_with()
+        # to reset na_helper from remembering the previous 'changed' value
+        my_obj = my_module()
+        if not self.onbox:
+            my_obj.server = self.server
+        with pytest.raises(AnsibleExitJson) as exc:
+            my_obj.apply()
+        assert not exc.value.args[0]['changed']
+
+    def test_if_all_methods_catch_exception(self):
+        module_args = {}
+        module_args.update(self.set_default_args())
+        set_module_args(module_args)
+        my_obj = my_module()
+        if not self.onbox:
+            my_obj.server = MockONTAPConnection('snapshot_fail')
+        with pytest.raises(AnsibleFailJson) as exc:
+            my_obj.create_snapshot()
+        assert 'Error creating snapshot ansible:' in exc.value.args[0]['msg']
+        with pytest.raises(AnsibleFailJson) as exc:
+            my_obj.delete_snapshot()
+        assert 'Error deleting snapshot ansible:' in exc.value.args[0]['msg']
+        with pytest.raises(AnsibleFailJson) as exc:
+            my_obj.modify_snapshot()
+        assert 'Error modifying snapshot ansible:' in exc.value.args[0]['msg']