From 2b07f4ba9d8e584af4da0a0fea66d486c6d9055a Mon Sep 17 00:00:00 2001 From: Yuwei Zhou Date: Tue, 11 Jan 2022 12:14:26 +0800 Subject: [PATCH 1/6] =?UTF-8?q?fix:=20=F0=9F=90=9B=20App=20update=20warns?= =?UTF-8?q?=20when=20=20active=20deployment=20not=20found?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../azext_spring_cloud/_app_validator.py | 30 ++++++++++++++-- .../azext_spring_cloud/_deployment_factory.py | 2 +- .../azext_spring_cloud/_params.py | 25 ++++++++------ src/spring-cloud/azext_spring_cloud/_utils.py | 8 +++++ src/spring-cloud/azext_spring_cloud/app.py | 34 ++++++++++++------- .../tests/latest/test_asc_app.py | 22 +++++++++--- .../tests/latest/test_asc_app_validator.py | 2 +- 7 files changed, 92 insertions(+), 31 deletions(-) diff --git a/src/spring-cloud/azext_spring_cloud/_app_validator.py b/src/spring-cloud/azext_spring_cloud/_app_validator.py index 8add1879262..d66706a06dc 100644 --- a/src/spring-cloud/azext_spring_cloud/_app_validator.py +++ b/src/spring-cloud/azext_spring_cloud/_app_validator.py @@ -4,7 +4,7 @@ # -------------------------------------------------------------------------------------------- # pylint: disable=too-few-public-methods, unused-argument, redefined-builtin - +from knack.log import get_logger from azure.cli.core.azclierror import InvalidArgumentValueError from msrestazure.azure_exceptions import CloudError from azure.core.exceptions import (ResourceNotFoundError) @@ -12,6 +12,9 @@ from ._client_factory import cf_spring_cloud_20220101preview +logger = get_logger(__name__) + + # 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." @@ -27,6 +30,18 @@ def fulfill_deployment_param(cmd, namespace): namespace.deployment = _ensure_active_deployment_exist_and_get(client, namespace.resource_group, namespace.service, namespace.name) +def fulfill_deployment_param_or_warning(cmd, namespace): + client = cf_spring_cloud_20220101preview(cmd.cli_ctx) + if not namespace.name or not namespace.service or not namespace.resource_group: + return + if namespace.deployment: + namespace.deployment = _ensure_deployment_exist(client, namespace.resource_group, namespace.service, namespace.name, namespace.deployment) + else: + namespace.deployment = _get_active_deployment(client, namespace.resource_group, namespace.service, namespace.name) + if not namespace.deployment: + logger.warning(NO_PRODUCTION_DEPLOYMENT_SET_ERROR) + + def active_deployment_exist(cmd, namespace): if not namespace.name or not namespace.service or not namespace.resource_group: return @@ -36,13 +51,22 @@ def active_deployment_exist(cmd, namespace): raise InvalidArgumentValueError(NO_PRODUCTION_DEPLOYMENT_SET_ERROR) -def active_deployment_exist_under_app(cmd, namespace): +def active_deployment_exist_or_warning(cmd, namespace): + if not namespace.name or not namespace.service or not namespace.resource_group: + return + client = cf_spring_cloud_20220101preview(cmd.cli_ctx) + deployment = _get_active_deployment(client, namespace.resource_group, namespace.service, namespace.name) + if not deployment: + logger.warning(NO_PRODUCTION_DEPLOYMENT_SET_ERROR) + + +def active_deployment_exist_under_app_or_warning(cmd, namespace): if not namespace.app or not namespace.service or not namespace.resource_group: return client = cf_spring_cloud_20220101preview(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) + logger.warning(NO_PRODUCTION_DEPLOYMENT_SET_ERROR) def ensure_not_active_deployment(cmd, namespace): diff --git a/src/spring-cloud/azext_spring_cloud/_deployment_factory.py b/src/spring-cloud/azext_spring_cloud/_deployment_factory.py index 32acd0d5b4c..1cd9fa5bd2a 100644 --- a/src/spring-cloud/azext_spring_cloud/_deployment_factory.py +++ b/src/spring-cloud/azext_spring_cloud/_deployment_factory.py @@ -50,7 +50,7 @@ def _format_resource_request(self, cpu=None, memory=None, **_): memory=memory ) - def _get_env(self, env, **_): + def _get_env(self, env=None, **_): return env def format_source(self, **kwargs): diff --git a/src/spring-cloud/azext_spring_cloud/_params.py b/src/spring-cloud/azext_spring_cloud/_params.py index 09b223c8bdd..2234aa3f6dd 100644 --- a/src/spring-cloud/azext_spring_cloud/_params.py +++ b/src/spring-cloud/azext_spring_cloud/_params.py @@ -17,9 +17,9 @@ from ._validators_enterprise import (only_support_enterprise, validate_git_uri, validate_acs_patterns, validate_routes) -from ._app_validator import (fulfill_deployment_param, active_deployment_exist, active_deployment_exist_under_app, +from ._app_validator import (fulfill_deployment_param, active_deployment_exist, active_deployment_exist_under_app_or_warning, ensure_not_active_deployment, validate_deloy_path, validate_deloyment_create_path, - validate_cpu, validate_memory) + validate_cpu, validate_memory, fulfill_deployment_param_or_warning, active_deployment_exist_or_warning) from ._utils import ApiType from .vendored_sdks.appplatform.v2022_01_01_preview.models._app_platform_management_client_enums import SupportedRuntimeValue, TestKeyType @@ -156,6 +156,9 @@ def load_arguments(self, _): help='A json file path for the persistent storages to be mounted to the app') c.argument('loaded_public_certificate_file', type=str, options_list=['--loaded-public-certificate-file', '-f'], help='A json file path indicates the certificates which would be loaded to app') + c.argument('deployment', options_list=['--deployment', '-d'], + help='Name of an existing deployment of the app. Default to the production deployment if not specified.', + validator=fulfill_deployment_param_or_warning) with self.argument_context('spring-cloud app append-persistent-storage') as c: c.argument('storage_name', type=str, @@ -168,16 +171,16 @@ def load_arguments(self, _): c.argument('mount_options', nargs='+', help='[optional] The mount options for the persistent storage volume.', default=None) c.argument('read_only', arg_type=get_three_state_flag(), help='[optional] If true, the persistent storage volume will be read only.', default=False) - for scope in ['spring-cloud app update', 'spring-cloud app start', 'spring-cloud app stop', 'spring-cloud app restart', 'spring-cloud app deploy', 'spring-cloud app scale', 'spring-cloud app set-deployment', 'spring-cloud app show-deploy-log']: + for scope in ['spring-cloud app start', 'spring-cloud app stop', 'spring-cloud app restart', 'spring-cloud app deploy', 'spring-cloud app scale', 'spring-cloud app set-deployment', 'spring-cloud app show-deploy-log']: with self.argument_context(scope) as c: c.argument('deployment', options_list=[ '--deployment', '-d'], help='Name of an existing deployment of the app. Default to the production deployment if not specified.', validator=fulfill_deployment_param) - c.argument('main_entry', options_list=[ - '--main-entry', '-m'], help="The path to the .NET executable relative to zip root.") - for scope in ['spring-cloud app identity', 'spring-cloud app unset-deployment']: - with self.argument_context(scope) as c: - c.argument('name', name_type, help='Name of app.', validator=active_deployment_exist) + with self.argument_context('spring-cloud app unset-deployment') as c: + c.argument('name', name_type, help='Name of app.', validator=active_deployment_exist) + + with self.argument_context('spring-cloud app identity') as c: + c.argument('name', name_type, help='Name of app.', validator=active_deployment_exist_or_warning) with self.argument_context('spring-cloud app identity assign') as c: c.argument('scope', help="The scope the managed identity has access to") @@ -218,6 +221,8 @@ def prepare_logs_argument(c): help="A string containing jvm options, use '=' instead of ' ' for this argument to avoid bash parse error, eg: --jvm-options='-Xms1024m -Xmx2048m'") c.argument('env', env_type) c.argument('disable_probe', arg_type=get_three_state_flag(), help='If true, disable the liveness and readiness probe.') + 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 scale') as c: c.argument('cpu', arg_type=cpu_type) @@ -289,7 +294,7 @@ def prepare_logs_argument(c): with self.argument_context('spring-cloud app binding') as c: c.argument('app', app_name_type, help='Name of app.', - validator=active_deployment_exist_under_app) + validator=active_deployment_exist_under_app_or_warning) 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']: @@ -391,7 +396,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=active_deployment_exist_under_app) + c.argument('app', app_name_type, help='Name of app.', validator=active_deployment_exist_under_app_or_warning) 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/_utils.py b/src/spring-cloud/azext_spring_cloud/_utils.py index e4633c82889..2dec595e24d 100644 --- a/src/spring-cloud/azext_spring_cloud/_utils.py +++ b/src/spring-cloud/azext_spring_cloud/_utils.py @@ -252,6 +252,14 @@ def get_spring_cloud_sku(client, resource_group, name): return client.services.get(resource_group, name).sku +def convert_argument_to_parameter_list(args): + return ', '.join([convert_argument_to_parameter(x) for x in args]) + + +def convert_argument_to_parameter(arg): + return '--{}'.format(arg.replace('_', '-')) + + def wait_till_end(cmd, *pollers): if not pollers: return diff --git a/src/spring-cloud/azext_spring_cloud/app.py b/src/spring-cloud/azext_spring_cloud/app.py index c2e18ac4a10..5e0a675839b 100644 --- a/src/spring-cloud/azext_spring_cloud/app.py +++ b/src/spring-cloud/azext_spring_cloud/app.py @@ -6,9 +6,9 @@ # pylint: disable=wrong-import-order from knack.log import get_logger from azure.cli.core.util import sdk_no_wait -from azure.cli.core.azclierror import ValidationError +from azure.cli.core.azclierror import (ValidationError, InvalidArgumentValueError) from .custom import app_get -from ._utils import (get_spring_cloud_sku, wait_till_end) +from ._utils import (get_spring_cloud_sku, wait_till_end, convert_argument_to_parameter_list) from ._deployment_factory import (deployment_selector, deployment_settings_options_from_resource, deployment_source_options_from_resource, @@ -149,9 +149,9 @@ def app_update(cmd, client, resource_group, service, name, 'resource_group': resource_group, 'service': service, 'app': name, - 'deployment': deployment.name, + 'sku': deployment.sku if deployment else get_spring_cloud_sku(client, resource_group, service), + 'deployment': deployment.name if deployment else None, 'deployment_resource': deployment, - 'sku': deployment.sku } deployment_kwargs = { @@ -160,8 +160,9 @@ def app_update(cmd, client, resource_group, service, name, 'runtime_version': runtime_version, 'jvm_options': jvm_options, 'main_entry': main_entry, - 'source_type': deployment.properties.source.type + 'source_type': deployment.properties.source.type if deployment else None } + app_kwargs = { 'public': assign_endpoint, 'enable_persistent_storage': enable_persistent_storage, @@ -171,6 +172,12 @@ def app_update(cmd, client, resource_group, service, name, 'https_only': https_only, } + if deployment is None: + updated_deployment_kwargs = {k: v for k, v in deployment_kwargs.items() if v} + if updated_deployment_kwargs: + raise InvalidArgumentValueError('{} cannot be set when there is no active deployment.' + .format(convert_argument_to_parameter_list(updated_deployment_kwargs.keys()))) + deployment_factory = deployment_selector(**deployment_kwargs, **basic_kwargs) app_factory = app_selector(**basic_kwargs) deployment_kwargs.update(deployment_factory.source_factory @@ -179,15 +186,18 @@ def app_update(cmd, client, resource_group, service, name, app_resource = app_factory.format_resource(**app_kwargs, **basic_kwargs) deployment_resource = deployment_factory.format_resource(**deployment_kwargs, **basic_kwargs) - app_poller = client.apps.begin_update(resource_group, service, name, app_resource) - poller = client.deployments.begin_update(resource_group, - service, - name, - DEFAULT_DEPLOYMENT_NAME, - deployment_resource) + pollers = [ + client.apps.begin_update(resource_group, service, name, app_resource) + ] + if deployment_kwargs: + pollers.append(client.deployments.begin_update(resource_group, + service, + name, + DEFAULT_DEPLOYMENT_NAME, + deployment_resource)) if no_wait: return - wait_till_end(cmd, app_poller, poller) + wait_till_end(cmd, *pollers) return app_get(cmd, client, resource_group, service, name) diff --git a/src/spring-cloud/azext_spring_cloud/tests/latest/test_asc_app.py b/src/spring-cloud/azext_spring_cloud/tests/latest/test_asc_app.py index 064853774de..4e31de91e78 100644 --- a/src/spring-cloud/azext_spring_cloud/tests/latest/test_asc_app.py +++ b/src/spring-cloud/azext_spring_cloud/tests/latest/test_asc_app.py @@ -357,10 +357,13 @@ def _execute(self, *args, **kwargs): app_update(_get_test_cmd(), client, *args, **kwargs) call_args = client.deployments.begin_update.call_args_list - self.assertEqual(1, len(call_args)) - self.assertEqual(5, len(call_args[0][0])) - self.assertEqual(args[0:3] + ('default',), call_args[0][0][0:4]) - self.patch_deployment_resource = call_args[0][0][4] + if kwargs.get('deployment', None): + self.assertEqual(1, len(call_args)) + self.assertEqual(5, len(call_args[0][0])) + self.assertEqual(args[0:3] + ('default',), call_args[0][0][0:4]) + self.patch_deployment_resource = call_args[0][0][4] + else: + self.patch_deployment_resource = None call_args = client.apps.begin_update.call_args_list self.assertEqual(1, len(call_args)) @@ -368,6 +371,17 @@ def _execute(self, *args, **kwargs): self.assertEqual(args[0:3], call_args[0][0][0:3]) self.patch_app_resource = call_args[0][0][3] + def test_app_update_without_deployment(self): + self._execute('rg', 'asc', 'app', assign_endpoint=True) + + self.assertIsNone(self.patch_deployment_resource) + resource = self.patch_app_resource + self.assertEqual(True, resource.properties.public) + + def test_invalid_app_update_deployment_settings_without_deployment(self): + with self.assertRaisesRegexp(CLIError, '--jvm-options cannot be set when there is no active deployment.'): + self._execute('rg', 'asc', 'app', jvm_options='test-option') + def test_app_update_jvm_options(self): self._execute('rg', 'asc', 'app', deployment=self._get_deployment(), jvm_options='test-option') resource = self.patch_deployment_resource 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 06cf409a14f..2c42f2a89ac 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,7 @@ from azure.cli.core.azclierror import InvalidArgumentValueError from msrestazure.azure_exceptions import CloudError from azure.core.exceptions import ResourceNotFoundError -from ..._app_validator import (fulfill_deployment_param, active_deployment_exist, active_deployment_exist_under_app, +from ..._app_validator import (fulfill_deployment_param, active_deployment_exist, validate_cpu, validate_memory, validate_deloyment_create_path, validate_deloy_path) From ca424202a30583709ba8a1062e4dece73f4de19e Mon Sep 17 00:00:00 2001 From: Yuwei Zhou Date: Tue, 11 Jan 2022 13:03:42 +0800 Subject: [PATCH 2/6] fix lint --- src/spring-cloud/azext_spring_cloud/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spring-cloud/azext_spring_cloud/app.py b/src/spring-cloud/azext_spring_cloud/app.py index 5e0a675839b..21a811498f5 100644 --- a/src/spring-cloud/azext_spring_cloud/app.py +++ b/src/spring-cloud/azext_spring_cloud/app.py @@ -176,7 +176,7 @@ def app_update(cmd, client, resource_group, service, name, updated_deployment_kwargs = {k: v for k, v in deployment_kwargs.items() if v} if updated_deployment_kwargs: raise InvalidArgumentValueError('{} cannot be set when there is no active deployment.' - .format(convert_argument_to_parameter_list(updated_deployment_kwargs.keys()))) + .format(convert_argument_to_parameter_list(updated_deployment_kwargs.keys()))) deployment_factory = deployment_selector(**deployment_kwargs, **basic_kwargs) app_factory = app_selector(**basic_kwargs) From 1d42ee5e87a5ee416d6bf160b750c8ea156dd64c Mon Sep 17 00:00:00 2001 From: Yuwei Zhou Date: Tue, 11 Jan 2022 14:49:16 +0800 Subject: [PATCH 3/6] upgrade parameter --- src/spring-cloud/azext_spring_cloud/custom.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spring-cloud/azext_spring_cloud/custom.py b/src/spring-cloud/azext_spring_cloud/custom.py index b56d787618c..5237fd6e537 100644 --- a/src/spring-cloud/azext_spring_cloud/custom.py +++ b/src/spring-cloud/azext_spring_cloud/custom.py @@ -347,7 +347,7 @@ def app_scale(cmd, client, resource_group, service, name, resource_group, service, name, deployment.name, deployment_resource) -def app_get_build_log(cmd, client, resource_group, service, name, deployment): +def app_get_build_log(cmd, client, resource_group, service, name, deployment=None): if deployment.properties.source.type != "Source": raise CLIError("{} deployment has no build logs.".format(deployment.properties.source.type)) return stream_logs(client.deployments, resource_group, service, name, deployment.name) From efea222de48f28c9efaf6db47b4a7f075a54e2f3 Mon Sep 17 00:00:00 2001 From: Yuwei Zhou Date: Wed, 12 Jan 2022 10:34:13 +0800 Subject: [PATCH 4/6] fix comment --- src/spring-cloud/azext_spring_cloud/app.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/spring-cloud/azext_spring_cloud/app.py b/src/spring-cloud/azext_spring_cloud/app.py index 21a811498f5..ef22365e6fc 100644 --- a/src/spring-cloud/azext_spring_cloud/app.py +++ b/src/spring-cloud/azext_spring_cloud/app.py @@ -6,7 +6,7 @@ # pylint: disable=wrong-import-order from knack.log import get_logger from azure.cli.core.util import sdk_no_wait -from azure.cli.core.azclierror import (ValidationError, InvalidArgumentValueError) +from azure.cli.core.azclierror import (ValidationError, ArgumentUsageError) from .custom import app_get from ._utils import (get_spring_cloud_sku, wait_till_end, convert_argument_to_parameter_list) from ._deployment_factory import (deployment_selector, @@ -175,8 +175,8 @@ def app_update(cmd, client, resource_group, service, name, if deployment is None: updated_deployment_kwargs = {k: v for k, v in deployment_kwargs.items() if v} if updated_deployment_kwargs: - raise InvalidArgumentValueError('{} cannot be set when there is no active deployment.' - .format(convert_argument_to_parameter_list(updated_deployment_kwargs.keys()))) + raise ArgumentUsageError ('{} cannot be set when there is no active deployment.' + .format(convert_argument_to_parameter_list(updated_deployment_kwargs.keys()))) deployment_factory = deployment_selector(**deployment_kwargs, **basic_kwargs) app_factory = app_selector(**basic_kwargs) From 442254a5331ebed1f3e7ae563ff6bf98786256e8 Mon Sep 17 00:00:00 2001 From: Yuwei Zhou Date: Wed, 12 Jan 2022 10:50:22 +0800 Subject: [PATCH 5/6] fix lint --- src/spring-cloud/azext_spring_cloud/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spring-cloud/azext_spring_cloud/app.py b/src/spring-cloud/azext_spring_cloud/app.py index ef22365e6fc..b7fe3b8ff0c 100644 --- a/src/spring-cloud/azext_spring_cloud/app.py +++ b/src/spring-cloud/azext_spring_cloud/app.py @@ -175,7 +175,7 @@ def app_update(cmd, client, resource_group, service, name, if deployment is None: updated_deployment_kwargs = {k: v for k, v in deployment_kwargs.items() if v} if updated_deployment_kwargs: - raise ArgumentUsageError ('{} cannot be set when there is no active deployment.' + raise ArgumentUsageError('{} cannot be set when there is no active deployment.' .format(convert_argument_to_parameter_list(updated_deployment_kwargs.keys()))) deployment_factory = deployment_selector(**deployment_kwargs, **basic_kwargs) From e66f7b6eb7a81057a35e280f6111d0fd87a30a49 Mon Sep 17 00:00:00 2001 From: Yuwei Zhou Date: Wed, 12 Jan 2022 12:50:42 +0800 Subject: [PATCH 6/6] fix lint --- src/spring-cloud/azext_spring_cloud/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spring-cloud/azext_spring_cloud/app.py b/src/spring-cloud/azext_spring_cloud/app.py index b7fe3b8ff0c..10a3f69bd4a 100644 --- a/src/spring-cloud/azext_spring_cloud/app.py +++ b/src/spring-cloud/azext_spring_cloud/app.py @@ -176,7 +176,7 @@ def app_update(cmd, client, resource_group, service, name, updated_deployment_kwargs = {k: v for k, v in deployment_kwargs.items() if v} if updated_deployment_kwargs: raise ArgumentUsageError('{} cannot be set when there is no active deployment.' - .format(convert_argument_to_parameter_list(updated_deployment_kwargs.keys()))) + .format(convert_argument_to_parameter_list(updated_deployment_kwargs.keys()))) deployment_factory = deployment_selector(**deployment_kwargs, **basic_kwargs) app_factory = app_selector(**basic_kwargs)