canonical / loki-k8s-operator

This charmed operator automates the operational procedures of ryunning Grafana Loki, an open-source logs backend, in monolithic mode
https://charmhub.io/loki-k8s
Apache License 2.0
10 stars 15 forks source link

The LogForwarder should put charm to block state when Juju < 3.4 #410

Open rgildein opened 1 month ago

rgildein commented 1 month ago

Bug Description

If the charm is deployed on <Juju 3.4, LogForwarder only provides a warning log message that Juju 3.4 is required, however this behavior is misleading, and I believed a better solution would be to put the charm into a block state until the relation is removed. The charm works fine with this relation, but the user can easily assume that all logs are going to Loki until something goes wrong and the service needs to be debugged.

To Reproduce

$ juju deploy grafana-agent-k8s
$ juju relate <my-charm>:logging grafana-agent-k8s

# e.g.
$ juju status
Model         Controller          Cloud/Region        Version  SLA          Timestamp
test-version  microk8s-localhost  microk8s/localhost  3.1.8    unsupported  15:58:25Z

App                Version                  Status   Scale  Charm              Channel         Rev  Address         Exposed  Message
grafana-agent-k8s  0.35.2                   waiting      1  grafana-agent-k8s  stable           64  10.152.183.69   no       installing agent
mlflow-minio       res:oci-image@1755999    active       1  minio              ckf-1.7/stable  214  10.152.183.122  no       
mlflow-mysql       8.0.35-0ubuntu0.22.04.1  active       1  mysql-k8s          8.0/stable      127  10.152.183.227  no       
mlflow-server                               active       1  mlflow-server                        0  10.152.183.39   no       

Unit                  Workload  Agent  Address      Ports          Message
grafana-agent-k8s/0*  blocked   idle   10.1.216.14                 grafana-cloud-config: off, logging-consumer: off
mlflow-minio/0*       active    idle   10.1.216.16  9000-9001/TCP  
mlflow-mysql/0*       active    idle   10.1.216.11                 Primary
mlflow-server/0*      active    idle   10.1.216.17       

Environment

$ microk8s version
MicroK8s v1.25.16 revision 6575
$ juju version
3.1.8-genericlinux-amd64

Relevant log output

unit-mlflow-server-0: 15:52:34 INFO juju.worker.uniter.operation ran "logging-relation-created" hook (via hook dispatching script: dispatch)
unit-mlflow-server-0: 15:52:35 WARNING unit.mlflow-server/0.juju-log logging:6: Juju version 3.1.8 does not support Pebble log forwarding. Juju >= 3.4 is needed.
unit-mlflow-server-0: 15:52:35 WARNING unit.mlflow-server/0.juju-log logging:6: 2 containers are present in metadata.yaml and refresh_event was not specified. Defaulting to update_status. Metrics IP may not be set in a timely fashion.
unit-mlflow-server-0: 15:52:35 WARNING unit.mlflow-server/0.juju-log logging:6: The relation 'logging' is not ready yet.
unit-mlflow-server-0: 15:52:35 WARNING unit.mlflow-server/0.juju-log logging:6: No Loki endpoints available

Additional context

No response

sed-i commented 2 weeks ago

Essentially this is a communication challenge between the lib and the charm, because we don't have a decentralized status setting mechanism (yet). We have a convention in the team that charm libs do not touch charm status, because they don't have the full context.

We could emit a custom event or add a get_status method (to be used in a charm's collect-unit-status), but in either case the charm author would need to read the docs to become aware of the details.

(We don't want an exception to avoid breaking the charm on upgrade).

So it seems like charm code should check for juju version itself. Leaving the issue open to add a code example in the module docstring for this.

Related: