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

ec2_vol support MultiAttach disk #365

Merged
merged 12 commits into from
Jun 24, 2021

Conversation

abikouo
Copy link
Contributor

@abikouo abikouo commented May 10, 2021

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
Copy link
Member

@markuman markuman left a comment

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.

plugins/modules/ec2_vol.py Outdated Show resolved Hide resolved
plugins/modules/ec2_vol.py Show resolved Hide resolved
plugins/modules/ec2_vol.py Outdated Show resolved Hide resolved
@ansibullbot
Copy link

@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
Copy link

@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
Copy link
Member

goneri commented May 27, 2021

recheck

@alinabuzachis
Copy link
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
Copy link
Collaborator

jillr commented Jun 18, 2021

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

@ansibullbot
Copy link

@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
Copy link
Contributor Author

abikouo commented Jun 22, 2021

gate

@abikouo abikouo requested a review from jillr June 22, 2021 16:21
plugins/modules/ec2_vol.py Show resolved Hide resolved
plugins/modules/ec2_vol.py Outdated Show resolved Hide resolved
Co-authored-by: Markus Bergholz <[email protected]>
Copy link
Member

@markuman markuman left a comment

Choose a reason for hiding this comment

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

LGTM

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.

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

@@ -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).
Copy link
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
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
…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
…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
…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
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
Development

Successfully merging this pull request may close these issues.

7 participants