Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(spring-cloud): 🐛 App update warns when active deployment not found #4308

Merged
merged 7 commits into from
Jan 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -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):
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 @@ -17,9 +17,9 @@
from ._validators_enterprise import (only_support_enterprise, validate_git_uri, validate_acs_patterns, validate_routes,
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 @@ -160,6 +160,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)
yuwzho marked this conversation as resolved.
Show resolved Hide resolved

with self.argument_context('spring-cloud app append-persistent-storage') as c:
c.argument('storage_name', type=str,
Expand All @@ -172,16 +175,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 @@ -222,6 +225,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)
Expand Down Expand Up @@ -293,7 +298,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 @@ -395,7 +400,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 @@ -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 = {
Expand All @@ -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,
Expand All @@ -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 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 @@ -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)


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 @@ -357,17 +357,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