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

Fix reported issues failing-ok and pp_size in module aix_lvg #58651

Open
wants to merge 5 commits into
base: devel
from

Conversation

Projects
None yet
4 participants
@d-little
Copy link
Contributor

commented Jul 2, 2019

SUMMARY

Fixes two issues described in #58522:

  1. If VG fails to create, exist with fail_json instead of returning OK.
  2. Added checks to main() to confirm that supplied pp_size is a valid value before continuing.
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

aix_lvg

ADDITIONAL INFORMATION

See #58522 for full issue details.

Test output:

With test task:

---
- name: "Test AIX LVM Stuff"
  hosts: all
  become: yes
  tasks:
  - name: manage_aix_lvm | creating new AIX volume group(s)
      force: true
      pp_size: "{{ item }}"
      pvs: "hdisk4"
      vg: "vg_exp1"
      state: present
    become: true
    with_items:
        - "-32"
        - 0
        - 129
        - "INVALID"
        - 128

Before:- False OK's returned from invalid data and failed creations

ok: [TEST] => (item=-32)
ok: [TEST] => (item=0)
ok: [TEST] => (item=129)
failed: [TEST] (item=INVALID) => {"ansible_loop_var": "item", "changed": false, "item": "INVALID", "msg": "argument pp_size is of type <type 'str'> and we were unable to convert to int: <type 'str'> cannot be converted to an int"}
ok: [TEST] => (item=256)


ok: [TEST] => {
    "changed": false,
    "invocation": {
        "module_args": {
            "force": true,
            "pp_size": 2048,
            "pvs": [
                "hdisk4"
            ],
            "state": "present",
            "vg": "vg_exp1",
            "vg_type": "normal"
        }
    },
    "msg": "Creating volume group 'vg_exp1' failed.",
    "state": "present"
}


After:

failed: [TEST] (item=-32) => {"ansible_loop_var": "item", "changed": false, "item": "-32", "msg": "pp_size must be >0 and a multiple of 2 (supplied: '-32')."}
failed: [TEST] (item=0) => {"ansible_loop_var": "item", "changed": false, "item": 0, "msg": "pp_size must be >0 and a multiple of 2 (supplied: '0')."}
failed: [TEST] (item=129) => {"ansible_loop_var": "item", "changed": false, "item": 129, "msg": "pp_size must be >0 and a multiple of 2 (supplied: '129')."}
failed: [TEST] (item=INVALID) => {"ansible_loop_var": "item", "changed": false, "item": "INVALID", "msg": "argument pp_size is of type <type 'str'> and we were unable to convert to int: <type 'str'> cannot be converted to an int"}
changed: [TEST] => (item=128)



fatal: [TEST]: FAILED! => {
    "changed": false,
    "invocation": {
        "module_args": {
            "force": true,
            "pp_size": 2048,
            "pvs": [
                "hdisk4"
            ],
            "state": "present",
            "vg": "vg_expx1",
            "vg_type": "normal"
        }
    },
    "msg": "Failed to create volume group 'vg_expx1' using command '/usr/sbin/mkvg  -s 2048 -f -y vg_expx1 hdisk4'. Error message: 0516-1197 /usr/sbin/mkvg: The -s parameter for Size must be a power\n\tof 2 between 1 and 1024.\n0516-862 /usr/sbin/mkvg: Unable to create volume group.\n"
}


@d-little d-little marked this pull request as ready for review Jul 2, 2019

@d-little d-little changed the title aix_lvg fixes: #58522 aix_lvg fixes Jul 2, 2019

@d-little d-little changed the title aix_lvg fixes Fix reported issues failing-ok and pp_size in module aix_lvg Jul 2, 2019

@mator

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

@d-little , how do you run (command line) your playbook ? thanks.

@ansibot ansibot removed the needs_triage label Jul 3, 2019

@d-little

This comment has been minimized.

Copy link
Contributor Author

commented Jul 3, 2019

@mator , I've cleaned up that test playbook a tiny bit. just the one task for my testing purposes.

# ansible-playbook -i ~/test.ini ~/test_aix_lvg.yml
# cat ~/test_aix_lvg.yml
---
- name: Test AIX module aix_lvg
  hosts: TESTHOST
  become: true
  vars:
    hdisk: hdisk4
    vgname: vg_tstexp
  tasks:
    - name: "Test AIX LVM with Invalid PP Values"
      become: yes
      aix_lvg:
        force: true
        pp_size: "{{ item }}"
        pvs: "{{ hdisk }}"
        vg: "{{ vgname }}"
        state: present
      with_items:
        - "-32"
        - 0
        - 129
        - "INVALID"

Run with the updated module code:

# ansible-playbook -i ~/test.ini ~/test_aix_lvg.yml

PLAY [Test AIX module aix_lvg] ***********************************************************************************************************************************************************************************************************

TASK [Gathering Facts] *******************************************************************************************************************************************************************************************************************
ok: [TESTHOST]

TASK [Test AIX LVM with Invalid PP Values] ***********************************************************************************************************************************************************************************************
failed: [TESTHOST] (item=-32) => {"ansible_loop_var": "item", "changed": false, "item": "-32", "msg": "pp_size must be >0 and a multiple of 2 (supplied: '-32')."}
failed: [TESTHOST] (item=0) => {"ansible_loop_var": "item", "changed": false, "item": 0, "msg": "pp_size must be >0 and a multiple of 2 (supplied: '0')."}
failed: [TESTHOST] (item=129) => {"ansible_loop_var": "item", "changed": false, "item": 129, "msg": "pp_size must be >0 and a multiple of 2 (supplied: '129')."}
failed: [TESTHOST] (item=INVALID) => {"ansible_loop_var": "item", "changed": false, "item": "INVALID", "msg": "argument pp_size is of type <type 'str'> and we were unable to convert to int: <type 'str'> cannot be converted to an int"}
        to retry, use: --limit @/root/test_aix_lvg.retry

PLAY RECAP *******************************************************************************************************************************************************************************************************************************
TESTHOST                   : ok=1    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0
@mator

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2019

thanks.

just checked on my system, looks good to me.

$ cat aix-pl1.yml
---
- name: Test AIX module aix_lvg
  hosts: aix7
  become: true
  vars:
    hdisk: hdisk1
    vgname: vg_exp1
  tasks:
    - name: "Test AIX LVM with Invalid PP Values"
      become: yes
      aix_lvg:
        force: true
        pp_size: "{{ item }}"
        pvs: "{{ hdisk }}"
        vg: "{{ vgname }}"
        state: present
      with_items:
        - "-4"
        - 0
        - 17
        - "b0rked"

$ ANSIBLE_NOCOWS=1 ansible-playbook aix-pl1.yml 

PLAY [Test AIX module aix_lvg] *****************************************************************************************************************

TASK [Gathering Facts] *************************************************************************************************************************
ok: [aix7]

TASK [Test AIX LVM with Invalid PP Values] *****************************************************************************************************
failed: [aix7] (item=-4) => {"ansible_loop_var": "item", "changed": false, "item": "-4", "msg": "pp_size must be >= 1 and <= 1024, and a multiple of 2 (supplied: '-4')."}
failed: [aix7] (item=0) => {"ansible_loop_var": "item", "changed": false, "item": 0, "msg": "pp_size must be >= 1 and <= 1024, and a multiple of 2 (supplied: '0')."}
failed: [aix7] (item=17) => {"ansible_loop_var": "item", "changed": false, "item": 17, "msg": "pp_size must be >= 1 and <= 1024, and a multiple of 2 (supplied: '17')."}
failed: [aix7] (item=b0rked) => {"ansible_loop_var": "item", "changed": false, "item": "b0rked", "msg": "argument pp_size is of type <type 'str'> and we were unable to convert to int: <type 'str'> cannot be converted to an int"}

PLAY RECAP *************************************************************************************************************************************
aix7                       : ok=1    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0 
@mator

mator approved these changes Jul 4, 2019

Copy link
Contributor

left a comment

LGTM

@jpmahowald

This comment has been minimized.

ppsize must be a power of 2, not just a multiple of 2.

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.