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

Fix ec2_group for numbered protocols (GRE) #42765

Merged
merged 6 commits into from Sep 5, 2018
Merged

Conversation

ryansb
Copy link
Contributor

@ryansb ryansb commented Jul 13, 2018

SUMMARY

Reported in #42740

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ec2_group

ANSIBLE VERSION

ADDITIONAL INFORMATION

@ansibot
Copy link
Contributor

ansibot commented Jul 13, 2018

cc @adq
click here for bot help

@ansibot ansibot added WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. affects_2.7 This issue/PR affects Ansible v2.7 aws bug This issue/PR relates to a bug. cloud module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jul 13, 2018
@ryansb ryansb removed the needs_triage Needs a first human triage before being processed. label Jul 13, 2018
@ansibot
Copy link
Contributor

ansibot commented Jul 13, 2018

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

lib/ansible/modules/cloud/amazon/ec2_group.py:322:20: E127 continuation line over-indented for visual indent

click here for bot help

@s-hertel s-hertel self-requested a review July 16, 2018 14:04
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jul 24, 2018
@ryansb ryansb changed the title WIP: Fix ec2_group for numbered protocols (GRE) Fix ec2_group for numbered protocols (GRE) Aug 15, 2018
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. test This PR relates to tests. and removed WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Aug 15, 2018
@ansibot
Copy link
Contributor

ansibot commented Aug 15, 2018

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

lib/ansible/modules/cloud/amazon/ec2_group.py:322:20: E126 continuation line over-indented for hanging indent

click here for bot help

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Aug 15, 2018
vpc_id: '{{ vpc_result.vpc.id }}'
description: '{{ ec2_group_description }}'
tags:
foo: 1
Copy link
Contributor

Choose a reason for hiding this comment

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

This task and the one below need credentials

description: '{{ ec2_group_description }}'
tags:
foo: 1
- name: Readd a tag with a numeric value
Copy link
Contributor

Choose a reason for hiding this comment

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

s/Readd/Read

@s-hertel
Copy link
Contributor

s-hertel commented Aug 16, 2018

This task is failing repeatedly for me.

TASK [ec2_group : test modifying rule and egress rule descriptions (expected changed=true)] ***********************************************
task path: /Users/shertel/Workspace/ansible/test/integration/targets/ec2_group/tasks/main.yml:1177
...
The full traceback is:
Traceback (most recent call last):
  File "/tmp/ansible_hIDfGf/ansible_module_ec2_group.py", line 709, in authorize
    client.authorize_security_group_ingress(GroupId=group_id, IpPermissions=ip_permissions)
  File "/Users/shertel/Workspace/ansible/venv/python2.7.13/lib/python2.7/site-packages/botocore/client.py", line 314, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/Users/shertel/Workspace/ansible/venv/python2.7.13/lib/python2.7/site-packages/botocore/client.py", line 612, in _make_api_call
    raise error_class(parsed_response, operation_name)
ClientError: An error occurred (InvalidPermission.Duplicate) when calling the AuthorizeSecurityGroupIngress operation: the specified rule "peer: 1001:d00::/24, TCP, from port: 8281, to port: 8281, ALLOW" already exists

fatal: [localhost]: FAILED! => {
    "boto3_version": "1.7.52",
    "botocore_version": "1.10.78",
    "changed": false,
    "error": {
        "code": "InvalidPermission.Duplicate",
        "message": "the specified rule \"peer: 1001:d00::/24, TCP, from port: 8281, to port: 8281, ALLOW\" already exists"
    },
    "invocation": {
        "module_args": {
            ...
        }
    },
    "msg": "Unable to authorize ingress rules: [{'ToPort': 8281, 'IpProtocol': u'tcp', 'Ipv6Ranges': [{'CidrIpv6': '1001:d00::/24', 'Description': 'ipv6 rule desc 2'}], 'FromPort': 8281}]: An error occurred (InvalidPermission.Duplicate) when calling the AuthorizeSecurityGroupIngress operation: the specified rule \"peer: 1001:d00::/24, TCP, from port: 8281, to port: 8281, ALLOW\" already exists",
    "response_metadata": {
        "http_headers": {
            "connection": "close",
            "date": "Thu, 16 Aug 2018 17:38:24 GMT",
            "server": "AmazonEC2",
            "transfer-encoding": "chunked"
        },
        "http_status_code": 400,
        "request_id": "0765a398-b022-424f-a0c7-e7333d366a93",
        "retry_attempts": 0
    }
}

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Aug 24, 2018
@mattclay
Copy link
Member

CI failure in integration tests: https://app.shippable.com/github/ansible/ansible/runs/78997/67/tests

@mattclay mattclay added the ci_verified Changes made in this PR are causing tests to fail. label Aug 28, 2018
@s-hertel s-hertel force-pushed the sg-fixes branch 3 times, most recently from f28f761 to bab7f3f Compare September 5, 2018 14:58
Add credentials to tests

pep8
@s-hertel
Copy link
Contributor

s-hertel commented Sep 5, 2018

bot_status

@ansibot
Copy link
Contributor

ansibot commented Sep 5, 2018

Components

lib/ansible/module_utils/ec2.py
support: core
maintainers:

lib/ansible/modules/cloud/amazon/ec2_group.py
support: core
maintainers: adq

test/integration/targets/ec2_group/tasks/main.yml
support: core
maintainers: adq

test/integration/targets/ec2_group/tasks/numeric_protos.yml
support: core
maintainers: adq

Metadata

waiting_on: ansible
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
too many files or commits: False
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): 0
community_shipits (namespace maintainers): 0
ansible_shipits (core team members): 1
shipit_actors (maintainers or core team members): ryansb
shipit_actors_other: []
automerge: automerge shipit test failed

click here for bot help

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Sep 5, 2018
@ryansb ryansb merged commit 20f2177 into ansible:devel Sep 5, 2018
ryansb added a commit to ryansb/ansible that referenced this pull request Sep 5, 2018
* Fix spurious `changed=True` when int is passed as tag

* Fix for all AWS module using compare_aws_tags

* Handle improperly stringified protocols and allow inconsistency between None/-1 on non-tcp protocols

* Add integration test that reproduces the same bug

* Return false if the comparsison is not equal

(cherry picked from commit 20f2177)
@ryansb ryansb deleted the sg-fixes branch September 5, 2018 17:38
abadger pushed a commit that referenced this pull request Sep 6, 2018
* Fix spurious `changed=True` when int is passed as tag

* Fix for all AWS module using compare_aws_tags

* Handle improperly stringified protocols and allow inconsistency between None/-1 on non-tcp protocols

* Add integration test that reproduces the same bug

* Return false if the comparsison is not equal

(cherry picked from commit 20f2177)
ndswartz pushed a commit to ndswartz/ansible that referenced this pull request Nov 28, 2018
* Fix spurious `changed=True` when int is passed as tag

* Fix for all AWS module using compare_aws_tags

* Handle improperly stringified protocols and allow inconsistency between None/-1 on non-tcp protocols

* Add integration test that reproduces the same bug

* Return false if the comparsison is not equal

(cherry picked from commit 20f2177)
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.7 This issue/PR affects Ansible v2.7 aws bug This issue/PR relates to a bug. cloud core_review In order to be merged, this PR must follow the core review workflow. module This issue/PR relates to a module. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants