Fix checks processing for multiple connectors

servo/checks.py

@@ -3,6 +3,7 @@
 import functools
 import hashlib
 import inspect
+import re
 import sys
 import types
 from typing import (
@@ -182,6 +183,11 @@ async def run(
         await run_check_handler(check, handler, *args, **kwargs)
         return check
 
+    @property
+    def escaped_name(self) -> str:
+        """Return check name compatible with calls to str.format"""
+        return re.sub(r"\{(.*?)\}", r"{{\1}}", self.name)
+
     @property
     def passed(self) -> bool:
         """Return a boolean value that Indicates if the check passed.

servo/cli.py

@@ -1312,7 +1312,7 @@ async def check_servo(servo_: servo.Servo) -> bool:
 
                 passing = set()
                 progress = servo.DurationProgress(servo.Duration(wait or 0))
-                ready = True
+                ready = False
                 while not progress.finished:
                     if not progress.started:
                         # run at least one time
@@ -1328,14 +1328,15 @@ async def check_servo(servo_: servo.Servo) -> bool:
                     ) or []
 
                     if progressive:
-                        if result := next(iter(results), None):
-                            checks: List[servo.Check] = result.value
+                        if results:
+                            checks: List[servo.Check] = functools.reduce(lambda a, b: a + b.value, results, [])
                             failure = None
                             for check in checks:
                                 if check.success:
                                     # FIXME: This should hold Check objects but hashing isn't matching
                                     if check.id not in passing:
-                                        servo.logger.success(f"✅ Check '{check.name}' passed", component=check.id)
+                                        # calling loguru with kwargs (component) triggers a str.format call which trips up on names with single curly braces
+                                        servo.logger.success(f"✅ Check '{check.escaped_name}' passed", component=check.id)
                                         passing.add(check.id)
                                 else:
                                     failure = check
@@ -1381,18 +1382,21 @@ async def fn() -> None:
                             typer.echo(f"WARNING: No checks found -- returning.")
                     else:
                         table = []
+                        # Don't return ready if no checks ran but initial value for this var must be true for subsequent "&=" ops to work
+                        and_checks_passed = bool(results)
                         if verbose:
                             headers = ["CONNECTOR", "CHECK", "ID", "TAGS", "STATUS", "MESSAGE"]
                             for result in results:
                                 checks: List[servo.Check] = result.value
                                 names, ids, tags, statuses, comments = [], [], [], [], []
+                                and_checks_passed &= bool(checks) # set ready False if connector responds with empty list
                                 for check in checks:
                                     names.append(check.name)
                                     ids.append(check.id)
                                     tags.append(", ".join(check.tags) if check.tags else "-")
                                     statuses.append(_check_status_to_str(check))
                                     comments.append(textwrap.shorten(check.message or "-", 70))
-                                    ready &= check.success
+                                    and_checks_passed &= check.success
 
                                 if not names:
                                     continue
@@ -1413,14 +1417,14 @@ async def fn() -> None:
                                 if not checks:
                                     continue
 
-                                success = True
+                                success = bool(checks) # Don't return ready on empty lists of checks
                                 errors = []
                                 for check in checks:
                                     success &= check.passed
                                     check.success or errors.append(
                                         f"{check.name}: {textwrap.wrap(check.message or '-')}"
                                     )
-                                ready &= success
+                                and_checks_passed &= success
                                 status = "√ PASSED" if success else "X FAILED"
                                 message = functools.reduce(
                                     lambda m, e: m
@@ -1431,6 +1435,7 @@ async def fn() -> None:
                                 row = [result.connector.name, status, message]
                                 table.append(row)
 
+                        ready = and_checks_passed
                         # Output table
                         if not quiet:
                             typer.echo(tabulate(table, headers, tablefmt="plain"))

servo/connectors/kubernetes.py

@@ -4377,13 +4377,13 @@ class KubernetesChecks(servo.BaseChecks):
     config: KubernetesConfiguration
 
     @servo.require("Connectivity to Kubernetes")
-    async def check_connectivity(self) -> None:
+    async def check_kubernetes_connectivity(self) -> None:
         async with kubernetes_asyncio.client.api_client.ApiClient() as api:
             v1 =kubernetes_asyncio.client.VersionApi(api)
             await v1.get_code()
 
     @servo.warn("Kubernetes version")
-    async def check_version(self) -> None:
+    async def check_kubernetes_version(self) -> None:
         async with kubernetes_asyncio.client.api_client.ApiClient() as api:
             v1 =kubernetes_asyncio.client.VersionApi(api)
             version = await v1.get_code()
@@ -4392,7 +4392,7 @@ async def check_version(self) -> None:
             assert int(int("".join(c for c in version.minor if c.isdigit()))) >= 16
 
     @servo.require("Required permissions")
-    async def check_permissions(self) -> None:
+    async def check_kubernetes_permissions(self) -> None:
         async with kubernetes_asyncio.client.api_client.ApiClient() as api:
             v1 = kubernetes_asyncio.client.AuthorizationV1Api(api)
             required_permissions = self.config.permissions
@@ -4420,18 +4420,18 @@ async def check_permissions(self) -> None:
                         ), f'Not allowed to "{verb}" resource "{resource}"'
 
     @servo.require('Namespace "{self.config.namespace}" is readable')
-    async def check_namespace(self) -> None:
+    async def check_kubernetes_namespace(self) -> None:
         await Namespace.read(self.config.namespace)
 
     @servo.multicheck('Deployment "{item.name}" is readable')
-    async def check_deployments(self) -> Tuple[Iterable, servo.CheckHandler]:
+    async def check_kubernetes_deployments(self) -> Tuple[Iterable, servo.CheckHandler]:
         async def check_dep(dep_config: DeploymentConfiguration) -> None:
             await Deployment.read(dep_config.name, dep_config.namespace)
 
         return (self.config.deployments or []), check_dep
 
     @servo.multicheck('Rollout "{item.name}" is readable')
-    async def check_rollouts(self) -> Tuple[Iterable, servo.CheckHandler]:
+    async def check_kubernetes_rollouts(self) -> Tuple[Iterable, servo.CheckHandler]:
         async def check_rol(rol_config: RolloutConfiguration) -> None:
             await Rollout.read(rol_config.name, rol_config.namespace)
 
@@ -4461,7 +4461,7 @@ async def _check_container_resource_requirements(
                 )
 
     @servo.multicheck('Containers in the "{item.name}" Deployment have resource requirements')
-    async def check_resource_requirements(self) -> Tuple[Iterable, servo.CheckHandler]:
+    async def check_kubernetes_resource_requirements(self) -> Tuple[Iterable, servo.CheckHandler]:
         async def check_dep_resource_requirements(
             dep_config: DeploymentConfiguration,
         ) -> None:
@@ -4472,7 +4472,7 @@ async def check_dep_resource_requirements(
 
 
     @servo.multicheck('Containers in the "{item.name}" Rollout have resource requirements')
-    async def check_rollout_resource_requirements(self) -> Tuple[Iterable, servo.CheckHandler]:
+    async def check_kubernetes_rollout_resource_requirements(self) -> Tuple[Iterable, servo.CheckHandler]:
         async def check_rol_resource_requirements(
             rol_config: RolloutConfiguration,
         ) -> None:
@@ -4483,7 +4483,7 @@ async def check_rol_resource_requirements(
 
 
     @servo.multicheck('Deployment "{item.name}" is ready')
-    async def check_deployments_are_ready(self) -> Tuple[Iterable, servo.CheckHandler]:
+    async def check_kubernetes_deployments_are_ready(self) -> Tuple[Iterable, servo.CheckHandler]:
         async def check_deployment(dep_config: DeploymentConfiguration) -> None:
             deployment = await Deployment.read(dep_config.name, dep_config.namespace)
             if not await deployment.is_ready():
@@ -4492,7 +4492,7 @@ async def check_deployment(dep_config: DeploymentConfiguration) -> None:
         return (self.config.deployments or []), check_deployment
 
     @servo.multicheck('Rollout "{item.name}" is ready')
-    async def check_rollouts_are_ready(self) -> Tuple[Iterable, servo.CheckHandler]:
+    async def check_kubernetes_rollouts_are_ready(self) -> Tuple[Iterable, servo.CheckHandler]:
         async def check_rollout(rol_config: RolloutConfiguration) -> None:
             rollout = await Rollout.read(rol_config.name, rol_config.namespace)
             if not await rollout.is_ready():

servo/connectors/opsani_dev.py

@@ -311,15 +311,15 @@ async def check_permissions(self) -> None:
                         ), f'Not allowed to "{verb}" resource "{resource}"'
 
     @servo.checks.require('Namespace "{self.config.namespace}" is readable')
-    async def check_kubernetes_namespace(self) -> None:
+    async def check_opsani_dev_kubernetes_namespace(self) -> None:
         await servo.connectors.kubernetes.Namespace.read(self.config.namespace)
 
     @servo.checks.require('{self.controller_type_name} "{self.config_controller_name}" is readable')
-    async def check_kubernetes_controller(self) -> None:
+    async def check_opsani_dev_kubernetes_controller(self) -> None:
         await self.controller_class.read(self.config_controller_name, self.config.namespace)
 
     @servo.checks.require('Container "{self.config.container}" is readable')
-    async def check_kubernetes_container(self) -> None:
+    async def check_opsani_dev_kubernetes_container(self) -> None:
         controller = await self.controller_class.read(self.config_controller_name, self.config.namespace)
         container = controller.find_container(self.config.container)
         assert (
@@ -399,11 +399,11 @@ async def check_controller_readiness(self) -> None:
             raise RuntimeError(f'{self.controller_type_name} "{controller.name}" is not ready')
 
     @servo.checks.require("service")
-    async def check_kubernetes_service(self) -> None:
+    async def check_opsani_dev_kubernetes_service(self) -> None:
         await servo.connectors.kubernetes.Service.read(self.config.service, self.config.namespace)
 
     @servo.checks.warn("service type")
-    async def check_kubernetes_service_type(self) -> None:
+    async def check_opsani_dev_kubernetes_service_type(self) -> None:
         service = await servo.connectors.kubernetes.Service.read(
             self.config.service, self.config.namespace
         )
@@ -414,7 +414,7 @@ async def check_kubernetes_service_type(self) -> None:
             )
 
     @servo.checks.check("service port")
-    async def check_kubernetes_service_port(self) -> None:
+    async def check_opsani_dev_kubernetes_service_port(self) -> None:
         service = await servo.connectors.kubernetes.Service.read(
             self.config.service, self.config.namespace
         )

tests/connectors/kubernetes_test.py

@@ -1475,7 +1475,7 @@ async def test_check_deployment_readiness_failure(self, config: KubernetesConfig
         while target_deploy.is_ready():
             await asyncio.sleep(0.1)
 
-        result = await KubernetesChecks(config).run_one(id="check_deployments_are_ready_item_0")
+        result = await KubernetesChecks(config).run_one(id="check_kubernetes_deployments_are_ready_item_0")
         assert result.success == False and result.message == "caught exception (RuntimeError): Deployment \"fiber-http\" is not ready"
 
 

tests/connectors/opsani_dev_test.py

@@ -170,7 +170,7 @@ async def load_manifests(
         async def test_resource_exists(
             self, resource: str, checks: servo.connectors.opsani_dev.OpsaniDevChecks
         ) -> None:
-            result = await checks.run_one(id=f"check_kubernetes_{resource}")
+            result = await checks.run_one(id=f"check_opsani_dev_kubernetes_{resource}")
             assert result.success
 
         async def test_target_container_resources_within_limits(
@@ -371,7 +371,7 @@ class TestChecksOriginalState:
         async def test_rollout_resource_exists(
             self, resource: str, rollout_checks: servo.connectors.opsani_dev.OpsaniDevRolloutChecks
         ) -> None:
-            result = await rollout_checks.run_one(id=f"check_kubernetes_{resource}")
+            result = await rollout_checks.run_one(id=f"check_opsani_dev_kubernetes_{resource}")
             assert result.success
 
         async def test_rollout_check_rsrc_limits(
@@ -569,7 +569,7 @@ async def multiport_service(self, kube, checks: servo.connectors.opsani_dev.Opsa
     async def test_requires_port_config_when_multiple_exist(
         self, kube, checks: servo.connectors.opsani_dev.OpsaniDevChecks, multiport_service
     ) -> None:
-        result = await checks.run_one(id=f"check_kubernetes_service_port")
+        result = await checks.run_one(id=f"check_opsani_dev_kubernetes_service_port")
         assert not result.success
         assert result.exception
         assert result.message == 'caught exception (ValueError): service defines more than one port: a `port` (name or number) must be specified in the configuration'
@@ -578,15 +578,15 @@ async def test_resolve_port_by_name(
         self, kube, checks: servo.connectors.opsani_dev.OpsaniDevChecks, multiport_service
     ) -> None:
         checks.config.port = 'elite'
-        result = await checks.run_one(id=f"check_kubernetes_service_port")
+        result = await checks.run_one(id=f"check_opsani_dev_kubernetes_service_port")
         assert result.success
         assert result.message == 'Service Port: elite 31337:31337/TCP'
 
     async def test_resolve_port_by_number(
         self, kube, checks: servo.connectors.opsani_dev.OpsaniDevChecks, multiport_service
     ) -> None:
         checks.config.port = 80
-        result = await checks.run_one(id=f"check_kubernetes_service_port")
+        result = await checks.run_one(id=f"check_opsani_dev_kubernetes_service_port")
         assert result.success
         assert result.message == 'Service Port: http 80:8480/TCP'
 
@@ -603,7 +603,7 @@ async def test_cannot_resolve_port_by_number(
         self, kube, checks: servo.connectors.opsani_dev.OpsaniDevChecks, multiport_service
     ) -> None:
         checks.config.port = 187
-        result = await checks.run_one(id=f"check_kubernetes_service_port")
+        result = await checks.run_one(id=f"check_opsani_dev_kubernetes_service_port")
         assert not result.success
         assert result.message == 'caught exception (LookupError): could not find a port numbered: 187'
     # Errors: