Skip to content

lvm_pv: new module for LVM Physical Volumes #10070

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jun 7, 2025

Conversation

klention
Copy link
Contributor

@klention klention commented Apr 25, 2025

SUMMARY

This pull request introduces a new lvm_pv module for managing LVM Physical Volumes. The module provides comprehensive functionality for:

  • Creating and removing physical volumes
  • Resizing existing PVs (with automatic device rescan capability)
  • Forceful operations when needed
  • Full idempotency and check mode support

Key features:

  • Supports all basic PV operations (pvcreate, pvremove, pvresize)
  • Automatically handles device rescans before resize operations
  • Provides clear status messages and change detection
  • Includes thorough parameter validation and error handling

The module fills a gap in Ansible's storage management capabilities, complementing the existing lvg and lvol modules.

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

lvm_pv

ADDITIONAL INFORMATION

Example usage:

- name: Creating physical volume on /dev/sdb
  community.general.lvm_pv:
    device: /dev/sdb

- name: Creating and resizing (if needed) physical volume
  community.general.lvm_pv:
    device: /dev/sdb
    resize: true

- name: Removing physical volume that is not part of any volume group
  community.general.lvm_pv:
    device: /dev/sdb
    state: absent

- name: Force removing physical volume that is already part of a volume group
  community.general.lvm_pv:
    device: /dev/sdb
    force: true
    state: absent

@ansibullbot ansibullbot added module module new_contributor Help guide this first time contributor new_plugin New plugin plugins plugin (any type) labels Apr 25, 2025
@ansibullbot

This comment has been minimized.

@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Apr 25, 2025
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-10 Automatically create a backport for the stable-10 branch labels Apr 26, 2025
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution! Two first quick suggestions:

@ansibullbot ansibullbot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI and removed ci_verified Push fixes to PR branch to re-run CI labels Apr 26, 2025
@ansibullbot

This comment has been minimized.

@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI labels Apr 26, 2025
@ansibullbot ansibullbot removed the ci_verified Push fixes to PR branch to re-run CI label Apr 26, 2025
@ansibullbot ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Apr 26, 2025
Copy link
Collaborator

@russoz russoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @klention

Thanks for your contribution. I left some initial comments.

You might want to consider using CmdRunner. See https://docs.ansible.com/ansible/latest/collections/community/general/docsite/guide_cmdrunner.html for more details.

Copy link
Collaborator

@russoz russoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of (minor) suggestions, other than that LGTM

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't forget to add tests, otherwise this won't get merged. (See https://github.com/ansible-collections/community.general/blob/main/CONTRIBUTING.md#creating-new-modules-or-plugins.)

@ansibullbot ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label May 10, 2025
@felixfontein
Copy link
Collaborator

ping @klention

@ansibullbot ansibullbot removed the stale_ci CI is older than 7 days, rerun before merging label May 23, 2025
@ansibullbot ansibullbot added integration tests/integration tests tests needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed stale_ci CI is older than 7 days, rerun before merging labels Jun 4, 2025
@klention klention marked this pull request as draft June 4, 2025 18:39
@ansibullbot ansibullbot added the WIP Work in progress label Jun 4, 2025
@klention klention force-pushed the lpv branch 2 times, most recently from 09be8cd to 1effe6b Compare June 4, 2025 20:28
@klention klention marked this pull request as ready for review June 4, 2025 20:47
@ansibullbot ansibullbot removed WIP Work in progress needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jun 4, 2025
@russoz russoz changed the title Added LVM Physical Volume module lvm_pv: new module for LVM Physical Volumes Jun 7, 2025
Copy link
Collaborator

@russoz russoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @klention Thanks for the updates, it is looking very good now.

I just spotted a couple of minor things, but other than that it is good to go.

@klention
Copy link
Contributor Author

klention commented Jun 7, 2025

Hi @russoz,

Thank you for your guidance and support!
I just applied your suggestions.
Please let me know if anything else is needed.

Copy link
Collaborator

@russoz russoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for your work on the module!

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better adjust the YAML list indentation to the style all other modules and plugins use:

@felixfontein felixfontein merged commit 367b28d into ansible-collections:main Jun 7, 2025
110 checks passed
@felixfontein
Copy link
Collaborator

@klention thanks for your contribution!
@russoz thanks for reviewing!

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Jun 7, 2025
@klention klention deleted the lpv branch June 7, 2025 18:30
@klention klention restored the lpv branch June 7, 2025 18:45
@klention klention deleted the lpv branch June 7, 2025 18:45
vbotka pushed a commit to vbotka/community.general that referenced this pull request Jun 10, 2025
* Added LVM Physical Volume module

* Fixed CI checks

* python 2.7 compatibility

* Fixed another fprint line not compatible with python 2.x

* Applied cosmetic changes

* Removed msg from RETURN section

* Updated the 'absent state' block logic

* Added integration tests

* Updated logic for creating loop devices on Alpine Linux

* Updated loop device path

* Minor, cosmetic changes

* Adjust indentation.

---------

Co-authored-by: Felix Fontein <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration tests/integration module module new_contributor Help guide this first time contributor new_plugin New plugin plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants