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

[Spring Cloud] Integrate tanzu components with service and app #4300

Merged
merged 17 commits into from
Jan 11, 2022

Conversation

ninpan-ms
Copy link
Contributor


The pr is part of main...VSChina:enterprise.

  • Integrate Tanzu components creation with service create commands.
  • Add --config-file-pattern parameter to app deploy/update/deployment create

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally?

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your PR is merged into master branch, a new PR will be created to update src/index.json automatically.
The precondition is to put your code inside this repo and upgrade the version in the PR but do not modify src/index.json.

@ninpan-ms ninpan-ms changed the title Integrate tanzu components with service and app [Spring Cloud] Integrate tanzu components with service and app Jan 6, 2022
@ninpan-ms
Copy link
Contributor Author

#4294

src/spring-cloud/azext_spring_cloud/_tanzu_component.py Outdated Show resolved Hide resolved
@@ -76,8 +80,13 @@ def before_create(self, **_):
pass

def after_create(self, no_wait=None, **kwargs):
service = self.client.services.get(self.resource_group, self.name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sku is already in the kwargs. Don't need to get from remote.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

logger.warning(" - Creating Application Configuration Service ..")
acs_resource = models.ConfigurationServiceResource()
return client.configuration_services.begin_create_or_update(resource_group, service, DEFAULT_NAME, acs_resource)
return
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redundant return?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

def create_service_registry(cmd, client, resource_group, service, enable_service_registry):
if enable_service_registry:
logger.warning(" - Creating Service Registry ..")
return client.service_registries.begin_create_or_update(resource_group, service, DEFAULT_NAME)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no service registry body, is this on purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's expected

@yonzhan
Copy link
Collaborator

yonzhan commented Jan 6, 2022

Spring Cloud

@yonzhan yonzhan added this to the Jan 2022 (2022-02-08) milestone Jan 6, 2022
Copy link
Member

@wangzelin007 wangzelin007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I ask you to add some tests for those new commands?

@ninpan-ms ninpan-ms closed this Jan 7, 2022
@ninpan-ms ninpan-ms reopened this Jan 7, 2022
@ninpan-ms ninpan-ms closed this Jan 7, 2022
@ninpan-ms ninpan-ms reopened this Jan 7, 2022
Comment on lines 96 to 127
c.argument('enable_application_configuration_service',
arg_type=get_three_state_flag(),
default=False,
is_preview=True,
options_list=['--enable-application-configuration-service', '--enable-acs'],
help='(Enterprise Tier Only) Enable Application Configuration Service.')
c.argument('enable_service_registry',
arg_type=get_three_state_flag(),
default=False,
is_preview=True,
options_list=['--enable-service-registry', '--enable-sr'],
help='(Enterprise Tier Only) Enable Service Registry.')
c.argument('enable_gateway',
arg_group="Spring Cloud Gateway",
arg_type=get_three_state_flag(),
default=False,
is_preview=True,
help='(Enterprise Tier Only) Enable Spring Cloud Gateway.')
c.argument('gateway_instance_count',
arg_group="Spring Cloud Gateway",
type=int,
validator=validate_gateway_instance_count,
is_preview=True,
help='(Enterprise Tier Only) Number of Spring Cloud Gateway instances.')
c.argument('enable_api_portal',
arg_group="API portal",
arg_type=get_three_state_flag(),
default=False,
is_preview=True,
help='(Enterprise Tier Only) Enable API portal.')
Copy link
Contributor

@zhoxing-ms zhoxing-ms Jan 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we consider using action='store_true' instead of get_three_state_flag() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated, thanks for suggestion!

@@ -219,6 +257,13 @@ def prepare_logs_argument(c):
c.argument('env', env_type)
c.argument('disable_probe', arg_type=get_three_state_flag(), help='If true, disable the liveness and readiness probe.')

for scope in ['update', 'deployment create', 'deploy']:
with self.argument_context('spring-cloud app {}'.format(scope)) as c:
c.argument('config_file_patterns', type=str,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type=str is the default setting, so we don't need an explicit declaration

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

Comment on lines 80 to 87
if namespace.enable_application_configuration_service:
raise InvalidArgumentValueError('--enable-application-configuration-service {}'.format(suffix))
if namespace.enable_service_registry:
raise InvalidArgumentValueError('--enable-service-registry {}'.format(suffix))
if namespace.enable_gateway:
raise InvalidArgumentValueError('--enable-gateway {}'.format(suffix))
if namespace.enable_api_portal:
raise InvalidArgumentValueError('--enable-api-portal {}'.format(suffix))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ArgumentUsageError may be the more appropriate error type, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

Comment on lines 111 to 120
if namespace.enable_gateway is False:
raise InvalidArgumentValueError("--gateway-instance-count can only be set when enable gateway.")
if namespace.gateway_instance_count < 1:
raise InvalidArgumentValueError("--gateway-instance-count must be greater than 0")


def validate_api_portal_instance_count(namespace):
if namespace.api_portal_instance_count is not None:
if namespace.enable_api_portal is False:
raise InvalidArgumentValueError("--api-portal-instance-count can only be set when enable API portal.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

@ninpan-ms ninpan-ms force-pushed the ninpan/integrate-tanzu branch 2 times, most recently from 90c1fbd to 168448b Compare January 11, 2022 04:59
ninpan-ms and others added 9 commits January 11, 2022 15:09
Co-authored-by: Yuwei Zhou <yuwzho@microsoft.com>
Co-authored-by: Yuwei Zhou <yuwzho@microsoft.com>
Co-authored-by: Yuwei Zhou <yuwzho@microsoft.com>
@zhoxing-ms zhoxing-ms merged commit 44ce91b into Azure:main Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants