From 06f6fca2b9f38709e0561ba97430c730e3da1724 Mon Sep 17 00:00:00 2001 From: Yuwei Zhou Date: Mon, 6 Dec 2021 14:59:53 +0800 Subject: [PATCH] =?UTF-8?q?refactor:=20=F0=9F=92=A1=20Move=20ensure=5Facti?= =?UTF-8?q?ve=5Fdeployment=5Fexist=20to=20validator?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are some commands request there is an active Deployment under the App. This PR moves the logic from every commands to a validator --- .../azext_spring_cloud/_app_validator.py | 27 ++++++++- .../azext_spring_cloud/_params.py | 13 ++--- .../azext_spring_cloud/_transformers.py | 2 +- src/spring-cloud/azext_spring_cloud/custom.py | 29 +--------- .../tests/latest/test_asc_app_validator.py | 56 ++++++++++++++++++- 5 files changed, 90 insertions(+), 37 deletions(-) diff --git a/src/spring-cloud/azext_spring_cloud/_app_validator.py b/src/spring-cloud/azext_spring_cloud/_app_validator.py index c53429b908d..ef136de6e8a 100644 --- a/src/spring-cloud/azext_spring_cloud/_app_validator.py +++ b/src/spring-cloud/azext_spring_cloud/_app_validator.py @@ -7,11 +7,13 @@ from azure.cli.core.azclierror import InvalidArgumentValueError from msrestazure.azure_exceptions import CloudError +from azure.core.exceptions import (ResourceNotFoundError) from ._client_factory import cf_spring_cloud # pylint: disable=line-too-long,raise-missing-from NO_PRODUCTION_DEPLOYMENT_ERROR = "No production deployment found, use --deployment to specify deployment or create deployment with: az spring-cloud app deployment create" +NO_PRODUCTION_DEPLOYMENT_SET_ERROR = "This app has no production deployment, use \"az spring-cloud app deployment create\" to create a deployment and \"az spring-cloud app set-deployment\" to set production deployment." def fulfill_deployment_param(cmd, namespace): @@ -24,6 +26,24 @@ def fulfill_deployment_param(cmd, namespace): namespace.deployment = _ensure_active_deployment_exist_and_get(client, namespace.resource_group, namespace.service, namespace.name) +def active_deployment_exist(cmd, namespace): + if not namespace.name or not namespace.service or not namespace.resource_group: + return + client = cf_spring_cloud(cmd.cli_ctx) + deployment = _get_active_deployment(client, namespace.resource_group, namespace.service, namespace.name) + if not deployment: + raise InvalidArgumentValueError(NO_PRODUCTION_DEPLOYMENT_SET_ERROR) + + +def active_deployment_exist_under_app(cmd, namespace): + if not namespace.app or not namespace.service or not namespace.resource_group: + return + client = cf_spring_cloud(cmd.cli_ctx) + deployment = _get_active_deployment(client, namespace.resource_group, namespace.service, namespace.app) + if not deployment: + raise InvalidArgumentValueError(NO_PRODUCTION_DEPLOYMENT_SET_ERROR) + + def _ensure_deployment_exist(client, resource_group, service, app, deployment): try: return client.deployments.get(resource_group, service, app, deployment) @@ -39,5 +59,8 @@ def _ensure_active_deployment_exist_and_get(client, resource_group, service, nam def _get_active_deployment(client, resource_group, service, name): - deployments = client.deployments.list(resource_group, service, name) - return next(iter(x for x in deployments if x.properties.active), None) + try: + deployments = client.deployments.list(resource_group, service, name) + return next(iter(x for x in deployments if x.properties.active), None) + except ResourceNotFoundError: + raise InvalidArgumentValueError('Deployments not found under App {}'.format(name)) diff --git a/src/spring-cloud/azext_spring_cloud/_params.py b/src/spring-cloud/azext_spring_cloud/_params.py index 5c1d79f7578..6ca60c3cf22 100644 --- a/src/spring-cloud/azext_spring_cloud/_params.py +++ b/src/spring-cloud/azext_spring_cloud/_params.py @@ -14,7 +14,7 @@ validate_tracing_parameters_asc_create, validate_tracing_parameters_asc_update, validate_app_insights_parameters, validate_instance_count, validate_java_agent_parameters, validate_jar) -from ._app_validator import (fulfill_deployment_param) +from ._app_validator import (fulfill_deployment_param, active_deployment_exist, active_deployment_exist_under_app) from ._utils import ApiType from .vendored_sdks.appplatform.v2020_07_01.models import RuntimeVersion, TestKeyType @@ -154,6 +154,9 @@ def load_arguments(self, _): c.argument('main_entry', options_list=[ '--main-entry', '-m'], help="The path to the .NET executable relative to zip root.") + with self.argument_context('spring-cloud app identity') as c: + c.argument('name', name_type, help='Name of app.', validator=active_deployment_exist) + with self.argument_context('spring-cloud app identity assign') as c: c.argument('scope', help="The scope the managed identity has access to") c.argument('role', help="Role name or id the managed identity will be assigned") @@ -246,13 +249,9 @@ def prepare_logs_argument(c): c.argument('file_path', help='The mount file path for your dump file.') c.argument('duration', type=str, default="60s", help='Duration of JFR.') - with self.argument_context('spring-cloud app deployment') as c: - c.argument('app', app_name_type, help='Name of app.', - validator=validate_app_name) - with self.argument_context('spring-cloud app binding') as c: c.argument('app', app_name_type, help='Name of app.', - validator=validate_app_name) + validator=active_deployment_exist_under_app) c.argument('name', name_type, help='Name of service binding.') for scope in ['spring-cloud app binding cosmos add', 'spring-cloud app binding mysql add', 'spring-cloud app binding redis add']: @@ -354,7 +353,7 @@ def prepare_logs_argument(c): with self.argument_context('spring-cloud app custom-domain') as c: c.argument('service', service_name_type) - c.argument('app', app_name_type, help='Name of app.', validator=validate_app_name) + c.argument('app', app_name_type, help='Name of app.', validator=active_deployment_exist_under_app) c.argument('domain_name', help='Name of custom domain.') with self.argument_context('spring-cloud app custom-domain bind') as c: diff --git a/src/spring-cloud/azext_spring_cloud/_transformers.py b/src/spring-cloud/azext_spring_cloud/_transformers.py index bb0d8f8fb9e..00bab49d449 100644 --- a/src/spring-cloud/azext_spring_cloud/_transformers.py +++ b/src/spring-cloud/azext_spring_cloud/_transformers.py @@ -30,7 +30,7 @@ def transform_app_table_output(result): item['Production Deployment'] = item['properties']['activeDeploymentName'] item['Public Url'] = item['properties']['url'] - if 'activeDeployment' in item['properties']: + if item['properties'].get('activeDeployment', None): isStarted = item['properties']['activeDeployment']['properties']['status'].upper() == "RUNNING" instance_count = item['properties']['activeDeployment']['sku']['capacity'] instances = item['properties']['activeDeployment']['properties']['instances'] diff --git a/src/spring-cloud/azext_spring_cloud/custom.py b/src/spring-cloud/azext_spring_cloud/custom.py index 4f49e71be35..0489c9b58c6 100644 --- a/src/spring-cloud/azext_spring_cloud/custom.py +++ b/src/spring-cloud/azext_spring_cloud/custom.py @@ -462,12 +462,6 @@ def _default_deployment_resource_builder(cpu, memory, env, jvm_options, runtime_ return deployment_resource -def _check_active_deployment_exist(client, resource_group, service, app): - active_deployment_name = client.apps.get(resource_group, service, app).properties.active_deployment_name - if not active_deployment_name: - logger.warning(NO_PRODUCTION_DEPLOYMENT_SET_ERROR) - - def app_update(cmd, client, resource_group, service, name, assign_endpoint=None, deployment=None, @@ -633,12 +627,9 @@ def app_get(cmd, client, service, name): app = client.apps.get(resource_group, service, name) - deployment_name = app.properties.active_deployment_name - if deployment_name: - deployment = client.deployments.get( - resource_group, service, name, deployment_name) - app.properties.active_deployment = deployment - else: + deployments = client.deployments.list(resource_group, service, name) + app.properties.active_deployment = next((x for x in deployments if x.properties.active), None) + if not app.properties.active_deployment: logger.warning(NO_PRODUCTION_DEPLOYMENT_SET_ERROR) return app @@ -762,7 +753,6 @@ def app_tail_log(cmd, client, resource_group, service, name, def app_identity_assign(cmd, client, resource_group, service, name, role=None, scope=None): - _check_active_deployment_exist(client, resource_group, service, name) app_resource = models_20210601preview.AppResource() identity = models_20210601preview.ManagedIdentityProperties(type="systemassigned") properties = models_20210601preview.AppResourceProperties() @@ -820,7 +810,6 @@ def app_identity_remove(cmd, client, resource_group, service, name): def app_identity_show(cmd, client, resource_group, service, name): - _check_active_deployment_exist(client, resource_group, service, name) app = client.apps.get(resource_group, service, name) return app.identity @@ -1279,12 +1268,10 @@ def config_repo_list(cmd, client, resource_group, name): def binding_list(cmd, client, resource_group, service, app): - _check_active_deployment_exist(client, resource_group, service, app) return client.bindings.list(resource_group, service, app) def binding_get(cmd, client, resource_group, service, app, name): - _check_active_deployment_exist(client, resource_group, service, app) return client.bindings.get(resource_group, service, app, name) @@ -1298,7 +1285,6 @@ def binding_cosmos_add(cmd, client, resource_group, service, app, name, database_name=None, key_space=None, collection_name=None): - _check_active_deployment_exist(client, resource_group, service, app) resource_id_dict = parse_resource_id(resource_id) resource_type = resource_id_dict['resource_type'] resource_name = resource_id_dict['resource_name'] @@ -1332,7 +1318,6 @@ def binding_cosmos_update(cmd, client, resource_group, service, app, name, database_name=None, key_space=None, collection_name=None): - _check_active_deployment_exist(client, resource_group, service, app) binding = client.bindings.get(resource_group, service, app, name).properties resource_id = binding.resource_id resource_name = binding.resource_name @@ -1360,7 +1345,6 @@ def binding_mysql_add(cmd, client, resource_group, service, app, name, key, username, database_name): - _check_active_deployment_exist(client, resource_group, service, app) resource_id_dict = parse_resource_id(resource_id) resource_type = resource_id_dict['resource_type'] resource_name = resource_id_dict['resource_name'] @@ -1383,7 +1367,6 @@ def binding_mysql_update(cmd, client, resource_group, service, app, name, key=None, username=None, database_name=None): - _check_active_deployment_exist(client, resource_group, service, app) binding_parameters = {} binding_parameters['username'] = username binding_parameters['databaseName'] = database_name @@ -1399,7 +1382,6 @@ def binding_mysql_update(cmd, client, resource_group, service, app, name, def binding_redis_add(cmd, client, resource_group, service, app, name, resource_id, disable_ssl=None): - _check_active_deployment_exist(client, resource_group, service, app) use_ssl = not disable_ssl resource_id_dict = parse_resource_id(resource_id) resource_type = resource_id_dict['resource_type'] @@ -1426,7 +1408,6 @@ def binding_redis_add(cmd, client, resource_group, service, app, name, def binding_redis_update(cmd, client, resource_group, service, app, name, disable_ssl=None): - _check_active_deployment_exist(client, resource_group, service, app) binding = client.bindings.get(resource_group, service, app, name).properties resource_id = binding.resource_id resource_name = binding.resource_name @@ -1866,7 +1847,6 @@ def domain_bind(cmd, client, resource_group, service, app, domain_name, certificate=None, enable_end_to_end_tls=None): - _check_active_deployment_exist(client, resource_group, service, app) properties = models.CustomDomainProperties() if certificate is not None: certificate_response = client.certificates.get(resource_group, service, certificate) @@ -1899,12 +1879,10 @@ def _update_app_e2e_tls(cmd, resource_group, service, app, enable_end_to_end_tls def domain_show(cmd, client, resource_group, service, app, domain_name): - _check_active_deployment_exist(client, resource_group, service, app) return client.custom_domains.get(resource_group, service, app, domain_name) def domain_list(cmd, client, resource_group, service, app): - _check_active_deployment_exist(client, resource_group, service, app) return client.custom_domains.list(resource_group, service, app) @@ -1912,7 +1890,6 @@ def domain_update(cmd, client, resource_group, service, app, domain_name, certificate=None, enable_end_to_end_tls=None): - _check_active_deployment_exist(client, resource_group, service, app) properties = models.CustomDomainProperties() if certificate is not None: certificate_response = client.certificates.get(resource_group, service, certificate) diff --git a/src/spring-cloud/azext_spring_cloud/tests/latest/test_asc_app_validator.py b/src/spring-cloud/azext_spring_cloud/tests/latest/test_asc_app_validator.py index 14b48d4fa25..20c10732a97 100644 --- a/src/spring-cloud/azext_spring_cloud/tests/latest/test_asc_app_validator.py +++ b/src/spring-cloud/azext_spring_cloud/tests/latest/test_asc_app_validator.py @@ -7,7 +7,9 @@ from argparse import Namespace from azure.cli.core.azclierror import InvalidArgumentValueError from msrestazure.azure_exceptions import CloudError -from ..._app_validator import (fulfill_deployment_param) +from azure.core.exceptions import ResourceNotFoundError +from ..._app_validator import (fulfill_deployment_param, active_deployment_exist, active_deployment_exist_under_app) + try: import unittest.mock as mock @@ -37,6 +39,58 @@ def _get_deployment(resource_group, service, app, deployment, active): return resource +class TestActiveDeploymentExist(unittest.TestCase): + @mock.patch('azext_spring_cloud._app_validator.cf_spring_cloud', autospec=True) + def test_deployment_found(self, client_factory_mock): + client = mock.MagicMock() + client.deployments.list.return_value = [ + _get_deployment('rg1', 'asc1', 'app1', 'green1', False), + _get_deployment('rg1', 'asc1', 'app1', 'default', True), + ] + client_factory_mock.return_value = client + + ns = Namespace(name='app1', service='asc1', resource_group='rg1', deployment=None) + active_deployment_exist(_get_test_cmd(), ns) + + @mock.patch('azext_spring_cloud._app_validator.cf_spring_cloud', autospec=True) + def test_deployment_without_active_exist(self, client_factory_mock): + client = mock.MagicMock() + client.deployments.list.return_value = [ + _get_deployment('rg1', 'asc1', 'app1', 'green1', False) + ] + client_factory_mock.return_value = client + + ns = Namespace(name='app1', service='asc1', resource_group='rg1', deployment=None) + with self.assertRaises(InvalidArgumentValueError) as context: + active_deployment_exist(_get_test_cmd(), ns) + self.assertEqual('This app has no production deployment, use \"az spring-cloud app deployment create\" to create a deployment and \"az spring-cloud app set-deployment\" to set production deployment.', str(context.exception)) + + @mock.patch('azext_spring_cloud._app_validator.cf_spring_cloud', autospec=True) + def test_no_deployments(self, client_factory_mock): + client = mock.MagicMock() + client.deployments.list.return_value = [] + client_factory_mock.return_value = client + + ns = Namespace(name='app1', service='asc1', resource_group='rg1', deployment=None) + with self.assertRaises(InvalidArgumentValueError) as context: + active_deployment_exist(_get_test_cmd(), ns) + self.assertEqual('This app has no production deployment, use \"az spring-cloud app deployment create\" to create a deployment and \"az spring-cloud app set-deployment\" to set production deployment.', str(context.exception)) + + @mock.patch('azext_spring_cloud._app_validator.cf_spring_cloud', autospec=True) + def test_app_not_found(self, client_factory_mock): + client = mock.MagicMock() + resp = mock.MagicMock() + resp.status_code = 404 + resp.text = '{"Message": "Not Found"}' + client.deployments.list.side_effect = ResourceNotFoundError(resp, error='App not found.') + client_factory_mock.return_value = client + + ns = Namespace(name='app1', service='asc1', resource_group='rg1', deployment=None) + with self.assertRaises(InvalidArgumentValueError) as context: + active_deployment_exist(_get_test_cmd(), ns) + self.assertEqual('Deployments not found under App app1', str(context.exception)) + + class TestFulfillDeploymentParameter(unittest.TestCase): @mock.patch('azext_spring_cloud._app_validator.cf_spring_cloud', autospec=True) def test_deployment_provide(self, client_factory_mock):