Create ovh_block_volume_snapshot.py #123

jawadevops · 2025-06-05T16:01:20Z

add snapshot volume

exemple:

name: Create a snapshot of a block volume in OVH
hosts: localhost
gather_facts: false
tasks:
- name: Take snapshot of volume
  synthesio.ovh.ovh_block_volume_snapshot:
  service_name: "{{ service_name }}"
  volume_id: "{{ volume_id }}"
  snapshot_name: "{{ snapshot_name }}"
  description: "{{ snapshot_description }}"

add snapshot volume

gemini-code-assist

Hello @jawadevops, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

Hello! Gemini here, providing a summary of this pull request. This PR introduces a new Ansible module, ovh_block_volume_snapshot, to the synthesio.ovh collection. The primary goal is to enable users to create snapshots of their OVH Public Cloud block storage volumes directly via Ansible playbooks. The module requires the OVH project ID (service_name), the ID of the target volume (volume_id), and a name for the new snapshot (snapshot_name). An optional description can also be provided for the snapshot.

Highlights

New Ansible Module: Adds a new module ovh_block_volume_snapshot to the collection.
OVH Block Volume Snapshots: Provides functionality to create snapshots for OVH Public Cloud block storage volumes.
Module Parameters: Requires service_name, volume_id, snapshot_name, and optionally accepts description.

Changelog

plugins/modules/ovh_block_volume_snapshot.py
- Added a new Ansible module to create snapshots of OVH block storage volumes.
- Implemented logic to call the OVH API endpoint /cloud/project/{service_name}/volume/{volume_id}/snapshot with provided parameters.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Cloud volume grows large,
Snapshot taken, quick and fast,
Data now is safe.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a new Ansible module ovh_block_volume_snapshot to create snapshots of OVH Public Cloud block storage volumes. The module is well-structured and follows the standard Ansible module pattern, including the necessary DOCUMENTATION, EXAMPLES, and RETURN blocks. The integration with the existing synthesio.ovh.ovh module utility is also correctly implemented.

However, there are a couple of areas that are critical for Ansible modules, particularly around idempotency and testing, which should be addressed before merging. I've provided specific comments on these points.

Summary of Findings

Lack of Idempotency: The module is not idempotent. It attempts to create a snapshot every time it runs, which will fail if a snapshot with the same name already exists. Logic should be added to check for existing snapshots and report changed=False if found (High severity, commented).
Missing Tests: There are no unit or integration tests included for this new module. Tests are crucial for verifying correctness, handling edge cases, and preventing regressions (High severity, not commented on code as it's a missing file).
Generic Exception Handling: The except Exception block is too broad and might hide specific errors. It's better to catch more specific exceptions or rely on the error handling in wrap_call (Medium severity, commented).
Author Field Placeholder: The author field in the documentation still contains the placeholder "Your Name" (Medium severity, commented).
Default Description: The default description "Snapshot created by Ansible" is functional but could potentially be slightly more specific (Low severity, not commented).
Check Mode Support: The module explicitly disables check mode (supports_check_mode=False). While sometimes necessary, supporting check mode is generally preferred for Ansible modules (Low severity, not commented).

Merge Readiness

This pull request introduces a valuable new module. However, the lack of idempotency and the absence of tests are significant concerns (High severity issues). These issues should be addressed before the module is merged to ensure its reliability and adherence to Ansible best practices. I am unable to approve this pull request directly; please have other reviewers approve this code after the suggested changes are made.

gemini-code-assist · 2025-06-05T16:02:06Z

plugins/modules/ovh_block_volume_snapshot.py

+    try:
+        result = client.wrap_call(
+            "POST",
+            f"/cloud/project/{service_name}/volume/{volume_id}/snapshot",
+            name=snapshot_name,
+            description=description
+        )
+        module.exit_json(changed=True, msg=f"Snapshot '{snapshot_name}' created successfully.", result=result)
+    except Exception as e:
+        module.fail_json(msg=f"Failed to create snapshot: {str(e)}")


Ansible modules should ideally be idempotent. This means running the module multiple times with the same parameters should result in the same final state and report changed=False after the first successful run.

The current implementation attempts to create the snapshot every time. If a snapshot with the given snapshot_name already exists for the volume, the OVH API will likely return an error, causing the module to fail on subsequent runs.

To make this module idempotent, consider adding logic to check if a snapshot with the specified snapshot_name already exists for the given volume_id before attempting to create it. If it exists, the module should report changed=False and exit successfully. This would likely involve making a GET call to the OVH API (e.g., listing snapshots for the volume and filtering by name) before the POST call to create the snapshot.

This might be a good point. Did you run the module multiple times with the same name?
Does it replace the snapshot, create a new one with the same name but a different creation date, or throw an error?

gemini-code-assist · 2025-06-05T16:02:06Z

plugins/modules/ovh_block_volume_snapshot.py

+    except Exception as e:
+        module.fail_json(msg=f"Failed to create snapshot: {str(e)}")


The except Exception as e: block is very broad and will catch any type of exception, including those not related to the API call itself. While client.wrap_call handles specific OVH API errors and calls module.fail_json, catching a generic Exception here might hide unexpected issues.

It's generally better practice to catch more specific exceptions if possible, or rely on the error handling within wrap_call for API-related issues. If there are other non-API exceptions you anticipate, catch those specifically. If not, you might not need this outer try...except block, or you could refine it.

gemini-code-assist · 2025-06-05T16:02:06Z

plugins/modules/ovh_block_volume_snapshot.py

+        required: false
+        type: str
+author:
+    - Your Name


The author field in the DOCUMENTATION block is currently set to "Your Name". Please update this field to reflect the actual author(s) or team responsible for this module.

author: - Synthesio SRE team <sre@synthesio.com>

tortuegenialez

Hi,
we are trying to keep a consistency with modules.
As this is operating on public cloud it should rather be named public_cloud_block_volume_snapshot.py

tortuegenialez · 2025-06-06T14:53:11Z

plugins/modules/ovh_block_volume_snapshot.py

+            description=description
+        )
+        module.exit_json(changed=True, msg=f"Snapshot '{snapshot_name}' created successfully.", result=result)
+    except Exception as e:


Exceptions are handled in plugins/module_utils/ovh.py

tortuegenialez · 2025-06-06T14:56:20Z

plugins/modules/ovh_block_volume_snapshot.py

+    try:
+        result = client.wrap_call(
+            "POST",
+            f"/cloud/project/{service_name}/volume/{volume_id}/snapshot",
+            name=snapshot_name,
+            description=description
+        )
+        module.exit_json(changed=True, msg=f"Snapshot '{snapshot_name}' created successfully.", result=result)
+    except Exception as e:
+        module.fail_json(msg=f"Failed to create snapshot: {str(e)}")


This might be a good point. Did you run the module multiple times with the same name?
Does it replace the snapshot, create a new one with the same name but a different creation date, or throw an error?

Create ovh_block_volume_snapshot.py

9586c8d

add snapshot volume

gemini-code-assist bot reviewed Jun 5, 2025

View reviewed changes

gemini-code-assist bot suggested changes Jun 5, 2025

View reviewed changes

tortuegenialez requested changes Jun 6, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Create ovh_block_volume_snapshot.py #123

Create ovh_block_volume_snapshot.py #123

Uh oh!

jawadevops commented Jun 5, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Jun 5, 2025

Uh oh!

tortuegenialez Jun 6, 2025

Uh oh!

gemini-code-assist bot Jun 5, 2025

Uh oh!

gemini-code-assist bot Jun 5, 2025

Uh oh!

tortuegenialez left a comment

Uh oh!

tortuegenialez Jun 6, 2025

Uh oh!

tortuegenialez Jun 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		except Exception as e:
		module.fail_json(msg=f"Failed to create snapshot: {str(e)}")

Create ovh_block_volume_snapshot.py #123

Are you sure you want to change the base?

Create ovh_block_volume_snapshot.py #123

Uh oh!

Conversation

jawadevops commented Jun 5, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Changelog

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Summary of Findings

Merge Readiness

Uh oh!

gemini-code-assist bot Jun 5, 2025

Choose a reason for hiding this comment

Uh oh!

tortuegenialez Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jun 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jun 5, 2025

Choose a reason for hiding this comment

Uh oh!

tortuegenialez left a comment

Choose a reason for hiding this comment

Uh oh!

tortuegenialez Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

tortuegenialez Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants