From 4ea658540ec83d3b0cd7ac894d695783f9f8818c Mon Sep 17 00:00:00 2001
From: Frode Nordahl <frode.nordahl@canonical.com>
Date: Wed, 24 Jun 2020 13:59:54 +0200
Subject: [PATCH] Improve messaging for required relations

At present the `certificates` relation will report as missing
until Vault is equipped with a root CA to issue certificates.

The `ovsdb-peer` relation is also not listed as required even
though peers are required for the charm to operate.

Change-Id: Id1534170e5e42caf0d508005ca5baea2415164cc
Closes-Bug: #1884931
---
 src/lib/charm/openstack/ovn_central.py    | 53 ++++++++++++++++++++++-
 src/tests/tests.yaml                      |  4 +-
 unit_tests/test_lib_charms_ovn_central.py | 23 +++++++++-
 3 files changed, 76 insertions(+), 4 deletions(-)

diff --git a/src/lib/charm/openstack/ovn_central.py b/src/lib/charm/openstack/ovn_central.py
index f93c0a8..6efc11f 100644
--- a/src/lib/charm/openstack/ovn_central.py
+++ b/src/lib/charm/openstack/ovn_central.py
@@ -29,6 +29,10 @@ import charms_openstack.charm
 charms_openstack.charm.use_defaults('charm.default-select-release')
 
 
+PEER_RELATION = 'ovsdb-peer'
+CERT_RELATION = 'certificates'
+
+
 # NOTE(fnordahl): We should split the ``OVNConfigurationAdapter`` in
 # ``layer-ovn`` into common and chassis specific parts so we can re-use the
 # common parts here.
@@ -63,7 +67,7 @@ class BaseOVNCentralCharm(charms_openstack.charm.OpenStackCharm):
     services = ['ovn-central']
     release_pkg = 'ovn-central'
     configuration_class = OVNCentralConfigurationAdapter
-    required_relations = ['certificates']
+    required_relations = [PEER_RELATION, CERT_RELATION]
     python_version = 3
     source_config_key = 'source'
 
@@ -100,6 +104,53 @@ class BaseOVNCentralCharm(charms_openstack.charm.OpenStackCharm):
         self.configure_source()
         super().install()
 
+    def states_to_check(self, required_relations=None):
+        """Override upstream method to add custom messaging.
+
+        Note that this method will only override the messaging for certain
+        relations, any relations we don't know about will get the default
+        treatment from the parent method.
+
+        Please take a look at parent method for parameter and return type
+        declaration.
+        """
+        # Retrieve default state map
+        states_to_check = super().states_to_check(
+            required_relations=required_relations)
+
+        if not states_to_check:
+            return None, None
+
+        if PEER_RELATION in states_to_check:
+            # for peer relation we want default messaging for all states but
+            # connected.
+            states_to_check[PEER_RELATION] = [
+                ('{}.connected'.format(PEER_RELATION),
+                 'blocked',
+                 'Charm requires peers to operate, add more units. A minimum '
+                 'of 3 is required for HA')
+            ] + [
+                states for states in states_to_check[PEER_RELATION]
+                if 'connected' not in states[0]
+            ]
+
+        if CERT_RELATION in states_to_check:
+            # for certificates relation we want to replace all messaging
+            states_to_check[CERT_RELATION] = [
+                # certificates relation has no connected state
+                ('{}.available'.format(CERT_RELATION),
+                 'blocked',
+                 "'{}' missing".format(CERT_RELATION)),
+                # we cannot proceed until Vault have provided server
+                # certificates
+                ('{}.server.certs.available'.format(CERT_RELATION),
+                 'waiting',
+                 "'{}' awaiting server certificate data"
+                 .format(CERT_RELATION)),
+            ]
+
+        return states_to_check
+
     @staticmethod
     def ovn_sysconfdir():
         return '/etc/ovn'
diff --git a/src/tests/tests.yaml b/src/tests/tests.yaml
index 15df5f7..eed2511 100644
--- a/src/tests/tests.yaml
+++ b/src/tests/tests.yaml
@@ -10,8 +10,8 @@ dev_bundles:
 - focal
 target_deploy_status:
   ovn-central:
-    workload-status: blocked
-    workload-status-message: "'certificates' missing"
+    workload-status: waiting
+    workload-status-message: "'ovsdb-peer' incomplete, 'certificates' awaiting server certificate data"
   vault:
     workload-status: blocked
     workload-status-message: Vault needs to be initialized
diff --git a/unit_tests/test_lib_charms_ovn_central.py b/unit_tests/test_lib_charms_ovn_central.py
index d16a6bc..cf731f4 100644
--- a/unit_tests/test_lib_charms_ovn_central.py
+++ b/unit_tests/test_lib_charms_ovn_central.py
@@ -12,8 +12,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import collections
 import io
-import mock
+import unittest.mock as mock
 
 import charms_openstack.test_utils as test_utils
 
@@ -98,6 +99,26 @@ class TestOVNCentralCharm(Helper):
         self.install.assert_called_once_with()
         self.configure_source.assert_called_once_with()
 
+    def test_states_to_check(self):
+        self.maxDiff = None
+        expect = collections.OrderedDict([
+            ('ovsdb-peer', [
+                ('ovsdb-peer.connected',
+                 'blocked',
+                 'Charm requires peers to operate, add more units. A minimum '
+                 'of 3 is required for HA'),
+                ('ovsdb-peer.available',
+                 'waiting',
+                 "'ovsdb-peer' incomplete")]),
+            ('certificates', [
+                ('certificates.available', 'blocked',
+                 "'certificates' missing"),
+                ('certificates.server.certs.available',
+                 'waiting',
+                 "'certificates' awaiting server certificate data")]),
+        ])
+        self.assertDictEqual(self.target.states_to_check(), expect)
+
     def test__default_port_list(self):
         self.assertEquals(
             self.target._default_port_list(),