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 wait_for_modify_volume_complete option #1558

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

mschurenko
Copy link

@mschurenko mschurenko commented May 19, 2023

SUMMARY
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ec2_vol

ADDITIONAL INFORMATION

I ran into the following issue when trying to modify an unattached volume:

boto3_version: 1.26.122
  botocore_version: 1.29.122
  error:
    code: OperationNotPermitted
    message: 'Cannot attach volume vol-02f06b9e2f1e6d1e0 when it is in modification state: MODIFYING'
  msg: 'Error while attaching EBS volume: An error occurred (OperationNotPermitted) when calling the AttachVolume operation: Cannot attach volume vol-02f06b9e2f1e6d1e0 when it is in modification state: MODIFYING'

There seems to be no waiter, but if we poll describe_volumes_modifications we can wait until ModificationState is complete.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/22d983eab41546ddbe025192da27cc59

✔️ ansible-galaxy-importer SUCCESS in 3m 44s
✔️ build-ansible-collection SUCCESS in 14m 11s
✔️ ansible-test-splitter SUCCESS in 4m 53s
✔️ integration-amazon.aws-1 SUCCESS in 14m 46s
Skipped 43 jobs

@mschurenko
Copy link
Author

Hi. Just bumping in case this wasn't noticed.

@ansibullbot
Copy link

@ansibullbot ansibullbot added community_review feature This issue/PR relates to a feature request module module new_contributor Help guide this first time contributor plugins plugin (any type) labels Jul 12, 2023
Copy link
Contributor

@hakbailey hakbailey 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 submitting this PR! Could you add the new option to the documentation block?

plugins/modules/ec2_vol.py Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Jul 24, 2023

Docs Build 📝

Thank you for contribution!✨

The docsite for this PR is available for download as an artifact from this run:
https://github.com/ansible-collections/amazon.aws/actions/runs/5732736742

You can compare to the docs for the main branch here:
https://ansible-collections.github.io/amazon.aws/branch/main

File changes:

  • M collections/amazon/aws/ec2_vol_module.html
Click to see the diff comparison.

NOTE: only file modifications are shown here. New and deleted files are excluded.
See the file list and check the published docs to see those files.

diff --git a/home/runner/work/amazon.aws/amazon.aws/docsbuild/base/collections/amazon/aws/ec2_vol_module.html b/home/runner/work/amazon.aws/amazon.aws/docsbuild/head/collections/amazon/aws/ec2_vol_module.html
index 96ff50b..f06554e 100644
--- a/home/runner/work/amazon.aws/amazon.aws/docsbuild/base/collections/amazon/aws/ec2_vol_module.html
+++ b/home/runner/work/amazon.aws/amazon.aws/docsbuild/head/collections/amazon/aws/ec2_vol_module.html
@@ -502,6 +502,28 @@ see <a class="reference internal" href="#ansible-collections-amazon-aws-ec2-vol-
 </div></td>
 </tr>
 <tr class="row-even"><td><div class="ansible-option-cell">
+<div class="ansibleOptionAnchor" id="parameter-wait"></div><p class="ansible-option-title" id="ansible-collections-amazon-aws-ec2-vol-module-parameter-wait"><strong>wait</strong></p>
+<a class="ansibleOptionLink" href="#parameter-wait" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-type">boolean</span></p>
+<p><span class="ansible-option-versionadded">added in amazon.aws 6.3.0</span></p>
+</div></td>
+<td><div class="ansible-option-cell"><p>Wait for volume modification to complete.</p>
+<p class="ansible-option-line"><span class="ansible-option-choices">Choices:</span></p>
+<ul class="simple">
+<li><p><code class="ansible-option-default-bold docutils literal notranslate"><span class="pre">false</span></code> <span class="ansible-option-choices-default-mark">← (default)</span></p></li>
+<li><p><code class="ansible-option-choices-entry docutils literal notranslate"><span class="pre">true</span></code></p></li>
+</ul>
+</div></td>
+</tr>
+<tr class="row-odd"><td><div class="ansible-option-cell">
+<div class="ansibleOptionAnchor" id="parameter-wait_timeout"></div><p class="ansible-option-title" id="ansible-collections-amazon-aws-ec2-vol-module-parameter-wait-timeout"><strong>wait_timeout</strong></p>
+<a class="ansibleOptionLink" href="#parameter-wait_timeout" title="Permalink to this option"></a><p class="ansible-option-type-line"><span class="ansible-option-type">integer</span></p>
+<p><span class="ansible-option-versionadded">added in amazon.aws 6.3.0</span></p>
+</div></td>
+<td><div class="ansible-option-cell"><p>How long before wait gives up, in seconds.</p>
+<p class="ansible-option-line"><span class="ansible-option-default-bold">Default:</span> <code class="ansible-option-default docutils literal notranslate"><span class="pre">900</span></code></p>
+</div></td>
+</tr>
+<tr class="row-even"><td><div class="ansible-option-cell">
 <div class="ansibleOptionAnchor" id="parameter-zone"></div>
 <div class="ansibleOptionAnchor" id="parameter-availability_zone"></div>
 <div class="ansibleOptionAnchor" id="parameter-aws_zone"></div>

@ansibullbot
Copy link

@mschurenko this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibullbot ansibullbot added merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html and removed community_review labels Jul 24, 2023
@mschurenko mschurenko force-pushed the add-wait-for-modify-complete branch from c0764af to 3b68a5c Compare July 24, 2023 19:16
@ansibullbot ansibullbot added community_review and removed merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Jul 24, 2023
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/fa31e40ffa3a4f20a9121b53aee3f058

✔️ ansible-galaxy-importer SUCCESS in 3m 53s
✔️ build-ansible-collection SUCCESS in 13m 36s
✔️ ansible-test-splitter SUCCESS in 5m 15s
✔️ integration-amazon.aws-1 SUCCESS in 12m 49s
Skipped 43 jobs

Copy link
Contributor

@hakbailey hakbailey left a comment

Choose a reason for hiding this comment

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

Whoops, just noticed there isn't a changelog fragment for this, could you add that too? Thanks!

@mschurenko
Copy link
Author

mschurenko commented Jul 25, 2023

Whoops, just noticed there isn't a changelog fragment for this, could you add that too? Thanks!

So should I add something to CHANGELOG.rst?

Sorry, I see it's documented here https://docs.ansible.com/ansible/latest/community/collection_development_process.html#creating-a-changelog-fragment

@mschurenko
Copy link
Author

Whoops, just noticed there isn't a changelog fragment for this, could you add that too? Thanks!

So should I add something to CHANGELOG.rst?

Sorry, I see it's documented here https://docs.ansible.com/ansible/latest/community/collection_development_process.html#creating-a-changelog-fragment

Ok I think (hope) I did it correctly.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/db0933b73703460ca5575c23fff56dd0

✔️ ansible-galaxy-importer SUCCESS in 10m 30s
✔️ build-ansible-collection SUCCESS in 12m 54s
✔️ ansible-test-splitter SUCCESS in 6m 02s
✔️ integration-amazon.aws-1 SUCCESS in 13m 17s
Skipped 43 jobs

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/bb49ffb2e5294d02b256e3671ce7fa02

✔️ ansible-galaxy-importer SUCCESS in 5m 15s
✔️ build-ansible-collection SUCCESS in 13m 32s
✔️ ansible-test-splitter SUCCESS in 5m 27s
✔️ integration-amazon.aws-1 SUCCESS in 13m 53s
Skipped 43 jobs

plugins/modules/ec2_vol.py Outdated Show resolved Hide resolved
@softwarefactory-project-zuul
Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/0d028cf058964cae9be01796aab22663

✔️ ansible-galaxy-importer SUCCESS in 3m 44s
✔️ build-ansible-collection SUCCESS in 13m 25s
✔️ ansible-test-splitter SUCCESS in 4m 43s
integration-amazon.aws-1 FAILURE in 25m 54s
Skipped 43 jobs

@ansibullbot
Copy link

@mschurenko this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibullbot ansibullbot added merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html and removed community_review labels Aug 1, 2023
@ansibullbot ansibullbot added community_review and removed merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Aug 1, 2023
@softwarefactory-project-zuul
Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/303df7e6e286475a89651fe19a8bd37d

✔️ ansible-galaxy-importer SUCCESS in 4m 52s
✔️ build-ansible-collection SUCCESS in 14m 42s
✔️ ansible-test-splitter SUCCESS in 5m 33s
integration-amazon.aws-1 FAILURE in 39m 18s
Skipped 43 jobs

@alinabuzachis alinabuzachis added this to the 6.4.0 milestone Aug 4, 2023
@hakbailey
Copy link
Contributor

recheck

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/d66b52319e6045c99581d1eb5f688c99

✔️ ansible-galaxy-importer SUCCESS in 4m 52s
✔️ build-ansible-collection SUCCESS in 12m 35s
✔️ ansible-test-splitter SUCCESS in 5m 04s
✔️ integration-amazon.aws-1 SUCCESS in 18m 20s
Skipped 43 jobs

mod_state = ""
_wait_till = module.params.get("wait_timeout") + time.time()
while _wait_till > time.time():
mod_response = ec2_conn.describe_volumes_modifications(VolumeIds=[volume["volume_id"]])
Copy link
Collaborator

Choose a reason for hiding this comment

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

One small thing. I guess we should enclose mod_response = ec2_conn.describe_volumes_modifications(VolumeIds=[volume["volume_id"]]) in a try-except block.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, I can add that

@@ -769,6 +805,9 @@ def main():
if multi_attach is True and volume_type not in ("io1", "io2"):
module.fail_json(msg="multi_attach is only supported for io1 and io2 volumes.")

if wait is True and modify_volume is False:
module.fail_json(msg="wait does nothing if modify_volume is False.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess it should work for now. We would probably also like to add the option to wait for deletion in the future.

Copy link
Author

Choose a reason for hiding this comment

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

We are using this module currently and have decided to wait for either completed or optimizing as waiting for completed can take a very long time. Should we do the same here? Or should we have the option to wait for one or more modification states?

How about having an option like this?

wait_modification_states:
    description:
      - A list of modification states to wait for. Valid options are "completed" and "optimizing"
    type: list
    default: ["completed", "optimizing"]

@hakbailey hakbailey added the waiting_on_contributor Needs help. Feel free to engage to get things unblocked label Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community_review feature This issue/PR relates to a feature request module module new_contributor Help guide this first time contributor plugins plugin (any type) waiting_on_contributor Needs help. Feel free to engage to get things unblocked
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants