Merge ~tcuthbert/charm-telegraf:master into charm-telegraf:master

Proposed by Thomas Cuthbert on 2021-01-11
Status: Merged
Approved by: Xav Paice on 2021-01-26
Approved revision: 631a400466a51aa887ee9fe5be9e9171ad2784d7
Merged at revision: bc10de9a5a8c0e07f101bf5664fda6d6b7c076df
Proposed branch: ~tcuthbert/charm-telegraf:master
Merge into: charm-telegraf:master
Diff against target: 35 lines (+10/-4)
2 files modified
src/reactive/telegraf.py (+1/-3)
src/tests/unit/test_telegraf.py (+9/-1)
Reviewer Review Type Date Requested Status
Xav Paice 2021-01-11 Approve on 2021-01-26
Peter Sabaini Approve on 2021-01-18
Haw Loeung +1 2021-01-11 Approve on 2021-01-14
Review via email: mp+396060@code.launchpad.net

Commit message

Workaround for LP#1910974: telegraf haproxy input broken with Juju >= 2.8.7

To post a comment you must log in.

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Junien Fridrick (axino) wrote :

Some questions inline

Haw Loeung (hloeung) wrote :

I think the best solution here is to update the haproxy[1] and content-cache[2] to pass through the listen address for haproxy statistics, they both already pass through the username, password, and port. Then have the telegraf charm check the presence of this and use it if it's something other than 0.0.0.0. If it doesn't exist, try work it out with the existing code (rel["private-address"] and hookenv.unit_private_ip()).

For the HAProxy charm, it defaults to statistics on 0.0.0.0 so why it doesn't appear broken with the latest Juju version - localhost or private-address, HAProxy statistics will answer on both interfaces. For the Content-cache charm, it configures statistics to listen on 127.0.0.1.

[1]https://bazaar.launchpad.net/~haproxy-team/charm-haproxy/trunk/view/head:/hooks/hooks.py#L1437
[2]https://git.launchpad.net/~hloeung/content-cache-charm/tree/reactive/content_cache.py#n483

Thomas Cuthbert (tcuthbert) wrote :

> I think the best solution here is to update the haproxy[1] and content-
> cache[2] to pass through the listen address for haproxy statistics, they both
> already pass through the username, password, and port. Then have the telegraf
> charm check the presence of this and use it if it's something other than
> 0.0.0.0. If it doesn't exist, try work it out with the existing code (rel
> ["private-address"] and hookenv.unit_private_ip()).
>
> For the HAProxy charm, it defaults to statistics on 0.0.0.0 so why it doesn't
> appear broken with the latest Juju version - localhost or private-address,
> HAProxy statistics will answer on both interfaces. For the Content-cache
> charm, it configures statistics to listen on 127.0.0.1.
>
> [1]https://bazaar.launchpad.net/~haproxy-team/charm-
> haproxy/trunk/view/head:/hooks/hooks.py#L1437
> [2]https://git.launchpad.net/~hloeung/content-cache-
> charm/tree/reactive/content_cache.py#n483

okay I've updated, see https://code.launchpad.net/~tcuthbert/content-cache-charm/+git/content-cache-charm/+merge/396233 also.

Haw Loeung (hloeung) :
Thomas Cuthbert (tcuthbert) wrote :

comment inline

Thomas Cuthbert (tcuthbert) wrote :

comment inline mk2

Haw Loeung (hloeung) :
Haw Loeung (hloeung) wrote :

LGTM, but let's see what BootStack reviewers say.

review: Approve (+1)
Peter Sabaini (peter-sabaini) wrote :

Approve lgtm

review: Approve
Xav Paice (xavpaice) wrote :

LGTM

review: Approve

Change successfully merged at revision bc10de9a5a8c0e07f101bf5664fda6d6b7c076df

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/reactive/telegraf.py b/src/reactive/telegraf.py
2index c0b9e0c..e106ee4 100644
3--- a/src/reactive/telegraf.py
4+++ b/src/reactive/telegraf.py
5@@ -910,9 +910,7 @@ def haproxy_input(haproxy):
6 enabled = False
7 if not enabled:
8 continue
9- addr = rel["private-address"]
10- if addr == hookenv.unit_private_ip():
11- addr = "localhost"
12+ addr = rel.get("listener-address", rel["private-address"])
13 port = rel["port"]
14 user = rel["user"]
15 password = rel.get("password", None)
16diff --git a/src/tests/unit/test_telegraf.py b/src/tests/unit/test_telegraf.py
17index afc3625..2cfbdc8 100644
18--- a/src/tests/unit/test_telegraf.py
19+++ b/src/tests/unit/test_telegraf.py
20@@ -879,7 +879,15 @@ def test_haproxy_input(monkeypatch, config):
21 telegraf.haproxy_input("test")
22 expected = """
23 [[inputs.haproxy]]
24- servers = ["http://foo:bar@localhost:1234"]
25+ servers = ["http://foo:bar@1.2.3.4:1234"]
26+"""
27+ assert configs_dir().join("haproxy.conf").read().strip() == expected.strip()
28+
29+ relations[0]["listener-address"] = "127.0.0.1"
30+ telegraf.haproxy_input("test")
31+ expected = """
32+[[inputs.haproxy]]
33+ servers = ["http://foo:bar@127.0.0.1:1234"]
34 """
35 assert configs_dir().join("haproxy.conf").read().strip() == expected.strip()
36

Subscribers

People subscribed via source and target branches