Skip to content

Commit

Permalink
refactor: 💡 Move ensure_active_deployment_exist to validator
Browse files Browse the repository at this point in the history
There are some commands request there is an active Deployment under the
App. This PR moves the logic from every commands to a validator
  • Loading branch information
yuwzho committed Dec 6, 2021
1 parent 2435c8f commit 06f6fca
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 37 deletions.
27 changes: 25 additions & 2 deletions src/spring-cloud/azext_spring_cloud/_app_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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)
Expand All @@ -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))
13 changes: 6 additions & 7 deletions src/spring-cloud/azext_spring_cloud/_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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']:
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion src/spring-cloud/azext_spring_cloud/_transformers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand Down
29 changes: 3 additions & 26 deletions src/spring-cloud/azext_spring_cloud/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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)


Expand All @@ -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']
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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']
Expand All @@ -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
Expand All @@ -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']
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -1899,20 +1879,17 @@ 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)


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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit 06f6fca

Please sign in to comment.