Skip to content

xdsinterop: extend the ports to use#25881

Merged
menghanl merged 1 commit intogrpc:masterfrom
menghanl:xds_interop_more_ports
Apr 6, 2021
Merged

xdsinterop: extend the ports to use#25881
menghanl merged 1 commit intogrpc:masterfrom
menghanl:xds_interop_more_ports

Conversation

@menghanl
Copy link
Copy Markdown
Contributor

@menghanl menghanl commented Apr 5, 2021

This is to add more ports for forwarding-rule.

It's in theory not necessary, because forwarding-rule doesn't need to use the
same port as the services. This is a limitation of the test framework, and can
be fixed in the future.

This is to add more ports for forwarding-rule.

It's in theory not necessary, because forwarding-rule doesn't need to use the
same port as the services. This is a limitation of the test framework, and can
be fixed in the future.
@menghanl menghanl changed the title Extend the ports to use xdsinterop: extend the ports to use Apr 5, 2021
@menghanl menghanl requested a review from lidizheng April 5, 2021 21:19
@menghanl menghanl added the release notes: no Indicates if PR should not be in release notes label Apr 5, 2021
@lidizheng lidizheng self-assigned this Apr 5, 2021
Copy link
Copy Markdown
Contributor

@lidizheng lidizheng left a comment

Choose a reason for hiding this comment

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

Looks good, but why do we change tools/internal_ci/linux/grpc_python_bazel_test.cfg?

build_file: "grpc/tools/internal_ci/linux/grpc_bazel.sh"
timeout_mins: 240
build_file: "grpc/tools/internal_ci/linux/grpc_xds.sh"
timeout_mins: 180
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This changes doesn't seem right. Maybe you wanted to create a new .cfg file?

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.

This is to hijack the python test to run the xds tests. Will revert the commit later.

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.

This is reverted.

help='GCP network to use')
argp.add_argument('--service_port_range',
default='8080:8110',
default='8080:8280',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

optional: we can expand the port range to a much larger numbers. E.g., 1k? 50k?

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.

We can only use the ports allowed by the pre-configured firewall rule.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh, TIL. In that case, looks good.

Copy link
Copy Markdown
Contributor

@lidizheng lidizheng left a comment

Choose a reason for hiding this comment

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

Bazel Python passed, the test cases looks fine.

@menghanl menghanl force-pushed the xds_interop_more_ports branch from a130edb to 58c2b9b Compare April 6, 2021 00:28
@menghanl menghanl merged commit 239a440 into grpc:master Apr 6, 2021
@menghanl menghanl deleted the xds_interop_more_ports branch April 6, 2021 23:53
menghanl added a commit that referenced this pull request Apr 7, 2021
This is to add more ports for forwarding-rule.

It's in theory not necessary, because forwarding-rule doesn't need to use the
same port as the services. This is a limitation of the test framework, and can
be fixed in the future.
menghanl added a commit to menghanl/grpc that referenced this pull request Apr 7, 2021
This is to add more ports for forwarding-rule.

It's in theory not necessary, because forwarding-rule doesn't need to use the
same port as the services. This is a limitation of the test framework, and can
be fixed in the future.
menghanl added a commit to menghanl/grpc that referenced this pull request Apr 7, 2021
This is to add more ports for forwarding-rule.

It's in theory not necessary, because forwarding-rule doesn't need to use the
same port as the services. This is a limitation of the test framework, and can
be fixed in the future.
menghanl added a commit to menghanl/grpc that referenced this pull request Apr 7, 2021
This is to add more ports for forwarding-rule.

It's in theory not necessary, because forwarding-rule doesn't need to use the
same port as the services. This is a limitation of the test framework, and can
be fixed in the future.
menghanl added a commit to menghanl/grpc that referenced this pull request Apr 7, 2021
This is to add more ports for forwarding-rule.

It's in theory not necessary, because forwarding-rule doesn't need to use the
same port as the services. This is a limitation of the test framework, and can
be fixed in the future.
menghanl added a commit to menghanl/grpc that referenced this pull request Apr 7, 2021
This is to add more ports for forwarding-rule.

It's in theory not necessary, because forwarding-rule doesn't need to use the
same port as the services. This is a limitation of the test framework, and can
be fixed in the future.
menghanl added a commit to menghanl/grpc that referenced this pull request Apr 7, 2021
This is to add more ports for forwarding-rule.

It's in theory not necessary, because forwarding-rule doesn't need to use the
same port as the services. This is a limitation of the test framework, and can
be fixed in the future.
menghanl added a commit to menghanl/grpc that referenced this pull request Apr 7, 2021
This is to add more ports for forwarding-rule.

It's in theory not necessary, because forwarding-rule doesn't need to use the
same port as the services. This is a limitation of the test framework, and can
be fixed in the future.
menghanl added a commit to menghanl/grpc that referenced this pull request Apr 7, 2021
This is to add more ports for forwarding-rule.

It's in theory not necessary, because forwarding-rule doesn't need to use the
same port as the services. This is a limitation of the test framework, and can
be fixed in the future.
menghanl added a commit to menghanl/grpc that referenced this pull request Apr 7, 2021
This is to add more ports for forwarding-rule.

It's in theory not necessary, because forwarding-rule doesn't need to use the
same port as the services. This is a limitation of the test framework, and can
be fixed in the future.
srini100 pushed a commit that referenced this pull request Apr 8, 2021
This is to add more ports for forwarding-rule.

It's in theory not necessary, because forwarding-rule doesn't need to use the
same port as the services. This is a limitation of the test framework, and can
be fixed in the future.
srini100 pushed a commit that referenced this pull request Apr 8, 2021
This is to add more ports for forwarding-rule.

It's in theory not necessary, because forwarding-rule doesn't need to use the
same port as the services. This is a limitation of the test framework, and can
be fixed in the future.
srini100 pushed a commit that referenced this pull request Apr 8, 2021
This is to add more ports for forwarding-rule.

It's in theory not necessary, because forwarding-rule doesn't need to use the
same port as the services. This is a limitation of the test framework, and can
be fixed in the future.
srini100 pushed a commit that referenced this pull request Apr 8, 2021
This is to add more ports for forwarding-rule.

It's in theory not necessary, because forwarding-rule doesn't need to use the
same port as the services. This is a limitation of the test framework, and can
be fixed in the future.
srini100 pushed a commit that referenced this pull request Apr 8, 2021
This is to add more ports for forwarding-rule.

It's in theory not necessary, because forwarding-rule doesn't need to use the
same port as the services. This is a limitation of the test framework, and can
be fixed in the future.
srini100 pushed a commit that referenced this pull request Apr 8, 2021
This is to add more ports for forwarding-rule.

It's in theory not necessary, because forwarding-rule doesn't need to use the
same port as the services. This is a limitation of the test framework, and can
be fixed in the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes: no Indicates if PR should not be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants