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 13:05:02 UTC

[GitHub] [trafficcontrol] zrhoffman opened a new pull request #5853: Use `t3c` for cache configuration in CDN in a Box

zrhoffman opened a new pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853


   <!--
   ************ 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.
   -->
   
   - [x] This PR fixes #5831 by using `t3c` to configure caching proxy servers in CDN in a Box (24e8c808fb).<!-- You can check for an issue here: https://github.com/apache/trafficcontrol/issues -->
   - Moves the Python ORT implementation to the `/experimental` directory (01d6e52d17)
   - Makes the systemctl shim used by the ORT Integration Tests start `trafficserver` (bf35702165) and adds it to CDN in a Box (cfd3bf3dd8)
   - Adds a debugging configuration for the `t3c` components in CDN in a Box (f9fdfbcfdc)
   
   ## 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. -->
   
   - CDN in a Box
   - Traffic Ops ORT (only changes `systemctl` shim used by tests)
   - CI tests (triggers the *Documentation Build* workflow when the Python ORT implementation is modified)
   
   ## 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. -->
   - Test the CDN in a Box Delivery Services
   - Try to debug a `t3c` component, set a breakpoint, ensure it hits that breakpoint
   
   ## 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] CDN in a Box is a test, no further tests necessary
   - [ ] This PR includes documentation OR I have explained why documentation is unnecessary
   - [x] This PR includes an update to CHANGELOG.md
   - [x] This PR includes any and all required license headers
   - [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] ocket8888 commented on a change in pull request #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634404450



##########
File path: cache-config/testing/docker/ort_test/systemctl.sh
##########
@@ -19,35 +19,33 @@
 
 # This is a work around for testing t3c which uses systemctl.
 # systemctl does not work in a container very well so this script
-# replaces systemctl in the container and always returns a 
+# replaces systemctl in the container and always returns a
 # sucessful result to t3c.
 
 USAGE="\nsystemctl COMMAND NAME\n"
 
-if [ -z $1 ] || [ -z $2 ]; then
-  echo -e $USAGE
+if [[ -z "$1" ]] || [[ -z "$2" ]]; then
+  echo -e "$USAGE"
   exit 0
 else
-  COMMAND=$1
-  NAME=$2
+  COMMAND="$1"
+  NAME="${2%.service}"
 fi
 
-if [ "$2" != "trafficserver" ]; then
-  echo -e "\nFailed to start ${NAME}.service: Unit not found.n"
+if [[ "$NAME" != "trafficserver" ]]; then
+  echo -e "\nFailed to start ${NAME}.service: Unit not found.\n"
   exit 0
 fi
 
-case $COMMAND in 
-  enable)
-    ;;
-  restart)
-    ;;
-  status)
-    ;;
-  start)
-    ;;
-  stop)
-    ;;
+case "$COMMAND" in
+  restart) command_found=true;;
+  start) command_found=true;;
+  status) command_found=true;;
+  stop) command_found=true;;
 esac
 
+if [[ "$command_found" == true ]]; then

Review comment:
       should this just be doing nothing if the command wasn't found?

##########
File path: infrastructure/cdn-in-a-box/cache/Dockerfile
##########
@@ -51,109 +54,84 @@ RUN dnf -y install epel-release && \
         make                    \
         numactl-libs            \
         perl                    \
-        perl-Carp               \
-        perl-constant           \
-        perl-Data-Dumper        \
-        perl-Encode             \
-        perl-Exporter           \
-        perl-File-Path          \
-        perl-File-Temp          \
-        perl-Filter             \
-        perl-Getopt-Long        \
-        perl-HTTP-Tiny          \
-        perl-libs               \
-        perl-macros             \
-        perl-parent             \
-        perl-PathTools          \
-        perl-Pod-Escapes        \
-        perl-podlators          \
-        perl-Pod-Perldoc        \
-        perl-Pod-Simple         \
-        perl-Pod-Usage          \
-        perl-Scalar-List-Utils  \
-        perl-Socket             \
-        perl-Storable           \
-        perl-Text-ParseWords    \
-        perl-threads            \
-        perl-threads-shared     \
-        perl-Time-HiRes         \
-        perl-Time-Local         \
-        perl-URI                \
         tcl                     \
         $additional_packages && \
     if [[ "${RHEL_VERSION%%.*}" -eq 8 ]]; then \
-        set -- \
-            # Pretend that we have the right library versions.
-            # TODO: Use a proper CentOS 7 or 8 RPM once trafficserver
-            # is in EPEL again (see apache/trafficserver#6855)
-            libtcl8.6.so        libtcl8.5.so     \
-            libncursesw.so.6    libncursesw.so.5 \
-            libtinfo.so.6       libtinfo.so.5    \
-            || exit 1; \
-    fi && \
-    cd /usr/lib64 && \
-    while [[ $# -gt 0 ]]; do \
-        source="$1" && \
-        shift && \
-        target="$1" && \
-        shift && \
-        ln -s "$source" "$target" || exit 1; \
-    done
-
-ADD https://ci.trafficserver.apache.org/RPMS/CentOS7/trafficserver-7.1.4-2.el7.x86_64.rpm /trafficserver.rpm
-ADD https://ci.trafficserver.apache.org/RPMS/CentOS7/trafficserver-devel-7.1.4-2.el7.x86_64.rpm /trafficserver-devel.rpm
+        # Pretend that we have the right library versions.
+        # TODO: Use a proper CentOS 7 or 8 RPM once trafficserver
+        # is in EPEL again (see apache/trafficserver#6855)
+        cd /usr/lib64 && \
+        ln -s libtcl8.6.so        libtcl8.5.so && \
+        ln -s libncursesw.so.6    libncursesw.so.5 && \
+        ln -s libtinfo.so.6       libtinfo.so.5  || \
+            exit 1; \
+    fi
 
-RUN rpm -Uvh --nodeps /trafficserver.rpm /trafficserver-devel.rpm && \
-    dnf install -y jq python3-psutil python3-setuptools python3-pip logrotate && \
+RUN dnf install -y bind-utils kyotocabinet-libs initscripts iproute net-tools nmap-ncat gettext autoconf automake libtool gcc-c++ cronie glibc-devel openssl-devel && \
+    dnf install -y jq logrotate && \
     dnf clean all
 
-RUN dnf install -y bind-utils kyotocabinet-libs initscripts iproute net-tools nmap-ncat gettext autoconf automake libtool gcc-c++ cronie glibc-devel openssl-devel
+FROM common-traffic-server-dependencies AS common-cache-config-layers
 
-RUN python3 -m pip install --upgrade pip && python3 -m pip install requests urllib3 distro
+ADD https://ci.trafficserver.apache.org/RPMS/CentOS7/trafficserver-7.1.4-2.el7.x86_64.rpm /trafficserver.rpm
+ADD https://ci.trafficserver.apache.org/RPMS/CentOS7/trafficserver-devel-7.1.4-2.el7.x86_64.rpm /trafficserver-devel.rpm
 
-ADD traffic_server/plugins/astats_over_http/astats_over_http.c traffic_server/plugins/astats_over_http/Makefile.am /
+COPY traffic_server/plugins/astats_over_http/astats_over_http.c traffic_server/plugins/astats_over_http/Makefile.am /
 
-RUN tsxs -v -c astats_over_http.c -o astats_over_http.so
+RUN rpm -Uvh --nodeps /trafficserver.rpm /trafficserver-devel.rpm && \
+    tsxs -v -c astats_over_http.c -o astats_over_http.so
 
-# The symbolic link here is a shim for broken atstccfg behavior - remove when it's fixed.
-RUN mkdir -p /usr/libexec/trafficserver /opt/ort /opt/trafficserver/etc/trafficserver/ /opt/init.d && ln -s /opt/trafficserver/etc/trafficserver/ssl /etc/trafficserver/ssl && tsxs -v -o astats_over_http.so -i
+# The symbolic links here are used because trafficserver-7.1.4-2.el7.x86_64.rpm has / as TS_ROOT,
+# but the Cache Configs assume that /opt/trafficserver is TS_ROOT. The symlinks can be removed
+# once a Traffic Server RPM using /opt/trafficserver as TS_ROOT is used.
+RUN ln -s / /opt/trafficserver && \
+    mkdir /libexec /etc/trafficserver/ssl && \
+    ln -s /usr/lib64/trafficserver/plugins /libexec/trafficserver && \
+    tsxs -v -o astats_over_http.so -i
 
-RUN dnf remove -y gcc-c++ glibc-devel autoconf automake libtool && rm -f /astats_over_http.c /Makefile.am
+RUN dnf remove -y gcc-c++ glibc-devel autoconf automake libtool && \
+    rm -f /astats_over_http.c /Makefile.am
 
-# You need to do this because the RPM in the ATS archives is just all kinds of messed-up
-RUN chmod 755 /usr/lib64/trafficserver /etc/trafficserver/body_factory /etc/trafficserver/body_factory/default
-RUN mkdir -p /var/trafficserver /opt/ort && \
-    dd if=/dev/zero bs=1M count=1000 of=/var/trafficserver/cache && \
-    chown -R ats:ats /etc/trafficserver/ /var/trafficserver/ /opt/ort /usr/lib64/trafficserver/ && \
-    sed -i 's/STRING 8080 8080:ipv6/STRING 80 80:ipv6/' /etc/trafficserver/records.config
+# traffic_server needs different ownership than trafficserver-7.1.4-2.el7.x86_64.rpm sets
+RUN chown -R ats:ats /etc/trafficserver/ /usr/lib64/trafficserver/
 
-RUN setcap CAP_NET_BIND_SERVICE=+eip /bin/traffic_server && setcap CAP_NET_BIND_SERVICE=+eip /bin/traffic_manager && setcap CAP_NET_BIND_SERVICE=+eip /bin/trafficserver && setcap CAP_NET_BIND_SERVICE=+eip /bin/traffic_cop
 
 WORKDIR /opt
 
-ADD infrastructure/cdn-in-a-box/ort /opt/ort/
-ADD traffic_control/clients/python /opt/Apache-TrafficControl/
-
-RUN touch /var/log/ort.log && \
-	pip3 install ./Apache-TrafficControl && \
-	pip3 install ./ort && \
-	cp ort/traffic_ops_ort.crontab /etc/cron.d/traffic_ops_ort-cron-template && \
-	cp ort/traffic_ops_ort.logrotate /etc/logrotate.d/ort
+ADD infrastructure/cdn-in-a-box/cache/traffic_ops_ort.crontab /etc/cron.d/traffic_ops_ort-cron-template
+ADD infrastructure/cdn-in-a-box/cache/traffic_ops_ort.logrotate /etc/logrotate.d/ort
 
 ADD infrastructure/cdn-in-a-box/cache/run.sh infrastructure/cdn-in-a-box/traffic_ops/to-access.sh infrastructure/cdn-in-a-box/enroller/server_template.json /
 
 COPY infrastructure/cdn-in-a-box/dns/set-dns.sh \
      infrastructure/cdn-in-a-box/dns/insert-self-into-dns.sh \
+     infrastructure/cdn-in-a-box/cache/uname \
      /usr/local/sbin/
+# Copy systemctl.sh to /usr/bin specifically because t3c runs /bin/systemctl by absolute path
+COPY cache-config/testing/docker/ort_test/systemctl.sh /usr/bin/systemctl
 
 ARG ORT_RPM=infrastructure/cdn-in-a-box/cache/trafficcontrol-cache-config.rpm
 ADD $ORT_RPM /
-RUN rpm -Uvh --nodeps /$(basename $ORT_RPM) &&\
+RUN rpm -Uvh /$(basename $ORT_RPM) &&\
     rm /$(basename $ORT_RPM)
 CMD /run.sh
 
-FROM common-cache-server-layers AS mid
+FROM common-traffic-server-dependencies AS get-delve
+RUN dnf -y install golang && \
+    go get -u github.com/go-delve/delve/cmd/dlv
+
+FROM common-cache-config-layers AS mid
 COPY infrastructure/cdn-in-a-box/mid/init.d/ /opt/init.d/
 
-FROM common-cache-server-layers AS edge
+FROM mid AS mid-debug
+COPY --from=get-delve /root/go/bin /usr/bin
+COPY infrastructure/cdn-in-a-box/cache/init-debug-scripts.sh /opt/init.d/
+
+FROM common-cache-config-layers AS edge
 COPY infrastructure/cdn-in-a-box/edge/init.d/ /opt/init.d/
+
+FROM edge AS edge-debug
+COPY --from=get-delve /root/go/bin /usr/bin
+COPY infrastructure/cdn-in-a-box/cache/init-debug-scripts.sh /opt/init.d/
+
+FROM edge

Review comment:
       what does this hanging `FROM` line do?

##########
File path: infrastructure/cdn-in-a-box/cache/run.sh
##########
@@ -17,14 +17,78 @@
 # specific language governing permissions and limitations
 # under the License.
 
-set -e
-set -x
-set -m
+set -o errexit -o xtrace -o monitor
+
+mkdir /tmp/ort
+
+set-dns.sh
+insert-self-into-dns.sh
+
+source /to-access.sh
+
+# Wait on SSL certificate generation
+until [[ -f "$X509_CA_ENV_FILE" ]]
+do
+  echo "Waiting on Shared SSL certificate generation"
+  sleep 3
+done
+
+# Source the CIAB-CA shared SSL environment
+until [[ -n "$X509_GENERATION_COMPLETE" ]]
+do
+  echo "Waiting on X509 vars to be defined"
+  sleep 1
+  source "$X509_CA_ENV_FILE"

Review comment:
       Inconsistent indentation using 2 spaces - existing code in this file used 4 spaces. I wouldn't object to switching to tabs, but it should be done consistently and globally through the file

##########
File path: infrastructure/cdn-in-a-box/cache/init-debug-scripts.sh
##########
@@ -0,0 +1,57 @@
+#!/usr/bin/env bash
+
+# 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.
+
+set -o errexit -o nounset
+
+maybe_debug() {
+	set -o errexit -o nounset
+	hostname="$1"
+	shift
+	debug_port="$1"
+	shift
+	actual_binary="$1"
+	shift
+	hostname="${hostname//-/_}" # replace - with _
+	hostname="${hostname^^}" # uppercase
+	debug_variable_name="T3C_DEBUG_COMPONENT_${hostname}"
+	if [[ "${!debug_variable_name}" == "${actual_binary%.actual}" ]]; then
+		command=(dlv --listen=":${debug_port}" --headless=true --api-version=2 exec "/usr/bin/${actual_binary}" --)
+	else
+		command=("$actual_binary")
+	fi
+	exec "${command[@]}" "$@"
+}
+
+hostname="$(hostname --short)"
+for t3c_tool in $(compgen -c t3c | sort | uniq); do
+	(path="$(type -p "$t3c_tool")"
+	cd "$(dirname "$path")"
+	dlv_script="${t3c_tool}.debug"
+	actual_binary="${t3c_tool}.actual"
+	touch "$dlv_script"
+	chmod +x "$dlv_script"
+	<<-DLV_SCRIPT cat > "$dlv_script"
+	#!/usr/bin/env bash
+	$(type maybe_debug | tail -n+2)

Review comment:
       Do you need to print the source of the function?

##########
File path: infrastructure/cdn-in-a-box/cache/init-debug-scripts.sh
##########
@@ -0,0 +1,57 @@
+#!/usr/bin/env bash
+
+# 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.
+
+set -o errexit -o nounset
+
+maybe_debug() {
+	set -o errexit -o nounset
+	hostname="$1"
+	shift
+	debug_port="$1"
+	shift
+	actual_binary="$1"
+	shift
+	hostname="${hostname//-/_}" # replace - with _
+	hostname="${hostname^^}" # uppercase
+	debug_variable_name="T3C_DEBUG_COMPONENT_${hostname}"
+	if [[ "${!debug_variable_name}" == "${actual_binary%.actual}" ]]; then
+		command=(dlv --listen=":${debug_port}" --headless=true --api-version=2 exec "/usr/bin/${actual_binary}" --)
+	else
+		command=("$actual_binary")
+	fi
+	exec "${command[@]}" "$@"
+}
+
+hostname="$(hostname --short)"
+for t3c_tool in $(compgen -c t3c | sort | uniq); do
+	(path="$(type -p "$t3c_tool")"
+	cd "$(dirname "$path")"
+	dlv_script="${t3c_tool}.debug"
+	actual_binary="${t3c_tool}.actual"
+	touch "$dlv_script"

Review comment:
       touch is unnecessary if you move the `chmod` below the `cat`.

##########
File path: infrastructure/cdn-in-a-box/cache/run.sh
##########
@@ -17,14 +17,78 @@
 # specific language governing permissions and limitations
 # under the License.
 
-set -e
-set -x
-set -m
+set -o errexit -o xtrace -o monitor
+
+mkdir /tmp/ort
+
+set-dns.sh
+insert-self-into-dns.sh
+
+source /to-access.sh
+
+# Wait on SSL certificate generation
+until [[ -f "$X509_CA_ENV_FILE" ]]
+do
+  echo "Waiting on Shared SSL certificate generation"
+  sleep 3
+done
+
+# Source the CIAB-CA shared SSL environment
+until [[ -n "$X509_GENERATION_COMPLETE" ]]
+do
+  echo "Waiting on X509 vars to be defined"
+  sleep 1
+  source "$X509_CA_ENV_FILE"
+done
+
+# Trust the CIAB-CA at the System level
+cp "$X509_CA_CERT_FULL_CHAIN_FILE" /etc/pki/ca-trust/source/anchors
+update-ca-trust extract
+
+while ! to-ping 2>/dev/null; do
+	echo "waiting for Traffic Ops"
+	sleep 5

Review comment:
       Inconsistent indentation using tabs - existing code in this file used 4 spaces. I wouldn't object to switching to tabs, but it should be done consistently and globally through the file

##########
File path: infrastructure/cdn-in-a-box/cache/run.sh
##########
@@ -17,14 +17,78 @@
 # specific language governing permissions and limitations
 # under the License.
 
-set -e
-set -x
-set -m
+set -o errexit -o xtrace -o monitor
+
+mkdir /tmp/ort
+
+set-dns.sh
+insert-self-into-dns.sh
+
+source /to-access.sh
+
+# Wait on SSL certificate generation
+until [[ -f "$X509_CA_ENV_FILE" ]]
+do
+  echo "Waiting on Shared SSL certificate generation"
+  sleep 3

Review comment:
       Inconsistent indentation using 2 spaces - existing code in this file used 4 spaces. I wouldn't object to switching to tabs (tabs > 4-space > 2-space >>> 8-space), but it should be done consistently and globally through the file

##########
File path: infrastructure/cdn-in-a-box/cache/Dockerfile
##########
@@ -51,109 +54,84 @@ RUN dnf -y install epel-release && \
         make                    \
         numactl-libs            \
         perl                    \
-        perl-Carp               \
-        perl-constant           \
-        perl-Data-Dumper        \
-        perl-Encode             \
-        perl-Exporter           \
-        perl-File-Path          \
-        perl-File-Temp          \
-        perl-Filter             \
-        perl-Getopt-Long        \
-        perl-HTTP-Tiny          \
-        perl-libs               \
-        perl-macros             \
-        perl-parent             \
-        perl-PathTools          \
-        perl-Pod-Escapes        \
-        perl-podlators          \
-        perl-Pod-Perldoc        \
-        perl-Pod-Simple         \
-        perl-Pod-Usage          \
-        perl-Scalar-List-Utils  \
-        perl-Socket             \
-        perl-Storable           \
-        perl-Text-ParseWords    \
-        perl-threads            \
-        perl-threads-shared     \
-        perl-Time-HiRes         \
-        perl-Time-Local         \
-        perl-URI                \
         tcl                     \
         $additional_packages && \
     if [[ "${RHEL_VERSION%%.*}" -eq 8 ]]; then \
-        set -- \
-            # Pretend that we have the right library versions.
-            # TODO: Use a proper CentOS 7 or 8 RPM once trafficserver
-            # is in EPEL again (see apache/trafficserver#6855)
-            libtcl8.6.so        libtcl8.5.so     \
-            libncursesw.so.6    libncursesw.so.5 \
-            libtinfo.so.6       libtinfo.so.5    \
-            || exit 1; \
-    fi && \
-    cd /usr/lib64 && \
-    while [[ $# -gt 0 ]]; do \
-        source="$1" && \
-        shift && \
-        target="$1" && \
-        shift && \
-        ln -s "$source" "$target" || exit 1; \
-    done
-
-ADD https://ci.trafficserver.apache.org/RPMS/CentOS7/trafficserver-7.1.4-2.el7.x86_64.rpm /trafficserver.rpm
-ADD https://ci.trafficserver.apache.org/RPMS/CentOS7/trafficserver-devel-7.1.4-2.el7.x86_64.rpm /trafficserver-devel.rpm
+        # Pretend that we have the right library versions.
+        # TODO: Use a proper CentOS 7 or 8 RPM once trafficserver
+        # is in EPEL again (see apache/trafficserver#6855)
+        cd /usr/lib64 && \
+        ln -s libtcl8.6.so        libtcl8.5.so && \
+        ln -s libncursesw.so.6    libncursesw.so.5 && \
+        ln -s libtinfo.so.6       libtinfo.so.5  || \
+            exit 1; \
+    fi
 
-RUN rpm -Uvh --nodeps /trafficserver.rpm /trafficserver-devel.rpm && \
-    dnf install -y jq python3-psutil python3-setuptools python3-pip logrotate && \
+RUN dnf install -y bind-utils kyotocabinet-libs initscripts iproute net-tools nmap-ncat gettext autoconf automake libtool gcc-c++ cronie glibc-devel openssl-devel && \
+    dnf install -y jq logrotate && \
     dnf clean all
 
-RUN dnf install -y bind-utils kyotocabinet-libs initscripts iproute net-tools nmap-ncat gettext autoconf automake libtool gcc-c++ cronie glibc-devel openssl-devel
+FROM common-traffic-server-dependencies AS common-cache-config-layers
 
-RUN python3 -m pip install --upgrade pip && python3 -m pip install requests urllib3 distro
+ADD https://ci.trafficserver.apache.org/RPMS/CentOS7/trafficserver-7.1.4-2.el7.x86_64.rpm /trafficserver.rpm
+ADD https://ci.trafficserver.apache.org/RPMS/CentOS7/trafficserver-devel-7.1.4-2.el7.x86_64.rpm /trafficserver-devel.rpm
 
-ADD traffic_server/plugins/astats_over_http/astats_over_http.c traffic_server/plugins/astats_over_http/Makefile.am /
+COPY traffic_server/plugins/astats_over_http/astats_over_http.c traffic_server/plugins/astats_over_http/Makefile.am /
 
-RUN tsxs -v -c astats_over_http.c -o astats_over_http.so
+RUN rpm -Uvh --nodeps /trafficserver.rpm /trafficserver-devel.rpm && \
+    tsxs -v -c astats_over_http.c -o astats_over_http.so
 
-# The symbolic link here is a shim for broken atstccfg behavior - remove when it's fixed.
-RUN mkdir -p /usr/libexec/trafficserver /opt/ort /opt/trafficserver/etc/trafficserver/ /opt/init.d && ln -s /opt/trafficserver/etc/trafficserver/ssl /etc/trafficserver/ssl && tsxs -v -o astats_over_http.so -i
+# The symbolic links here are used because trafficserver-7.1.4-2.el7.x86_64.rpm has / as TS_ROOT,
+# but the Cache Configs assume that /opt/trafficserver is TS_ROOT. The symlinks can be removed
+# once a Traffic Server RPM using /opt/trafficserver as TS_ROOT is used.
+RUN ln -s / /opt/trafficserver && \
+    mkdir /libexec /etc/trafficserver/ssl && \
+    ln -s /usr/lib64/trafficserver/plugins /libexec/trafficserver && \
+    tsxs -v -o astats_over_http.so -i
 
-RUN dnf remove -y gcc-c++ glibc-devel autoconf automake libtool && rm -f /astats_over_http.c /Makefile.am
+RUN dnf remove -y gcc-c++ glibc-devel autoconf automake libtool && \
+    rm -f /astats_over_http.c /Makefile.am
 
-# You need to do this because the RPM in the ATS archives is just all kinds of messed-up
-RUN chmod 755 /usr/lib64/trafficserver /etc/trafficserver/body_factory /etc/trafficserver/body_factory/default
-RUN mkdir -p /var/trafficserver /opt/ort && \
-    dd if=/dev/zero bs=1M count=1000 of=/var/trafficserver/cache && \
-    chown -R ats:ats /etc/trafficserver/ /var/trafficserver/ /opt/ort /usr/lib64/trafficserver/ && \
-    sed -i 's/STRING 8080 8080:ipv6/STRING 80 80:ipv6/' /etc/trafficserver/records.config
+# traffic_server needs different ownership than trafficserver-7.1.4-2.el7.x86_64.rpm sets
+RUN chown -R ats:ats /etc/trafficserver/ /usr/lib64/trafficserver/
 
-RUN setcap CAP_NET_BIND_SERVICE=+eip /bin/traffic_server && setcap CAP_NET_BIND_SERVICE=+eip /bin/traffic_manager && setcap CAP_NET_BIND_SERVICE=+eip /bin/trafficserver && setcap CAP_NET_BIND_SERVICE=+eip /bin/traffic_cop
 
 WORKDIR /opt
 
-ADD infrastructure/cdn-in-a-box/ort /opt/ort/
-ADD traffic_control/clients/python /opt/Apache-TrafficControl/
-
-RUN touch /var/log/ort.log && \
-	pip3 install ./Apache-TrafficControl && \
-	pip3 install ./ort && \
-	cp ort/traffic_ops_ort.crontab /etc/cron.d/traffic_ops_ort-cron-template && \
-	cp ort/traffic_ops_ort.logrotate /etc/logrotate.d/ort
+ADD infrastructure/cdn-in-a-box/cache/traffic_ops_ort.crontab /etc/cron.d/traffic_ops_ort-cron-template
+ADD infrastructure/cdn-in-a-box/cache/traffic_ops_ort.logrotate /etc/logrotate.d/ort

Review comment:
       Local files should be copied with `COPY`

##########
File path: infrastructure/cdn-in-a-box/traffic_monitor/Dockerfile
##########
@@ -20,7 +20,7 @@
 ############################################################
 
 ARG RHEL_VERSION=8
-FROM centos:${RHEL_VERSION} as trafficmonitor
+FROM centos:${RHEL_VERSION} as trafficmonitor-dependencies

Review comment:
       I think these types of changes are best left to a separate body of work, since they don't relate to _"Us[ing] `t3c` for cache configuration in CDN-in-a-Box"_

##########
File path: infrastructure/cdn-in-a-box/cache/init-debug-scripts.sh
##########
@@ -0,0 +1,57 @@
+#!/usr/bin/env bash
+
+# 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.
+
+set -o errexit -o nounset
+
+maybe_debug() {
+	set -o errexit -o nounset
+	hostname="$1"
+	shift
+	debug_port="$1"
+	shift
+	actual_binary="$1"
+	shift
+	hostname="${hostname//-/_}" # replace - with _
+	hostname="${hostname^^}" # uppercase
+	debug_variable_name="T3C_DEBUG_COMPONENT_${hostname}"
+	if [[ "${!debug_variable_name}" == "${actual_binary%.actual}" ]]; then
+		command=(dlv --listen=":${debug_port}" --headless=true --api-version=2 exec "/usr/bin/${actual_binary}" --)
+	else
+		command=("$actual_binary")
+	fi
+	exec "${command[@]}" "$@"
+}
+
+hostname="$(hostname --short)"
+for t3c_tool in $(compgen -c t3c | sort | uniq); do
+	(path="$(type -p "$t3c_tool")"
+	cd "$(dirname "$path")"
+	dlv_script="${t3c_tool}.debug"
+	actual_binary="${t3c_tool}.actual"
+	touch "$dlv_script"
+	chmod +x "$dlv_script"
+	<<-DLV_SCRIPT cat > "$dlv_script"
+	#!/usr/bin/env bash
+	$(type maybe_debug | tail -n+2)
+	maybe_debug "${hostname}" "${DEBUG_PORT}" "${actual_binary}" "\$@"
+	DLV_SCRIPT
+	mv "$t3c_tool" "$actual_binary"
+	ln -s "$dlv_script" "$t3c_tool"
+	)

Review comment:
       why wrap in <kbd>(</kbd>/<kbd>)</kbd>?

##########
File path: infrastructure/cdn-in-a-box/cache/uname
##########
@@ -0,0 +1,35 @@
+#!/usr/bin/env bash
+# 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.
+
+set -o errexit -o nounset
+get_rhel_version() {
+	local redhat_release=/etc/redhat-release
+	local default_version="${RHEL_VERSION:-8}"
+	if [ -e $redhat_release ]; then
+		releasever="$(rpm -q --qf '%{version}' -f $redhat_release)"
+		releasever="${releasever%%.*}"
+	else
+		releasever=${default_version}
+	fi;
+
+	echo "el${releasever}"
+}
+rhel_version="$(get_rhel_version)"
+kernel_version="3.10.0-862.11.6.${rhel_version}"

Review comment:
       Do we need to spoof the kernel version? Things that need to interact with the kernel will actually be interacting with the host kernel, won't they?

##########
File path: infrastructure/cdn-in-a-box/traffic_ops_data/profiles/020-ATS_MID_TIER_CACHE.json
##########
@@ -15,7 +15,7 @@
 			"configFile": "records.config",
 			"name": "CONFIG proxy.config.config_dir",
 			"secure": false,
-			"value": "STRING /etc/trafficserver"
+			"value": "STRING /opt/trafficserver/etc/trafficserver"

Review comment:
       Does `t3c` not support installation to `/`?

##########
File path: infrastructure/cdn-in-a-box/traffic_ops_data/profiles/020-ATS_MID_TIER_CACHE.json
##########
@@ -255,118 +255,176 @@
 			"configFile": "ip_allow.config",
 			"name": "location",
 			"secure": false,
-			"value": "/etc/trafficserver"
+			"value": "/opt/trafficserver/etc/trafficserver"
 		},
 		{
 			"configFile": "storage.config",
 			"name": "Drive_Prefix",
 			"secure": false,
 			"value": "/var/trafficserver/"
 		},
+		{
+			"configFile": "records.config",
+			"name": "CONFIG proxy.config.ssl.CA.cert.path",
+			"secure": false,
+			"value": "STRING /opt/trafficserver/etc/trafficserver/ssl"
+		},
+		{
+			"configFile": "records.config",
+			"name": "CONFIG proxy.config.ssl.client.CA.cert.path",
+			"secure": false,
+			"value": "STRING /opt/trafficserver/etc/trafficserver/ssl"
+		},
+		{
+			"configFile": "records.config",
+			"name": "CONFIG proxy.config.ssl.client.cert.path",
+			"secure": false,
+			"value": "STRING /opt/trafficserver/etc/trafficserver/ssl"
+		},
+		{
+			"configFile": "records.config",
+			"name": "CONFIG proxy.config.ssl.client.private_key.path",
+			"secure": false,
+			"value": "STRING /opt/trafficserver/etc/trafficserver/ssl"
+		},
+		{
+			"configFile": "records.config",
+			"name": "CONFIG proxy.config.ssl.ocsp.enabled",
+			"secure": false,
+			"value": "INT 1"
+		},
+		{
+			"configFile": "records.config",
+			"name": "CONFIG proxy.config.ssl.server.cert.path",
+			"secure": false,
+			"value": "STRING /opt/trafficserver/etc/trafficserver/ssl"
+		},
+		{
+			"configFile": "records.config",
+			"name": "CONFIG proxy.config.ssl.server.private_key.path",
+			"secure": false,
+			"value": "STRING /opt/trafficserver/etc/trafficserver/ssl"
+		},
+		{
+			"configFile": "records.config",
+			"name": "CONFIG proxy.config.ssl.server.ticket_key.filename",
+			"secure": false,
+			"value": "STRING NULL"
+		},

Review comment:
       Do these need to be added to the edge's profile as well?

##########
File path: infrastructure/cdn-in-a-box/cache/run.sh
##########
@@ -17,14 +17,78 @@
 # specific language governing permissions and limitations
 # under the License.
 
-set -e
-set -x
-set -m
+set -o errexit -o xtrace -o monitor
+
+mkdir /tmp/ort
+
+set-dns.sh
+insert-self-into-dns.sh
+
+source /to-access.sh
+
+# Wait on SSL certificate generation
+until [[ -f "$X509_CA_ENV_FILE" ]]
+do
+  echo "Waiting on Shared SSL certificate generation"
+  sleep 3
+done
+
+# Source the CIAB-CA shared SSL environment
+until [[ -n "$X509_GENERATION_COMPLETE" ]]
+do
+  echo "Waiting on X509 vars to be defined"
+  sleep 1
+  source "$X509_CA_ENV_FILE"
+done
+
+# Trust the CIAB-CA at the System level
+cp "$X509_CA_CERT_FULL_CHAIN_FILE" /etc/pki/ca-trust/source/anchors
+update-ca-trust extract
+
+while ! to-ping 2>/dev/null; do
+	echo "waiting for Traffic Ops"
+	sleep 5
+done
+
+export TO_USER=$TO_ADMIN_USER
+export TO_PASSWORD=$TO_ADMIN_PASSWORD
+
+# wait until the CDN has been registered
+found=
+while [[ -z $found ]]; do
+    echo 'waiting for enroller setup'
+    sleep 3
+    found=$(to-get api/2.0/cdns?name="$CDN_NAME" | jq -r '.response[].name')
+done
 
 for f in /opt/init.d/*; do
     echo "$f"
-    $f
+    source "$f"
+done
+
+# Wait for SSL keys to exist
+until [[ $(to-get "api/2.0/cdns/name/$CDN_NAME/sslkeys" | jq '.response | length') -gt 0 ]]; do
+	echo 'waiting for SSL keys to exist'
+	sleep 3

Review comment:
       Inconsistent indentation using tabs - existing code in this file used 4 spaces. I wouldn't object to switching to tabs, but it should be done consistently and globally through the file

##########
File path: infrastructure/cdn-in-a-box/cache/run.sh
##########
@@ -17,14 +17,78 @@
 # specific language governing permissions and limitations
 # under the License.
 
-set -e
-set -x
-set -m
+set -o errexit -o xtrace -o monitor
+
+mkdir /tmp/ort
+
+set-dns.sh
+insert-self-into-dns.sh
+
+source /to-access.sh
+
+# Wait on SSL certificate generation
+until [[ -f "$X509_CA_ENV_FILE" ]]
+do
+  echo "Waiting on Shared SSL certificate generation"
+  sleep 3
+done
+
+# Source the CIAB-CA shared SSL environment
+until [[ -n "$X509_GENERATION_COMPLETE" ]]
+do
+  echo "Waiting on X509 vars to be defined"
+  sleep 1
+  source "$X509_CA_ENV_FILE"
+done
+
+# Trust the CIAB-CA at the System level
+cp "$X509_CA_CERT_FULL_CHAIN_FILE" /etc/pki/ca-trust/source/anchors
+update-ca-trust extract
+
+while ! to-ping 2>/dev/null; do
+	echo "waiting for Traffic Ops"
+	sleep 5
+done
+
+export TO_USER=$TO_ADMIN_USER
+export TO_PASSWORD=$TO_ADMIN_PASSWORD
+
+# wait until the CDN has been registered
+found=
+while [[ -z $found ]]; do
+    echo 'waiting for enroller setup'
+    sleep 3
+    found=$(to-get api/2.0/cdns?name="$CDN_NAME" | jq -r '.response[].name')
+done
 
 for f in /opt/init.d/*; do
     echo "$f"
-    $f
+    source "$f"
+done
+
+# Wait for SSL keys to exist
+until [[ $(to-get "api/2.0/cdns/name/$CDN_NAME/sslkeys" | jq '.response | length') -gt 0 ]]; do
+	echo 'waiting for SSL keys to exist'
+	sleep 3
 done
 
-# tail -f /dev/null
-/bin/bash
+# If /tmp/trafficcontrol-cache-config does not already exist when running t3c-apply, t3c-apply will create it and fail silently
+mkdir /tmp/trafficcontrol-cache-config
+
+# hostname is already defined in /etc/init.d/99-run.sh
+hostname="${hostname//-/_}" # replace - with _
+hostname="${hostname^^}" # uppercase
+debug_variable_name="T3C_DEBUG_COMPONENT_${hostname}"
+debug_binary="${!debug_variable_name}"
+if ! type -p "$debug_binary"; then
+	t3c apply --run-mode=badass --traffic-ops-url="$TO_URL" --traffic-ops-user="$TO_USER" --traffic-ops-password="$TO_PASSWORD" --git=yes --dispersion=0 --log-location-error=stdout --log-location-warning=stdout --log-location-info=stdout all || { echo "Failed"; }

Review comment:
       Inconsistent indentation using tabs - existing code in this file used 4 spaces. I wouldn't object to switching to tabs, but it should be done consistently and globally through the file

##########
File path: infrastructure/cdn-in-a-box/cache/uname
##########
@@ -0,0 +1,35 @@
+#!/usr/bin/env bash
+# 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.
+
+set -o errexit -o nounset
+get_rhel_version() {
+	local redhat_release=/etc/redhat-release
+	local default_version="${RHEL_VERSION:-8}"
+	if [ -e $redhat_release ]; then
+		releasever="$(rpm -q --qf '%{version}' -f $redhat_release)"
+		releasever="${releasever%%.*}"
+	else
+		releasever=${default_version}
+	fi;
+
+	echo "el${releasever}"
+}
+rhel_version="$(get_rhel_version)"
+kernel_version="3.10.0-862.11.6.${rhel_version}"
+architecture=x86_64

Review comment:
       Do we need to spoof the architecture? All running code will, in fact, be using the host's architecture, not necessarily `x86_64`.




-- 
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 commented on a change in pull request #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634694000



##########
File path: infrastructure/cdn-in-a-box/traffic_ops_data/profiles/020-ATS_MID_TIER_CACHE.json
##########
@@ -15,7 +15,7 @@
 			"configFile": "records.config",
 			"name": "CONFIG proxy.config.config_dir",
 			"secure": false,
-			"value": "STRING /etc/trafficserver"
+			"value": "STRING /opt/trafficserver/etc/trafficserver"

Review comment:
       that's unfortunate




-- 
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 commented on a change in pull request #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r636290326



##########
File path: infrastructure/cdn-in-a-box/traffic_monitor/Dockerfile
##########
@@ -64,7 +63,7 @@ CMD /run.sh
 HEALTHCHECK --interval=10s --timeout=1s \
     CMD bash -c 'source /to-access.sh && [[ "$(curl -s http://trafficmonitor.infra.ciab.test/api/traffic-ops-uri)" == "$TO_URL" ]]'
 
-FROM trafficmonitor-dependencies as get-delve
+FROM trafficmonitor as get-delve

Review comment:
       Seems like this is reverting the changes from #5866 
   
   Same thing in Enroller/Ops etc.




-- 
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 commented on a change in pull request #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634646120



##########
File path: infrastructure/cdn-in-a-box/traffic_monitor/Dockerfile
##########
@@ -20,7 +20,7 @@
 ############################################################
 
 ARG RHEL_VERSION=8
-FROM centos:${RHEL_VERSION} as trafficmonitor
+FROM centos:${RHEL_VERSION} as trafficmonitor-dependencies

Review comment:
       Yes I think ideally those optimizations would be made at a later date, but they're small enough that we can probably just squeeze them in here. Alternatively, if you have the time and it's not annoying, you could make the optimizations to the other services and submit that as a separate PR as a prerequisite to this one. I can promise to merge that today, if you so choose and once its checks pass.




-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r635502371



##########
File path: traffic_ops_ort/traffic_ops_ort.pl
##########
@@ -0,0 +1,2662 @@
+#!/usr/bin/perl

Review comment:
       oh no




-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman edited a comment on pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#issuecomment-844387566


   Updated the t3c integration tests GHA workflow to run when anything in `/cache-config/testing` is modified, since this PR updates `/cache-config/testing/docker/ort_test/systemctl.sh`. The updates to `systemctl.sh` cause the t3c integration tests to fail because `tc-fixtures.json` does not include a valid ATS version:
   
   https://github.com/apache/trafficcontrol/blob/04b6bb59188be169f22ea575b1e37d3dd67a6fe8/cache-config/testing/ort-tests/tc-fixtures.json#L1759-L1765
   
   <strike>GH issue incoming.</strike> See https://github.com/apache/trafficcontrol/pull/5853#issuecomment-845263483


-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#issuecomment-842588101


   > Moving ORT.py like this breaks the autodoc for it, causing a bunch of broken links. See the docs build action output for details.
   
   Fixed `ortPath` in 04c5e66964.


-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634444531



##########
File path: infrastructure/cdn-in-a-box/cache/init-debug-scripts.sh
##########
@@ -0,0 +1,57 @@
+#!/usr/bin/env bash
+
+# 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.
+
+set -o errexit -o nounset
+
+maybe_debug() {
+	set -o errexit -o nounset
+	hostname="$1"
+	shift
+	debug_port="$1"
+	shift
+	actual_binary="$1"
+	shift
+	hostname="${hostname//-/_}" # replace - with _
+	hostname="${hostname^^}" # uppercase
+	debug_variable_name="T3C_DEBUG_COMPONENT_${hostname}"
+	if [[ "${!debug_variable_name}" == "${actual_binary%.actual}" ]]; then
+		command=(dlv --listen=":${debug_port}" --headless=true --api-version=2 exec "/usr/bin/${actual_binary}" --)
+	else
+		command=("$actual_binary")
+	fi
+	exec "${command[@]}" "$@"
+}
+
+hostname="$(hostname --short)"
+for t3c_tool in $(compgen -c t3c | sort | uniq); do
+	(path="$(type -p "$t3c_tool")"
+	cd "$(dirname "$path")"
+	dlv_script="${t3c_tool}.debug"
+	actual_binary="${t3c_tool}.actual"
+	touch "$dlv_script"

Review comment:
       Moved `chmod` below the `cat` in 17b12aeae8.




-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#issuecomment-843240527


   Rebased onto master to get `t3c-check` from #5281.


-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman edited a comment on pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#issuecomment-845263483


   The t3c integration tests don't seem to be built around Traffic Server running. Copied the `systemctl` shim into CDN in a Box and reverted the one used by the `t3c` integration tests in 65240c03a0.
   
   Edit 2021-06-01: Opened #5903 for running ATS in the t3c integration tests


-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#issuecomment-845464550


   Awesome, can't wait.


-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634644569



##########
File path: infrastructure/cdn-in-a-box/cache/uname
##########
@@ -0,0 +1,35 @@
+#!/usr/bin/env bash
+# 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.
+
+set -o errexit -o nounset
+get_rhel_version() {
+	local redhat_release=/etc/redhat-release
+	local default_version="${RHEL_VERSION:-8}"
+	if [ -e $redhat_release ]; then
+		releasever="$(rpm -q --qf '%{version}' -f $redhat_release)"
+		releasever="${releasever%%.*}"
+	else
+		releasever=${default_version}
+	fi;
+
+	echo "el${releasever}"
+}
+rhel_version="$(get_rhel_version)"
+kernel_version="3.10.0-862.11.6.${rhel_version}"
+architecture=x86_64

Review comment:
       You're right, no need to spoof. Still removed the `uname` shim in 549aa07ce.




-- 
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] jrushford edited a comment on pull request #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
jrushford edited a comment on pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#issuecomment-845462714


   > The t3c integration tests don't seem to be built around Traffic Server running. Copied the `systemctl` shim into CDN in a Box and reverted the one used by the `t3c` integration tests in [65240c0](https://github.com/apache/trafficcontrol/commit/65240c03a000d039807ca93eb682c73e5825de34).
   
   @zrhoffman Until recently the t3c integration tests didn't really need Trafficserver running however, that will be changing.  In order to completely test t3c-apply, we will actually need to start Trafficserver.  I have a ticket, [CDN-13118](https://ccp.sys.comcast.net/browse/CDN-13118) to fix this.  I'll ask you to review the PR once it's up.


-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634645073



##########
File path: infrastructure/cdn-in-a-box/cache/run.sh
##########
@@ -17,14 +17,78 @@
 # specific language governing permissions and limitations
 # under the License.
 
-set -e
-set -x
-set -m
+set -o errexit -o xtrace -o monitor
+
+mkdir /tmp/ort
+
+set-dns.sh
+insert-self-into-dns.sh
+
+source /to-access.sh
+
+# Wait on SSL certificate generation
+until [[ -f "$X509_CA_ENV_FILE" ]]
+do
+  echo "Waiting on Shared SSL certificate generation"
+  sleep 3
+done
+
+# Source the CIAB-CA shared SSL environment
+until [[ -n "$X509_GENERATION_COMPLETE" ]]
+do
+  echo "Waiting on X509 vars to be defined"
+  sleep 1
+  source "$X509_CA_ENV_FILE"
+done
+
+# Trust the CIAB-CA at the System level
+cp "$X509_CA_CERT_FULL_CHAIN_FILE" /etc/pki/ca-trust/source/anchors
+update-ca-trust extract
+
+while ! to-ping 2>/dev/null; do
+	echo "waiting for Traffic Ops"
+	sleep 5

Review comment:
       Spacing made consistent in 4d2c0da326.




-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#issuecomment-845263483


   The t3c integration tests don't seem to be built around Traffic Server running. Copied the `systemctl` shim into CDN in a Box and reverted the one used by the `t3c` integration tests in 65240c03a0.


-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634459369



##########
File path: infrastructure/cdn-in-a-box/cache/uname
##########
@@ -0,0 +1,35 @@
+#!/usr/bin/env bash
+# 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.
+
+set -o errexit -o nounset
+get_rhel_version() {
+	local redhat_release=/etc/redhat-release
+	local default_version="${RHEL_VERSION:-8}"
+	if [ -e $redhat_release ]; then
+		releasever="$(rpm -q --qf '%{version}' -f $redhat_release)"
+		releasever="${releasever%%.*}"
+	else
+		releasever=${default_version}
+	fi;
+
+	echo "el${releasever}"
+}
+rhel_version="$(get_rhel_version)"
+kernel_version="3.10.0-862.11.6.${rhel_version}"

Review comment:
       Not for `t3c`, we don't. In `traffic_ops_ort.pl`, we did:
   
   https://github.com/apache/trafficcontrol/blob/a289d9ce008df420b41f4b3ac488e094860b10f0/traffic_ops_ort/traffic_ops_ort.pl#L343-L348
   
   Removed the `uname` shim in 549aa07ce.




-- 
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 commented on a change in pull request #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634640982



##########
File path: infrastructure/cdn-in-a-box/cache/init-debug-scripts.sh
##########
@@ -0,0 +1,57 @@
+#!/usr/bin/env bash
+
+# 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.
+
+set -o errexit -o nounset
+
+maybe_debug() {
+	set -o errexit -o nounset
+	hostname="$1"
+	shift
+	debug_port="$1"
+	shift
+	actual_binary="$1"
+	shift
+	hostname="${hostname//-/_}" # replace - with _
+	hostname="${hostname^^}" # uppercase
+	debug_variable_name="T3C_DEBUG_COMPONENT_${hostname}"
+	if [[ "${!debug_variable_name}" == "${actual_binary%.actual}" ]]; then
+		command=(dlv --listen=":${debug_port}" --headless=true --api-version=2 exec "/usr/bin/${actual_binary}" --)
+	else
+		command=("$actual_binary")
+	fi
+	exec "${command[@]}" "$@"
+}
+
+hostname="$(hostname --short)"
+for t3c_tool in $(compgen -c t3c | sort | uniq); do
+	(path="$(type -p "$t3c_tool")"
+	cd "$(dirname "$path")"
+	dlv_script="${t3c_tool}.debug"
+	actual_binary="${t3c_tool}.actual"
+	touch "$dlv_script"
+	chmod +x "$dlv_script"
+	<<-DLV_SCRIPT cat > "$dlv_script"
+	#!/usr/bin/env bash
+	$(type maybe_debug | tail -n+2)
+	maybe_debug "${hostname}" "${DEBUG_PORT}" "${actual_binary}" "\$@"
+	DLV_SCRIPT
+	mv "$t3c_tool" "$actual_binary"
+	ln -s "$dlv_script" "$t3c_tool"
+	)

Review comment:
       Is it going to mess with anything if it's indented?
   ```bash
   (
       like;
       this;
   )
   ```




-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634685407



##########
File path: infrastructure/cdn-in-a-box/cache/init-debug-scripts.sh
##########
@@ -0,0 +1,57 @@
+#!/usr/bin/env bash
+
+# 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.
+
+set -o errexit -o nounset
+
+maybe_debug() {
+	set -o errexit -o nounset
+	hostname="$1"
+	shift
+	debug_port="$1"
+	shift
+	actual_binary="$1"
+	shift
+	hostname="${hostname//-/_}" # replace - with _
+	hostname="${hostname^^}" # uppercase
+	debug_variable_name="T3C_DEBUG_COMPONENT_${hostname}"
+	if [[ "${!debug_variable_name}" == "${actual_binary%.actual}" ]]; then
+		command=(dlv --listen=":${debug_port}" --headless=true --api-version=2 exec "/usr/bin/${actual_binary}" --)
+	else
+		command=("$actual_binary")
+	fi
+	exec "${command[@]}" "$@"
+}
+
+hostname="$(hostname --short)"
+for t3c_tool in $(compgen -c t3c | sort | uniq); do
+	(path="$(type -p "$t3c_tool")"
+	cd "$(dirname "$path")"
+	dlv_script="${t3c_tool}.debug"
+	actual_binary="${t3c_tool}.actual"
+	touch "$dlv_script"
+	chmod +x "$dlv_script"
+	<<-DLV_SCRIPT cat > "$dlv_script"
+	#!/usr/bin/env bash
+	$(type maybe_debug | tail -n+2)

Review comment:
       Yes, otherwise it wouldn't be defined within `DLV_SCRIPT`.




-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r636302137



##########
File path: infrastructure/cdn-in-a-box/traffic_monitor/Dockerfile
##########
@@ -64,7 +63,7 @@ CMD /run.sh
 HEALTHCHECK --interval=10s --timeout=1s \
     CMD bash -c 'source /to-access.sh && [[ "$(curl -s http://trafficmonitor.infra.ciab.test/api/traffic-ops-uri)" == "$TO_URL" ]]'
 
-FROM trafficmonitor-dependencies as get-delve
+FROM trafficmonitor as get-delve

Review comment:
       Rebased and took out the commits that modify those files.




-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634459369



##########
File path: infrastructure/cdn-in-a-box/cache/uname
##########
@@ -0,0 +1,35 @@
+#!/usr/bin/env bash
+# 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.
+
+set -o errexit -o nounset
+get_rhel_version() {
+	local redhat_release=/etc/redhat-release
+	local default_version="${RHEL_VERSION:-8}"
+	if [ -e $redhat_release ]; then
+		releasever="$(rpm -q --qf '%{version}' -f $redhat_release)"
+		releasever="${releasever%%.*}"
+	else
+		releasever=${default_version}
+	fi;
+
+	echo "el${releasever}"
+}
+rhel_version="$(get_rhel_version)"
+kernel_version="3.10.0-862.11.6.${rhel_version}"

Review comment:
       Not for `t3c`, we don't. In `traffic_ops_ort.pl`, we did:
   
   https://github.com/apache/trafficcontrol/blob/a289d9ce008df420b41f4b3ac488e094860b10f0/traffic_ops_ort/traffic_ops_ort.pl#L343-L348
   
   Removed the `uname` shim in 549aa07ce5.




-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#issuecomment-844383814


   Rebased to fix a merge conflict in CHANGELOG.md


-- 
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] jrushford commented on pull request #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
jrushford commented on pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#issuecomment-845462714


   > The t3c integration tests don't seem to be built around Traffic Server running. Copied the `systemctl` shim into CDN in a Box and reverted the one used by the `t3c` integration tests in [65240c0](https://github.com/apache/trafficcontrol/commit/65240c03a000d039807ca93eb682c73e5825de34).
   
   @zrhoffman Until recently the t3c integration tests didn't really need Trafficserver running however, that will be changing.  In order to completely test t3c-apply, we will actually need to start Trafficserver.  I have a ticket, [CDN-13118](https://ccp.sys.comcast.net/browse/CDN-13118) to fix this.


-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634447007



##########
File path: infrastructure/cdn-in-a-box/cache/run.sh
##########
@@ -17,14 +17,78 @@
 # specific language governing permissions and limitations
 # under the License.
 
-set -e
-set -x
-set -m
+set -o errexit -o xtrace -o monitor
+
+mkdir /tmp/ort
+
+set-dns.sh
+insert-self-into-dns.sh
+
+source /to-access.sh
+
+# Wait on SSL certificate generation
+until [[ -f "$X509_CA_ENV_FILE" ]]
+do
+  echo "Waiting on Shared SSL certificate generation"
+  sleep 3

Review comment:
       Made the indentation consistently use tabs in 4d2c0da326.




-- 
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] jrushford commented on pull request #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
jrushford commented on pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#issuecomment-845473276


   > Updated the t3c integration tests GHA workflow to run when anything in `/cache-config/testing` is modified, since this PR updates `/cache-config/testing/docker/ort_test/systemctl.sh`. The updates to `systemctl.sh` cause the t3c integration tests to fail because `tc-fixtures.json` does not include a valid ATS version:
   > 
   > https://github.com/apache/trafficcontrol/blob/04b6bb59188be169f22ea575b1e37d3dd67a6fe8/cache-config/testing/ort-tests/tc-fixtures.json#L1759-L1765
   > 
   > GH issue incoming. See [#5853 (comment)](https://github.com/apache/trafficcontrol/pull/5853#issuecomment-845263483)
   
   Rob has a PR in to fix this issue.  the value "*" will be replaced with the original value "CHANGEME".  Somehow this was changed recently.  Anyway, after the GHA workflow builds the trafficserver RPM, "CHANGEME" is modified with a sed script to reflect the correct ATS version.


-- 
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 commented on a change in pull request #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r635500866



##########
File path: traffic_ops_ort/traffic_ops_ort.pl
##########
@@ -0,0 +1,2662 @@
+#!/usr/bin/perl

Review comment:
       Looks like you're necromancing, which while not strictly illegal has been banned guild-wide in all nine provinces by Arch Mage Hannibal Traven since 3E 431.
   
   Best practice is to not practice necromancy.




-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634691716



##########
File path: cache-config/testing/docker/ort_test/systemctl.sh
##########
@@ -19,35 +19,33 @@
 
 # This is a work around for testing t3c which uses systemctl.
 # systemctl does not work in a container very well so this script
-# replaces systemctl in the container and always returns a 
+# replaces systemctl in the container and always returns a
 # sucessful result to t3c.
 
 USAGE="\nsystemctl COMMAND NAME\n"
 
-if [ -z $1 ] || [ -z $2 ]; then
-  echo -e $USAGE
+if [[ -z "$1" ]] || [[ -z "$2" ]]; then
+  echo -e "$USAGE"
   exit 0
 else
-  COMMAND=$1
-  NAME=$2
+  COMMAND="$1"
+  NAME="${2%.service}"
 fi
 
-if [ "$2" != "trafficserver" ]; then
-  echo -e "\nFailed to start ${NAME}.service: Unit not found.n"
+if [[ "$NAME" != "trafficserver" ]]; then
+  echo -e "\nFailed to start ${NAME}.service: Unit not found.\n"
   exit 0
 fi
 
-case $COMMAND in 
-  enable)
-    ;;
-  restart)
-    ;;
-  status)
-    ;;
-  start)
-    ;;
-  stop)
-    ;;
+case "$COMMAND" in
+  restart) command_found=true;;
+  start) command_found=true;;
+  status) command_found=true;;
+  stop) command_found=true;;
 esac
 
+if [[ "$command_found" == true ]]; then

Review comment:
       Exiting with an error if the command wasn't found in 749d9dee14.




-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634684973



##########
File path: infrastructure/cdn-in-a-box/cache/Dockerfile
##########
@@ -51,109 +54,84 @@ RUN dnf -y install epel-release && \
         make                    \
         numactl-libs            \
         perl                    \
-        perl-Carp               \
-        perl-constant           \
-        perl-Data-Dumper        \
-        perl-Encode             \
-        perl-Exporter           \
-        perl-File-Path          \
-        perl-File-Temp          \
-        perl-Filter             \
-        perl-Getopt-Long        \
-        perl-HTTP-Tiny          \
-        perl-libs               \
-        perl-macros             \
-        perl-parent             \
-        perl-PathTools          \
-        perl-Pod-Escapes        \
-        perl-podlators          \
-        perl-Pod-Perldoc        \
-        perl-Pod-Simple         \
-        perl-Pod-Usage          \
-        perl-Scalar-List-Utils  \
-        perl-Socket             \
-        perl-Storable           \
-        perl-Text-ParseWords    \
-        perl-threads            \
-        perl-threads-shared     \
-        perl-Time-HiRes         \
-        perl-Time-Local         \
-        perl-URI                \
         tcl                     \
         $additional_packages && \
     if [[ "${RHEL_VERSION%%.*}" -eq 8 ]]; then \
-        set -- \
-            # Pretend that we have the right library versions.
-            # TODO: Use a proper CentOS 7 or 8 RPM once trafficserver
-            # is in EPEL again (see apache/trafficserver#6855)
-            libtcl8.6.so        libtcl8.5.so     \
-            libncursesw.so.6    libncursesw.so.5 \
-            libtinfo.so.6       libtinfo.so.5    \
-            || exit 1; \
-    fi && \
-    cd /usr/lib64 && \
-    while [[ $# -gt 0 ]]; do \
-        source="$1" && \
-        shift && \
-        target="$1" && \
-        shift && \
-        ln -s "$source" "$target" || exit 1; \
-    done
-
-ADD https://ci.trafficserver.apache.org/RPMS/CentOS7/trafficserver-7.1.4-2.el7.x86_64.rpm /trafficserver.rpm
-ADD https://ci.trafficserver.apache.org/RPMS/CentOS7/trafficserver-devel-7.1.4-2.el7.x86_64.rpm /trafficserver-devel.rpm
+        # Pretend that we have the right library versions.
+        # TODO: Use a proper CentOS 7 or 8 RPM once trafficserver
+        # is in EPEL again (see apache/trafficserver#6855)
+        cd /usr/lib64 && \
+        ln -s libtcl8.6.so        libtcl8.5.so && \
+        ln -s libncursesw.so.6    libncursesw.so.5 && \
+        ln -s libtinfo.so.6       libtinfo.so.5  || \
+            exit 1; \
+    fi
 
-RUN rpm -Uvh --nodeps /trafficserver.rpm /trafficserver-devel.rpm && \
-    dnf install -y jq python3-psutil python3-setuptools python3-pip logrotate && \
+RUN dnf install -y bind-utils kyotocabinet-libs initscripts iproute net-tools nmap-ncat gettext autoconf automake libtool gcc-c++ cronie glibc-devel openssl-devel && \
+    dnf install -y jq logrotate && \
     dnf clean all
 
-RUN dnf install -y bind-utils kyotocabinet-libs initscripts iproute net-tools nmap-ncat gettext autoconf automake libtool gcc-c++ cronie glibc-devel openssl-devel
+FROM common-traffic-server-dependencies AS common-cache-config-layers
 
-RUN python3 -m pip install --upgrade pip && python3 -m pip install requests urllib3 distro
+ADD https://ci.trafficserver.apache.org/RPMS/CentOS7/trafficserver-7.1.4-2.el7.x86_64.rpm /trafficserver.rpm
+ADD https://ci.trafficserver.apache.org/RPMS/CentOS7/trafficserver-devel-7.1.4-2.el7.x86_64.rpm /trafficserver-devel.rpm
 
-ADD traffic_server/plugins/astats_over_http/astats_over_http.c traffic_server/plugins/astats_over_http/Makefile.am /
+COPY traffic_server/plugins/astats_over_http/astats_over_http.c traffic_server/plugins/astats_over_http/Makefile.am /
 
-RUN tsxs -v -c astats_over_http.c -o astats_over_http.so
+RUN rpm -Uvh --nodeps /trafficserver.rpm /trafficserver-devel.rpm && \
+    tsxs -v -c astats_over_http.c -o astats_over_http.so
 
-# The symbolic link here is a shim for broken atstccfg behavior - remove when it's fixed.
-RUN mkdir -p /usr/libexec/trafficserver /opt/ort /opt/trafficserver/etc/trafficserver/ /opt/init.d && ln -s /opt/trafficserver/etc/trafficserver/ssl /etc/trafficserver/ssl && tsxs -v -o astats_over_http.so -i
+# The symbolic links here are used because trafficserver-7.1.4-2.el7.x86_64.rpm has / as TS_ROOT,
+# but the Cache Configs assume that /opt/trafficserver is TS_ROOT. The symlinks can be removed
+# once a Traffic Server RPM using /opt/trafficserver as TS_ROOT is used.
+RUN ln -s / /opt/trafficserver && \
+    mkdir /libexec /etc/trafficserver/ssl && \
+    ln -s /usr/lib64/trafficserver/plugins /libexec/trafficserver && \
+    tsxs -v -o astats_over_http.so -i
 
-RUN dnf remove -y gcc-c++ glibc-devel autoconf automake libtool && rm -f /astats_over_http.c /Makefile.am
+RUN dnf remove -y gcc-c++ glibc-devel autoconf automake libtool && \
+    rm -f /astats_over_http.c /Makefile.am
 
-# You need to do this because the RPM in the ATS archives is just all kinds of messed-up
-RUN chmod 755 /usr/lib64/trafficserver /etc/trafficserver/body_factory /etc/trafficserver/body_factory/default
-RUN mkdir -p /var/trafficserver /opt/ort && \
-    dd if=/dev/zero bs=1M count=1000 of=/var/trafficserver/cache && \
-    chown -R ats:ats /etc/trafficserver/ /var/trafficserver/ /opt/ort /usr/lib64/trafficserver/ && \
-    sed -i 's/STRING 8080 8080:ipv6/STRING 80 80:ipv6/' /etc/trafficserver/records.config
+# traffic_server needs different ownership than trafficserver-7.1.4-2.el7.x86_64.rpm sets
+RUN chown -R ats:ats /etc/trafficserver/ /usr/lib64/trafficserver/
 
-RUN setcap CAP_NET_BIND_SERVICE=+eip /bin/traffic_server && setcap CAP_NET_BIND_SERVICE=+eip /bin/traffic_manager && setcap CAP_NET_BIND_SERVICE=+eip /bin/trafficserver && setcap CAP_NET_BIND_SERVICE=+eip /bin/traffic_cop
 
 WORKDIR /opt
 
-ADD infrastructure/cdn-in-a-box/ort /opt/ort/
-ADD traffic_control/clients/python /opt/Apache-TrafficControl/
-
-RUN touch /var/log/ort.log && \
-	pip3 install ./Apache-TrafficControl && \
-	pip3 install ./ort && \
-	cp ort/traffic_ops_ort.crontab /etc/cron.d/traffic_ops_ort-cron-template && \
-	cp ort/traffic_ops_ort.logrotate /etc/logrotate.d/ort
+ADD infrastructure/cdn-in-a-box/cache/traffic_ops_ort.crontab /etc/cron.d/traffic_ops_ort-cron-template
+ADD infrastructure/cdn-in-a-box/cache/traffic_ops_ort.logrotate /etc/logrotate.d/ort

Review comment:
       Using `COPY` for local files in 41cb537aba.




-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r635503891



##########
File path: traffic_ops_ort/traffic_ops_ort.pl
##########
@@ -0,0 +1,2662 @@
+#!/usr/bin/perl

Review comment:
       Removed in 210a4027d3. There's a cache configs bug I have yet to report and I used `traffic_ops_ort.pl` in CDN in a Box (which works with the `uname` shim still there) to rule it being a regression from `traffic_ops_ort.pl` to `t3c`.




-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#issuecomment-844387566


   Updated the t3c integration tests GHA workflow to run when anything in `/cache-config/testing` is modified, since this PR updates `/cache-config/testing/docker/ort_test/systemctl.sh`. The updates to `systemctl.sh` cause the t3c integration tests to fail because `tc-fixtures.json` does not include a valid ATS version:
   
   https://github.com/apache/trafficcontrol/blob/04b6bb59188be169f22ea575b1e37d3dd67a6fe8/cache-config/testing/ort-tests/tc-fixtures.json#L1759-L1765
   
   GH issue incoming.


-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634447418



##########
File path: infrastructure/cdn-in-a-box/cache/run.sh
##########
@@ -17,14 +17,78 @@
 # specific language governing permissions and limitations
 # under the License.
 
-set -e
-set -x
-set -m
+set -o errexit -o xtrace -o monitor
+
+mkdir /tmp/ort
+
+set-dns.sh
+insert-self-into-dns.sh
+
+source /to-access.sh
+
+# Wait on SSL certificate generation
+until [[ -f "$X509_CA_ENV_FILE" ]]
+do
+  echo "Waiting on Shared SSL certificate generation"
+  sleep 3
+done
+
+# Source the CIAB-CA shared SSL environment
+until [[ -n "$X509_GENERATION_COMPLETE" ]]
+do
+  echo "Waiting on X509 vars to be defined"
+  sleep 1
+  source "$X509_CA_ENV_FILE"
+done
+
+# Trust the CIAB-CA at the System level
+cp "$X509_CA_CERT_FULL_CHAIN_FILE" /etc/pki/ca-trust/source/anchors
+update-ca-trust extract
+
+while ! to-ping 2>/dev/null; do
+	echo "waiting for Traffic Ops"
+	sleep 5
+done
+
+export TO_USER=$TO_ADMIN_USER
+export TO_PASSWORD=$TO_ADMIN_PASSWORD
+
+# wait until the CDN has been registered
+found=
+while [[ -z $found ]]; do
+    echo 'waiting for enroller setup'
+    sleep 3
+    found=$(to-get api/2.0/cdns?name="$CDN_NAME" | jq -r '.response[].name')
+done
 
 for f in /opt/init.d/*; do
     echo "$f"
-    $f
+    source "$f"
+done
+
+# Wait for SSL keys to exist
+until [[ $(to-get "api/2.0/cdns/name/$CDN_NAME/sslkeys" | jq '.response | length') -gt 0 ]]; do
+	echo 'waiting for SSL keys to exist'
+	sleep 3

Review comment:
       Spacing made consistent in 4d2c0da326.




-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634453554



##########
File path: infrastructure/cdn-in-a-box/traffic_monitor/Dockerfile
##########
@@ -20,7 +20,7 @@
 ############################################################
 
 ARG RHEL_VERSION=8
-FROM centos:${RHEL_VERSION} as trafficmonitor
+FROM centos:${RHEL_VERSION} as trafficmonitor-dependencies

Review comment:
       While it's true that  it doesn't *directly* relate to using `t3c` for cache configuration CDN in a Box, it is directly related to one of the changes that this PR involves. 942b4396de reorganizes the cache Dockerfile to avoid unnecessarily rebuilding the layer that installs `dlv`, and if that optimization was applied only to the cache Dockerfile, then the debugging stages across our CiaB Dockerfiles would be inconsistent.
   
   So I'd say that including the changes to non-cache Dockerfiles in 942b4396de leads to less entropy over our CiaB Dockerfiles, not less. For the sake of consistency, I'd be tempted to not even apply that optimization to our cache Dockerfile in this PR if not applying it to the other ones in 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] zrhoffman commented on a change in pull request #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634643155



##########
File path: infrastructure/cdn-in-a-box/traffic_ops_data/profiles/020-ATS_MID_TIER_CACHE.json
##########
@@ -255,118 +255,176 @@
 			"configFile": "ip_allow.config",
 			"name": "location",
 			"secure": false,
-			"value": "/etc/trafficserver"
+			"value": "/opt/trafficserver/etc/trafficserver"
 		},
 		{
 			"configFile": "storage.config",
 			"name": "Drive_Prefix",
 			"secure": false,
 			"value": "/var/trafficserver/"
 		},
+		{
+			"configFile": "records.config",
+			"name": "CONFIG proxy.config.ssl.CA.cert.path",
+			"secure": false,
+			"value": "STRING /opt/trafficserver/etc/trafficserver/ssl"
+		},
+		{
+			"configFile": "records.config",
+			"name": "CONFIG proxy.config.ssl.client.CA.cert.path",
+			"secure": false,
+			"value": "STRING /opt/trafficserver/etc/trafficserver/ssl"
+		},
+		{
+			"configFile": "records.config",
+			"name": "CONFIG proxy.config.ssl.client.cert.path",
+			"secure": false,
+			"value": "STRING /opt/trafficserver/etc/trafficserver/ssl"
+		},
+		{
+			"configFile": "records.config",
+			"name": "CONFIG proxy.config.ssl.client.private_key.path",
+			"secure": false,
+			"value": "STRING /opt/trafficserver/etc/trafficserver/ssl"
+		},
+		{
+			"configFile": "records.config",
+			"name": "CONFIG proxy.config.ssl.ocsp.enabled",
+			"secure": false,
+			"value": "INT 1"
+		},
+		{
+			"configFile": "records.config",
+			"name": "CONFIG proxy.config.ssl.server.cert.path",
+			"secure": false,
+			"value": "STRING /opt/trafficserver/etc/trafficserver/ssl"
+		},
+		{
+			"configFile": "records.config",
+			"name": "CONFIG proxy.config.ssl.server.private_key.path",
+			"secure": false,
+			"value": "STRING /opt/trafficserver/etc/trafficserver/ssl"
+		},
+		{
+			"configFile": "records.config",
+			"name": "CONFIG proxy.config.ssl.server.ticket_key.filename",
+			"secure": false,
+			"value": "STRING NULL"
+		},

Review comment:
       Nah, they were already there. With these added, the only configuration difference between the Edge and Mid profiles is that Edge is a reverse proxy:
   
   ```diff
   --- 010-ATS_EDGE_TIER_CACHE.json	2021-05-18 12:21:57.791484259 -0600
   +++ 020-ATS_MID_TIER_CACHE.json	2021-05-18 12:21:57.791484259 -0600
   @@ -1,7 +1,7 @@
    {
    	"cdnName": "$CDN_NAME",
   -	"description": "Edge Cache - Apache Traffic Server",
   -	"name": "EDGE_TIER_ATS_CACHE",
   +	"description": "Mid Cache - Apache Traffic Server",
   +	"name": "MID_TIER_ATS_CACHE",
    	"routingDisabled": false,
    	"type": "ATS_PROFILE",
    	"params": [
   @@ -93,7 +93,7 @@
    			"configFile": "records.config",
    			"name": "CONFIG proxy.config.reverse_proxy.enabled",
    			"secure": false,
   -			"value": "INT 1"
   +			"value": "INT 0"
    		},
    		{
    			"configFile": "records.config",
   ```




-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634453554



##########
File path: infrastructure/cdn-in-a-box/traffic_monitor/Dockerfile
##########
@@ -20,7 +20,7 @@
 ############################################################
 
 ARG RHEL_VERSION=8
-FROM centos:${RHEL_VERSION} as trafficmonitor
+FROM centos:${RHEL_VERSION} as trafficmonitor-dependencies

Review comment:
       While it's true that  it doesn't *directly* relate to using `t3c` for cache configuration CDN in a Box, it is directly related to one of the changes that this PR involves. 942b4396de reorganizes the cache Dockerfile to avoid unnecessarily rebuilding the layer that installs `dlv`, and if that optimization was applied only to the cache Dockerfile, then the debugging stages across our CiaB Dockerfiles would be inconsistent.
   
   So I'd say that including the changes to non-cache Dockerfiles in 942b4396de leads to less entropy over our CiaB Dockerfiles, not more. For the sake of consistency, I'd be tempted to not even apply that optimization to our cache Dockerfile in this PR if not applying it to the other ones in 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] zrhoffman commented on a change in pull request #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634447304



##########
File path: infrastructure/cdn-in-a-box/cache/run.sh
##########
@@ -17,14 +17,78 @@
 # specific language governing permissions and limitations
 # under the License.
 
-set -e
-set -x
-set -m
+set -o errexit -o xtrace -o monitor
+
+mkdir /tmp/ort
+
+set-dns.sh
+insert-self-into-dns.sh
+
+source /to-access.sh
+
+# Wait on SSL certificate generation
+until [[ -f "$X509_CA_ENV_FILE" ]]
+do
+  echo "Waiting on Shared SSL certificate generation"
+  sleep 3
+done
+
+# Source the CIAB-CA shared SSL environment
+until [[ -n "$X509_GENERATION_COMPLETE" ]]
+do
+  echo "Waiting on X509 vars to be defined"
+  sleep 1
+  source "$X509_CA_ENV_FILE"

Review comment:
       Spacing made consistent in 4d2c0da326.




-- 
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] jrushford commented on pull request #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
jrushford commented on pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#issuecomment-845465607


   > Awesome, can't wait.
   
   I'll work on it soon.  currently ATS won't start on the ort_test container and it looks like some additional parameters need to be added to tc-fixtures.json which ultimately has them added to the trafficserver records.config


-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634669193



##########
File path: infrastructure/cdn-in-a-box/traffic_monitor/Dockerfile
##########
@@ -20,7 +20,7 @@
 ############################################################
 
 ARG RHEL_VERSION=8
-FROM centos:${RHEL_VERSION} as trafficmonitor
+FROM centos:${RHEL_VERSION} as trafficmonitor-dependencies

Review comment:
       Opened #5866 for this optimization in the other Dockerfiles, removed it from this PR in 062205cfc4.




-- 
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 commented on a change in pull request #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634693635



##########
File path: infrastructure/cdn-in-a-box/cache/init-debug-scripts.sh
##########
@@ -0,0 +1,57 @@
+#!/usr/bin/env bash
+
+# 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.
+
+set -o errexit -o nounset
+
+maybe_debug() {
+	set -o errexit -o nounset
+	hostname="$1"
+	shift
+	debug_port="$1"
+	shift
+	actual_binary="$1"
+	shift
+	hostname="${hostname//-/_}" # replace - with _
+	hostname="${hostname^^}" # uppercase
+	debug_variable_name="T3C_DEBUG_COMPONENT_${hostname}"
+	if [[ "${!debug_variable_name}" == "${actual_binary%.actual}" ]]; then
+		command=(dlv --listen=":${debug_port}" --headless=true --api-version=2 exec "/usr/bin/${actual_binary}" --)
+	else
+		command=("$actual_binary")
+	fi
+	exec "${command[@]}" "$@"
+}
+
+hostname="$(hostname --short)"
+for t3c_tool in $(compgen -c t3c | sort | uniq); do
+	(path="$(type -p "$t3c_tool")"
+	cd "$(dirname "$path")"
+	dlv_script="${t3c_tool}.debug"
+	actual_binary="${t3c_tool}.actual"
+	touch "$dlv_script"
+	chmod +x "$dlv_script"
+	<<-DLV_SCRIPT cat > "$dlv_script"
+	#!/usr/bin/env bash
+	$(type maybe_debug | tail -n+2)
+	maybe_debug "${hostname}" "${DEBUG_PORT}" "${actual_binary}" "\$@"
+	DLV_SCRIPT
+	mv "$t3c_tool" "$actual_binary"
+	ln -s "$dlv_script" "$t3c_tool"
+	)

Review comment:
       I think the latter looks better, but could be confusing to people who just think you're trying to delimit the function contents in a manner similar to many programming languages. The former is more clearly defining scope




-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman edited a comment on pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#issuecomment-849189401


   > @zrhoffman Rob has a PR in to fix this issue, I think it's #5875. the value "*" will be replaced with the original value "CHANGEME". Somehow this was changed recently. Anyway, after the GHA workflow builds the trafficserver RPM, "CHANGEME" is modified with a sed script to reflect the correct ATS version.
   
   @jrushford Related to that, I opened PR #5876 to use `jq` to manipulate the JSON to set the ATS RPM version in `tc-fixtures.json`, which carries the benefit not needing the placeholder text to be there in order to update it.


-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634644929



##########
File path: infrastructure/cdn-in-a-box/cache/run.sh
##########
@@ -17,14 +17,78 @@
 # specific language governing permissions and limitations
 # under the License.
 
-set -e
-set -x
-set -m
+set -o errexit -o xtrace -o monitor
+
+mkdir /tmp/ort
+
+set-dns.sh
+insert-self-into-dns.sh
+
+source /to-access.sh
+
+# Wait on SSL certificate generation
+until [[ -f "$X509_CA_ENV_FILE" ]]
+do
+  echo "Waiting on Shared SSL certificate generation"
+  sleep 3
+done
+
+# Source the CIAB-CA shared SSL environment
+until [[ -n "$X509_GENERATION_COMPLETE" ]]
+do
+  echo "Waiting on X509 vars to be defined"
+  sleep 1
+  source "$X509_CA_ENV_FILE"
+done
+
+# Trust the CIAB-CA at the System level
+cp "$X509_CA_CERT_FULL_CHAIN_FILE" /etc/pki/ca-trust/source/anchors
+update-ca-trust extract
+
+while ! to-ping 2>/dev/null; do
+	echo "waiting for Traffic Ops"
+	sleep 5
+done
+
+export TO_USER=$TO_ADMIN_USER
+export TO_PASSWORD=$TO_ADMIN_PASSWORD
+
+# wait until the CDN has been registered
+found=
+while [[ -z $found ]]; do
+    echo 'waiting for enroller setup'
+    sleep 3
+    found=$(to-get api/2.0/cdns?name="$CDN_NAME" | jq -r '.response[].name')
+done
 
 for f in /opt/init.d/*; do
     echo "$f"
-    $f
+    source "$f"
+done
+
+# Wait for SSL keys to exist
+until [[ $(to-get "api/2.0/cdns/name/$CDN_NAME/sslkeys" | jq '.response | length') -gt 0 ]]; do
+	echo 'waiting for SSL keys to exist'
+	sleep 3
 done
 
-# tail -f /dev/null
-/bin/bash
+# If /tmp/trafficcontrol-cache-config does not already exist when running t3c-apply, t3c-apply will create it and fail silently
+mkdir /tmp/trafficcontrol-cache-config
+
+# hostname is already defined in /etc/init.d/99-run.sh
+hostname="${hostname//-/_}" # replace - with _
+hostname="${hostname^^}" # uppercase
+debug_variable_name="T3C_DEBUG_COMPONENT_${hostname}"
+debug_binary="${!debug_variable_name}"
+if ! type -p "$debug_binary"; then
+	t3c apply --run-mode=badass --traffic-ops-url="$TO_URL" --traffic-ops-user="$TO_USER" --traffic-ops-password="$TO_PASSWORD" --git=yes --dispersion=0 --log-location-error=stdout --log-location-warning=stdout --log-location-info=stdout all || { echo "Failed"; }

Review comment:
       Spacing made consistent in 4d2c0da326.




-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman edited a comment on pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#issuecomment-845263483


   The t3c integration tests don't seem to be built around Traffic Server running. Copied the `systemctl` shim into CDN in a Box and reverted the one used by the `t3c` integration tests in 65240c03a0.
   Edit 2021-06-01: Opened #5903 for running ATS in the t3c integration tests


-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634445157



##########
File path: infrastructure/cdn-in-a-box/cache/init-debug-scripts.sh
##########
@@ -0,0 +1,57 @@
+#!/usr/bin/env bash
+
+# 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.
+
+set -o errexit -o nounset
+
+maybe_debug() {
+	set -o errexit -o nounset
+	hostname="$1"
+	shift
+	debug_port="$1"
+	shift
+	actual_binary="$1"
+	shift
+	hostname="${hostname//-/_}" # replace - with _
+	hostname="${hostname^^}" # uppercase
+	debug_variable_name="T3C_DEBUG_COMPONENT_${hostname}"
+	if [[ "${!debug_variable_name}" == "${actual_binary%.actual}" ]]; then
+		command=(dlv --listen=":${debug_port}" --headless=true --api-version=2 exec "/usr/bin/${actual_binary}" --)
+	else
+		command=("$actual_binary")
+	fi
+	exec "${command[@]}" "$@"
+}
+
+hostname="$(hostname --short)"
+for t3c_tool in $(compgen -c t3c | sort | uniq); do
+	(path="$(type -p "$t3c_tool")"
+	cd "$(dirname "$path")"
+	dlv_script="${t3c_tool}.debug"
+	actual_binary="${t3c_tool}.actual"
+	touch "$dlv_script"
+	chmod +x "$dlv_script"
+	<<-DLV_SCRIPT cat > "$dlv_script"
+	#!/usr/bin/env bash
+	$(type maybe_debug | tail -n+2)
+	maybe_debug "${hostname}" "${DEBUG_PORT}" "${actual_binary}" "\$@"
+	DLV_SCRIPT
+	mv "$t3c_tool" "$actual_binary"
+	ln -s "$dlv_script" "$t3c_tool"
+	)

Review comment:
       To avoid having to `cd` back 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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#issuecomment-849189401


   > @zrhoffman Rob has a PR in to fix this issue, I think it's #5875. the value "*" will be replaced with the original value "CHANGEME". Somehow this was changed recently. Anyway, after the GHA workflow builds the trafficserver RPM, "CHANGEME" is modified with a sed script to reflect the correct ATS version.
   
   @jrushford Related to that, I opened PR #5876 to use `jq` to manipulate the JSON to set the ATS RPM version in `jc-fixtures.json`, which carries the benefit not needing the placeholder text to be there in order to update it.


-- 
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] rawlinp merged pull request #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
rawlinp merged pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853


   


-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634443435



##########
File path: infrastructure/cdn-in-a-box/cache/Dockerfile
##########
@@ -51,109 +54,84 @@ RUN dnf -y install epel-release && \
         make                    \
         numactl-libs            \
         perl                    \
-        perl-Carp               \
-        perl-constant           \
-        perl-Data-Dumper        \
-        perl-Encode             \
-        perl-Exporter           \
-        perl-File-Path          \
-        perl-File-Temp          \
-        perl-Filter             \
-        perl-Getopt-Long        \
-        perl-HTTP-Tiny          \
-        perl-libs               \
-        perl-macros             \
-        perl-parent             \
-        perl-PathTools          \
-        perl-Pod-Escapes        \
-        perl-podlators          \
-        perl-Pod-Perldoc        \
-        perl-Pod-Simple         \
-        perl-Pod-Usage          \
-        perl-Scalar-List-Utils  \
-        perl-Socket             \
-        perl-Storable           \
-        perl-Text-ParseWords    \
-        perl-threads            \
-        perl-threads-shared     \
-        perl-Time-HiRes         \
-        perl-Time-Local         \
-        perl-URI                \
         tcl                     \
         $additional_packages && \
     if [[ "${RHEL_VERSION%%.*}" -eq 8 ]]; then \
-        set -- \
-            # Pretend that we have the right library versions.
-            # TODO: Use a proper CentOS 7 or 8 RPM once trafficserver
-            # is in EPEL again (see apache/trafficserver#6855)
-            libtcl8.6.so        libtcl8.5.so     \
-            libncursesw.so.6    libncursesw.so.5 \
-            libtinfo.so.6       libtinfo.so.5    \
-            || exit 1; \
-    fi && \
-    cd /usr/lib64 && \
-    while [[ $# -gt 0 ]]; do \
-        source="$1" && \
-        shift && \
-        target="$1" && \
-        shift && \
-        ln -s "$source" "$target" || exit 1; \
-    done
-
-ADD https://ci.trafficserver.apache.org/RPMS/CentOS7/trafficserver-7.1.4-2.el7.x86_64.rpm /trafficserver.rpm
-ADD https://ci.trafficserver.apache.org/RPMS/CentOS7/trafficserver-devel-7.1.4-2.el7.x86_64.rpm /trafficserver-devel.rpm
+        # Pretend that we have the right library versions.
+        # TODO: Use a proper CentOS 7 or 8 RPM once trafficserver
+        # is in EPEL again (see apache/trafficserver#6855)
+        cd /usr/lib64 && \
+        ln -s libtcl8.6.so        libtcl8.5.so && \
+        ln -s libncursesw.so.6    libncursesw.so.5 && \
+        ln -s libtinfo.so.6       libtinfo.so.5  || \
+            exit 1; \
+    fi
 
-RUN rpm -Uvh --nodeps /trafficserver.rpm /trafficserver-devel.rpm && \
-    dnf install -y jq python3-psutil python3-setuptools python3-pip logrotate && \
+RUN dnf install -y bind-utils kyotocabinet-libs initscripts iproute net-tools nmap-ncat gettext autoconf automake libtool gcc-c++ cronie glibc-devel openssl-devel && \
+    dnf install -y jq logrotate && \
     dnf clean all
 
-RUN dnf install -y bind-utils kyotocabinet-libs initscripts iproute net-tools nmap-ncat gettext autoconf automake libtool gcc-c++ cronie glibc-devel openssl-devel
+FROM common-traffic-server-dependencies AS common-cache-config-layers
 
-RUN python3 -m pip install --upgrade pip && python3 -m pip install requests urllib3 distro
+ADD https://ci.trafficserver.apache.org/RPMS/CentOS7/trafficserver-7.1.4-2.el7.x86_64.rpm /trafficserver.rpm
+ADD https://ci.trafficserver.apache.org/RPMS/CentOS7/trafficserver-devel-7.1.4-2.el7.x86_64.rpm /trafficserver-devel.rpm
 
-ADD traffic_server/plugins/astats_over_http/astats_over_http.c traffic_server/plugins/astats_over_http/Makefile.am /
+COPY traffic_server/plugins/astats_over_http/astats_over_http.c traffic_server/plugins/astats_over_http/Makefile.am /
 
-RUN tsxs -v -c astats_over_http.c -o astats_over_http.so
+RUN rpm -Uvh --nodeps /trafficserver.rpm /trafficserver-devel.rpm && \
+    tsxs -v -c astats_over_http.c -o astats_over_http.so
 
-# The symbolic link here is a shim for broken atstccfg behavior - remove when it's fixed.
-RUN mkdir -p /usr/libexec/trafficserver /opt/ort /opt/trafficserver/etc/trafficserver/ /opt/init.d && ln -s /opt/trafficserver/etc/trafficserver/ssl /etc/trafficserver/ssl && tsxs -v -o astats_over_http.so -i
+# The symbolic links here are used because trafficserver-7.1.4-2.el7.x86_64.rpm has / as TS_ROOT,
+# but the Cache Configs assume that /opt/trafficserver is TS_ROOT. The symlinks can be removed
+# once a Traffic Server RPM using /opt/trafficserver as TS_ROOT is used.
+RUN ln -s / /opt/trafficserver && \
+    mkdir /libexec /etc/trafficserver/ssl && \
+    ln -s /usr/lib64/trafficserver/plugins /libexec/trafficserver && \
+    tsxs -v -o astats_over_http.so -i
 
-RUN dnf remove -y gcc-c++ glibc-devel autoconf automake libtool && rm -f /astats_over_http.c /Makefile.am
+RUN dnf remove -y gcc-c++ glibc-devel autoconf automake libtool && \
+    rm -f /astats_over_http.c /Makefile.am
 
-# You need to do this because the RPM in the ATS archives is just all kinds of messed-up
-RUN chmod 755 /usr/lib64/trafficserver /etc/trafficserver/body_factory /etc/trafficserver/body_factory/default
-RUN mkdir -p /var/trafficserver /opt/ort && \
-    dd if=/dev/zero bs=1M count=1000 of=/var/trafficserver/cache && \
-    chown -R ats:ats /etc/trafficserver/ /var/trafficserver/ /opt/ort /usr/lib64/trafficserver/ && \
-    sed -i 's/STRING 8080 8080:ipv6/STRING 80 80:ipv6/' /etc/trafficserver/records.config
+# traffic_server needs different ownership than trafficserver-7.1.4-2.el7.x86_64.rpm sets
+RUN chown -R ats:ats /etc/trafficserver/ /usr/lib64/trafficserver/
 
-RUN setcap CAP_NET_BIND_SERVICE=+eip /bin/traffic_server && setcap CAP_NET_BIND_SERVICE=+eip /bin/traffic_manager && setcap CAP_NET_BIND_SERVICE=+eip /bin/trafficserver && setcap CAP_NET_BIND_SERVICE=+eip /bin/traffic_cop
 
 WORKDIR /opt
 
-ADD infrastructure/cdn-in-a-box/ort /opt/ort/
-ADD traffic_control/clients/python /opt/Apache-TrafficControl/
-
-RUN touch /var/log/ort.log && \
-	pip3 install ./Apache-TrafficControl && \
-	pip3 install ./ort && \
-	cp ort/traffic_ops_ort.crontab /etc/cron.d/traffic_ops_ort-cron-template && \
-	cp ort/traffic_ops_ort.logrotate /etc/logrotate.d/ort
+ADD infrastructure/cdn-in-a-box/cache/traffic_ops_ort.crontab /etc/cron.d/traffic_ops_ort-cron-template
+ADD infrastructure/cdn-in-a-box/cache/traffic_ops_ort.logrotate /etc/logrotate.d/ort
 
 ADD infrastructure/cdn-in-a-box/cache/run.sh infrastructure/cdn-in-a-box/traffic_ops/to-access.sh infrastructure/cdn-in-a-box/enroller/server_template.json /
 
 COPY infrastructure/cdn-in-a-box/dns/set-dns.sh \
      infrastructure/cdn-in-a-box/dns/insert-self-into-dns.sh \
+     infrastructure/cdn-in-a-box/cache/uname \
      /usr/local/sbin/
+# Copy systemctl.sh to /usr/bin specifically because t3c runs /bin/systemctl by absolute path
+COPY cache-config/testing/docker/ort_test/systemctl.sh /usr/bin/systemctl
 
 ARG ORT_RPM=infrastructure/cdn-in-a-box/cache/trafficcontrol-cache-config.rpm
 ADD $ORT_RPM /
-RUN rpm -Uvh --nodeps /$(basename $ORT_RPM) &&\
+RUN rpm -Uvh /$(basename $ORT_RPM) &&\
     rm /$(basename $ORT_RPM)
 CMD /run.sh
 
-FROM common-cache-server-layers AS mid
+FROM common-traffic-server-dependencies AS get-delve
+RUN dnf -y install golang && \
+    go get -u github.com/go-delve/delve/cmd/dlv
+
+FROM common-cache-config-layers AS mid
 COPY infrastructure/cdn-in-a-box/mid/init.d/ /opt/init.d/
 
-FROM common-cache-server-layers AS edge
+FROM mid AS mid-debug
+COPY --from=get-delve /root/go/bin /usr/bin
+COPY infrastructure/cdn-in-a-box/cache/init-debug-scripts.sh /opt/init.d/
+
+FROM common-cache-config-layers AS edge
 COPY infrastructure/cdn-in-a-box/edge/init.d/ /opt/init.d/
+
+FROM edge AS edge-debug
+COPY --from=get-delve /root/go/bin /usr/bin
+COPY infrastructure/cdn-in-a-box/cache/init-debug-scripts.sh /opt/init.d/
+
+FROM edge

Review comment:
       Makes it so that the image builds using the `edge` target by default instead of the `edge-debug` target if no target is specified. Added a comment explaining why in 5c3def0336.




-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634681750



##########
File path: infrastructure/cdn-in-a-box/traffic_ops_data/profiles/020-ATS_MID_TIER_CACHE.json
##########
@@ -15,7 +15,7 @@
 			"configFile": "records.config",
 			"name": "CONFIG proxy.config.config_dir",
 			"secure": false,
-			"value": "STRING /etc/trafficserver"
+			"value": "STRING /opt/trafficserver/etc/trafficserver"

Review comment:
       It does, but CDN in a Box also should use a newer ATS version (currently using ATS 7.1.4), and building `./pkg -vo ats` yields an ATS RPM that uses `/opt/trafficserver` as the TS root directory, so using `/opt/trafficserver` seems inevitable.




-- 
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] jrushford edited a comment on pull request #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
jrushford edited a comment on pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#issuecomment-845473276


   > Updated the t3c integration tests GHA workflow to run when anything in `/cache-config/testing` is modified, since this PR updates `/cache-config/testing/docker/ort_test/systemctl.sh`. The updates to `systemctl.sh` cause the t3c integration tests to fail because `tc-fixtures.json` does not include a valid ATS version:
   > 
   > https://github.com/apache/trafficcontrol/blob/04b6bb59188be169f22ea575b1e37d3dd67a6fe8/cache-config/testing/ort-tests/tc-fixtures.json#L1759-L1765
   > 
   > GH issue incoming. See [#5853 (comment)](https://github.com/apache/trafficcontrol/pull/5853#issuecomment-845263483)
   
   @zrhoffman Rob has a PR in to fix this issue, I think it's #5875.  the value "*" will be replaced with the original value "CHANGEME".  Somehow this was changed recently.  Anyway, after the GHA workflow builds the trafficserver RPM, "CHANGEME" is modified with a sed script to reflect the correct ATS version.


-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634695469



##########
File path: infrastructure/cdn-in-a-box/cache/init-debug-scripts.sh
##########
@@ -0,0 +1,57 @@
+#!/usr/bin/env bash
+
+# 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.
+
+set -o errexit -o nounset
+
+maybe_debug() {
+	set -o errexit -o nounset
+	hostname="$1"
+	shift
+	debug_port="$1"
+	shift
+	actual_binary="$1"
+	shift
+	hostname="${hostname//-/_}" # replace - with _
+	hostname="${hostname^^}" # uppercase
+	debug_variable_name="T3C_DEBUG_COMPONENT_${hostname}"
+	if [[ "${!debug_variable_name}" == "${actual_binary%.actual}" ]]; then
+		command=(dlv --listen=":${debug_port}" --headless=true --api-version=2 exec "/usr/bin/${actual_binary}" --)
+	else
+		command=("$actual_binary")
+	fi
+	exec "${command[@]}" "$@"
+}
+
+hostname="$(hostname --short)"
+for t3c_tool in $(compgen -c t3c | sort | uniq); do
+	(path="$(type -p "$t3c_tool")"
+	cd "$(dirname "$path")"
+	dlv_script="${t3c_tool}.debug"
+	actual_binary="${t3c_tool}.actual"
+	touch "$dlv_script"
+	chmod +x "$dlv_script"
+	<<-DLV_SCRIPT cat > "$dlv_script"
+	#!/usr/bin/env bash
+	$(type maybe_debug | tail -n+2)
+	maybe_debug "${hostname}" "${DEBUG_PORT}" "${actual_binary}" "\$@"
+	DLV_SCRIPT
+	mv "$t3c_tool" "$actual_binary"
+	ln -s "$dlv_script" "$t3c_tool"
+	)

Review comment:
       Indenting in c6bfe3da98




-- 
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 #5853: Use `t3c` for cache configuration in CDN in a Box

Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5853:
URL: https://github.com/apache/trafficcontrol/pull/5853#discussion_r634687748



##########
File path: infrastructure/cdn-in-a-box/cache/init-debug-scripts.sh
##########
@@ -0,0 +1,57 @@
+#!/usr/bin/env bash
+
+# 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.
+
+set -o errexit -o nounset
+
+maybe_debug() {
+	set -o errexit -o nounset
+	hostname="$1"
+	shift
+	debug_port="$1"
+	shift
+	actual_binary="$1"
+	shift
+	hostname="${hostname//-/_}" # replace - with _
+	hostname="${hostname^^}" # uppercase
+	debug_variable_name="T3C_DEBUG_COMPONENT_${hostname}"
+	if [[ "${!debug_variable_name}" == "${actual_binary%.actual}" ]]; then
+		command=(dlv --listen=":${debug_port}" --headless=true --api-version=2 exec "/usr/bin/${actual_binary}" --)
+	else
+		command=("$actual_binary")
+	fi
+	exec "${command[@]}" "$@"
+}
+
+hostname="$(hostname --short)"
+for t3c_tool in $(compgen -c t3c | sort | uniq); do
+	(path="$(type -p "$t3c_tool")"
+	cd "$(dirname "$path")"
+	dlv_script="${t3c_tool}.debug"
+	actual_binary="${t3c_tool}.actual"
+	touch "$dlv_script"
+	chmod +x "$dlv_script"
+	<<-DLV_SCRIPT cat > "$dlv_script"
+	#!/usr/bin/env bash
+	$(type maybe_debug | tail -n+2)
+	maybe_debug "${hostname}" "${DEBUG_PORT}" "${actual_binary}" "\$@"
+	DLV_SCRIPT
+	mv "$t3c_tool" "$actual_binary"
+	ln -s "$dlv_script" "$t3c_tool"
+	)

Review comment:
       That would work, or it could be grouped with the loop:
   
   ```shell
   for t3c_tool in $(compgen -c t3c | sort | uniq); do (
   	path="$(type -p "$t3c_tool")"
   	# [...]
   	ln -s "$dlv_script" "$t3c_tool"
   ) done
   ```
   
   Thoughts?




-- 
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