[BUG] consul module incorrectly reports error on success
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 15.7k
- Forks
- 5.6k
- Avg merge
- 2d 44m
- Merged PRs (30d)
- 80
Description
Description
When putting requests to Consul, no response is received. We just get a HTTP 200 to indicate success.
This means salt.utils.http.query throws exception Expecting value: line 1 column 1 (char 0) in _query here as there is nothing to decode. Modifying salt.utils.http.query to use decode=False resolves this
Setup
Here is an example state that installs/configures consul and can manifest the issue
repo:
pkgrepo.managed:
- name: hashicorp
- humanname: Hashicorp Stable - $basearch
- baseurl: https://rpm.releases.hashicorp.com/RHEL/$releasever/$basearch/stable
- gpgcheck: 1
- gpgkey: https://rpm.releases.hashicorp.com/gpg
consul:
pkg.installed
config:
file.managed:
- name: /etc/consul.d/consul.hcl
- contents: |
data_dir = "/opt/consul"
server = true
bind_addr = "0.0.0.0"
advertise_addr = "127.0.0.1"
bootstrap_expect = 1
consul service:
service.running:
- name: consul
- enable: True
- reload: True
- watch:
- file: /etc/consul.d/consul.hcl
consul.agent_service_register:
module.run:
- consul_url: "http://localhost:8500"
- kwargs:
name: "testing"
id: "testing"
port: 9100
address: "localhost"
check:
interval: "5m"
http: "http://localhost:9100/metrics"
If you run the above you will see the service is correctly created:
[vagrant@master ~]$ consul catalog services
consul
testing
[vagrant@master ~]$
But salt incorrectly reports an error:
----------
ID: consul.agent_service_register
Function: module.run
Result: False
Comment: Module function consul.agent_service_register threw an exception. Exception: Expecting value: line 1 column 1 (char 0)
Started: 23:26:17.659090
Duration: 5842.556 ms
Changes:
Steps to Reproduce the behavior
See above, or vagrant up https://github.com/tomdoherty/salt-docker
Expected behavior
Error should not be thrown
Versions Report
salt --versions-report
(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)[vagrant@master ~]$ salt --versions-report
Salt Version:
Salt: 3004
Dependency Versions:
cffi: 1.11.5
cherrypy: unknown
dateutil: 2.6.1
docker-py: Not Installed
gitdb: Not Installed
gitpython: Not Installed
Jinja2: 2.10.1
libgit2: Not Installed
M2Crypto: 0.35.2
Mako: Not Installed
msgpack: 0.6.2
msgpack-pure: Not Installed
mysql-python: Not Installed
pycparser: 2.14
pycrypto: Not Installed
pycryptodome: Not Installed
pygit2: Not Installed
Python: 3.6.8 (default, Mar 19 2021, 05:13:41)
python-gnupg: Not Installed
PyYAML: 3.12
PyZMQ: 19.0.0
smmap: Not Installed
timelib: Not Installed
Tornado: 4.5.3
ZMQ: 4.3.4
System Versions:
dist: centos 8
locale: UTF-8
machine: x86_64
release: 5.10.47-linuxkit
system: Linux
version: CentOS Linux 8
Additional context
below patch fixes this
diff --git a/salt/modules/consul.py b/salt/modules/consul.py
index 814bc47ff5..13c79715b1 100644
--- a/salt/modules/consul.py
+++ b/salt/modules/consul.py
@@ -84,7 +84,7 @@ def _query(
method=method,
params=query_params,
data=data,
- decode=True,
+ decode=False,
status=True,
header_dict=headers,
opts=__opts__,
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in salt/modules/consul.py at _query, especially the call to salt.utils.http.query around line 82, and review how the Consul registration request handles an empty successful response. Reproduce the provided state or equivalent service-registration request and verify that HTTP 200 responses with no body no longer produce an exception while the service is created.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- devops
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100