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 consul module service deregistration #34847

Merged
merged 2 commits into from
Mar 6, 2018

Conversation

alvaroaleman
Copy link
Contributor

@alvaroaleman alvaroaleman commented Jan 14, 2018

Upstream pr in the consul-python library:
python-consul/python-consul#174

This is based on work from https://github.com/bobh

Fixes #34628

SUMMARY
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

consul.py

ANSIBLE VERSION
devel

@ansibot
Copy link
Contributor

ansibot commented Jan 14, 2018

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

lib/ansible/modules/clustering/consul.py:346:0: NameError: name 'consul' is not defined

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

lib/ansible/modules/clustering/consul.py:346:0: NameError: name 'consul' is not defined

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

lib/ansible/modules/clustering/consul.py:346:0: NameError: name 'consul' is not defined

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

lib/ansible/modules/clustering/consul.py:346:0: NameError: name 'consul' is not defined

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

lib/ansible/modules/clustering/consul.py:346:0: NameError: name 'consul' is not defined

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

lib/ansible/modules/clustering/consul.py:353:26: E128 continuation line under-indented for visual indent
lib/ansible/modules/clustering/consul.py:354:26: E128 continuation line under-indented for visual indent
lib/ansible/modules/clustering/consul.py:355:26: E128 continuation line under-indented for visual indent
lib/ansible/modules/clustering/consul.py:356:26: E128 continuation line under-indented for visual indent

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

lib/ansible/modules/clustering/consul.py:0:0: E321 Exception attempting to import module for argument_spec introspection

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Jan 14, 2018

@ansibot ansibot added affects_2.5 This issue/PR affects Ansible v2.5 bugfix_pull_request ci_verified Changes made in this PR are causing tests to fail. module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. labels Jan 14, 2018
@alvaroaleman alvaroaleman force-pushed the fix-34628-consul-deregistration branch from 283a7b5 to 90501d2 Compare January 14, 2018 12:23
@ansibot
Copy link
Contributor

ansibot commented Jan 14, 2018

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

lib/ansible/modules/clustering/consul.py:346:0: NameError: name 'consul' is not defined

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

lib/ansible/modules/clustering/consul.py:346:0: NameError: name 'consul' is not defined

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

lib/ansible/modules/clustering/consul.py:346:0: NameError: name 'consul' is not defined

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

lib/ansible/modules/clustering/consul.py:346:0: NameError: name 'consul' is not defined

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

lib/ansible/modules/clustering/consul.py:346:0: NameError: name 'consul' is not defined

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

lib/ansible/modules/clustering/consul.py:0:0: E321 Exception attempting to import module for argument_spec introspection

click here for bot help

@alvaroaleman alvaroaleman force-pushed the fix-34628-consul-deregistration branch from 90501d2 to ffa114b Compare January 14, 2018 12:37
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Jan 14, 2018
Upstream pr in the python-consul library:
python-consul/python-consul#174

This is based on work from https://github.com/bobh

Fixes ansible#34628
@ansibot
Copy link
Contributor

ansibot commented Jan 14, 2018

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

lib/ansible/modules/clustering/consul.py:346:0: NameError: name 'consul' is not defined

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

lib/ansible/modules/clustering/consul.py:346:0: NameError: name 'consul' is not defined

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

lib/ansible/modules/clustering/consul.py:346:0: NameError: name 'consul' is not defined

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

lib/ansible/modules/clustering/consul.py:346:0: NameError: name 'consul' is not defined

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

lib/ansible/modules/clustering/consul.py:346:0: NameError: name 'consul' is not defined

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

lib/ansible/modules/clustering/consul.py:0:0: E321 Exception attempting to import module for argument_spec introspection

click here for bot help

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Jan 14, 2018
@alvaroaleman alvaroaleman force-pushed the fix-34628-consul-deregistration branch from ffa114b to 9967419 Compare January 14, 2018 12:56
@ansibot ansibot added community_review In order to be merged, this PR must follow the community 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 Jan 14, 2018
@alvaroaleman
Copy link
Contributor Author

ready_for_review

@sivel sivel removed the needs_triage Needs a first human triage before being processed. label Jan 15, 2018
@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 23, 2018
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request 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 Mar 2, 2018
@alvaroaleman alvaroaleman force-pushed the fix-34628-consul-deregistration branch from c24c684 to ae87740 Compare March 6, 2018 13:15
@jnogol
Copy link

jnogol commented Mar 6, 2018

Tested. Now deregistration works like a charm @alvaroaleman @gundalow

@gundalow gundalow added this to To Do in 2.5.x blocker list via automation Mar 6, 2018
@gundalow
Copy link
Contributor

gundalow commented Mar 6, 2018

rebuild_merge

@ansibot ansibot merged commit c9cb001 into ansible:devel Mar 6, 2018
@alvaroaleman alvaroaleman deleted the fix-34628-consul-deregistration branch March 6, 2018 16:59
@gundalow
Copy link
Contributor

gundalow commented Mar 6, 2018

Does this need cherry-picking into stable-2.5?

@alvaroaleman
Copy link
Contributor Author

That would be very helpful, yes

gundalow pushed a commit to gundalow/ansible that referenced this pull request Mar 6, 2018
* Fix consul module service deregistration

Upstream pr in the python-consul library:
python-consul/python-consul#174

This is based on work from https://github.com/bobh

Fixes ansible#34628

* Pass ACL token when deregistering consul service

(cherry picked from commit c9cb001)
gundalow added a commit that referenced this pull request Mar 6, 2018
* Fix consul module service deregistration

Upstream pr in the python-consul library:
python-consul/python-consul#174

This is based on work from https://github.com/bobh

Fixes #34628

* Pass ACL token when deregistering consul service

(cherry picked from commit c9cb001)
@gundalow
Copy link
Contributor

gundalow commented Mar 6, 2018

Merged into devel and cherry-picked into stable-2.5 in 471e75a

@nitzmahone nitzmahone moved this from To Do to 2.5.1 Holding in 2.5.x blocker list Mar 9, 2018
@nitzmahone nitzmahone moved this from 2.5.1 Holding to Done in 2.5.x blocker list Mar 9, 2018
abadger pushed a commit that referenced this pull request Mar 29, 2018
* Fix consul module service deregistration

Upstream pr in the python-consul library:
python-consul/python-consul#174

This is based on work from https://github.com/bobh

Fixes #34628

* Pass ACL token when deregistering consul service

(cherry picked from commit c9cb001)
@ansible ansible locked and limited conversation to collaborators Apr 26, 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 bug This issue/PR relates to a bug. community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. support:community This issue/PR relates to code supported by the Ansible community.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

consul.py: state=absent is broken
5 participants