Skip to content

ec2_vol support MultiAttach disk - #365

Merged
ansible-zuul[bot] merged 12 commits into
ansible-collections:mainfrom
abikouo:ec2_vol_multiattach
Jun 24, 2021
Merged

ec2_vol support MultiAttach disk#365
ansible-zuul[bot] merged 12 commits into
ansible-collections:mainfrom
abikouo:ec2_vol_multiattach

Conversation

@abikouo

@abikouo abikouo commented May 10, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor
SUMMARY

new parameter multi_attach to support MultiAttach on disk creation/update

#225

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ec2_vol

ADDITIONAL INFORMATION

@abikouo abikouo changed the title WIP - ec2_vol support MultiAttach disk ec2_vol support MultiAttach disk May 10, 2021
@abikouo abikouo mentioned this pull request May 14, 2021
Closed
markuman
markuman reviewed May 19, 2021
View reviewed changes

@markuman markuman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it's a must have feature. but sadly it collides with #334 which (imho) should be land in main before this one.

Comment thread plugins/modules/ec2_vol.py Outdated
Comment thread plugins/modules/ec2_vol.py
Comment thread plugins/modules/ec2_vol.py Outdated
@tremble tremble mentioned this pull request May 19, 2021
Merged
@ansibullbot

ansibullbot commented May 21, 2021

Copy link
Copy Markdown

cc @jillr @lwade @s-hertel @tremble @wimnat
click here for bot help

@ansibullbot ansibullbot added community_review feature This issue/PR relates to a feature request has_issue integration tests/integration module module needs_triage plugins plugin (any type) tests tests labels May 21, 2021
@ansibullbot

ansibullbot commented May 26, 2021

Copy link
Copy Markdown

@abikouo 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 May 26, 2021
@abikouo
abikouo force-pushed the ec2_vol_multiattach branch from b96f8e7 to 77a60f1 Compare May 26, 2021 08:37
@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 May 26, 2021
@goneri

goneri commented May 27, 2021

Copy link
Copy Markdown
Member

recheck

@alinabuzachis

alinabuzachis commented Jun 4, 2021

Copy link
Copy Markdown
Collaborator

@abikouo thank you for working on this. Can you please rebase your PR?

@ansibullbot ansibullbot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review labels Jun 4, 2021
@jillr

jillr commented Jun 18, 2021

Copy link
Copy Markdown
Contributor

@abikouo can you please rebase this and clean up the git conflicts? Throughput support has been added in main from #370

@ansibullbot

ansibullbot commented Jun 22, 2021

Copy link
Copy Markdown

@abikouo 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! and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jun 22, 2021
@abikouo
abikouo force-pushed the ec2_vol_multiattach branch from 00f8c5e to 82cbb63 Compare June 22, 2021 08:14
@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 Jun 22, 2021
@abikouo

abikouo commented Jun 22, 2021

Copy link
Copy Markdown
Contributor Author

gate

@abikouo
abikouo requested a review from jillr June 22, 2021 16:21
markuman
markuman suggested changes Jun 22, 2021
View reviewed changes
Comment thread plugins/modules/ec2_vol.py
Comment thread plugins/modules/ec2_vol.py Outdated
@abikouo @markuman
Update plugins/modules/ec2_vol.py
b270771 
Co-authored-by: Markus Bergholz <[email protected]>
markuman
markuman approved these changes Jun 22, 2021
View reviewed changes

@markuman markuman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

tremble
tremble requested changes Jun 23, 2021
View reviewed changes

@tremble tremble left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Generally looks good to me, we only need the update to attachment_set to be listed as a breaking change.

Comment thread changelogs/fragments/362-ec2_vol-add-multi-attach-parameter.yml
@@ -0,0 +1,3 @@
minor_changes:
- ec2_vol - add parameter ``multi_attach`` to support Multi-Attach on volume creation/update (https://github.com/ansible-collections/amazon.aws/pull/362).
- ec2_vol_info - return ``attachment_set`` is now a list of attachments with Multi-Attach support on disk. (https://github.com/ansible-collections/amazon.aws/pull/362).

@tremble tremble Jun 23, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a breaking change and needs to be documented as such. Next release is a major release so I think we can get away with this (? @jillr ?)

@abikouo abikouo added the gate label Jun 24, 2021
@ansible-zuul
ansible-zuul Bot merged commit 1879a62 into ansible-collections:main Jun 24, 2021
@alinabuzachis alinabuzachis mentioned this pull request Jun 30, 2021
Closed
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
@mdavis-xyz
Add abort multipart upload and expire obj del markers to s3 lifecycle (…
d443ae5 
…ansible-collections#794)

Add abort multipart upload and expire obj del markers to s3 lifecycle

Depends-On: ansible/ansible-zuul-jobs#1247
SUMMARY
Fixes ansible-collections#365 ansible-collections#796
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
s3_lifecycle
ADDITIONAL INFORMATION
I have not run integration tests yet because of ansible-collections#793.
I'm unsure about how to name and structure the new arguments.
Do I nest them to match the API, or flatten them to match existing arguments?

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Matthew Davis <None>
Reviewed-by: Mark Chappell <None>
Reviewed-by: None <None>
Reviewed-by: Markus Bergholz <[email protected]>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
@mdavis-xyz @abikouo
Add abort multipart upload and expire obj del markers to s3 lifecycle (…
7791c46 
…ansible-collections#794)

Add abort multipart upload and expire obj del markers to s3 lifecycle

Depends-On: ansible/ansible-zuul-jobs#1247
SUMMARY
Fixes ansible-collections#365 ansible-collections#796
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
s3_lifecycle
ADDITIONAL INFORMATION
I have not run integration tests yet because of ansible-collections#793.
I'm unsure about how to name and structure the new arguments.
Do I nest them to match the API, or flatten them to match existing arguments?

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Matthew Davis <None>
Reviewed-by: Mark Chappell <None>
Reviewed-by: None <None>
Reviewed-by: Markus Bergholz <[email protected]>
@abikouo
abikouo deleted the ec2_vol_multiattach branch October 24, 2023 15:44
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Oct 24, 2023
@mdavis-xyz
Add abort multipart upload and expire obj del markers to s3 lifecycle (…
ed3a7a0 
…ansible-collections#794)

Add abort multipart upload and expire obj del markers to s3 lifecycle

Depends-On: ansible/ansible-zuul-jobs#1247
SUMMARY
Fixes ansible-collections#365 ansible-collections#796
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
s3_lifecycle
ADDITIONAL INFORMATION
I have not run integration tests yet because of ansible-collections#793.
I'm unsure about how to name and structure the new arguments.
Do I nest them to match the API, or flatten them to match existing arguments?

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Matthew Davis <None>
Reviewed-by: Mark Chappell <None>
Reviewed-by: None <None>
Reviewed-by: Markus Bergholz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tremble tremble tremble approved these changes
@jillr jillr Awaiting requested review from jillr
+1 more reviewer
@markuman markuman markuman approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

community_review feature This issue/PR relates to a feature request has_issue integration tests/integration module module plugins plugin (any type) tests tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@abikouo @ansibullbot @goneri @alinabuzachis @jillr @tremble @markuman