Skip to content
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

Add grain to list LVM volumes implement issue 57629 #57631

Conversation

piterpunk
Copy link
Collaborator

@piterpunk piterpunk commented Jun 10, 2020

What does this PR do?

This initial support implements a grain called "lvm" which returns a dictionary with the volume groups, each one with a list of their logical volumes inside.

LVM is an important information about a Linux machine. Usually Linux servers have most or all their disk filesystems using a logical volume as mountpoint.

What issues does this PR fix or reference?

Implements and resolves #57629

New Behavior

To have a grain with the list of LVM volumes:

root@marvin:~# salt-call grains.get lvm
local:
    ----------
    marvinvg00:
        - opt
        - root
        - swap
        - tmp
        - usr
        - var
    marvinvg01:
        - home
        - srv
        - usrlocal

Merge requirements satisfied?

salt-runtests-20200610192428.log

Commits signed with GPG?

No

@piterpunk piterpunk requested a review from a team as a code owner June 10, 2020 22:26
@ghost ghost requested review from DmitryKuzmenko and removed request for a team June 10, 2020 22:26
@piterpunk piterpunk changed the title Add grain to list LVN volumes implement issue 57629 Add grain to list LVM volumes implement issue 57629 Jun 10, 2020
@sagetherage sagetherage added the Feature new functionality including changes to functionality and code refactors, etc. label Jun 11, 2020
@dwoz dwoz added the Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases label Jun 16, 2020
@DmitryKuzmenko
Copy link
Contributor

@piterpunk thank you for contribution! Could you please cover your changes with tests?

@piterpunk
Copy link
Collaborator Author

@piterpunk thank you for contribution! Could you please cover your changes with tests?

Hi Dmitry, tests added. I hope they are Ok.

The tests for Linux and AIX are not the same because in AIX the LVM is part of base system, so it's always installed and with at least one volume group created.

@DmitryKuzmenko
Copy link
Contributor

@piterpunk thank you! I've restarted macos x mojave tests the failure looked not related to your changes.

@piterpunk
Copy link
Collaborator Author

@DmitryKuzmenko , seeing the console logs, the failure at MacOSX seems to be while setting up the environment, it didn't even began to run the tests.

DmitryKuzmenko
DmitryKuzmenko previously approved these changes Jun 19, 2020
@DmitryKuzmenko DmitryKuzmenko added Has Testcase and removed Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases labels Jun 19, 2020
@DmitryKuzmenko
Copy link
Contributor

@piterpunk Everything passed. Thank you for your work!

@sagetherage sagetherage added the Magnesium Mg release after Na prior to Al label Jul 22, 2020
piterpunk added 5 commits August 7, 2020 04:26
- LVM is an important information about a Linux machine.
  Usually Linux servers have most or all their disk
  filesystems using a logical volume as mountpoint.
- This initial support implements a grain called "lvm"
  which returns a dictionary with the volume groups, each
  one with a list of their logical volumes inside:

      VolumeGroup00:
          - LogicalVolume00
          - LogicalVolume01
          - ...
          - LogicalVolumeNN
      VolumeGroup01:
      ...
      VolumeGroupNN:
          - LogicalVolume00
          - LogicalVolume01
          - ...
          - LogicalVolumeNN
- Added tests/unit/grains/test_lvm.py file
- Tests covers _linux_lvm and _aix_lvm
@piterpunk piterpunk force-pushed the new-feature-add-grain-to-list-LVM-volumes_Implement-Issue-57629 branch from aae0adf to 7c5ea47 Compare August 7, 2020 12:20
@piterpunk
Copy link
Collaborator Author

@DmitryKuzmenko , I did some changes to pass the new pre-commit tests, removing the usage of six and Py2 support.

@DmitryKuzmenko
Copy link
Contributor

@piterpunk thank you!

@DmitryKuzmenko
Copy link
Contributor

Tests are passing now (again).

@piterpunk
Copy link
Collaborator Author

Tests are passing now (again).

@DmitryKuzmenko , it's a lottery. Most tests fails by timeout, no available space, or error cloning the repository. I don't know if and how I can re-run only the failed tests, so my alternative is to merge from master until all tests passed.

@dwoz dwoz merged commit 301d989 into saltstack:master Aug 24, 2020
27 checks passed
@piterpunk piterpunk deleted the new-feature-add-grain-to-list-LVM-volumes_Implement-Issue-57629 branch September 29, 2020 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature new functionality including changes to functionality and code refactors, etc. Magnesium Mg release after Na prior to Al
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE REQUEST]: Grain with the LVM volumes
4 participants