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.

Sign up for GitHub

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

Jump to bottom

Formally start the ec2 deprecation process #424

Merged
merged 1 commit into from
Aug 5, 2021
Merged

Formally start the ec2 deprecation process #424

merged 1 commit into from
Aug 5, 2021

Conversation

tremble
Copy link
Contributor

@tremble tremble commented Jul 30, 2021 •
edited
Loading

SUMMARY

The ec2 module is based upon the deprecated boto (not boto3/botocore) SDK and most functionality has been replaced by the ec2_instance module.

Formally start the deprecation process and give it 2 major releases.

For practical purposes the ec2 module has a number of limitations (things like not supporting all regions) and we're still getting bugs filed against it for quirky behaviour that's fixed in ec2_instance. By formally starting the deprecation process we can move most folks over to ec2_instance where possible and avoid trying to support 2 modules at the same time.

The major missing feature with the ec2_instance module is spot instances. Support for spot instances is underway in #407

Should spot instance support not be available by 3.0.0 we can always extend the deprecation period.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ec2

ADDITIONAL INFORMATION

@ansibullbot
Copy link

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

@ansibullbot ansibullbot added community_review feature This issue/PR relates to a feature request module module needs_triage plugins plugin (any type) labels Jul 30, 2021
@tremble tremble requested a review from jillr July 30, 2021 09:03
@ansibullbot ansibullbot added the tests tests label Jul 30, 2021
@jillr
Copy link
Collaborator

jillr commented Jul 30, 2021

This PR makes me so happy 😁

jillr
jillr reviewed Jul 30, 2021
View reviewed changes
Copy link
Collaborator

@jillr jillr left a comment

Choose a reason for hiding this comment

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

Everywhere we say "in favor of ec2_instance" we should say "ec2_instance and ec2_spot_instance", since that's been the sticking point

@jillr
Copy link
Collaborator

jillr commented Jul 30, 2021

recheck

@tremble
Copy link
Contributor Author

tremble commented Jul 31, 2021

Everywhere we say "in favor of ec2_instance" we should say "ec2_instance and ec2_spot_instance", since that's been the sticking point

The reason I didn't mention ec2_spot_instance in the changes themselves was that IMO it's worth flagging the module as "deprecated" even if ec2_spot_instance isn't ready in time for 2.0.0. We've had folks using it for "normal" ec2 instances and hitting bugs / limitations resulting in issues, IMO getting them to move to ec2_instance brings more value than delaying for spot instance support.

Hence the Should spot instance support not be available by 3.0.0 we can always extend the deprecation period. entry in the PR description.

@tremble
Copy link
Contributor Author

tremble commented Aug 2, 2021

recheck

1 similar comment
@tremble
Copy link
Contributor Author

tremble commented Aug 5, 2021

recheck

@tremble
Copy link
Contributor Author

tremble commented Aug 5, 2021

From IRC

<@jillr> I think we can merge 424, if spot instance doesn't go in 2.0 it will be in 2.1 fairly shortly thereafter

@tremble tremble added the gate label Aug 5, 2021
ansible-zuul[bot]
ansible-zuul bot approved these changes Aug 5, 2021
View reviewed changes
Copy link
Contributor

@ansible-zuul ansible-zuul bot left a comment

Choose a reason for hiding this comment

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

LGTM!

@tremble
Copy link
Contributor Author

tremble commented Aug 5, 2021

recheck

@ansible-zuul ansible-zuul bot merged commit af03411 into ansible-collections:main Aug 5, 2021
@tremble tremble mentioned this pull request Aug 18, 2021
Closed
@tremble tremble deleted the deprecate/ec2 branch November 26, 2021 12:42
alinabuzachis pushed a commit to alinabuzachis/amazon.aws that referenced this pull request Sep 16, 2022
@pjrm @tremble @alinabuzachis
Fix state=get on route53 module (Issue ansible-collections#423) (ansi…
e7ffed7 
…ble-collections#424)

* Fix state=get on route53 module

This bug was introduced when refactoring from boto to boto3 library. This happens because the method "get_hosted_zone" only returns the DelegationSet when the DNS zone is external. Therefore this breaks when trying to get internal records.

The solution is to search for getting DNS records of type ''NS'' with the same name as the hosted zone.

* Update changelogs/fragments/406-route53-state-get.yml

Co-authored-by: Mark Chappell <mchappel@redhat.com>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections/community.aws@331ada1
goneri pushed a commit to alinabuzachis/amazon.aws that referenced this pull request Sep 21, 2022
@pjrm @tremble @goneri
Fix state=get on route53 module (Issue ansible-collections#423) (ansi…
e3ac87d 
…ble-collections#424)

* Fix state=get on route53 module

This bug was introduced when refactoring from boto to boto3 library. This happens because the method "get_hosted_zone" only returns the DelegationSet when the DNS zone is external. Therefore this breaks when trying to get internal records.

The solution is to search for getting DNS records of type ''NS'' with the same name as the hosted zone.

* Update changelogs/fragments/406-route53-state-get.yml

Co-authored-by: Mark Chappell <mchappel@redhat.com>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections/community.aws@331ada1
goneri pushed a commit to alinabuzachis/amazon.aws that referenced this pull request Sep 21, 2022
@pjrm @tremble @goneri
Fix state=get on route53 module (Issue ansible-collections#423) (ansi…
55adbfc 
…ble-collections#424)

* Fix state=get on route53 module

This bug was introduced when refactoring from boto to boto3 library. This happens because the method "get_hosted_zone" only returns the DelegationSet when the DNS zone is external. Therefore this breaks when trying to get internal records.

The solution is to search for getting DNS records of type ''NS'' with the same name as the hosted zone.

* Update changelogs/fragments/406-route53-state-get.yml

Co-authored-by: Mark Chappell <mchappel@redhat.com>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections/community.aws@331ada1
goneri pushed a commit to alinabuzachis/amazon.aws that referenced this pull request Sep 21, 2022
@pjrm @tremble @goneri
Fix state=get on route53 module (Issue ansible-collections#423) (ansi…
e1829f5 
…ble-collections#424)

* Fix state=get on route53 module

This bug was introduced when refactoring from boto to boto3 library. This happens because the method "get_hosted_zone" only returns the DelegationSet when the DNS zone is external. Therefore this breaks when trying to get internal records.

The solution is to search for getting DNS records of type ''NS'' with the same name as the hosted zone.

* Update changelogs/fragments/406-route53-state-get.yml

Co-authored-by: Mark Chappell <mchappel@redhat.com>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections/community.aws@331ada1
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
@pjrm @tremble
Fix state=get on route53 module (Issue ansible-collections#423) (ansi…
da184fc 
…ble-collections#424)

* Fix state=get on route53 module

This bug was introduced when refactoring from boto to boto3 library. This happens because the method "get_hosted_zone" only returns the DelegationSet when the DNS zone is external. Therefore this breaks when trying to get internal records.

The solution is to search for getting DNS records of type ''NS'' with the same name as the hosted zone.

* Update changelogs/fragments/406-route53-state-get.yml

Co-authored-by: Mark Chappell <mchappel@redhat.com>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
@pjrm @tremble @abikouo
Fix state=get on route53 module (Issue ansible-collections#423) (ansi…
5a654c3 
…ble-collections#424)

* Fix state=get on route53 module

This bug was introduced when refactoring from boto to boto3 library. This happens because the method "get_hosted_zone" only returns the DelegationSet when the DNS zone is external. Therefore this breaks when trying to get internal records.

The solution is to search for getting DNS records of type ''NS'' with the same name as the hosted zone.

* Update changelogs/fragments/406-route53-state-get.yml

Co-authored-by: Mark Chappell <mchappel@redhat.com>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Oct 24, 2023
@pjrm @tremble
Fix state=get on route53 module (Issue ansible-collections#423) (ansi…
331ada1 
…ble-collections#424)

* Fix state=get on route53 module

This bug was introduced when refactoring from boto to boto3 library. This happens because the method "get_hosted_zone" only returns the DelegationSet when the DNS zone is external. Therefore this breaks when trying to get internal records.

The solution is to search for getting DNS records of type ''NS'' with the same name as the hosted zone.

* Update changelogs/fragments/406-route53-state-get.yml

Co-authored-by: Mark Chappell <mchappel@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jillr jillr jillr left review comments

@ansible-zuul ansible-zuul[bot] ansible-zuul approved these changes

Assignees
No one assigned
Labels
community_review feature This issue/PR relates to a feature request module module plugins plugin (any type) tests tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tremble @ansibullbot @jillr