ec2_instance - Support AdditionalInfo option. #1828
Conversation
Docs Build 📝Thank you for contribution!✨ This PR has been merged and your docs changes will be incorporated when they are next published. |
|
Thanks for taking the time to submit this PR. #1825 was recently submitted which would support a broader range of placement options. I think it might be better to reduce this PR down to just adding the "additional_info" parameter. IMO grouping the placement options (as with #1825) is going to be more sustainable (EC2 instances have a huge number of potential options) Please also add some integration tests (see other examples in tests/integration/targets/ec2_instance_*) |
tremble
changed the title
Thank you for taking a look at this PR, I will update to only add additional_info now that #1825 will take care of other things I need. |
yugesk
changed the title
|
@tremble AdditionalInfo is a property reserved for internal use and it will fail for external users I think. How do I add integration tests to it? |
|
Could someone please review this PR? |
|
@tremble could you please review again? |
If it's reserved for internal use, what's the use case for adding it? |
We still need additional_info parameter for passing in custom options thats understandable by aws/boto3 commands. E.g. for doing a target launch we could pass in input like "--additional-info ignore-no-launches=true,target-droplet=x.x.x.x". This lets you launch a instance onto a test/production host. |
There was a problem hiding this comment.
We also need a changelog fragment, but otherwise I'm ok with this change: it's reasonably minimal.
However, I'd like to hear from @alinabuzachis or @jillr how they feel about supporting something that (if I understand correctly) can only be used by Amazon Engineers.
|
I'm a little confused what's going on with CI here, because this PR should fail sanity checks as is. It fails locally when I run it. We can't have an undocumented parameter. This is a pretty weird case. At the very least, the new parameter needs to be documented and the documentation should clearly state that it is unsupported and will likely fail. Probably something about it being added purely for Amazon's internal use. |
Some of the tests need to be manually approved when a new contributor submits a PR. This will be more important if/when integration tests move to github actions. |
This comment was marked as outdated.
This comment was marked as outdated.
|
@tremble thanks for fixing the black formatting errors. Why are still some tests failing? |
This comment was marked as outdated.
This comment was marked as outdated.
Some of the tests are slightly flakey. It's a side effect of the way many of the AWS APIs trigger a change and return "success" before the change has actually been applied. Over time we've been adding waiters to improve things uncovered by the more "unstable" tests, but the API for waiters results in extra API calls, and in turn sometimes causes problems with the AWS rate limiting. |
This comment was marked as outdated.
This comment was marked as outdated.
Work on ec2_instance integration test flakes SUMMARY As seen in #1828 the integration tests for ec2_instance are still a little flaky (ignoring Zuul also being flaky). This attempts to tweak two of the worst offenders to improve things ISSUE TYPE Bugfix Pull Request COMPONENT NAME ec2_instance ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis
Work on ec2_instance integration test flakes SUMMARY As seen in #1828 the integration tests for ec2_instance are still a little flaky (ignoring Zuul also being flaky). This attempts to tweak two of the worst offenders to improve things ISSUE TYPE Bugfix Pull Request COMPONENT NAME ec2_instance ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis (cherry picked from commit 02b1511)
Work on ec2_instance integration test flakes SUMMARY As seen in #1828 the integration tests for ec2_instance are still a little flaky (ignoring Zuul also being flaky). This attempts to tweak two of the worst offenders to improve things ISSUE TYPE Bugfix Pull Request COMPONENT NAME ec2_instance ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis (cherry picked from commit 02b1511)
[PR #1845/02b15111 backport][stable-7] Work on ec2_instance integration test flakes This is a backport of PR #1845 as merged into main (02b1511). SUMMARY As seen in #1828 the integration tests for ec2_instance are still a little flaky (ignoring Zuul also being flaky). This attempts to tweak two of the worst offenders to improve things ISSUE TYPE Bugfix Pull Request COMPONENT NAME ec2_instance ADDITIONAL INFORMATION Reviewed-by: Mark Chappell
[PR #1845/02b15111 backport][stable-6] Work on ec2_instance integration test flakes This is a backport of PR #1845 as merged into main (02b1511). SUMMARY As seen in #1828 the integration tests for ec2_instance are still a little flaky (ignoring Zuul also being flaky). This attempts to tweak two of the worst offenders to improve things ISSUE TYPE Bugfix Pull Request COMPONENT NAME ec2_instance ADDITIONAL INFORMATION Reviewed-by: Mark Chappell
|
recheck |
|
regate |
|
regate |
softwarefactory-project-zuul
bot
merged commit e02fb78
into
ansible-collections:main
Backport to stable-7: 💚 backport PR created✅ Backport PR branch: Backported as #1880 🤖 @patchback |
ec2_instance - Support AdditionalInfo option. This is a reserved parameter that was already supported by boto3. SUMMARY Enable support for passing additional_info while creating instance. ISSUE TYPE Feature Pull Request COMPONENT NAME ec2_instance Reviewed-by: Mark Chappell Reviewed-by: Yugesh Kambham Reviewed-by: Alina Buzachis (cherry picked from commit e02fb78)
[PR #1828/e02fb78a backport][stable-7] ec2_instance - Support AdditionalInfo option. This is a backport of PR #1828 as merged into main (e02fb78). This is a reserved parameter that was already supported by boto3. SUMMARY Enable support for passing additional_info while creating instance. ISSUE TYPE Feature Pull Request COMPONENT NAME ec2_instance Reviewed-by: Mark Chappell
This is a reserved parameter that was already supported by boto3.
SUMMARY
Enable support for passing additional_info while creating instance.
ISSUE TYPE
COMPONENT NAME
ec2_instance