-
Notifications
You must be signed in to change notification settings - Fork 340
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
base: main
Are you sure you want to change the base?
add wait_for_modify_volume_complete option #1558
Conversation
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 44s |
Hi. Just bumping in case this wasn't noticed. |
There was a problem hiding this 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?
Docs Build 📝Thank you for contribution!✨ The docsite for this PR is available for download as an artifact from this run: You can compare to the docs for the File changes:
Click to see the diff comparison.NOTE: only file modifications are shown here. New and deleted files are excluded. 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>
|
@mschurenko this PR contains the following merge commits: Please rebase your branch to remove these commits. |
c0764af
to
3b68a5c
Compare
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 53s |
There was a problem hiding this 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!
So should I add something to 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. |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 10m 30s |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 5m 15s |
Build failed. ✔️ ansible-galaxy-importer SUCCESS in 3m 44s |
@mschurenko this PR contains the following merge commits: Please rebase your branch to remove these commits. |
d7574b8
to
d0be10e
Compare
Build failed. ✔️ ansible-galaxy-importer SUCCESS in 4m 52s |
recheck |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 52s |
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"]]) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.") |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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"]
SUMMARY
ISSUE TYPE
COMPONENT NAME
ec2_vol
ADDITIONAL INFORMATION
I ran into the following issue when trying to modify an unattached volume:
There seems to be no waiter, but if we poll
describe_volumes_modifications
we can wait untilModificationState
iscomplete
.