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

LVG: add new parameter pvresize #52890

Open
wants to merge 8 commits into
base: devel
from

Conversation

Projects
None yet
3 participants
@cassio-santos
Copy link

cassio-santos commented Feb 24, 2019

SUMMARY

Prior to this adjustment, if the underlying physical device was resized, the physical volume built on it would not be resized.

An additional (optional) parameter was added with default to "yes" for ansible 2.8. When backporting, would probably change the default for "no" hence preserving compatibility only for already shipped versions. This is an important conceptual change, once the current behavior is misleading as described in the bug description.

In order to detect if the physical volume must be resized, a simple check verifies if the difference between the physical device size and the sum of the physical volume size and extent start size (reserved for metadata) is greater than one extent. If that is the case, the resize occurs

Fixes #29139

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

lvg

ADDITIONAL INFORMATION

Fix issue #29139
Prior to this adjustment, if the underlying physical device was resized, the physical volume would not resize the disk.
@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 24, 2019

Removing unnecessary command reference
There was no need for that command. Changed all to use pvdisplay
@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 24, 2019

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/modules/system/lvg.py:274:161: E501 line too long (162 > 160 characters)

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/system/lvg.py:0:0: E309 version_added for new option (pvresize) should be '2.8'. Currently StrictVersion ('0.0')

click here for bot help

cassio-santos added some commits Feb 24, 2019

Adds "version_added" attribute
Adds "version_added" attribute for pvresize parameter

cassio-santos added some commits Feb 24, 2019

Removing unnecessary command reference
lsblk is unnecessarily referenced. (Reintroduced by mistake)
Change pvresize default to true and reuses code
Changes pvresize default to "yes" (Shipping with ansible 2.8). When backporting, will change the default to "no" (Still needs some hard thinking on this).

It's an important conceptual change, as the current behavior is misleading.
@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 24, 2019

The test ansible-test sanity --test pep8 [explain] failed with 4 errors:

lib/ansible/modules/system/lvg.py:270:64: E226 missing whitespace around arithmetic operator
lib/ansible/modules/system/lvg.py:270:68: E226 missing whitespace around arithmetic operator
lib/ansible/modules/system/lvg.py:270:75: E226 missing whitespace around arithmetic operator
lib/ansible/modules/system/lvg.py:270:79: E226 missing whitespace around arithmetic operator

click here for bot help

@gundalow gundalow added feature and removed bug labels Feb 25, 2019

@ansibot ansibot removed the needs_triage label Feb 25, 2019

@Akasurde Akasurde changed the title Fix issue #29139 LVG: add new parameter pvresize Mar 4, 2019

@ansibot ansibot added the stale_ci label Mar 4, 2019

@cassio-santos

This comment has been minimized.

Copy link
Author

cassio-santos commented Mar 20, 2019

@Akasurde @gundalow , any more step required?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.