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 ec2 volume creation issue for io1 with iops #177

Merged

Conversation

miho120
Copy link
Contributor

@miho120 miho120 commented Oct 22, 2020

SUMMARY

Related to volume creation io1 with iops.
When the volume parameters set with set_fact using volume size variable it will cause create_block_device TypeError: unorderable types: int() > str() unhandled exception.
If we set volume_size just in set_fact, the volume_size parameter will always be a string.
In ec2.py file we have statement if int(volume['iops']) > MAX_IOPS_TO_SIZE_RATIO * size: and that means that iops parameter will always be int. So this statements triggers TypeError exception, because we're trying to compare by > int and string.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ec2.py

ADDITIONAL INFORMATION

Reproduction:
defaults main.yml

ec2_volume_size: 100
ec2_volume_iops: 1000

Task main.yml

- name: Defining data volume parameters
  set_fact:
    data_volume:
      device_name: /dev/xvdf
      volume_type: 'io1'
      volume_size: '{{ ec2_volume_size }}'
      iops: '{{ ec2_volume_iops }}'
      delete_on_termination: true

- name: "Provision instance(s)"
  ec2:
    ...
    volumes: "{{ [data_volume] }}"
    ...

Before change:

fatal: [localhost]: FAILED! => {"changed": false, "module_stderr": "Traceback (most recent call last):
  File \"<stdin>\", line 102, in <module>
  File \"<stdin>\", line 94, in _ansiballz_main
  File \"<stdin>\", line 40, in invoke_module
  File \"/usr/lib/python3.5/runpy.py\", line 196, in run_module
    return _run_module_code(code, init_globals, run_name, mod_spec)
  File \"/usr/lib/python3.5/runpy.py\", line 96, in _run_module_code
    mod_name, mod_spec, pkg_name, script_name)
  File \"/usr/lib/python3.5/runpy.py\", line 85, in _run_code
    exec(code, run_globals)
  File \"/tmp/ansible_ec2_payload_gfzsxvci/ansible_ec2_payload.zip/ansible_collections/amazon/aws/plugins/modules/ec2.py\", line 1735, in <module>
  File \"/tmp/ansible_ec2_payload_gfzsxvci/ansible_ec2_payload.zip/ansible_collections/amazon/aws/plugins/modules/ec2.py\", line 1721, in main
  File \"/tmp/ansible_ec2_payload_gfzsxvci/ansible_ec2_payload.zip/ansible_collections/amazon/aws/plugins/modules/ec2.py\", line 930, in enforce_count
  File \"/tmp/ansible_ec2_payload_gfzsxvci/ansible_ec2_payload.zip/ansible_collections/amazon/aws/plugins/modules/ec2.py\", line 1137, in create_instances
  File \"/tmp/ansible_ec2_payload_gfzsxvci/ansible_ec2_payload.zip/ansible_collections/amazon/aws/plugins/modules/ec2.py\", line 813, in create_block_device
TypeError: unorderable types: int() > str()
", "module_stdout": "", "msg": "MODULE FAILURE
See stdout/stderr for the exact error", "rc": 1}

After the change, it just works as expected

@miho120 miho120 changed the title fix-ansible-ec2-volume-iops-issue Fix ec2 volume creation issue for io1 with iops Oct 22, 2020
Copy link
Contributor

@tremble tremble 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 taking the time to submit this PR,

In theory the best way to deal with this would be to set suboptions down in the "argument_spec". However, ec2 is an 'old' boto based module that needs rewriting anyway and setting suboptions has a number of complex side effects. As such I think we go with this simple fix.

Please could you add a changelog fragment https://docs.ansible.com/ansible/latest/community/development_process.html#changelogs-how-to

Copy link
Contributor Author

@miho120 miho120 left a comment

Choose a reason for hiding this comment

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

Hi @tremble Changelog has been added

@miho120 miho120 requested a review from tremble November 11, 2020 13:03
…ps.yaml

Co-authored-by: Mark Chappell <mchappel@redhat.com>
@miho120 miho120 requested a review from tremble November 11, 2020 13:09
Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

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

LGTM, will merge once the tests complete successfully.

@tremble tremble merged commit 19eecc4 into ansible-collections:main Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants