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
diff --git a/src/reactive/telegraf.py b/src/reactive/telegraf.py
index c0b9e0c..e106ee4 100644
--- a/src/reactive/telegraf.py
+++ b/src/reactive/telegraf.py
@@ -910,9 +910,7 @@ def haproxy_input(haproxy):
910 enabled = False910 enabled = False
911 if not enabled:911 if not enabled:
912 continue912 continue
913 addr = rel["private-address"]913 addr = rel.get("listener-address", rel["private-address"])
914 if addr == hookenv.unit_private_ip():
915 addr = "localhost"
916 port = rel["port"]914 port = rel["port"]
917 user = rel["user"]915 user = rel["user"]
918 password = rel.get("password", None)916 password = rel.get("password", None)
diff --git a/src/tests/unit/test_telegraf.py b/src/tests/unit/test_telegraf.py
index afc3625..2cfbdc8 100644
--- a/src/tests/unit/test_telegraf.py
+++ b/src/tests/unit/test_telegraf.py
@@ -879,7 +879,15 @@ def test_haproxy_input(monkeypatch, config):
879 telegraf.haproxy_input("test")879 telegraf.haproxy_input("test")
880 expected = """880 expected = """
881[[inputs.haproxy]]881[[inputs.haproxy]]
882 servers = ["http://foo:bar@localhost:1234"]882 servers = ["http://foo:bar@1.2.3.4:1234"]
883"""
884 assert configs_dir().join("haproxy.conf").read().strip() == expected.strip()
885
886 relations[0]["listener-address"] = "127.0.0.1"
887 telegraf.haproxy_input("test")
888 expected = """
889[[inputs.haproxy]]
890 servers = ["http://foo:bar@127.0.0.1:1234"]
883"""891"""
884 assert configs_dir().join("haproxy.conf").read().strip() == expected.strip()892 assert configs_dir().join("haproxy.conf").read().strip() == expected.strip()
885893

Subscribers

People subscribed via source and target branches