Skip to content

Commit

Permalink
fix(spring-cloud): 🐛 App update warns when active deployment not found (
Browse files Browse the repository at this point in the history
#4308)

* fix: 🐛 App update warns when  active deployment not found

* fix lint

* upgrade parameter

* fix comment

* fix lint

* fix lint
  • Loading branch information
yuwzho authored Jan 12, 2022
1 parent 2c2be60 commit ec35a6e
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 32 deletions.
30 changes: 27 additions & 3 deletions src/spring-cloud/azext_spring_cloud/_app_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,17 @@
# --------------------------------------------------------------------------------------------

# 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)
from ._resource_quantity import (validate_cpu as validate_cpu_value, validate_memory as validate_memory_value)
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."
Expand All @@ -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
Expand All @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion src/spring-cloud/azext_spring_cloud/_deployment_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,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 _get_addon_configs(self, config_file_patterns=None, **_):
Expand Down
25 changes: 15 additions & 10 deletions src/spring-cloud/azext_spring_cloud/_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
validate_api_portal_instance_count,
validate_buildpack_binding_exist, validate_buildpack_binding_not_exist,
validate_buildpack_binding_properties, validate_buildpack_binding_secrets)
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


Expand Down Expand Up @@ -196,6 +196,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,
Expand All @@ -208,16 +211,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")
Expand Down Expand Up @@ -258,6 +261,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.")

for scope in ['update', 'deployment create', 'deploy']:
with self.argument_context('spring-cloud app {}'.format(scope)) as c:
Expand Down Expand Up @@ -336,7 +341,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']:
Expand Down Expand Up @@ -438,7 +443,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:
Expand Down
8 changes: 8 additions & 0 deletions src/spring-cloud/azext_spring_cloud/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 22 additions & 12 deletions src/spring-cloud/azext_spring_cloud/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, ArgumentUsageError)
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,
Expand Down Expand Up @@ -150,9 +150,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 = {
Expand All @@ -162,8 +162,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,
Expand All @@ -173,6 +174,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 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)
deployment_kwargs.update(deployment_factory.source_factory
Expand All @@ -181,15 +188,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)


Expand Down
2 changes: 1 addition & 1 deletion src/spring-cloud/azext_spring_cloud/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
22 changes: 18 additions & 4 deletions src/spring-cloud/azext_spring_cloud/tests/latest/test_asc_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,17 +360,31 @@ 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))
self.assertEqual(4, len(call_args[0][0]))
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down

0 comments on commit ec35a6e

Please sign in to comment.