Merge ~pjdc/prometheus-mongodb-exporter-charm/+git/prometheus-mongodb-exporter-charm:no-needless-blocked into prometheus-mongodb-exporter-charm:master

Proposed by Paul Collins
Status: Rejected
Rejected by: Haw Loeung
Proposed branch: ~pjdc/prometheus-mongodb-exporter-charm/+git/prometheus-mongodb-exporter-charm:no-needless-blocked
Merge into: prometheus-mongodb-exporter-charm:master
Diff against target: 57 lines (+21/-4)
1 file modified
reactive/prometheus_mongodb_exporter.py (+21/-4)
Reviewer Review Type Date Requested Status
Stuart Bishop (community) Needs Fixing
Canonical IS Reviewers Pending
Review via email: mp+372231@code.launchpad.net

Commit message

ensure installation happens immediately after setting mongodb_uri to juju-db://

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
Stuart Bishop (stub) wrote :

The maybe_block_on_mongodb_uri() handler may end up running before the is_mongodb_uri_usable() handler, causing a spurious blocked state.

Instead, I'd just do:

@when('config.default.mongodb_uri') # default is empty string
def block_on_no_uri():
    blocked('mongodb_uri must be set')

@when('config.changed.mongodb_uri')
def local_or_remote():
    l = mongodb_uri_is_juju_db()
    reactive.toggle_flag('prometheus-mongodb-exporter.monitor-juju-db', l)
    reactive.toggle_flag('prometheus-mongodb-exporter.monitor-remote-db', not l)

@when('prometheus-mongodb-exporter.monitor-juju-db')
@when_not('leadership.is_leader', 'leadership.set.mongodb_uri')
def wait_on_leader():
    waiting('Waiting for leader to provide mongodb credentials')

@when('prometheus-mongodb-exporter.monitor-juju-db')
@when('leadership.is_leader')
@when_not('leadership.set.mongodb_uri')
def create_juju_db_user():
    [ ... existing code ... ]

review: Needs Fixing

Unmerged commits

681810e... by Paul Collins

ensure installation happens immediately after setting mongodb_uri to juju-db://

Previously, install_prometheus_mongodb_exporter would bail when
the config item was "juju-db://" but the leader setting was empty,
because create_juju_db_user had not yet been invoked.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/reactive/prometheus_mongodb_exporter.py b/reactive/prometheus_mongodb_exporter.py
index ae2d3d3..ec32635 100644
--- a/reactive/prometheus_mongodb_exporter.py
+++ b/reactive/prometheus_mongodb_exporter.py
@@ -81,6 +81,25 @@ def maybe_monitor_juju_db():
81 remove_state('prometheus-mongodb-exporter.monitor-juju-db')81 remove_state('prometheus-mongodb-exporter.monitor-juju-db')
8282
8383
84@when_any('config.changed.mongodb_uri', 'leadership.changed.mongodb_uri')
85def is_mongodb_uri_usable():
86 if get_mongodb_uri():
87 set_state('prometheus-mongodb-exporter.mongodb_uri.usable')
88 else:
89 remove_state('prometheus-mongodb-exporter.mongodb_uri.usable')
90
91
92@when_not('prometheus-mongodb-exporter.mongodb_uri.usable')
93def maybe_block_on_mongodb_uri():
94 if get_mongodb_uri():
95 return
96 if mongodb_uri_is_juju_db():
97 # The config item is "juju-db://" but the leader setting is empty,
98 # which create_juju_db_user will fix shortly; let it have its chance.
99 return
100 blocked('mongodb_uri must be set')
101
102
84@when('prometheus-mongodb-exporter.monitor-juju-db')103@when('prometheus-mongodb-exporter.monitor-juju-db')
85@when('leadership.is_leader')104@when('leadership.is_leader')
86@when_not('leadership.set.mongodb_uri')105@when_not('leadership.set.mongodb_uri')
@@ -121,6 +140,7 @@ def create_juju_db_user():
121 leader_set(mongodb_uri='mongodb://{}:{}@localhost:37017/admin'.format(username, password))140 leader_set(mongodb_uri='mongodb://{}:{}@localhost:37017/admin'.format(username, password))
122141
123142
143@when('prometheus-mongodb-exporter.mongodb_uri.usable')
124@when_not('prometheus-mongodb-exporter.installed')144@when_not('prometheus-mongodb-exporter.installed')
125def install_prometheus_mongodb_exporter():145def install_prometheus_mongodb_exporter():
126 ''' Install the proxy application '''146 ''' Install the proxy application '''
@@ -130,10 +150,6 @@ def install_prometheus_mongodb_exporter():
130150
131 snap.install('prometheus-mongodb-exporter')151 snap.install('prometheus-mongodb-exporter')
132152
133 if not uri:
134 blocked('mongodb_uri must be set')
135 return
136
137 # Create a systemd unit file, and open the port.153 # Create a systemd unit file, and open the port.
138 port = hookenv.config()['port']154 port = hookenv.config()['port']
139 if not sensible_port(port):155 if not sensible_port(port):
@@ -168,6 +184,7 @@ def update_opened_port():
168 hookenv.open_port(new_port)184 hookenv.open_port(new_port)
169185
170186
187@when('prometheus-mongodb-exporter.mongodb_uri.usable')
171@when('prometheus-mongodb-exporter.installed')188@when('prometheus-mongodb-exporter.installed')
172@when_any('config.changed', 'leadership.changed')189@when_any('config.changed', 'leadership.changed')
173def update_daemon_args():190def update_daemon_args():

Subscribers

People subscribed via source and target branches