Hi, there are a few things to be reviewed:
* Makefile does not have a unittest target (probably because unit tests didn't exist). However, tox.ini does have a "unit" env. I think the Makefile misses the target, running "tox -e unit"
* Functional tests miss nagios-{{series}} and nrpe-{{series}} in the overlay.yaml.j2 template. It would be good to test when grafana gets related to nrpe, by checking that the check_grafana_http.cfg file exists in /etc/nagios/nrpe.d. There are examples of such check (file_stat) in ie. charm-openstack-service-checks
* Functional tests don't test a deployment of cs:grafana + juju upgrade-charm to the local version. I spotted a bug that can be seen at [1]. "nrpe-external-master.available" state gets removed and wipe_nrpe_checks is called. Looking at the debug lines, the http interface checks endpoint.website.joined state and removes website.available. I don't see any trace of calling the nrpe-external-master interface and do a similar thing (which OTOH only occurs when n-e-m-relation-{departed,broken} hooks are triggered - and debug lines don't show such run).
On this 3rd test (upgrade-charm) it would be could to verify that the old checks (check_grafana-server.cfg [apt] or check_snap.grafana.grafana.cfg [snap]) are removed (backward compatibility with a previous deployed environment). On fresh deploys, such checks won't exist, only check_grafana_http.cfg.
Hi, there are a few things to be reviewed:
* Makefile does not have a unittest target (probably because unit tests didn't exist). However, tox.ini does have a "unit" env. I think the Makefile misses the target, running "tox -e unit"
* Functional tests miss nagios-{{series}} and nrpe-{{series}} in the overlay.yaml.j2 template. It would be good to test when grafana gets related to nrpe, by checking that the check_grafana_ http.cfg file exists in /etc/nagios/nrpe.d. There are examples of such check (file_stat) in ie. charm-openstack -service- checks
* Functional tests don't test a deployment of cs:grafana + juju upgrade-charm to the local version. I spotted a bug that can be seen at [1]. "nrpe-external- master. available" state gets removed and wipe_nrpe_checks is called. Looking at the debug lines, the http interface checks endpoint. website. joined state and removes website.available. I don't see any trace of calling the nrpe-external- master interface and do a similar thing (which OTOH only occurs when n-e-m-relation- {departed, broken} hooks are triggered - and debug lines don't show such run).
On this 3rd test (upgrade-charm) it would be could to verify that the old checks (check_ grafana- server. cfg [apt] or check_snap. grafana. grafana. cfg [snap]) are removed (backward compatibility with a previous deployed environment). On fresh deploys, such checks won't exist, only check_grafana_ http.cfg.
1. https:/ /pastebin. ubuntu. com/p/2smQt4X3W d/