search

canonical / kafka-operator

Kafka VM operator
Apache License 2.0
6 stars 12 forks source link

[DPE-3333] Add integration test for broken tls #188

Closed Batalex closed 3 months ago

deusebio commented 3 months ago

Changes look fine, but can we also add the integration tests? It seems pretty straightforward that - if we force update-status events to not be triggered for say 20m - we make sure that the truststore/keystore are updated as expected (that's the meaning of this test, that we would like the certificate revocation to be done straight away, right?)

We already have the test_tls that add/removes relations. Adding this check should be pretty easy, I believe

Batalex commented 3 months ago

This will trigger a config_changed event across the charm wich will take care of restarting, because it will find that properties are no longer expected.

Hum, I think you are right. At least that would explain why I had so much trouble testing my changes, because _on_config_changed would take care of restarting the server anyway.

zmraul commented 3 months ago

What I'm thinking is that modifying the peer databag (from tls: "enabled" to tls: "") triggers _on_config_changed on all units except the one that did the change, in this case the leader unit. So the bug might be related to this condition specifically, where other units restart with correct properties, yet there is one that lags behind until update_status or something else triggers. I didn't look into the rabbit hole, but it would be something worth checking.