Merge ~aluria/charm-telegraf:remote-unit-name into ~telegraf-charmers/charm-telegraf:master

Proposed by Alvaro Uria
Status: Merged
Approved by: Alvaro Uria
Approved revision: 2920829b8dc4671c0685c86d575a639833ec699c
Merged at revision: 9bdf8979ff4edda16f34fc293e65bf65091f71e1
Proposed branch: ~aluria/charm-telegraf:remote-unit-name
Merge into: ~telegraf-charmers/charm-telegraf:master
Diff against target: 21 lines (+1/-9)
1 file modified
reactive/telegraf.py (+1/-9)
Reviewer Review Type Date Requested Status
Peter Sabaini (community) Approve
James Hebden (community) Needs Information
Review via email: mp+336901@code.launchpad.net
To post a comment you must log in.
Revision history for this message
James Hebden (ec0) wrote :

The reason for added the code you are removing was that when deployed to units which have multiple networks, we had observed the returned private address may not be routable between the telegraf unit and the prometheus server is was related to as a target. The below code fixed that by being sympathetic to the primary network address of the space the relation was bound to, rather than using the units private IP address.

This is the 4th case which we also need to test against - when the prometheus target relation is bound to a different space in order to gain access to a server only accessible on a network other than the unit's detected private address - for example when deployed alongside a Ceph monitor where the storage access network may be picked up for the private-address.

review: Needs Information
Revision history for this message
Alvaro Uria (aluria) wrote :

I understand this kind of check you mention is needed to send telegraf unit hostname (the target) to its relation (ie. prometheus) -- but this MP only affects which unit_name telegraf is related to (and each telegraf unit can only be related to a single unit, so I think there's no need of further IP matching verifications).

The issue I hit can be seen at https://pastebin.canonical.com/208764/

The IP verification to set telegraf's "hostname" key is managed in another part of the code:
https://git.launchpad.net/telegraf-charm/tree/reactive/telegraf.py#n652

Revision history for this message
Peter Sabaini (peter-sabaini) wrote :

+1 after Alvaros' persuasive argument :-)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/reactive/telegraf.py b/reactive/telegraf.py
index 7521b97..f8d0b27 100644
--- a/reactive/telegraf.py
+++ b/reactive/telegraf.py
@@ -111,15 +111,7 @@ def get_remote_unit_name():
111 rels = hookenv.relations_of_type(rel_type)111 rels = hookenv.relations_of_type(rel_type)
112 if rels and len(rels) >= 1:112 if rels and len(rels) >= 1:
113 rel = rels[0]113 rel = rels[0]
114 try:114 return rel['__unit__']
115 ip_addr = hookenv.network_get_primary_address('prometheus-client')
116 except NotImplementedError:
117 ip_addr = hookenv.unit_private_ip()
118 if rel['private-address'] == ip_addr:
119 return rel['__unit__']
120
121 if rel['private-address'] == hookenv.unit_private_ip():
122 return rel['__unit__']
123115
124116
125def render_base_inputs():117def render_base_inputs():

Subscribers

People subscribed via source and target branches

to all changes: