You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2020/10/19 15:57:53 UTC
[GitHub] [trafficcontrol] zrhoffman opened a new pull request #5168: Enable includeSystemTests in the API tests GitHub Action
zrhoffman opened a new pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168
<!--
************ STOP!! ************
If this Pull Request is intended to fix a security vulnerability, DO NOT submit it! Instead, contact
the Apache Software Foundation Security Team at security@trafficcontrol.apache.org and follow the
guidelines at https://www.apache.org/security/ regarding vulnerability disclosure.
-->
## What does this PR (Pull Request) do?
<!-- Explain the changes you made here. If this fixes an Issue, identify it by
replacing the text in the checkbox item with the Issue number e.g.
- [x] This PR fixes #9001 OR is not related to any Issue
^ This will automatically close Issue number 9001 when the Pull Request is
merged (The '#' is important).
Be sure you check the box properly, see the "The following criteria are ALL
met by this PR" section for details.
-->
This PR
- [x] is not related to any Issue <!-- You can check for an issue here: https://github.com/apache/trafficcontrol/issues -->
- adds an SMTP server to the Traffic Ops API tests GitHub Actions workflow
- adds a Riak server to the Traffic Ops API tests GitHub Action
- Enables the `"includeSystemTests"` in the Traffic Ops Test config used by the Traffic Ops API tests GitHub Action
## Which Traffic Control components are affected by this PR?
<!-- Please delete all components from this list that are NOT affected by this
Pull Request. Also, feel free to add the name of a tool or script that is
affected but not on the list.
Additionally, if this Pull Request does NOT affect documentation, please
explain why documentation is not required. -->
- Traffic Ops (only API tests fixtures changed)
- CI tests
## What is the best way to verify this PR?
<!-- Please include here ALL the steps necessary to test your Pull Request. If
it includes tests (and most should), outline here the steps needed to run the
tests. If not, lay out the manual testing procedure and please explain why
tests are unnecessary for this Pull Request. -->
- Check the result of the *Traffic Ops Go client/API integration tests* GitHub Action
- Look at `.github/actions/to-integration-tests/README.md` and check if it still makes sense
## The following criteria are ALL met by this PR
<!-- Check the boxes to signify that the associated statement is true. To
"check a box", replace the space inside of the square brackets with an 'x'.
e.g.
- [ x] <- Wrong
- [x ] <- Wrong
- [] <- Wrong
- [*] <- Wrong
- [x] <- Correct!
-->
- [x] This PR includes tests
- [x] This PR includes documentation
- [x] Affects only our CI tests, no changelog entry necessary
- [x] This PR includes any and all required license headers
- [x] This PR does not include a database migration
- [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://www.apache.org/security/) for details)
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] shamrickus commented on a change in pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
shamrickus commented on a change in pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#discussion_r508936305
##########
File path: .github/actions/to-integration-tests/entrypoint.sh
##########
@@ -26,6 +26,46 @@ download_go() {
}
download_go
+ciab_dir="${GITHUB_WORKSPACE}/infrastructure/cdn-in-a-box";
+trafficvault=trafficvault;
+start_traffic_vault() {
+<<-'BASH_LINES' cat >infrastructure/cdn-in-a-box/traffic_vault/prestart.d/00-0-standalone-config.sh;
+ TV_FQDN="${TV_HOST}.${INFRA_SUBDOMAIN}.${TLD_DOMAIN}" # Also used in 02-add-search-schema.sh
+ certs_dir=/etc/ssl/certs;
+ X509_INFRA_CERT_FILE="${certs_dir}/trafficvault.crt";
+ X509_INFRA_KEY_FILE="${certs_dir}/trafficvault.key";
+
+ # Generate x509 certificate
+ openssl req -new -x509 -nodes -newkey rsa:4096 -out "$X509_INFRA_CERT_FILE" -keyout "$X509_INFRA_KEY_FILE" -subj "/CN=${TV_FQDN}";
+
+ # Do not wait for CDN in a Box to generate SSL keys
+ sed -i '0,/^update-ca-certificates/d' /etc/riak/prestart.d/00-config.sh;
+
+ # Do not try to source to-access.sh
+ sed -i '/to-access\.sh/d' /etc/riak/{prestart.d,poststart.d}/*
+ BASH_LINES
+
+ DOCKER_BUILDKIT=1 docker build "$ciab_dir" -f "${ciab_dir}/traffic_vault/Dockerfile" -t "$trafficvault";
+ network="$(docker network ls --quiet --filter name=github_network)";
+ if [[ "$INPUT_VERSION" -lt 3 ]]; then
Review comment:
Looking at the logs for the V1/V2 tests (specifically the TO error logs):
```
ERROR: riak_services.go:374: 2020-10-19T17:24:58.753104875Z: GetPooledCluster failed, returning nil cluster
ERROR: deleteoldcerts.go:112: 2020-10-19T17:24:58.753126176Z: deleting old DS certificates: getting riak ds keys: with cluster error: getting riak pooled cluster: GetPooledCluster unable to return cluster
ERROR: riak_services.go:374: 2020-10-19T17:24:58.894709833Z: GetPooledCluster failed, returning nil cluster
```
Perhaps it would be good to have it enabled for old versions too? It's not causing any test failures, but those might be hiding legitimate regression bugs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#discussion_r509535557
##########
File path: .github/actions/to-integration-tests/entrypoint.sh
##########
@@ -26,6 +26,46 @@ download_go() {
}
download_go
+ciab_dir="${GITHUB_WORKSPACE}/infrastructure/cdn-in-a-box";
+trafficvault=trafficvault;
+start_traffic_vault() {
+<<-'BASH_LINES' cat >infrastructure/cdn-in-a-box/traffic_vault/prestart.d/00-0-standalone-config.sh;
+ TV_FQDN="${TV_HOST}.${INFRA_SUBDOMAIN}.${TLD_DOMAIN}" # Also used in 02-add-search-schema.sh
+ certs_dir=/etc/ssl/certs;
+ X509_INFRA_CERT_FILE="${certs_dir}/trafficvault.crt";
+ X509_INFRA_KEY_FILE="${certs_dir}/trafficvault.key";
+
+ # Generate x509 certificate
+ openssl req -new -x509 -nodes -newkey rsa:4096 -out "$X509_INFRA_CERT_FILE" -keyout "$X509_INFRA_KEY_FILE" -subj "/CN=${TV_FQDN}";
+
+ # Do not wait for CDN in a Box to generate SSL keys
+ sed -i '0,/^update-ca-certificates/d' /etc/riak/prestart.d/00-config.sh;
+
+ # Do not try to source to-access.sh
+ sed -i '/to-access\.sh/d' /etc/riak/{prestart.d,poststart.d}/*
+ BASH_LINES
+
+ DOCKER_BUILDKIT=1 docker build "$ciab_dir" -f "${ciab_dir}/traffic_vault/Dockerfile" -t "$trafficvault";
+ network="$(docker network ls --quiet --filter name=github_network)";
+ if [[ "$INPUT_VERSION" -lt 3 ]]; then
Review comment:
Having TV enabled the whole time would add 4-5 minutes to the workflow because
* Right now, it builds Traffic Vault while running the API tests
* It starts Traffic Vault in the background while running the API tests andit sort works out that it's up and running before it's used by `TestDeliveryServices` in v3
While log messages are useful, if there really is a bug related to that, ideally we would have an API for it instead of relying on scanning the logs.
One thing that makes scanning the logs trickier is that the Traffic Ops error log is only shown after the tests have finished running, so seeing log messages that correspond with certain points in the tests is not straightforward. I have changed 2 things in 1db99ec630 to make seeing Traffic Ops errors and warnings easier:
* Now the logs show up in real time instead of after tests have run
* Traffic Ops warnings are shown with a red background and warnings are shown with a yellow background, so they are easy to spot (not sure why, but I don't see any TO warnings anyway).
Will that work?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#discussion_r509720737
##########
File path: .github/actions/to-integration-tests/dist/to-integration-test.js
##########
@@ -0,0 +1,31 @@
+/*
Review comment:
Removed in 9bee219470
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman edited a comment on pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
zrhoffman edited a comment on pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#issuecomment-717360609
Changed tabs to spaces in c10adf023b.
Since `"strict"` is already set in `tsconfig.json`, `"strictNullChecks"` is [already set as well](https://github.com/microsoft/TypeScript-Website/commit/66f17ad4d0fa6b2b9fee791b0b7a729710aeb907#diff-c61f3a65b5d5f3b9d067e04829015516162b7aa9f9ececb29e3c138f0dd873caR86
).
> `--strict` | `boolean` | `false` | Enable all strict type checking options. <br/>Enabling `--strict` enables `--noImplicitAny`, `--noImplicitThis`, `--alwaysStrict`, `--strictBindCallApply`, `--strictNullChecks`, `--strictFunctionTypes` and `--strictPropertyInitialization`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 merged pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
ocket8888 merged pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] shamrickus commented on pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
shamrickus commented on pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#issuecomment-714593856
Could you add `.github/actions/to-integration-tests/node_modules` to the `.gitignore`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#discussion_r509717485
##########
File path: .github/actions/to-integration-tests/dist/to-integration-test.js
##########
@@ -0,0 +1,31 @@
+/*
Review comment:
Yes. TypeScript actions typically commit the transpiled JS file, the action itself is not responsible for transpiling.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#discussion_r509535557
##########
File path: .github/actions/to-integration-tests/entrypoint.sh
##########
@@ -26,6 +26,46 @@ download_go() {
}
download_go
+ciab_dir="${GITHUB_WORKSPACE}/infrastructure/cdn-in-a-box";
+trafficvault=trafficvault;
+start_traffic_vault() {
+<<-'BASH_LINES' cat >infrastructure/cdn-in-a-box/traffic_vault/prestart.d/00-0-standalone-config.sh;
+ TV_FQDN="${TV_HOST}.${INFRA_SUBDOMAIN}.${TLD_DOMAIN}" # Also used in 02-add-search-schema.sh
+ certs_dir=/etc/ssl/certs;
+ X509_INFRA_CERT_FILE="${certs_dir}/trafficvault.crt";
+ X509_INFRA_KEY_FILE="${certs_dir}/trafficvault.key";
+
+ # Generate x509 certificate
+ openssl req -new -x509 -nodes -newkey rsa:4096 -out "$X509_INFRA_CERT_FILE" -keyout "$X509_INFRA_KEY_FILE" -subj "/CN=${TV_FQDN}";
+
+ # Do not wait for CDN in a Box to generate SSL keys
+ sed -i '0,/^update-ca-certificates/d' /etc/riak/prestart.d/00-config.sh;
+
+ # Do not try to source to-access.sh
+ sed -i '/to-access\.sh/d' /etc/riak/{prestart.d,poststart.d}/*
+ BASH_LINES
+
+ DOCKER_BUILDKIT=1 docker build "$ciab_dir" -f "${ciab_dir}/traffic_vault/Dockerfile" -t "$trafficvault";
+ network="$(docker network ls --quiet --filter name=github_network)";
+ if [[ "$INPUT_VERSION" -lt 3 ]]; then
Review comment:
Having TV enabled the whole time would add 4-5 minutes to the workflow because
* Right now, it builds Traffic Vault while running the API tests
* It starts Traffic Vault in the background while running the API tests andit sort works out that it's up and running before it's used by `TestDeliveryServices` in v3
While log messages are useful, if there really is a bug related to that, ideally we would have an API for it instead of relying on scanning the logs.
One thing that makes scanning the logs trickier is that the Traffic Ops error log is only shown after the tests have finished running, so seeing log messages that correspond with certain points in the tests is not straightforward. I have changed 2 things in 9c915d9eb5 to make seeing Traffic Ops errors and warnings easier:
* Now the logs show up in real time instead of after tests have run
* Traffic Ops warnings are shown with a red background and warnings are shown with a yellow background, so they are easy to spot (not sure why, but I don't see any TO warnings anyway).
Will that work?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#issuecomment-717360609
Changed tabs to spaces in c10adf023b.
Since `"strict"` is already set in `tsconfig.json`, `"strictNullChecks"` is [already set as well](https://github.com/microsoft/TypeScript-Website/commit/66f17ad4d0fa6b2b9fee791b0b7a729710aeb907#diff-c61f3a65b5d5f3b9d067e04829015516162b7aa9f9ececb29e3c138f0dd873caR86
).
> --strict` | `boolean` | `false` | Enable all strict type checking options. <br/>Enabling `--strict` enables `--noImplicitAny`, `--noImplicitThis`, `--alwaysStrict`, `--strictBindCallApply`, `--strictNullChecks`, `--strictFunctionTypes` and `--strictPropertyInitialization`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman edited a comment on pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
zrhoffman edited a comment on pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#issuecomment-717360609
Using tabs for indentation in c10adf023b.
Since `"strict"` is already set in `tsconfig.json`, `"strictNullChecks"` is [already set as well](https://github.com/microsoft/TypeScript-Website/commit/66f17ad4d0fa6b2b9fee791b0b7a729710aeb907#diff-c61f3a65b5d5f3b9d067e04829015516162b7aa9f9ececb29e3c138f0dd873caR86
).
> Option | Type | Default | Description
> -----------------------------------------------|-----------|--------------------------------|----------------------------------------------------------------------
> `--strict` | `boolean` | `false` | Enable all strict type checking options. <br/>Enabling `--strict` enables `--noImplicitAny`, `--noImplicitThis`, `--alwaysStrict`, `--strictBindCallApply`, `--strictNullChecks`, `--strictFunctionTypes` and `--strictPropertyInitialization`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman edited a comment on pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
zrhoffman edited a comment on pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#issuecomment-717360609
Using tabs for indentation in c10adf023b.
Since `"strict"` is already set in `tsconfig.json`, `"strictNullChecks"` is [already set as well](https://github.com/microsoft/TypeScript-Website/commit/66f17ad4d0fa6b2b9fee791b0b7a729710aeb907#diff-c61f3a65b5d5f3b9d067e04829015516162b7aa9f9ececb29e3c138f0dd873caR86
).
> `--strict` | `boolean` | `false` | Enable all strict type checking options. <br/>Enabling `--strict` enables `--noImplicitAny`, `--noImplicitThis`, `--alwaysStrict`, `--strictBindCallApply`, `--strictNullChecks`, `--strictFunctionTypes` and `--strictPropertyInitialization`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#issuecomment-712299627
Rebased onto master
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] shamrickus commented on a change in pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
shamrickus commented on a change in pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#discussion_r508936305
##########
File path: .github/actions/to-integration-tests/entrypoint.sh
##########
@@ -26,6 +26,46 @@ download_go() {
}
download_go
+ciab_dir="${GITHUB_WORKSPACE}/infrastructure/cdn-in-a-box";
+trafficvault=trafficvault;
+start_traffic_vault() {
+<<-'BASH_LINES' cat >infrastructure/cdn-in-a-box/traffic_vault/prestart.d/00-0-standalone-config.sh;
+ TV_FQDN="${TV_HOST}.${INFRA_SUBDOMAIN}.${TLD_DOMAIN}" # Also used in 02-add-search-schema.sh
+ certs_dir=/etc/ssl/certs;
+ X509_INFRA_CERT_FILE="${certs_dir}/trafficvault.crt";
+ X509_INFRA_KEY_FILE="${certs_dir}/trafficvault.key";
+
+ # Generate x509 certificate
+ openssl req -new -x509 -nodes -newkey rsa:4096 -out "$X509_INFRA_CERT_FILE" -keyout "$X509_INFRA_KEY_FILE" -subj "/CN=${TV_FQDN}";
+
+ # Do not wait for CDN in a Box to generate SSL keys
+ sed -i '0,/^update-ca-certificates/d' /etc/riak/prestart.d/00-config.sh;
+
+ # Do not try to source to-access.sh
+ sed -i '/to-access\.sh/d' /etc/riak/{prestart.d,poststart.d}/*
+ BASH_LINES
+
+ DOCKER_BUILDKIT=1 docker build "$ciab_dir" -f "${ciab_dir}/traffic_vault/Dockerfile" -t "$trafficvault";
+ network="$(docker network ls --quiet --filter name=github_network)";
+ if [[ "$INPUT_VERSION" -lt 3 ]]; then
Review comment:
Looking at the logs for the V1/V2 tests (specifically the TO error logs):
```
ERROR: riak_services.go:374: 2020-10-19T17:24:58.753104875Z: GetPooledCluster failed, returning nil cluster
ERROR: deleteoldcerts.go:112: 2020-10-19T17:24:58.753126176Z: deleting old DS certificates: getting riak ds keys: with cluster error: getting riak pooled cluster: GetPooledCluster unable to return cluster
```
Perhaps it would be good to have TV enabled for old versions too? It's not causing any test failures, but those might be hiding legitimate regression bugs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#discussion_r509720275
##########
File path: .github/actions/to-integration-tests/dist/to-integration-test.js
##########
@@ -0,0 +1,31 @@
+/*
Review comment:
oh
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#issuecomment-713875997
Passed on re-run. It failed before due to #5182 which is not caused by this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] shamrickus commented on a change in pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
shamrickus commented on a change in pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#discussion_r509715645
##########
File path: .github/actions/to-integration-tests/dist/to-integration-test.js
##########
@@ -0,0 +1,31 @@
+/*
Review comment:
Did you mean to commit this file?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] shamrickus commented on a change in pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
shamrickus commented on a change in pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#discussion_r508936305
##########
File path: .github/actions/to-integration-tests/entrypoint.sh
##########
@@ -26,6 +26,46 @@ download_go() {
}
download_go
+ciab_dir="${GITHUB_WORKSPACE}/infrastructure/cdn-in-a-box";
+trafficvault=trafficvault;
+start_traffic_vault() {
+<<-'BASH_LINES' cat >infrastructure/cdn-in-a-box/traffic_vault/prestart.d/00-0-standalone-config.sh;
+ TV_FQDN="${TV_HOST}.${INFRA_SUBDOMAIN}.${TLD_DOMAIN}" # Also used in 02-add-search-schema.sh
+ certs_dir=/etc/ssl/certs;
+ X509_INFRA_CERT_FILE="${certs_dir}/trafficvault.crt";
+ X509_INFRA_KEY_FILE="${certs_dir}/trafficvault.key";
+
+ # Generate x509 certificate
+ openssl req -new -x509 -nodes -newkey rsa:4096 -out "$X509_INFRA_CERT_FILE" -keyout "$X509_INFRA_KEY_FILE" -subj "/CN=${TV_FQDN}";
+
+ # Do not wait for CDN in a Box to generate SSL keys
+ sed -i '0,/^update-ca-certificates/d' /etc/riak/prestart.d/00-config.sh;
+
+ # Do not try to source to-access.sh
+ sed -i '/to-access\.sh/d' /etc/riak/{prestart.d,poststart.d}/*
+ BASH_LINES
+
+ DOCKER_BUILDKIT=1 docker build "$ciab_dir" -f "${ciab_dir}/traffic_vault/Dockerfile" -t "$trafficvault";
+ network="$(docker network ls --quiet --filter name=github_network)";
+ if [[ "$INPUT_VERSION" -lt 3 ]]; then
Review comment:
Looking at the logs for the V1/V2 tests (specifically the TO error logs):
```
ERROR: riak_services.go:374: 2020-10-19T17:24:58.753104875Z: GetPooledCluster failed, returning nil cluster
ERROR: deleteoldcerts.go:112: 2020-10-19T17:24:58.753126176Z: deleting old DS certificates: getting riak ds keys: with cluster error: getting riak pooled cluster: GetPooledCluster unable to return cluster
```
Perhaps it would be good to have it enabled for old versions too? It's not causing any test failures, but those might be hiding legitimate regression bugs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#issuecomment-714605415
> Could you add `.github/actions/to-integration-tests/node_modules` to the `.gitignore`?
Gitignored in 8d06b84b28.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] shamrickus commented on pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
shamrickus commented on pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#issuecomment-713837329
ciab-build timed out
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#issuecomment-718150517
Rebased after #5203 was closed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] shamrickus commented on a change in pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
shamrickus commented on a change in pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#discussion_r509599516
##########
File path: .github/actions/to-integration-tests/entrypoint.sh
##########
@@ -26,6 +26,46 @@ download_go() {
}
download_go
+ciab_dir="${GITHUB_WORKSPACE}/infrastructure/cdn-in-a-box";
+trafficvault=trafficvault;
+start_traffic_vault() {
+<<-'BASH_LINES' cat >infrastructure/cdn-in-a-box/traffic_vault/prestart.d/00-0-standalone-config.sh;
+ TV_FQDN="${TV_HOST}.${INFRA_SUBDOMAIN}.${TLD_DOMAIN}" # Also used in 02-add-search-schema.sh
+ certs_dir=/etc/ssl/certs;
+ X509_INFRA_CERT_FILE="${certs_dir}/trafficvault.crt";
+ X509_INFRA_KEY_FILE="${certs_dir}/trafficvault.key";
+
+ # Generate x509 certificate
+ openssl req -new -x509 -nodes -newkey rsa:4096 -out "$X509_INFRA_CERT_FILE" -keyout "$X509_INFRA_KEY_FILE" -subj "/CN=${TV_FQDN}";
+
+ # Do not wait for CDN in a Box to generate SSL keys
+ sed -i '0,/^update-ca-certificates/d' /etc/riak/prestart.d/00-config.sh;
+
+ # Do not try to source to-access.sh
+ sed -i '/to-access\.sh/d' /etc/riak/{prestart.d,poststart.d}/*
+ BASH_LINES
+
+ DOCKER_BUILDKIT=1 docker build "$ciab_dir" -f "${ciab_dir}/traffic_vault/Dockerfile" -t "$trafficvault";
+ network="$(docker network ls --quiet --filter name=github_network)";
+ if [[ "$INPUT_VERSION" -lt 3 ]]; then
Review comment:
That's fine, I don't think these particular errors are anything significant, just something that I noticed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#issuecomment-717397988
The API tests failure is legitimate. See #5203
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] shamrickus commented on a change in pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
shamrickus commented on a change in pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#discussion_r509718987
##########
File path: .github/actions/to-integration-tests/dist/to-integration-test.js
##########
@@ -0,0 +1,31 @@
+/*
Review comment:
This file is named `to-integration-test.js` there is another in dist called `to-integration-tests.js` which corresponds to the `to-integration-tests.ts` file.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5168: Enable includeSystemTests in the API tests GitHub Action
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5168:
URL: https://github.com/apache/trafficcontrol/pull/5168#discussion_r509535557
##########
File path: .github/actions/to-integration-tests/entrypoint.sh
##########
@@ -26,6 +26,46 @@ download_go() {
}
download_go
+ciab_dir="${GITHUB_WORKSPACE}/infrastructure/cdn-in-a-box";
+trafficvault=trafficvault;
+start_traffic_vault() {
+<<-'BASH_LINES' cat >infrastructure/cdn-in-a-box/traffic_vault/prestart.d/00-0-standalone-config.sh;
+ TV_FQDN="${TV_HOST}.${INFRA_SUBDOMAIN}.${TLD_DOMAIN}" # Also used in 02-add-search-schema.sh
+ certs_dir=/etc/ssl/certs;
+ X509_INFRA_CERT_FILE="${certs_dir}/trafficvault.crt";
+ X509_INFRA_KEY_FILE="${certs_dir}/trafficvault.key";
+
+ # Generate x509 certificate
+ openssl req -new -x509 -nodes -newkey rsa:4096 -out "$X509_INFRA_CERT_FILE" -keyout "$X509_INFRA_KEY_FILE" -subj "/CN=${TV_FQDN}";
+
+ # Do not wait for CDN in a Box to generate SSL keys
+ sed -i '0,/^update-ca-certificates/d' /etc/riak/prestart.d/00-config.sh;
+
+ # Do not try to source to-access.sh
+ sed -i '/to-access\.sh/d' /etc/riak/{prestart.d,poststart.d}/*
+ BASH_LINES
+
+ DOCKER_BUILDKIT=1 docker build "$ciab_dir" -f "${ciab_dir}/traffic_vault/Dockerfile" -t "$trafficvault";
+ network="$(docker network ls --quiet --filter name=github_network)";
+ if [[ "$INPUT_VERSION" -lt 3 ]]; then
Review comment:
Having TV enabled the whole time would add 4-5 minutes to the workflow because
* Right now, it builds Traffic Vault while running the API tests
* It starts Traffic Vault in the background while running the API tests andit sort works out that it's up and running before it's used by `TestDeliveryServices` in v3
While log messages are useful, if there really is a bug related to that, ideally we would have an API for it instead of relying on scanning the logs.
One thing that makes scanning the logs trickier is that the Traffic Ops error log is only shown after the tests have finished running, so seeing log messages that correspond with certain points in the tests is not straightforward. I have changed 2 things in 9c915d9eb5 to make seeing Traffic Ops errors and warnings easier:
* Now the logs show up in real time instead of after tests have run
* Traffic Ops warnings are shown with a red background and warnings are shown with a yellow, so they are easy to spot (not sure why, but I don't see any TO warnings anyway).
Will that work?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org