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

Proposed by Thomas Cuthbert
Status: Merged
Approved by: Xav Paice
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 (community) Approve
Peter Sabaini (community) Approve
Haw Loeung +1 Approve
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.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

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

Revision history for this message
Junien F (axino) wrote :

Some questions inline

Revision history for this message
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

Revision history for this message
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.

Revision history for this message
Haw Loeung (hloeung) :
Revision history for this message
Thomas Cuthbert (tcuthbert) wrote :

comment inline

Revision history for this message
Thomas Cuthbert (tcuthbert) wrote :

comment inline mk2

Revision history for this message
Haw Loeung (hloeung) :
Revision history for this message
Haw Loeung (hloeung) wrote :

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

review: Approve (+1)
Revision history for this message
Peter Sabaini (peter-sabaini) wrote :

Approve lgtm

review: Approve
Revision history for this message
Xav Paice (xavpaice) wrote :

LGTM

review: Approve
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

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

to all changes: