From dceed59db2ed1f6b73897e76273a4659b2b196e0 Mon Sep 17 00:00:00 2001 From: Mark Beierl Date: Tue, 11 Apr 2023 21:03:56 +0000 Subject: [PATCH] OSM-986 Implements a check status activity that waits for the charm deployment to become ready. Change-Id: Icfbc5c9eb2ea921f981fb934fdcf43d6aa7bf651 Signed-off-by: Mark Beierl --- osm_lcm/nglcm.py | 1 + osm_lcm/temporal/juju_paas_activities.py | 83 ++++++++++--- osm_lcm/temporal/vdu_workflows.py | 20 +++- osm_lcm/tests/test_juju_paas_activities.py | 130 ++++++++++++++++++++- requirements-test.in | 3 +- requirements-test.txt | 4 +- 6 files changed, 220 insertions(+), 21 deletions(-) diff --git a/osm_lcm/nglcm.py b/osm_lcm/nglcm.py index 781a5bdd..06f803e6 100644 --- a/osm_lcm/nglcm.py +++ b/osm_lcm/nglcm.py @@ -114,6 +114,7 @@ class NGLcm: logger_module.addHandler(file_handler) if logger_config["loglevel"]: logger_module.setLevel(logger_config["loglevel"]) + logging.getLogger("juju.client.connection").setLevel(logging.CRITICAL) def _configure_logging(self): if self.main_config.globalConfig.logfile: diff --git a/osm_lcm/temporal/juju_paas_activities.py b/osm_lcm/temporal/juju_paas_activities.py index 3c61ec7a..98a0ae53 100644 --- a/osm_lcm/temporal/juju_paas_activities.py +++ b/osm_lcm/temporal/juju_paas_activities.py @@ -13,7 +13,10 @@ # implied. # See the License for the specific language governing permissions and # limitations under the License. +import asyncio import logging +from juju.application import Application +from juju.controller import Controller from temporalio import activity from n2vc.temporal_libjuju import ConnectionInfo, Libjuju from osm_common.temporal_constants import ( @@ -24,6 +27,7 @@ from osm_common.temporal_constants import ( ) from osm_common.dataclasses.temporal_dataclasses import ( CharmInfo, + CheckCharmStatusInput, ModelInfo, TestVimConnectivityInput, VduInstantiateInput, @@ -80,6 +84,17 @@ class JujuPaasConnector: connection_info = self._get_connection_info(vim_uuid) return Libjuju(connection_info) + async def _get_controller(self, vim_uuid) -> Controller: + connection_info = self._get_connection_info(vim_uuid) + controller = Controller() + await controller.connect( + endpoint=connection_info.endpoint, + username=connection_info.user, + password=connection_info.password, + cacert=connection_info.cacert, + ) + return controller + @activity.defn(name=ACTIVITY_TEST_VIM_CONNECTIVITY) async def test_vim_connectivity( self, test_connectivity_input: TestVimConnectivityInput @@ -178,8 +193,12 @@ class JujuPaasConnector: ) @activity.defn(name=ACTIVITY_CHECK_CHARM_STATUS) - async def check_charm_status(self, check_charm_status: VduInstantiateInput) -> None: - """Validates the credentials by attempting to connect to the given Juju Controller. + async def check_charm_status( + self, check_charm_status: CheckCharmStatusInput + ) -> None: + """Checks the ready status of the charm. This activity will block until the status of + the application is either "active" or "blocked". Additionally, it also blocks until + the workload status of each of its units is also either "active" or "blocked". Collaborators: DB Read: vim_accounts @@ -190,19 +209,57 @@ class JujuPaasConnector: or Juju controller is not reachable Activity Lifecycle: - This activity should complete relatively quickly (in a few seconds). - However, it would be reasonable to wait more than 72 seconds (network timeout) - incase there are network issues. + This activity will continue indefinitely until the specified charm deployment + has reached a ready state. Heartbeats are performed to ensure this activity + does not timeout. - This activity will not report a heartbeat due to its - short-running nature. - - It is recommended, although not necessary to implement a - back-off strategy for this activity, as it will naturally block - and wait on each connection attempt. + A start-to-close of something reasonable (such as 5 minutes) should be implemented + at the workflow level and such a timeout shall trigger workflow failure logic. """ - # TODO: Implement OSM-986 - pass + + controller = await self._get_controller(check_charm_status.vim_uuid) + model = await controller.get_model(check_charm_status.model_name) + application = model.applications[check_charm_status.application_name] + + ready = False + last_status = None + application_status = None + last_unit_status = {} + + while not ready: + activity.heartbeat() + await asyncio.sleep(check_charm_status.poll_interval) + # Perform the fetch of the status only once and keep it locally + application_status = application.status + if application_status != last_status: + last_status = application_status + self.logger.debug( + f"Application `{check_charm_status.application_name}` is {application_status}" + ) + + if application_status in ["active", "blocked"]: + # Check each unit to see if they are also ready + if not self._check_units_ready( + application=application, last_unit_status=last_unit_status + ): + continue + else: + continue + ready = True + + def _check_units_ready( + self, application: Application, last_unit_status: dict + ) -> bool: + for unit in application.units: + unit_workload_status = unit.workload_status + if unit_workload_status != last_unit_status.get(unit, None): + last_unit_status[unit] = unit_workload_status + self.logger.debug( + f"Application `{application.name}` Unit `{unit}` is {unit_workload_status}" + ) + if unit_workload_status not in ["active", "blocked"]: + return False + return True class CharmInfoUtils: diff --git a/osm_lcm/temporal/vdu_workflows.py b/osm_lcm/temporal/vdu_workflows.py index 09cd93d0..edb1235a 100644 --- a/osm_lcm/temporal/vdu_workflows.py +++ b/osm_lcm/temporal/vdu_workflows.py @@ -19,7 +19,10 @@ import logging from temporalio import workflow from temporalio.common import RetryPolicy -from osm_common.dataclasses.temporal_dataclasses import VduInstantiateInput +from osm_common.dataclasses.temporal_dataclasses import ( + VduInstantiateInput, + CheckCharmStatusInput, +) from osm_common.temporal_constants import ( ACTIVITY_DEPLOY_CHARM, @@ -43,6 +46,7 @@ class VduInstantiateWorkflow: @workflow.run async def run(self, input: VduInstantiateInput) -> None: try: + self.logger.info(f"Deploying VDU `{input.charm_info.app_name}`") await workflow.execute_activity( activity=ACTIVITY_DEPLOY_CHARM, arg=input, @@ -52,15 +56,25 @@ class VduInstantiateWorkflow: retry_policy=retry_policy, ) + self.logger.info( + f"Waiting for VDU `{input.charm_info.app_name}` to become ready" + ) await workflow.execute_activity( activity=ACTIVITY_CHECK_CHARM_STATUS, - arg=input, + arg=CheckCharmStatusInput( + vim_uuid=input.vim_uuid, + model_name=input.model_name, + application_name=input.charm_info.app_name, + ), activity_id=f"{ACTIVITY_CHECK_CHARM_STATUS}-{input.vim_uuid}", task_queue=LCM_TASK_QUEUE, - schedule_to_close_timeout=default_schedule_to_close_timeout, + start_to_close_timeout=timedelta(minutes=5), + heartbeat_timeout=timedelta(seconds=30), retry_policy=retry_policy, ) + self.logger.info(f"VDU `{input.charm_info.app_name}` is ready") + except Exception as e: self.logger.error(f"{WORKFLOW_VDU_INSTANTIATE} failed with {str(e)}") raise e diff --git a/osm_lcm/tests/test_juju_paas_activities.py b/osm_lcm/tests/test_juju_paas_activities.py index 7d94a2cd..b943c26c 100644 --- a/osm_lcm/tests/test_juju_paas_activities.py +++ b/osm_lcm/tests/test_juju_paas_activities.py @@ -15,13 +15,20 @@ # limitations under the License. import asynctest +import asyncio +import unittest.mock as mock + +from juju.application import Application from juju.errors import JujuError -from osm_common.dataclasses.temporal_dataclasses import ModelInfo +from juju.unit import Unit +from n2vc.temporal_libjuju import ConnectionInfo +from osm_common.dataclasses.temporal_dataclasses import CheckCharmStatusInput, ModelInfo from osm_common.dbbase import DbException from osm_lcm.temporal.juju_paas_activities import JujuPaasConnector -from n2vc.temporal_libjuju import ConnectionInfo +from parameterized import parameterized from temporalio.testing import ActivityEnvironment -from unittest.mock import Mock +from unittest.mock import AsyncMock, Mock + vim_id = "some-vim-uuid" namespace = "some-namespace" @@ -75,3 +82,120 @@ class TestJujuPaasConnector(asynctest.TestCase): await self.env.run(self.juju_paas_connector.create_model, model_info) mock_get_connection_info.assert_called_once_with(vim_id) mock_add_model.assert_not_called() + + +class JujuPaasActivitiesTest(asynctest.TestCase): + def setUp(self) -> None: + self.db = Mock() + self.env = ActivityEnvironment() + self.env.on_heartbeat = self.on_heartbeat + self.heartbeat_count = 0 + self.heartbeat_maximum = 5 + + self.controller = AsyncMock() + self.model = Mock() + self.controller.get_model.return_value = self.model + + async def get_controller(_: str): + return self.controller + + self.juju_paas = JujuPaasConnector(self.db) + self.juju_paas._get_controller = get_controller + + self.application_name = "application" + self.application = Mock(spec=Application) + self.application.name = self.application_name + self.model.applications = {self.application_name: self.application} + + def on_heartbeat(self, *args, **kwargs): + self.heartbeat_count += 1 + if self.heartbeat_count > self.heartbeat_maximum: + self.env.cancel() + + @parameterized.expand( + [ + ("App active, Unit active", ["active"], ["active"], 1), + ("App blocked, Unit active", ["blocked"], ["active"], 1), + ("App active, Unit blocked", ["active"], ["blocked"], 1), + ("App blocked, Unit blocked", ["blocked"], ["blocked"], 1), + ( + "App maint, then active, Unit active", + ["maintenance", "active"], + ["active"], + 2, + ), + ( + "App active, unit maint then active", + ["active", "active"], + ["maintenance", "active"], + 2, + ), + ] + ) + async def test_check_charm_status_application_with_one_unit( + self, _, app_events, unit_events, heartbeat_maximum + ): + arg = CheckCharmStatusInput( + application_name=self.application_name, + model_name="model", + vim_uuid="vim-uuid", + poll_interval=0, + ) + + self.heartbeat_maximum = heartbeat_maximum + units = [Mock(spec=Unit)] + self.application.units = units + + type(self.application).status = mock.PropertyMock(side_effect=app_events) + + type(units[0]).workload_status = mock.PropertyMock(side_effect=unit_events) + + await self.env.run(self.juju_paas.check_charm_status, arg) + + async def test_check_charm_status_cancel(self): + arg = CheckCharmStatusInput( + application_name=self.application_name, + model_name="model", + vim_uuid="vim-uuid", + poll_interval=0, + ) + + self.heartbeat_maximum = 3 + units = [Mock(spec=Unit)] + self.application.units = units + + type(self.application).status = mock.PropertyMock( + side_effect=["maintenance", "maintenance", "maintenance", "maintenance"] + ) + + type(units[0]).workload_status = mock.PropertyMock(side_effect=[]) + + with self.assertRaises(asyncio.exceptions.CancelledError): + await self.env.run(self.juju_paas.check_charm_status, arg) + + async def test_check_charm_status_multiple_units(self): + arg = CheckCharmStatusInput( + application_name=self.application_name, + model_name="model", + vim_uuid="vim-uuid", + poll_interval=0, + ) + + self.heartbeat_maximum = 4 + + units = [Mock(spec=Unit), Mock(spec=Unit)] + self.application.units = units + + type(self.application).status = mock.PropertyMock( + side_effect=["active", "active", "active", "active"] + ) + + type(units[0]).workload_status = mock.PropertyMock( + side_effect=["maintenance", "maintenance", "active", "active"] + ) + + type(units[1]).workload_status = mock.PropertyMock( + side_effect=["maintenance", "active", "maintenance", "active"] + ) + + await self.env.run(self.juju_paas.check_charm_status, arg) diff --git a/requirements-test.in b/requirements-test.in index 15fb5ee9..449f5a44 100644 --- a/requirements-test.in +++ b/requirements-test.in @@ -16,4 +16,5 @@ asynctest coverage mock -nose2 \ No newline at end of file +nose2 +parameterized diff --git a/requirements-test.txt b/requirements-test.txt index 11c468b4..9842e2b6 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -16,9 +16,11 @@ ####################################################################################### asynctest==0.13.0 # via -r requirements-test.in -coverage==7.2.1 +coverage==7.2.3 # via -r requirements-test.in mock==5.0.1 # via -r requirements-test.in nose2==0.12.0 # via -r requirements-test.in +parameterized==0.9.0 + # via -r requirements-test.in -- 2.25.1