Skip to content

Support spring cloud vnet injection - #1709

Merged
Feng Zhou (fengzhou-msft) merged 33 commits into
Azure:masterfrom
VSChina:yuwzho/vnet
Aug 12, 2020
Merged

Feng Zhou (fengzhou-msft) merged 33 commits into
Azure:masterfrom
VSChina:yuwzho/vnet

Conversation

@yuwzho

@yuwzho Yuwei Zhou (yuwzho) commented May 15, 2020

Copy link
Copy Markdown
Contributor

This PR supports creating a VNet-injected Azure Spring Cloud service. It add the following parameters in az spring-cloud create

  • vnet (OPTIONAL): User can specify a name or resource id of an existing VNet.
    • If only a name given, it will format the resource ID using ASC's subscription id and resource group.
    • If app-subnet and service-runtime-subnet are both provided, this field can be omitted.
  • app-subnet (REQUIRED): User can specify a name or subnet resource id. If only a name given, the vnet must be provided.
    • Cannot be a same one as service-runtime-subnet.
  • app-network-resource-group(OPTIONAL): A resource group where all network resources related to app will be created in.
  • service-runtime-subnet(REQUIRED): User can specify a name or subnet resource id. If only a name given, the vnet must be provided.
    • Cannot be a same one as app-subnet.
  • service-runtime-network-resource-group(OPTIONAL): A resource group where all network resources related to service runtime will be created in.
  • reserved-cidr-range(OPTIONAL): Specify 3 unused /16 IP ranges (e.g. 10.244.0.0/16,10.244.2.0/16,10.244.3.0/16), which are not overlapped with any network elements in the whole customer’s network environment. Azure Spring Cloud uses the IP ranges for hosting underlying infrastructure.
    If user doesn't set this value, CLI should parse VNet's ip_addresses, and set the value to 3 CIDR ranges non-overlap with vnet.address_space.ip_addresses. If the vnet's ip addresses do not have such space left, CLI should raise an error and ask for the user input. This set operation requires NetworkClient in validator.

Here add validator to validate:

  1. Two subnets must in the same VNet
  2. Two subnets must not be the same subnet.
  3. CIDR range must has 3 /16 IP ranges.
  4. If CIDR range is 3 IP ranges, validate they DO NOT overlap with each other.
  5. CIDR is valid IPv4 network.

It doesn't validate:

  1. Whether the subnets are exist or not
  2. Whether the subnets are used or not
  3. CIDR are used or not.

This checklist is used to make sure that common guidelines for a pull request are followed.

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.

@azuresdkci

Copy link
Copy Markdown

If this PR is for a new extension or change to an existing extension, use the following to try out the changes in this PR:

docker run -it microsoft/azure-cli:latest
export EXT=<NAME>
pip install --upgrade --target ~/.azure/cliextensions/$EXT "git+https://github.com/VSChina/azure-cli-extensions.git@yuwzho/vnet#subdirectory=src/$EXT&egg=$EXT"

Comment thread src/spring-cloud/azext_spring_cloud/_params.py Outdated
@yonzhan

Copy link
Copy Markdown
Collaborator

spring cloud

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

hi Xiaojian Xu (@arrownj) does the global VNET in core could be leveraged here? Could service commands customize the argument name in their command?

@yuwzho Yuwei Zhou (yuwzho) May 18, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Let me describe our API for vnet-injected here, so that Xiaojian Xu (@arrownj) can have a better understanding of it.

If user wants to create a vnet-injected spring cloud, he needs to specify the properties in API level

spring: {
  NetworkProfile: {
    serviceRuntimeSubnetId: <a subnet resource id we say subnet-A>,
    appSubnetId: <another subnet resource id, we say subnet-B>,
    serviceCidr: <1 or 3 cidr range which has prefix requirement>
  }
}

The subnet-A and subnet-B must be in the same VNet. Thus I define the CLI interface for subent-A and subnet-B with 3 parameters, to help user copy-paste the resource id, or do it only once.

  • app-subnet, service-runtime-subnet: Can give a name only if vnet specified, and then can format the resource id. Or given the resource id directly.
  • vnet: If a name, format the resource id in the spring-cloud's resource group. Or a given resource id.
    This can be omitted if the 2 subnets's resource id are given.

Comment thread src/spring-cloud/azext_spring_cloud/_validators.py Outdated
Comment thread src/spring-cloud/azext_spring_cloud/_validators.py Outdated
Comment thread src/spring-cloud/azext_spring_cloud/_params.py Outdated
@yuwzho

Copy link
Copy Markdown
Contributor Author

rebase 0.2.4 version

@yuwzho

Copy link
Copy Markdown
Contributor Author

rebase 0.3.0 version

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor issue, suggest to use service_runtime_vnet_resource_group to be specific.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here we keep the wording the same as portal

Comment thread src/spring-cloud/azext_spring_cloud/_validators.py Outdated
Comment thread src/spring-cloud/azext_spring_cloud/_validators.py Outdated
@yungezz

Copy link
Copy Markdown
Member

LGTM

@mmyyrroonn

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@fengzhou-msft

Copy link
Copy Markdown
Member

What's the status of this PR? Is it ready for review?

@yungezz Yunge Zhu (yungezz) removed this from the S170 milestone Aug 5, 2020
@yungezz

Copy link
Copy Markdown
Member

Yuchen Wang (@yucwan) what's next step plan of this PR? will someone else pick it up?

@yucwan

Copy link
Copy Markdown

Yuwei Zhou (@yuwzho) As VNet API is already in production. Shall we proceed and merge this pr now?

Yuwei Zhou (yuwzho) and others added 21 commits August 10, 2020 13:08
* Check vnet role assignment

* Resolve comments

* Use app id instead of spn

* Add link in warning
#6)

* update sdk

* add support for vnet

* lint check

* add overlap validation

* add resource id validation

* add test

* update wording

* add example

* update log streaming endpoint

* update logstreaming endpoint

* fix properties

* fix lint

* make reserved cidr range optional

* fix lint

* calculate cidr range from vnet spaces

* vendor sdk

* add two  resource group

* blacklist some ip ranges

* exclude linter

* Rename src/spring-cloud/azext_spring_cloud/linter_exclusions.yml to src/spring-cloud/linter_exclusions.yml

* fix name

* fix regex

* fix validation and can create normal service instance

* update parameters

* Subnets associated with any route table are not accepted for vnet case

* Fetch vnet instead of subnets

* Resolve comments

* Add ut

* Fix style check

Co-authored-by: yuwzho <yuwzho@microsoft.com>
@yuwzho

Copy link
Copy Markdown
Contributor Author

Yuwei Zhou (@yuwzho) As VNet API is already in production. Shall we proceed and merge this pr now?

I will publish this draft once the backend full ready.

@yuwzho
Yuwei Zhou (yuwzho) marked this pull request as ready for review August 12, 2020 03:51
@yuwzho

Copy link
Copy Markdown
Contributor Author

What's the status of this PR? Is it ready for review?

Feng Zhou (@fengzhou-msft) Just published the PR, please help me to review it. Thanks.

Comment thread src/spring-cloud/azext_spring_cloud/_validators.py Outdated
Co-authored-by: Feng Zhou <55177366+fengzhou-msft@users.noreply.github.com>
@fengzhou-msft
Feng Zhou (fengzhou-msft) merged commit 58e3e4b into Azure:master Aug 12, 2020
@yuwzho
Yuwei Zhou (yuwzho) deleted the yuwzho/vnet branch August 13, 2020 02:01
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.

8 participants