Repository: ansible/ansible
Pull request: Ovirt delete snapshots after x days 
##### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->

Fixes #41500

##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
- Bugfix Pull Request
- Feature Pull Request

##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below -->
ovirt
##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes -->
```paste below
ansible 2.6.3
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/mnecas/Desktop/Projects/Redhat/ansible/lib/ansible/modules/cloud/ovirt']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.15 (default, May 16 2018, 17:50:09) [GCC 8.1.1 20180502 (Red Hat 8.1.1-1)]

```

##### ADDITIONAL INFORMATION
<!--- Include additional information to help people understand the change here -->
<!--- A step-by-step reproduction of the problem is helpful if there is no related issue -->

<!--- Paste verbatim command output below, e.g. before and after your change -->
```paste below

```


Problem statement:
Allow for ovirt_snapshots to be deleted based on date differences
<!---
Verify first that your issue/request is not already reported on GitHub.
THIS FORM WILL BE READ BY A MACHINE, COMPLETE ALL SECTIONS AS DESCRIBED.
Also test if the latest release, and devel branch are affected too.
ALWAYS add information AFTER (OUTSIDE) these html comments.
Otherwise it may end up being automatically closed by our bot. -->

##### SUMMARY
<!--- Explain the problem briefly -->
I'd like to be able to easily delete snapshots older than X days using the ovirt_snapshots module.  The ovirt_snapshots_facts module returns a formatted date parameter for each snapshot.

##### ISSUE TYPE
 - Feature Idea

##### COMPONENT NAME
<!--- Insert, BELOW THIS COMMENT, the name of the module, plugin, task or feature.
Do not include extra details here, e.g. "vyos_command" not "the network module vyos_command" or the full path-->
ovirt_snapshots

##### ANSIBLE VERSION
<!--- Paste, BELOW THIS COMMENT, verbatim output from "ansible --version" between quotes below -->
```
ansible 2.5.2
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/usr/lib/python2.7/site-packages/ara/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /bin/ansible
  python version = 2.7.5 (default, Feb 20 2018, 09:19:12) [GCC 4.8.5 20150623 (Red Hat 4.8.5-28)]
```

##### CONFIGURATION
<!--- If using Ansible 2.4 or above, paste, BELOW THIS COMMENT, the results of "ansible-config dump --only-changed"
Otherwise, mention any settings you have changed/added/removed in ansible.cfg
(or using the ANSIBLE_* environment variables).-->
DEFAULT_ACTION_PLUGIN_PATH(/etc/ansible/ansible.cfg) = [u'/usr/lib/python2.7/site-packages/ara/plugins/actions']
DEFAULT_ASK_PASS(/etc/ansible/ansible.cfg) = False
DEFAULT_BECOME(/etc/ansible/ansible.cfg) = True
DEFAULT_BECOME_ASK_PASS(/etc/ansible/ansible.cfg) = False
DEFAULT_BECOME_METHOD(/etc/ansible/ansible.cfg) = sudo
DEFAULT_BECOME_USER(/etc/ansible/ansible.cfg) = root
DEFAULT_CALLBACK_PLUGIN_PATH(/etc/ansible/ansible.cfg) = [u'/usr/lib/python2.7/site-packages/ara/plugins/callbacks']
DEFAULT_FORKS(/etc/ansible/ansible.cfg) = 25
DEFAULT_LOAD_CALLBACK_PLUGINS(/etc/ansible/ansible.cfg) = True
DEFAULT_LOG_PATH(/etc/ansible/ansible.cfg) = /var/log/ansible.log
DEFAULT_MODULE_PATH(/etc/ansible/ansible.cfg) = [u'/usr/lib/python2.7/site-packages/ara/plugins/modules']
DEFAULT_PRIVATE_KEY_FILE(/etc/ansible/ansible.cfg) = /root/.ssh/id_rsa
DEFAULT_REMOTE_PORT(/etc/ansible/ansible.cfg) = 9022
DEFAULT_REMOTE_USER(/etc/ansible/ansible.cfg) = ansible
DEFAULT_STDOUT_CALLBACK(/etc/ansible/ansible.cfg) = yaml
HOST_KEY_CHECKING(/etc/ansible/ansible.cfg) = False
RETRY_FILES_ENABLED(/etc/ansible/ansible.cfg) = False

##### OS / ENVIRONMENT
<!--- Mention, BELOW THIS COMMENT, the OS you are running Ansible from, and the OS you are
managing, or say "N/A" for anything that is not platform-specific.
Also mention the specific version of what you are trying to control,
e.g. if this is a network bug the version of firmware on the network device.-->
Red Hat Enterprise Linux Server release 7.5 (Maipo)

Hints from issue discussion:
Files identified in the description:
* [lib/ansible/modules/cloud/ovirt/ovirt_snapshots.py](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/cloud/ovirt/ovirt_snapshots.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
cc @machacekondra @mwperina
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
We can implement this, but you can workaround with something like:

```yaml
  - name: List snaphosts
    ovirt_snapshots_facts:
      auth: "{{ ovirt_auth }}"
      vm: myvm

  - name: Remove snaphosts older then week
    ovirt_snapshots:
      auth: "{{ ovirt_auth }}"
      state: absent
      vm_name: myvm
      snapshot_id: "{{ item.id }}"
    with_items: "{{ ovirt_snapshots }}"
    when: "{{( ansible_date_time.date|to_datetime('%Y-%m-%d') - item.date[:-6]|to_datetime('%Y-%m-%d %H:%M:%S.%f')).days > 7 }}"

```
I ended up doing it this way:

```
  - name: Get all snapshots
    ovirt_snapshots_facts:
      auth:
        username: "admin@internal"
        password: "password"
        insecure: True
        url: https://rhevm.domain.com/ovirt-engine/api
      vm: "{{ ansible_hostname }}"
    delegate_to: localhost
  - shell: date '+%Y-%m-%d' --date='7 days ago'
    register: old_date
    delegate_to: localhost
  - name: Delete snapshots if they are older than 1 week
    ovirt_snapshots:
      auth:
        username: "admin@internal"
        password: "password"
        insecure: True
        url: https://rhevm.domain.com/ovirt-engine/api
      snapshot_id: "{{ item.id }}"
      state: absent
      timeout: 600
      vm_name: "{{ ansible_hostname }}"
    loop: "{{ ovirt_snapshots }}"
    when:
      - item.snapshot_type == "regular"
      - item.date[:10] < old_date.stdout
    delegate_to: localhost

Files identified in the description:
* [lib/ansible/modules/cloud/ovirt/_ovirt_snapshots.py](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/cloud/ovirt/_ovirt_snapshots.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
Files identified in the description:
None

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->

Commit message: put remove logic to function

Patch to review:
diff --git a/lib/ansible/modules/cloud/ovirt/ovirt_snapshot.py b/lib/ansible/modules/cloud/ovirt/ovirt_snapshot.py
index e988c3e820f8bc..6b84ea5cbac324 100644
--- a/lib/ansible/modules/cloud/ovirt/ovirt_snapshot.py
+++ b/lib/ansible/modules/cloud/ovirt/ovirt_snapshot.py
@@ -59,6 +59,11 @@
             - "restore_memory"
             - "save_memory"
         type: bool
+    keep_days_old:
+        description:
+            - "Number of days after which should snapshot be deleted."
+            - "It will check all snapshots of virtual machine and delete them, if they are older."
+        version_added: "2.8"
 notes:
     - "Note that without a guest agent the data on the created snapshot may be
        inconsistent."
@@ -98,6 +103,11 @@
     state: absent
     vm_name: rhel7
     snapshot_id: "{{ snapshot.id }}"
+
+# Delete all snapshots older than 2 days
+- ovirt_snapshot:
+    vm_name: test
+    keep_days_old: 2
 '''
 
 
@@ -112,6 +122,10 @@
                   at following url: http://ovirt.github.io/ovirt-engine-api-model/master/#types/snapshot."
     returned: On success if snapshot is found.
     type: dict
+snapshots:
+    description: List of deleted snapshots
+    returned: On success returns deleted snapshots
+    type: list
 '''
 
 
@@ -222,6 +236,23 @@ def restore_snapshot(module, vm_service, snapshots_service):
     }
 
 
+def remove_old_snapshosts(module, snapshots_service):
+    from datetime import datetime
+    import time
+    deleted_snapshot = []
+    changed = False
+    date_now = datetime.now()
+    for snapshot in snapshots_service.list():
+        if snapshot.vm and snapshot.vm.name == module.params.get('vm_name'):
+            diff = date_now - snapshot.date.replace(tzinfo=None)
+            if diff.days >= module.params.get('keep_days_old'):
+                snapshots_service.snapshot_service(snapshot.id).remove()
+                deleted_snapshot.append(get_dict_of_struct(snapshot))
+                changed = True
+                time.sleep(45)
+    return dict(snapshots=deleted_snapshot, changed=changed)
+
+
 def main():
     argument_spec = ovirt_full_argument_spec(
         state=dict(
@@ -231,6 +262,7 @@ def main():
         vm_name=dict(required=True),
         snapshot_id=dict(default=None),
         description=dict(default=None),
+        keep_days_old=dict(default=None, type='int'),
         use_memory=dict(
             default=None,
             type='bool',
@@ -266,7 +298,10 @@ def main():
     try:
         state = module.params['state']
         if state == 'present':
-            ret = create_snapshot(module, vm_service, snapshots_service)
+            if module.params.get('keep_days_old') is not None:
+                ret = remove_old_snapshosts(module, snapshots_service)
+            else:
+                ret = create_snapshot(module, vm_service, snapshots_service)
         elif state == 'restore':
             ret = restore_snapshot(module, vm_service, snapshots_service)
         elif state == 'absent':


Code review comments:
diff_hunk: @@ -222,6 +236,23 @@ def restore_snapshot(module, vm_service, snapshots_service):
     }
 
 
+def remove_old_snapshosts(module, snapshots_service):
+    from datetime import datetime
+    import time
+    deleted_snapshot = []
+    changed = False
+    date_now = datetime.now()
+    for snapshot in snapshots_service.list():
+        if snapshot.vm and snapshot.vm.name == module.params.get('vm_name'):
path: lib/ansible/modules/cloud/ovirt/ovirt_snapshot.py
text: @user1:
I think it should be just fine to check `if snapshot.vm is not None` because you are already using `snapshots_service` of that vm, so you don't need to check if names are same.

diff_hunk: @@ -222,6 +236,23 @@ def restore_snapshot(module, vm_service, snapshots_service):
     }
 
 
+def remove_old_snapshosts(module, snapshots_service):
+    from datetime import datetime
+    import time
+    deleted_snapshot = []
+    changed = False
+    date_now = datetime.now()
+    for snapshot in snapshots_service.list():
+        if snapshot.vm and snapshot.vm.name == module.params.get('vm_name'):
+            diff = date_now - snapshot.date.replace(tzinfo=None)
+            if diff.days >= module.params.get('keep_days_old'):
+                snapshots_service.snapshot_service(snapshot.id).remove()
+                deleted_snapshot.append(get_dict_of_struct(snapshot))
+                changed = True
+                time.sleep(45)
path: lib/ansible/modules/cloud/ovirt/ovirt_snapshot.py
text: @user1:
We need to wait until snaphsot is removed and vm ready to remove another snapshot, not any random wait time.

diff_hunk: @@ -222,6 +236,23 @@ def restore_snapshot(module, vm_service, snapshots_service):
     }
 
 
+def remove_old_snapshosts(module, snapshots_service):
+    from datetime import datetime
path: lib/ansible/modules/cloud/ovirt/ovirt_snapshot.py
text: @user1:
Please do this import at the begging of the file with other imports.