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 support for ECR Lifecycle Policies to ecs_ecr #48997

Merged
merged 21 commits into from Dec 19, 2019
Merged

Conversation

@SpamapS
Copy link
Contributor

SpamapS commented Nov 21, 2018

SUMMARY

Add support for ECR Lifecycle Policies to ecs_ecr. Fixes #32003

ISSUE TYPE
Feature Pull Request
COMPONENT NAME

ecs_ecr

ANSIBLE VERSION

ansible 2.4.0.0
config file = /Users/dlee/.ansible.cfg
configured module search path = [u'/Users/dlee/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
ansible python module location = /usr/local/Cellar/ansible/2.4.0.0/libexec/lib/python2.7/site-packages/ansible
executable location = /usr/local/bin/ansible
python version = 2.7.14 (default, Sep 25 2017, 09:53:22) [GCC 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.37)]

ADDITIONAL INFORMATION

Integration tests provided, and they pass.

PLAY RECAP *********************************************************************
localhost : ok=55 changed=8 unreachable=0 failed=0

@SpamapS

This comment has been minimized.

Copy link
Contributor Author

SpamapS commented Nov 21, 2018

Rebased #32137

@sivel sivel changed the title ##### SUMMARY Add support for ECR Lifecycle Policies to ecs_ecr Nov 21, 2018
@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Nov 21, 2018

Hi @SpamapS, thank you for submitting this pull-request!

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Nov 21, 2018

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Nov 21, 2018

The test ansible-test sanity --test validate-modules [explain] failed with 3 errors:

lib/ansible/modules/cloud/amazon/ecs_ecr.py:0:0: E309 version_added for new option (lifecycle_policy) should be 2.8. Currently 2.5
lib/ansible/modules/cloud/amazon/ecs_ecr.py:0:0: E309 version_added for new option (purge_lifecycle_policy) should be 2.8. Currently 2.5
lib/ansible/modules/cloud/amazon/ecs_ecr.py:0:0: E309 version_added for new option (purge_policy) should be 2.8. Currently 2.5

click here for bot help

@Maelstromeous

This comment has been minimized.

Copy link

Maelstromeous commented Jun 15, 2019

Would be most appreciated if this PR could be picked back up :)

@SpamapS SpamapS force-pushed the SpamapS:ecr-lifecycle branch to 0981e9e Jun 16, 2019
@SpamapS

This comment has been minimized.

Copy link
Contributor Author

SpamapS commented Jun 16, 2019

Rebased! I've had this vendored locally since the initial rebase I did... would be great to rejoin the fold in some future Ansible release. :)

leedm777 and others added 16 commits Oct 26, 2017
Marks the `delete_policy` parameter as deprecated, to be removed in
Ansible 2.6.
What I really want is --diff support, and changing results based on
verbosity is abnormal.
The original PR sat while a few releases happened.
This is not allowed and is generally bad practice.
This was added in the time the PR has been in development, so rework
things to use it.
This makes sure that lifecycle_policy is produced when passed in.

*Also a minor suggestion for simplification from PR.
@jillr jillr force-pushed the SpamapS:ecr-lifecycle branch from 96f2dd1 to d391e61 Dec 18, 2019
@jillr

This comment has been minimized.

Copy link
Contributor

jillr commented Dec 18, 2019

I believe that should do it, sanity and integration tests are passing locally. If CI passes and you don't have any objections to my changes there @SpamapS I'll give it a fresh set of eyes in the morning just to be safe but I think we should be about there.

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Dec 18, 2019

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/cloud/amazon/ecs_ecr.py:0:0: option-incorrect-version-added: version_added for new option (purge_policy) should be None. Currently '2.10'

click here for bot help

@ansibot ansibot added the ci_verified label Dec 18, 2019
Per sanity test fail.
@jillr
jillr approved these changes Dec 19, 2019
Copy link
Contributor

jillr left a comment

LGTM, thanks very much @SpamapS!

@jillr jillr merged commit 284f263 into ansible:devel Dec 19, 2019
1 check passed
1 check passed
Shippable Run 154009 status is SUCCESS.
Details
xuxiaowei0512 added a commit to xuxiaowei0512/ansible that referenced this pull request Dec 22, 2019
* Fix copy/pasta for ecs_ecr test names

* Add support for lifecycle policies to ecs_ecr

New feature for ecs_ecr to support [ECR Lifecycle Policies][].

Fixes ansible#32003

 [ECR Lifecycle Policies]: https://docs.aws.amazon.com/AmazonECR/latest/userguide/LifecyclePolicies.html

* Improve error message for ecs_ecr parsing errors

Replaces the exception and stack trace with a description of what's
actually going wrong from a user perspective.

* Rename delete policy to purge policy

Marks the `delete_policy` parameter as deprecated, to be removed in
Ansible 2.6.

* Add version_added to purge_policy

* Remove changing results based on verbosity

What I really want is --diff support, and changing results based on
verbosity is abnormal.

* Ensure repository name is lowercase

* Fix deprecation cycle to 4 releases

* Use a YAML anchor for credentials

* Remove filters from assertions

* Add minimal permissions needed

* Updating version_added and deprecation cycle

The original PR sat while a few releases happened.

* Bumping version added and deprecation version

We missed the 2.8 release.

* Removing bare except:

This is not allowed and is generally bad practice.

* Fix lint errors

* update ansible release metadata

* Use the new alias deprecation scheme

This was added in the time the PR has been in development, so rework
things to use it.

* Add test coverage

This makes sure that lifecycle_policy is produced when passed in.

*Also a minor suggestion for simplification from PR.

* Restore changes from 62871 lost in rebase

* Add changelog

* Remove version_added for new purge_policy option

Per sanity test fail.
@ansible ansible locked and limited conversation to collaborators Jan 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
10 participants
You can’t perform that action at this time.