-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Monitoring: add VPCSC system test. #7791
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
Conversation
|
|
||
| @staticmethod | ||
| def _do_test(delayed_inside, delayed_outside): | ||
| if ('GOOGLE_CLOUD_TESTS_IN_VPCSC' in os.environ) and (os.environ['GOOGLE_CLOUD_TESTS_IN_VPCSC'].lower() == 'True'.lower()): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can just write os.environ.get('GOOGLE_CLOUD_TESTS_IN_VPCSC', 'false').lower() == 'true'.
tseaver
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These new tests are all being skipped under CI. We should probably work out how to get them enabled before merging.
|
Hi @tseaver, the VPC team has their own Kokoro workflow to test this. I believe @bmccutchon knows more. Otherwise, if you'd like to run this test in the pull request workflow, please supply these environment variables: |
|
@tseaver It doesn't seem to me to make sense to have the tests maintained in a separate repository from the location where the CI runs. @busunkim96, @crwilcox please chime in. |
|
This is a requirement from the VPC team. Many teams are going to implement such tests. I have merged a similar one here: googleapis/google-cloud-java#5006, and the Spanner team has done it here https://github.com/googleapis/google-cloud-java/blob/master/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITVPCNegativeTest.java. |
|
@tseaver Echoing what @steinwaywhw said, we expect the client library teams to write and maintain these tests. The easiest way to do that is to keep them in this repo. I agree that it would make sense to have the CI workflow also be in this repo or run against affected pull requests, but that isn't feasible quite yet. |
|
@bmccutchon Would it make sense for us to run these tests against PRs in this repo, or is there some limitation that would keep us from doing that? Are there requirements beyond setting the environment variables @steinwaywhw mentioned? We would probably edit the nox session here to set the required environment variables. |
|
Hi @busunkim96, there are essentially two ways to run these tests: 1) from inside VPCSC, 2) from outside VPCSC. For case 1, you need to setup VPCSC environments, which is non-trivial. The VPC team has a setup, and they can run it. For case 2, you do not need a VPCSC setup, and the tests can be run by simply setting those two environment variables, and the test case still provides valuable informations on the effectiveness of VPC service control. |
|
Friendly ping 👋 |
|
Merging this now. I will create a follow-up PR to set the environment variables to make the tests run. |
|
@busunkim96 Thank you very much! If you have further questions, feel free to reach out to me. |
This new file is added in order to test client lib compatibility of VPC SC. The tests are auto-generated from the service definition of the monitoring API. The tests can be run inside or outside of VPC service perimeter. The input to the script should be the following environment variables.