Merge ~rgildein/charm-telegraf:bug1988312 into charm-telegraf:master

[Robert Gildein]

fix when case for configure_prometheus_client

Change when case for configure_prometheus_client to `@when("telegraf.installed")`.

fixes: https://bugs.launchpad.net/charm-telegraf/+bug/1988312
lint and unit tests: https://pastebin.ubuntu.com/p/98jbqWWNVM/
functional tests: https://pastebin.ubuntu.com/p/RxGvQbJWDB/

[Robert Gildein (rgildein)]

Functional tests are failing for `bionic-compute` and `focal-compute`
bundle due issue with sudoers.

```bash
# from unit
ubuntu@juju-5ead5f-zaza-12d45d4743f5-8:~$ sudo su
>>> /etc/sudoers.d/telegraf_ovs: syntax error near line 10 <<<
sudo: parse error in /etc/sudoers.d/telegraf_ovs near line 10
sudo: no valid sudoers sources found, quitting
sudo: unable to initialize policy plugin
```

Not sure if this is related, but it worked fine for `jammy-compute`.

[Robert Gildein (rgildein)]

Result from func, lint and unit tests: https://pastebin.ubuntu.com/p/pZXNcBt7F9/

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

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

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:8d2bd5d08b48d402a68927cc5a28b9b20cc7140a
https://jenkins.canonical.com/bootstack/job/lp-charm-telegraf-ci/146/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/877/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1298/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-telegraf-ci/146//rebuild

review: Approve (continuous-integration)

[Eric Chen (eric-chen)]

Do we cover the issue in function test? or unit test ?
https://bugs.launchpad.net/charm-telegraf/+bug/1988312
It's a critical bug that we should avoid it.

review: Needs Information

[Robert Gildein (rgildein)]

This is covered in unit and functional tests.
Unit and functional tests were broken after the MP [1].

The changes from MP [1] was reverted by MP [2] and I
adding them back but improved.

However, I am not sure why and if we need this, as I have
not been able to reproduce the issue described and fixed
by Haw Loeung in MP [1].

---
[1]: https://code.launchpad.net/~hloeung/charm-telegraf/+git/charm-telegraf/+merge/428004
[2]: https://code.launchpad.net/~jneo8/charm-telegraf/+git/charm-telegraf/+merge/429347

[Eric Chen (eric-chen)]

I hope we were talking about the same thing.
I would like to know can we dectect the error caused by [1] in unit or func test?

The simplist way to approve it: apply the patch in [1] and run unit/func again. It should failed

If not, it seems we need to add this basic environment used in [2].

series: jammy
applications:
  ubuntu:
    scale: 1
    charm: ch:ubuntu
  telegraf:
    charm: ch:telegraf
    channel: candidate
relations:
  - ["ubuntu", "telegraf"]

--
[1]: https://code.launchpad.net/~hloeung/charm-telegraf/+git/charm-telegraf/+merge/428004
[2]: https://bugs.launchpad.net/charm-telegraf/+bug/1988312

[Robert Gildein (rgildein)]

> I hope we were talking about the same thing.
> I would like to know can we dectect the error caused by [1] in unit or func
> test?
>
> The simplist way to approve it: apply the patch in [1] and run unit/func
> again. It should failed
>
> If not, it seems we need to add this basic environment used in [2].
>
> series: jammy
> applications:
> ubuntu:
> scale: 1
> charm: ch:ubuntu
> telegraf:
> charm: ch:telegraf
> channel: candidate
> relations:
> - ["ubuntu", "telegraf"]
>
> --
> [1]: https://code.launchpad.net/~hloeung/charm-telegraf/+git/charm-
> telegraf/+merge/428004
> [2]: https://bugs.launchpad.net/charm-telegraf/+bug/1988312

I have talked about the fact that I don't know at all why the MP [1] was
opened at all and therefore whether this is a necessary change at all.
I mentioned here [2], that I was unable to reproduce the error
described in MP [1].

I think it is covered by the unit tests because after [1] the unit test were
failing [3]. At the same time, the functional tests did not work earthed,
thanks to the [4] focal-snap.yaml bundle, which is very similar to the one
in [5].

To be sure, I once again tested the lint, unit and func tests on the
commit from MP [1]. Here are the results:
- lint and unit tests: https://pastebin.ubuntu.com/p/mhFg2HC6Zk/
- func tests: https://pastebin.ubuntu.com/p/p2MrX2YT4W/

To summarized this, I think that changing from
`@when("telegraf.configured")` to `@when("telegraf.installed")` is
fixing both [3] and [5], but I'm not sure if this fixed also issue
mention in [1], since I was not able to reproduced it.

---
[1]: https://code.launchpad.net/~hloeung/charm-telegraf/+git/charm-telegraf/+merge/428004
[2]: https://bugs.launchpad.net/charm-telegraf/+bug/1988312/comments/1
[3]: https://bugs.launchpad.net/charm-telegraf/+bug/1987542
[4]: https://git.launchpad.net/charm-telegraf/tree/src/tests/functional/tests/bundles/base-snap.yaml
[5]: https://bugs.launchpad.net/charm-telegraf/+bug/1988312

[Eric Chen (eric-chen)]

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 14575b2671384edaae01e2e7e68d67a518ce3529

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:8d2bd5d08b48d402a68927cc5a28b9b20cc7140a
https://jenkins.canonical.com/bootstack/job/lp-charm-telegraf-ci/151/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/885/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1305/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-telegraf-ci/151//rebuild

review: Needs Fixing (continuous-integration)

diff --git a/src/reactive/telegraf.py b/src/reactive/telegraf.py
index bab0236..abbb728 100644
--- a/src/reactive/telegraf.py
+++ b/src/reactive/telegraf.py
@@ -1713,6 +1713,7 @@ def configure_prometheus_client_with_relation(prometheus):
     clear_flag("endpoint.prometheus-client.changed")  # not automatic
 
 
+@when("telegraf.installed")
 @when_not("plugins.prometheus-client.configured")
 def configure_prometheus_client():
     hookenv.log("Configuring prometheus_client output plugin", level=hookenv.DEBUG)