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 2021/05/17 21:00:04 UTC
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5857: CIAB: Use postgres as the default traffic vault backend
zrhoffman commented on a change in pull request #5857:
URL: https://github.com/apache/trafficcontrol/pull/5857#discussion_r633857923
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/run-go.sh
##########
@@ -138,11 +143,13 @@ cd /opt/traffic_ops/app;
CDNCONF=/opt/traffic_ops/app/conf/cdn.conf
DBCONF=/opt/traffic_ops/app/conf/production/database.conf
RIAKCONF=/opt/traffic_ops/app/conf/production/riak.conf
+TV_DBCONF=/opt/traffic_ops/app/conf/production/tv.conf
mkdir -p /var/log/traffic_ops
touch "$TO_LOG_ERROR" "$TO_LOG_WARNING" "$TO_LOG_INFO" "$TO_LOG_DEBUG" "$TO_LOG_EVENT"
tail -qf "$TO_LOG_ERROR" "$TO_LOG_WARNING" "$TO_LOG_INFO" "$TO_LOG_DEBUG" "$TO_LOG_EVENT" &
-traffic_ops_golang_command=(./bin/traffic_ops_golang -cfg "$CDNCONF" -dbcfg "$DBCONF" -riakcfg "$RIAKCONF");
+traffic_ops_golang_command=(./bin/traffic_ops_golang -cfg "$CDNCONF" -dbcfg "$DBCONF");
+#-riakcfg "$RIAKCONF");
Review comment:
Can this commented line be removed?
##########
File path: traffic_ops/install/bin/_postinstall
##########
@@ -1004,6 +1018,17 @@ def generate_cdn_conf(questions, fname, automatic, root): # type: (list[Question
json.dump(existing_conf, conf_file, indent=indent)
print(file=conf_file)
logging.info("CDN configuration has been saved")
+ try:
+ traffic_vault_backend = existing_conf["traffic_ops_golang"]["traffic_vault_backend"]
+ except KeyError as e:
+ exception = ValueError("missing 'traffic_vault_backend' config_var")
+ exception.__cause__ = e
+ logging.warn("no traffic vault backend configured, using default postgres")
+
+ if traffic_vault_backend == "postgres":
+ return True
+ else:
+ return False
Review comment:
This can be `return traffic_vault_backend == "postgres"`
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/config.sh
##########
@@ -76,6 +81,19 @@ cdn_conf=/opt/traffic_ops/app/conf/cdn.conf
},
"use_ims": true,
"traffic_ops_golang" : {
+ "traffic_vault_backend": "$TV_BACKEND",
+ "traffic_vault_config": {
+ "dbname": "$TV_DB_NAME",
+ "hostname": "$TV_DB_SERVER.$INFRA_SUBDOMAIN.$TLD_DOMAIN",
+ "user": "$TV_DB_USER",
+ "password": "$TV_DB_USER_PASS",
+ "port": ${TV_DB_PORT:-5432},
+ "ssl": false,
+ "conn_max_lifetime_seconds": 60,
+ "max_connections": 500,
+ "max_idle_connections": 30,
+ "query_timeout_seconds": 10
Review comment:
`"query_timeout_seconds"` should be set to `${DEBUGGING_TIMEOUT:-60}` so debugging sessions don't time out.
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/config.sh
##########
@@ -76,6 +81,19 @@ cdn_conf=/opt/traffic_ops/app/conf/cdn.conf
},
"use_ims": true,
"traffic_ops_golang" : {
+ "traffic_vault_backend": "$TV_BACKEND",
+ "traffic_vault_config": {
+ "dbname": "$TV_DB_NAME",
+ "hostname": "$TV_DB_SERVER.$INFRA_SUBDOMAIN.$TLD_DOMAIN",
+ "user": "$TV_DB_USER",
+ "password": "$TV_DB_USER_PASS",
+ "port": ${TV_DB_PORT:-5432},
+ "ssl": false,
Review comment:
```json
"ssl": false,
```
is already in `cdn.conf`, no need to specify here
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/config.sh
##########
@@ -32,9 +32,14 @@
# TO_HOST
# TO_PORT
# TP_HOST
+# TV_DB_USER
+# TV_DB_NAME
+# TV_DB_SERVER
+# TV_DB_USER_PASS
+# TV_DB_PORT
Review comment:
Can these be alphabetized? Some of the rest of the list is also unsorted, might as well sort it all.
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/config.sh
##########
@@ -76,6 +81,19 @@ cdn_conf=/opt/traffic_ops/app/conf/cdn.conf
},
"use_ims": true,
"traffic_ops_golang" : {
+ "traffic_vault_backend": "$TV_BACKEND",
+ "traffic_vault_config": {
+ "dbname": "$TV_DB_NAME",
+ "hostname": "$TV_DB_SERVER.$INFRA_SUBDOMAIN.$TLD_DOMAIN",
+ "user": "$TV_DB_USER",
+ "password": "$TV_DB_USER_PASS",
+ "port": ${TV_DB_PORT:-5432},
+ "ssl": false,
+ "conn_max_lifetime_seconds": 60,
Review comment:
`"conn_max_lifetime_seconds"` should be set to `${DEBUGGING_TIMEOUT:-60}` so debugging sessions don't time out.
##########
File path: infrastructure/cdn-in-a-box/traffic_ops/run-go.sh
##########
@@ -30,16 +30,21 @@
# ADMIN_USER
# ADMIN_PASS
# DOMAIN
+# TV_DB_USER
+# TV_DB_NAME
+# TV_BACKEND
+# TV_DB_SERVER
+# TV_DB_USER_PASS
+# TV_DB_PORT
Review comment:
Same here, environment variables should be sorted.
##########
File path: traffic_ops/install/bin/_postinstall
##########
@@ -1004,6 +1018,17 @@ def generate_cdn_conf(questions, fname, automatic, root): # type: (list[Question
json.dump(existing_conf, conf_file, indent=indent)
print(file=conf_file)
logging.info("CDN configuration has been saved")
+ try:
+ traffic_vault_backend = existing_conf["traffic_ops_golang"]["traffic_vault_backend"]
+ except KeyError as e:
+ exception = ValueError("missing 'traffic_vault_backend' config_var")
+ exception.__cause__ = e
+ logging.warn("no traffic vault backend configured, using default postgres")
Review comment:
The `exception` built here should be raised, the warning is not sufficient.
--
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