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_instance - update build_run_instance_spec to skip TagSpecification if None #1151

Merged

Conversation

tremble
Copy link
Contributor

@tremble tremble commented Oct 11, 2022

SUMMARY

fixes: #1148

When no tags are supplied, build_run_instance_spec currently includes 'TagSpecification': None. This results in botocore throwing an exception.

Also renames instance_role to iam_instance_profile (keeping the original as an alias). While this could be split off, I'll just perform a partial backport for the bugfix when backporting to 4.x, while working through some unit tests the inaccuracy of the parameter name was apparent.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

plugins/modules/ec2_instance.py

ADDITIONAL INFORMATION

@github-actions
Copy link

github-actions bot commented Oct 11, 2022

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

@ansibullbot ansibullbot added bug This issue/PR relates to a bug community_review integration tests/integration module module needs_maintainer needs_triage new_plugin New plugin plugins plugin (any type) tests tests labels Oct 11, 2022
@softwarefactory-project-zuul

This comment was marked as resolved.

@tremble tremble force-pushed the issues/1148 branch 2 times, most recently from 2266692 to 83494d9 Compare October 11, 2022 14:34
import ansible_collections.amazon.aws.plugins.module_utils.arn as utils_arn
from ansible_collections.amazon.aws.plugins.module_utils.botocore import HAS_BOTO3

try:
Copy link
Member

Choose a reason for hiding this comment

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

This should never happen. botocore is in the unit-test requirements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can cause problems with the sanity tests if you don't add ImportError handling. The sanity tests are supposed to run in a very minimal environment.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

✔️ ansible-galaxy-importer SUCCESS in 4m 54s
✔️ build-ansible-collection SUCCESS in 6m 33s
ansible-test-sanity-aws-ansible-python38 FAILURE in 10m 38s
ansible-test-sanity-aws-ansible-2.12-python38 FAILURE in 9m 46s
ansible-test-sanity-aws-ansible-2.13-python38 FAILURE in 9m 55s
ansible-test-units-amazon-aws-python36 FAILURE in 9m 14s
ansible-test-units-amazon-aws-python38 FAILURE in 8m 12s
ansible-test-units-amazon-aws-python39 FAILURE in 8m 01s
cloud-tox-py3 FAILURE in 4m 36s
✔️ ansible-test-splitter SUCCESS in 3m 18s
✔️ integration-amazon.aws-1 SUCCESS in 37m 24s
✔️ integration-amazon.aws-2 SUCCESS in 40m 13s
✔️ integration-amazon.aws-3 SUCCESS in 44m 48s
✔️ integration-amazon.aws-4 SUCCESS in 31m 15s
⚠️ integration-amazon.aws-5 SKIPPED
⚠️ integration-amazon.aws-6 SKIPPED
⚠️ integration-amazon.aws-7 SKIPPED
⚠️ integration-amazon.aws-8 SKIPPED
⚠️ integration-amazon.aws-9 SKIPPED
⚠️ integration-amazon.aws-10 SKIPPED
⚠️ integration-amazon.aws-11 SKIPPED
⚠️ integration-amazon.aws-12 SKIPPED
⚠️ integration-amazon.aws-13 SKIPPED
⚠️ integration-amazon.aws-14 SKIPPED
⚠️ integration-amazon.aws-15 SKIPPED
⚠️ integration-amazon.aws-16 SKIPPED
⚠️ integration-amazon.aws-17 SKIPPED
⚠️ integration-amazon.aws-18 SKIPPED
✔️ integration-community.aws-1 SUCCESS in 7m 12s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED
⚠️ integration-community.aws-14 SKIPPED
⚠️ integration-community.aws-15 SKIPPED
⚠️ integration-community.aws-16 SKIPPED
⚠️ integration-community.aws-17 SKIPPED
⚠️ integration-community.aws-18 SKIPPED
✔️ ansible-test-changelog SUCCESS in 2m 45s

@tremble tremble added the mergeit Merge the PR (SoftwareFactory) label Oct 11, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

✔️ ansible-galaxy-importer SUCCESS in 5m 38s
✔️ build-ansible-collection SUCCESS in 5m 58s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 10m 18s
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 13m 09s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 10m 32s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 8m 58s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 8m 20s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 7m 27s
✔️ cloud-tox-py3 SUCCESS in 4m 04s
✔️ ansible-test-splitter SUCCESS in 3m 09s
✔️ integration-amazon.aws-1 SUCCESS in 49m 20s
✔️ integration-amazon.aws-2 SUCCESS in 45m 00s
✔️ integration-amazon.aws-3 SUCCESS in 49m 43s
✔️ integration-amazon.aws-4 SUCCESS in 45m 33s
⚠️ integration-amazon.aws-5 SKIPPED
⚠️ integration-amazon.aws-6 SKIPPED
⚠️ integration-amazon.aws-7 SKIPPED
⚠️ integration-amazon.aws-8 SKIPPED
⚠️ integration-amazon.aws-9 SKIPPED
⚠️ integration-amazon.aws-10 SKIPPED
⚠️ integration-amazon.aws-11 SKIPPED
⚠️ integration-amazon.aws-12 SKIPPED
⚠️ integration-amazon.aws-13 SKIPPED
⚠️ integration-amazon.aws-14 SKIPPED
⚠️ integration-amazon.aws-15 SKIPPED
⚠️ integration-amazon.aws-16 SKIPPED
⚠️ integration-amazon.aws-17 SKIPPED
⚠️ integration-amazon.aws-18 SKIPPED
⚠️ integration-community.aws-1 SKIPPED
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED
⚠️ integration-community.aws-14 SKIPPED
⚠️ integration-community.aws-15 SKIPPED
⚠️ integration-community.aws-16 SKIPPED
⚠️ integration-community.aws-17 SKIPPED
⚠️ integration-community.aws-18 SKIPPED
✔️ ansible-test-changelog SUCCESS in 3m 05s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit a9ad180 into ansible-collections:main Oct 11, 2022
@tremble tremble added backport-4 PR should be backported to the stable-4 branch backport-5 PR should be backported to the stable-5 branch labels Oct 12, 2022
@patchback
Copy link

patchback bot commented Oct 12, 2022

Backport to stable-5: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-5/a9ad1808f2e87837eed23d71cac3e35b1d433d23/pr-1151

Backported as #1160

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@patchback
Copy link

patchback bot commented Oct 12, 2022

Backport to stable-4: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply a9ad180 on top of patchback/backports/stable-4/a9ad1808f2e87837eed23d71cac3e35b1d433d23/pr-1151

Backporting merged PR #1151 into main

  1. Ensure you have a local repo clone of your fork. Unless you cloned it
    from the upstream, this would be your origin remote.
  2. Make sure you have an upstream repo added as a remote too. In these
    instructions you'll refer to it by the name upstream. If you don't
    have it, here's how you can add it:
    $ git remote add upstream https://github.com/ansible-collections/amazon.aws.git
  3. Ensure you have the latest copy of upstream and prepare a branch
    that will hold the backported code:
    $ git fetch upstream
    $ git checkout -b patchback/backports/stable-4/a9ad1808f2e87837eed23d71cac3e35b1d433d23/pr-1151 upstream/stable-4
  4. Now, cherry-pick PR ec2_instance - update build_run_instance_spec to skip TagSpecification if None #1151 contents into that branch:
    $ git cherry-pick -x a9ad1808f2e87837eed23d71cac3e35b1d433d23
    If it'll yell at you with something like fatal: Commit a9ad1808f2e87837eed23d71cac3e35b1d433d23 is a merge but no -m option was given., add -m 1 as follows intead:
    $ git cherry-pick -m1 -x a9ad1808f2e87837eed23d71cac3e35b1d433d23
  5. At this point, you'll probably encounter some merge conflicts. You must
    resolve them in to preserve the patch from PR ec2_instance - update build_run_instance_spec to skip TagSpecification if None #1151 as close to the
    original as possible.
  6. Push this branch to your fork on GitHub:
    $ git push origin patchback/backports/stable-4/a9ad1808f2e87837eed23d71cac3e35b1d433d23/pr-1151
  7. Create a PR, ensure that the CI is green. If it's not — update it so that
    the tests and any other checks pass. This is it!
    Now relax and wait for the maintainers to process your pull request
    when they have some cycles to do reviews. Don't worry — they'll tell you if
    any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Oct 12, 2022
…n if None (#1151)

ec2_instance - update build_run_instance_spec to skip TagSpecification if None

SUMMARY
fixes: #1148
When no tags are supplied, build_run_instance_spec currently includes 'TagSpecification': None.  This results in botocore throwing an exception.
Also renames instance_role to iam_instance_profile (keeping the original as an alias).  While this could be split off, I'll just perform a partial backport for the bugfix when backporting to 4.x, while working through some unit tests the inaccuracy of the parameter name was apparent.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
plugins/modules/ec2_instance.py
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Gonéri Le Bouder <[email protected]>
(cherry picked from commit a9ad180)
tremble added a commit to tremble/amazon.aws that referenced this pull request Oct 12, 2022
…n if None (ansible-collections#1151)

ec2_instance - update build_run_instance_spec to skip TagSpecification if None

SUMMARY
fixes: ansible-collections#1148
When no tags are supplied, build_run_instance_spec currently includes 'TagSpecification': None.  This results in botocore throwing an exception.
Also renames instance_role to iam_instance_profile (keeping the original as an alias).  While this could be split off, I'll just perform a partial backport for the bugfix when backporting to 4.x, while working through some unit tests the inaccuracy of the parameter name was apparent.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
plugins/modules/ec2_instance.py
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Gonéri Le Bouder <[email protected]>
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Oct 13, 2022
…n if None (#1151) (#1160)

[PR #1151/a9ad1808 backport][stable-5] ec2_instance - update build_run_instance_spec to skip TagSpecification if None

This is a backport of PR #1151 as merged into main (a9ad180).
SUMMARY
fixes: #1148
When no tags are supplied, build_run_instance_spec currently includes 'TagSpecification': None.  This results in botocore throwing an exception.
Also renames instance_role to iam_instance_profile (keeping the original as an alias).  While this could be split off, I'll just perform a partial backport for the bugfix when backporting to 4.x, while working through some unit tests the inaccuracy of the parameter name was apparent.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
plugins/modules/ec2_instance.py
ADDITIONAL INFORMATION

Reviewed-by: Mark Chappell <None>
saito-hideki pushed a commit to saito-hideki/amazon.aws that referenced this pull request Oct 18, 2022
…collections#1152)

lambda_info - refactor to fix bug when querying all lambdas

Depends-On: ansible/ansible-zuul-jobs#1558
SUMMARY

Fix bug that forces query: config when getting info for all lambdas. Refactored to return the expected info
Add extra cleanup at end of tests

Fixes ansible-collections#1151
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
lambda_info
ADDITIONAL INFORMATION
This module also currently returns a dict of dicts (as opposed to a list of dicts), but I wanted to keep the scope of this PR to fixing the bug.

Reviewed-by: Mark Chappell <None>
Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Jill R <None>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections/community.aws@0c76aed
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Oct 19, 2022
…n if None (#1151) (#1162)

[stable-4] ec2_instance - update build_run_instance_spec to skip TagSpecification if None

Partial backport of #1151
SUMMARY
When no tags are supplied, build_run_instance_spec currently includes 'TagSpecification': None.  This results in botocore throwing an exception.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
plugins/modules/ec2_instance.py
ADDITIONAL INFORMATION

Reviewed-by: Gonéri Le Bouder <[email protected]>
@tremble tremble deleted the issues/1148 branch October 21, 2022 07:33
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
…collections#1152)

lambda_info - refactor to fix bug when querying all lambdas

Depends-On: ansible/ansible-zuul-jobs#1558
SUMMARY

Fix bug that forces query: config when getting info for all lambdas. Refactored to return the expected info
Add extra cleanup at end of tests

Fixes ansible-collections#1151
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
lambda_info
ADDITIONAL INFORMATION
This module also currently returns a dict of dicts (as opposed to a list of dicts), but I wanted to keep the scope of this PR to fixing the bug.

Reviewed-by: Mark Chappell <None>
Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Jill R <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
…collections#1152)

lambda_info - refactor to fix bug when querying all lambdas

Depends-On: ansible/ansible-zuul-jobs#1558
SUMMARY

Fix bug that forces query: config when getting info for all lambdas. Refactored to return the expected info
Add extra cleanup at end of tests

Fixes ansible-collections#1151
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
lambda_info
ADDITIONAL INFORMATION
This module also currently returns a dict of dicts (as opposed to a list of dicts), but I wanted to keep the scope of this PR to fixing the bug.

Reviewed-by: Mark Chappell <None>
Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Jill R <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Oct 24, 2023
…collections#1152)

lambda_info - refactor to fix bug when querying all lambdas

Depends-On: ansible/ansible-zuul-jobs#1558
SUMMARY

Fix bug that forces query: config when getting info for all lambdas. Refactored to return the expected info
Add extra cleanup at end of tests

Fixes ansible-collections#1151
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
lambda_info
ADDITIONAL INFORMATION
This module also currently returns a dict of dicts (as opposed to a list of dicts), but I wanted to keep the scope of this PR to fixing the bug.

Reviewed-by: Mark Chappell <None>
Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Jill R <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-4 PR should be backported to the stable-4 branch backport-5 PR should be backported to the stable-5 branch bug This issue/PR relates to a bug community_review integration tests/integration mergeit Merge the PR (SoftwareFactory) module module needs_maintainer new_plugin New plugin plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ec2_instance fails to check TagSpecification for None
4 participants