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

AWS Network load balancer #33808

Merged
merged 10 commits into from May 24, 2018
Merged

AWS Network load balancer #33808

merged 10 commits into from May 24, 2018

Conversation

wimnat
Copy link
Contributor

@wimnat wimnat commented Dec 12, 2017

SUMMARY

AWS Network Load Balancer module

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

elb_network_lb

ANSIBLE VERSION
2.5
ADDITIONAL INFORMATION

Requires pending PR to move application load balancer code in to new class.
Work still to do:
subnet_mappings work
check any nlb only attributes

@ansibot
Copy link
Contributor

ansibot commented Dec 12, 2017

@ansibot ansibot added WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. affects_2.5 This issue/PR affects Ansible v2.5 aws cloud module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_module This PR includes a new module. new_plugin This PR includes a new plugin. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Dec 12, 2017
@ansibot
Copy link
Contributor

ansibot commented Dec 12, 2017

The test ansible-test sanity --test import --python 2.6 [?] failed with the following error:

lib/ansible/module_utils/aws/elb_utils.py:4:0: ImportError: No module named boto3

The test ansible-test sanity --test import --python 2.7 [?] failed with the following error:

lib/ansible/module_utils/aws/elb_utils.py:4:0: ImportError: No module named boto3

The test ansible-test sanity --test import --python 3.5 [?] failed with the following error:

lib/ansible/module_utils/aws/elb_utils.py:4:0: ImportError: No module named 'boto3'

The test ansible-test sanity --test import --python 3.6 [?] failed with the following error:

lib/ansible/module_utils/aws/elb_utils.py:4:0: ModuleNotFoundError: No module named 'boto3'

The test ansible-test sanity --test import --python 3.7 [?] failed with the following error:

lib/ansible/module_utils/aws/elb_utils.py:4:0: ModuleNotFoundError: No module named 'boto3'

The test ansible-test sanity --test pep8 [?] failed with the following errors:

lib/ansible/modules/cloud/amazon/elb_application_lb.py:383:161: E501 line too long (162 > 160 characters)
lib/ansible/modules/cloud/amazon/elb_application_lb.py:474:5: E303 too many blank lines (2)
lib/ansible/modules/cloud/amazon/elb_application_lb.py:475:5: E265 block comment should start with '# '
lib/ansible/modules/cloud/amazon/elb_application_lb.py:476:5: E265 block comment should start with '# '
lib/ansible/modules/cloud/amazon/elb_network_lb.py:315:161: E501 line too long (162 > 160 characters)

click here for bot help

@gundalow gundalow removed the needs_triage Needs a first human triage before being processed. label Dec 13, 2017
@s-hertel s-hertel mentioned this pull request Dec 18, 2017
@ansibot
Copy link
Contributor

ansibot commented Dec 20, 2017

The test ansible-test sanity --test import --python 2.6 [?] failed with the following error:

lib/ansible/module_utils/aws/elb_utils.py:4:0: ImportError: No module named boto3

The test ansible-test sanity --test import --python 2.7 [?] failed with the following error:

lib/ansible/module_utils/aws/elb_utils.py:4:0: ImportError: No module named boto3

The test ansible-test sanity --test import --python 3.5 [?] failed with the following error:

lib/ansible/module_utils/aws/elb_utils.py:4:0: ImportError: No module named 'boto3'

The test ansible-test sanity --test import --python 3.6 [?] failed with the following error:

lib/ansible/module_utils/aws/elb_utils.py:4:0: ModuleNotFoundError: No module named 'boto3'

The test ansible-test sanity --test import --python 3.7 [?] failed with the following error:

lib/ansible/module_utils/aws/elb_utils.py:4:0: ModuleNotFoundError: No module named 'boto3'

The test ansible-test sanity --test pep8 [?] failed with the following errors:

lib/ansible/module_utils/aws/elbv2.py:390:1: E303 too many blank lines (3)
lib/ansible/modules/cloud/amazon/elb_network_lb.py:315:161: E501 line too long (162 > 160 characters)

The test ansible-test sanity --test pylint [?] failed with the following error:

lib/ansible/module_utils/aws/elbv2.py:604:32: undefined-variable Undefined variable 'ALBListeners'

click here for bot help

@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 Dec 28, 2017
@ansibot ansibot added test This PR relates to tests. and removed 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 Dec 28, 2017
@ansibot
Copy link
Contributor

ansibot commented Dec 28, 2017

The test ansible-test sanity --test validate-modules [?] failed with the following error:

lib/ansible/modules/cloud/amazon/elb_network_lb.py:0:0: E101 Interpreter line is not "#!/usr/bin/python"

click here for bot help

@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 Jan 11, 2018

region, ec2_url, aws_connect_params = get_aws_connection_info(module, boto3=True)

if region:
Copy link
Contributor

Choose a reason for hiding this comment

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

If region is unset it is now handled in boto3_conn so you can remove this conditional.

- If yes, existing tags will be purged from the resource to match exactly what is defined by I(tags) parameter. If the I(tags) parameter is not set then
tags will not be modified.
required: false
default: yes
Copy link
Contributor

@s-hertel s-hertel Jan 15, 2018

Choose a reason for hiding this comment

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

I think this should default to false probably.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, ignore me, sorry. We've had this conversation before having default true is documented. https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/cloud/amazon/GUIDELINES.md#dealing-with-tags

description:
- If yes, existing listeners will be purged from the ELB to match exactly what is defined by I(listeners) parameter. If the I(listeners) parameter is
not set then listeners will not be modified
default: yes
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this safer to default to false?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think so, I just got burned by this in testing it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be at odds with the default for elb_application_lb

@wimnat
Copy link
Contributor Author

wimnat commented Jan 16, 2018

I'm going to hold off on any more work here until #33769 is merged to ease my rebasing pain :)

@linuxdynasty
Copy link
Contributor

@wimnat can I use this branch to test our your module?

@ansibot
Copy link
Contributor

ansibot commented May 19, 2018

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

lib/ansible/module_utils/aws/elbv2.py:266:0: trailing-whitespace Trailing whitespace

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

lib/ansible/module_utils/aws/elbv2.py:266:1: W293 blank line contains whitespace
lib/ansible/module_utils/aws/elbv2.py:381:161: E501 line too long (162 > 160 characters)

click here for bot help

@ansibot ansibot added 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. and removed core_review In order to be merged, this PR must follow the core review workflow. labels May 19, 2018
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label May 20, 2018
@ansibot
Copy link
Contributor

ansibot commented May 20, 2018

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

lib/ansible/module_utils/aws/elbv2.py:382:13: E125 continuation line with same indent as next logical line

click here for bot help

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label May 20, 2018
@ansibot
Copy link
Contributor

ansibot commented May 24, 2018

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

lib/ansible/module_utils/aws/elbv2.py:381:13: E125 continuation line with same indent as next logical line

click here for bot help

@s-hertel s-hertel requested a review from ryansb May 24, 2018 20:15
@s-hertel
Copy link
Contributor

bot_status

@ansibot
Copy link
Contributor

ansibot commented May 24, 2018

Components

lib/ansible/module_utils/aws/elbv2.py
support: core
maintainers:

lib/ansible/modules/cloud/amazon/elb_network_lb.py
support: community
maintainers:

test/integration/targets/elb_network_lb/aliases
support: community
maintainers:

test/integration/targets/elb_network_lb/defaults/main.yml
support: community
maintainers:

test/integration/targets/elb_network_lb/meta/main.yml
support: community
maintainers:

test/integration/targets/elb_network_lb/tasks/main.yml
support: community
maintainers:

test/integration/targets/elb_network_lb/tasks/test_creating_nlb.yml
support: community
maintainers:

test/integration/targets/elb_network_lb/tasks/test_deleting_nlb.yml
support: community
maintainers:

test/integration/targets/elb_network_lb/tasks/test_modifying_nlb_listeners.yml
support: community
maintainers:

test/integration/targets/elb_network_lb/tasks/test_nlb_bad_listener_options.yml
support: community
maintainers:

test/integration/targets/elb_network_lb/tasks/test_nlb_tags.yml
support: community
maintainers:

test/integration/targets/elb_network_lb/tasks/test_nlb_with_asg.yml
support: community
maintainers:

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): 1
ansible_shipits (core team members): 0
shipit_actors (maintainers or core team members): wimnat
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. labels May 24, 2018
@ryansb ryansb merged commit 858f0fc into ansible:devel May 24, 2018
@wimnat wimnat deleted the network_load_balancer branch May 25, 2018 03:58
gothicx pushed a commit to gothicx/ansible that referenced this pull request Jun 9, 2018
* New module - elb_network_lb

* Fix creating a load balancer without tags

* Linter

Fix purging tags

Remove extra imports

* add support for cross zone lb, doc update and fix tagging

* pep8 fixes

* Add integration tests for elb_network_lb module

* more pep8

* Remove non-applicable option for NLBs

* fix target protocol

* pep8
jacum pushed a commit to jacum/ansible that referenced this pull request Jun 26, 2018
* New module - elb_network_lb

* Fix creating a load balancer without tags

* Linter

Fix purging tags

Remove extra imports

* add support for cross zone lb, doc update and fix tagging

* pep8 fixes

* Add integration tests for elb_network_lb module

* more pep8

* Remove non-applicable option for NLBs

* fix target protocol

* pep8
@ansible ansible locked and limited conversation to collaborators May 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.5 This issue/PR affects Ansible v2.5 aws cloud core_review In order to be merged, this PR must follow the core review workflow. module This issue/PR relates to a module. new_module This PR includes a new module. new_plugin This PR includes a new plugin. support:community This issue/PR relates to code supported by the Ansible community. 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

7 participants