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/06/15 02:50:03 UTC
[GitHub] [trafficcontrol] jrushford opened a new pull request #5938: Updates ATS builds for cache-config integration tests.
jrushford opened a new pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938
Modifies building ATS for cache-config integration proper versions of openssl, cjose, and jansson are linked and
provided by the ATS rpm.
## What does this PR (Pull Request) do?
- Insures the ATS build used in integration tests is linked with openssl 1.1.1, close, and Jansson
- Add support for CentOS 8
## Which Traffic Control components are affected by this PR?
- Traffic Ops ORT
## What is the best way to verify this PR?
The GHA cache-config integration tests should pass. You may
run the tests manually also, see the cache-config/testing/README.md
## If this is a bug fix, what versions of Traffic Control are affected?
## The following criteria are ALL met by this PR
- [X] This PR includes tests OR I have explained why tests are unnecessary
- [X] This PR includes documentation OR I have explained why documentation is unnecessary
- [ ] This PR includes an update to CHANGELOG.md OR such an update is not necessary
- [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)
## Additional Information
--
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 #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r653847725
##########
File path: cache-config/testing/docker/docker-compose-ats-build.yml
##########
@@ -27,11 +27,19 @@ services:
trafficserver_build:
environment:
+ - RHEL_VERSION=8
- ATS_VERSION=8.1.x
+ - CJOSE_URL=https://github.com/cisco/cjose
+ - CJOSE_TAG=latest
+ - JANSSON_URL=https://github.com/akheron/jansson
+ - JANSSON_TAG=v2.11
+ - OPENSSL_URL=https://github.com/openssl/openssl
+ - OPENSSL_TAG=OpenSSL_1_1_1
+ - RUN_ATS_UNIT_TESTS=true
Review comment:
`RUN_ATS_UNIT_TESTS` should be false when run from GitHub Actions.
##########
File path: cache-config/testing/docker/ort_test/Dockerfile
##########
@@ -28,15 +23,16 @@
# The recommended minimum size for each block devices is 1G.
# For example, `sudo modprobe brd rd_size=1048576 rd_nr=2`
-FROM centos:7
+ARG RHEL_VERSION=${RHEL_VERSION}
+FROM centos:${RHEL_VERSION}
+ARG RHEL_VERSION=${RHEL_VERSION}
Review comment:
Build args are cleared at the beginning of each Docker layer, so
```dockerfile
ARG RHEL_VERSION=${RHEL_VERSION}
```
expands to
```dockerfile
ARG RHEL_VERSION=
```
which is functionally equivalent to
```dockerfile
ARG RHEL_VERSION
```
Trying to build the Dockerfile without providing a build arg fails:
```shell
[user@computer trafficcontrol]$ cd cache-config/testing/docker/ort_test
[user@computer ort_test]$ docker build .
[+] Building 0.8s (2/2) FINISHED
=> [internal] load build definition from Dockerfile 0.4s
=> => transferring dockerfile: 38B 0.0s
=> [internal] load .dockerignore 0.6s
=> => transferring context: 2B 0.0s
failed to solve with frontend dockerfile.v0: failed to create LLB definition: failed to parse stage name "centos:": invalid reference format
```
##########
File path: cache-config/testing/docker/docker-compose-ats-build.yml
##########
@@ -27,11 +27,19 @@ services:
trafficserver_build:
environment:
+ - RHEL_VERSION=8
Review comment:
Building the image using build arg `RHEL_VERSION=7` and running using that image and environment variable `RHEL_VERSION=8` (and vice versa) causes the ATS RPM to fail to build. So, either
* `RHEL_VERSION` should be copied from the build arg to the environment at Docker image build time like the t3c tests `traffic_ops` Dockerfile does:
https://github.com/apache/trafficcontrol/blob/251db037b425793229cc2239596e0b183b288a64/cache-config/testing/docker/traffic_ops/Dockerfile#L27 or
* `RHEL_VERSION` can be detected from the OS at run time: https://github.com/apache/trafficcontrol/blob/251db037b425793229cc2239596e0b183b288a64/build/functions.sh#L136-L137
##########
File path: cache-config/testing/docker/docker-compose-ats-build.yml
##########
@@ -27,11 +27,19 @@ services:
trafficserver_build:
environment:
+ - RHEL_VERSION=8
Review comment:
`RHEL_VERSION` is still defined in the environment in the compose file, which overrides `RHEL_VERSION` as set by an `ENV` instruction in the Dockerfile, so it should still be removed here.
--
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 a change in pull request #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r652330049
##########
File path: .github/workflows/ciab.yaml
##########
@@ -45,6 +45,7 @@ on:
- 'NOTICE'
- 'traffic_control/java/**'
- 'cache-config/testing/**'
+ 'cache-config/testing/docker/trafficserver/**'
Review comment:
@zrhoffman I guess I mis-understood your ask ^^^ above ^^^. What precisely do you want added/not added to ciab.yaml
--
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 a change in pull request #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r652314341
##########
File path: cache-config/testing/docker/ort_test/Dockerfile
##########
@@ -28,15 +28,17 @@
# The recommended minimum size for each block devices is 1G.
# For example, `sudo modprobe brd rd_size=1048576 rd_nr=2`
-FROM centos:7
+FROM centos:${RHEL_VERSION:-8}
MAINTAINER dev@trafficcontrol.apache.org
+RUN yum -y install epel-release
+
+RUN yum repolist
+
RUN yum install -y \
- https://download.postgresql.org/pub/repos/yum/reporpms/EL-7-x86_64/pgdg-redhat-repo-latest.noarch.rpm \
- epel-release initscripts postgresql13.x86_64 git gcc lua-5.1.4-15.el7 lua-devel-5.1.4-15.el7 \
- ImageMagick-c++-devel && \
- # jq is used in run.sh to update tc-fixtures.json with the ATS RPM version
- yum install -y jq
+ initscripts git jq \
+ git gcc jq \
Review comment:
fixed
##########
File path: cache-config/testing/docker/ort_test/Dockerfile
##########
@@ -28,15 +28,17 @@
# The recommended minimum size for each block devices is 1G.
# For example, `sudo modprobe brd rd_size=1048576 rd_nr=2`
-FROM centos:7
+FROM centos:${RHEL_VERSION:-8}
MAINTAINER dev@trafficcontrol.apache.org
+RUN yum -y install epel-release
+
+RUN yum repolist
+
RUN yum install -y \
- https://download.postgresql.org/pub/repos/yum/reporpms/EL-7-x86_64/pgdg-redhat-repo-latest.noarch.rpm \
- epel-release initscripts postgresql13.x86_64 git gcc lua-5.1.4-15.el7 lua-devel-5.1.4-15.el7 \
- ImageMagick-c++-devel && \
- # jq is used in run.sh to update tc-fixtures.json with the ATS RPM version
- yum install -y jq
+ initscripts git jq \
+ git gcc jq \
Review comment:
fixed
--
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 a change in pull request #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r652165330
##########
File path: cache-config/testing/docker/docker-compose-ats-build.yml
##########
@@ -27,11 +27,14 @@ services:
trafficserver_build:
environment:
- - ATS_VERSION=8.1.x
+ - CJOSE_URL=https://github.com/cisco/cjose
+ - CJOSE_TAG=latest
+ - JANSSON_URL=https://github.com/akheron/jansson
+ - JANSSON_TAG=v2.11
+ - OPENSSL_URL=https://github.com/openssl/openssl
+ - OPENSSL_TAG=OpenSSL_1_1_1
build:
context: ../../..
dockerfile: cache-config/testing/docker/trafficserver/Dockerfile
- args:
- RHEL_VERSION: ${RHEL_VERSION:-7}
Review comment:
@zrhoffman is this something new, docker-compose doesn't support Parameter expansion
`docker-compose -f docker-compose-ats-build.yml build trafficserver_build
ERROR: Invalid interpolation format for "build" option in service "trafficserver_build": "${RHEL_VERSION:-7}"`
--
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 #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r651578171
##########
File path: cache-config/testing/docker/trafficserver/trafficserver.spec
##########
@@ -16,59 +17,70 @@
# under the License.
#
# SPDX-License-Identifier: Apache-2.0
-#
+
%global src %{_topdir}/SOURCES/trafficserver
%global git_args --git-dir="%{src}/.git" --work-tree="%{src}"
-%global git_tag %(git %{git_args} describe --long | sed 's/^\\\(.*\\\)-\\\([0-9]\\\+\\\)-g\\\([0-9a-f]\\\+\\\)$/\\\1/' | sed 's/-/_/')
+%global tag %(git %{git_args} describe --long | sed 's/^\\\(.*\\\)-\\\([0-9]\\\+\\\)-g\\\([0-9a-f]\\\+\\\)$/\\\1/' | sed 's/-/_/')
%global distance %(git %{git_args} describe --long | sed 's/^\\\(.*\\\)-\\\([0-9]\\\+\\\)-g\\\([0-9a-f]\\\+\\\)$/\\\2/')
-%global commit %(git %{git_args} describe --long | sed 's/^\\\(.*\\\)-\\\([0-9]\\\+\\\)-g\\\([0-9a-f]\\\+\\\)$/\\\3/')
+%global commit %(git %{git_args} describe --long | sed 's/^\\\(.*\\\)-\\\([0-9]\\\+\\\)-g\\\([0-9a-f]\\\+\\\)$/\\\3/')
%global git_serial %(git %{git_args} rev-list HEAD | wc -l)
%global install_prefix "/opt"
%global api_stats "4096"
%global _find_debuginfo_dwz_opts %{nil}
-
-%global min_tag 8.1.0
-%global tag %(echo -e '%{min_tag}\\n%{git_tag}' | sort | tail -n 1 )
+%{!?_with_openssl_included: %{!?_without_openssl_included: %define _without_openssl_included --without-openssl_included}}
+%{?_with_openssl_included: %{?_without_openssl_included: %{error: both _with_openssl_included and _without_openssl_included}}}
+%{!?_with_openssl_included: %{!?_without_openssl_included: %{error: neither _with_openssl_included nor _without_openssl_included}}}
+%{?_without_openssl_included:BuildRequires: openssl-devel}
Name: trafficserver
Version: %{tag}
Epoch: %{git_serial}
Release: %{distance}.%{commit}%{?dist}
Summary: Apache Traffic Server
-Packager: ORT integration tests.
-Vendor: IPCDN
+Vendor: Apache
Group: Applications/Communications
License: Apache License, Version 2.0
-URL: https://github.com/apache/trafficcontrol
+URL: https://github.com/apache/trafficserver
BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
-Requires: tcl, hwloc, pcre, libcap, brotli, libmaxminddb, openssl
-BuildRequires: autoconf, automake, libtool, pcre, libcap-devel, pcre-devel
-%{?el7:BuildRequires: devtoolset-9}
-%{?el8:BuildRequires: gcc-toolset-9-gcc, gcc-toolset-9-gcc-c++}
-Source: trafficserver
+Requires: tcl, hwloc, pcre, libcap
+BuildRequires: autoconf, automake, libtool, gcc-c++, glibc-devel, expat-devel, pcre, libcap-devel, pcre-devel, perl-ExtUtils-MakeMaker, tcl-devel, hwloc-devel
+Source: src
%description
-Apache Traffic Server with Comcast modifications and environment specific modifications
+Apache Traffic Server with Apache Traffic Control modifications and environment specific modifications
%prep
%setup -c -T
cp -far %{src}/. .
+cp -far %{src}/../traffic_server_jemalloc ..
Review comment:
`-a` implies `-r`, so `cp -far` can be just `cp -fa`.
##########
File path: cache-config/testing/docker/trafficserver/run.sh
##########
@@ -18,32 +18,58 @@
# under the License.
#
-function initBuildArea() {
- cd /root
-
- # prep build environment
- [ -e rpmbuild ] && rm -rf rpmbuild
- [ ! -e rpmbuild ] || { echo "Failed to clean up rpm build directory 'rpmbuild': $?" >&2; exit 1; }
- mkdir -p rpmbuild/{BUILD,BUILDROOT,RPMS,SPECS,SOURCES,SRPMS} || { echo "Failed to create build directory '$RPMBUILD': $?" >&2;
- exit 1; }
+function die() {
+ { test -n "$@" && echo "$@"; exit 1; } >&2
}
-setowner() {
- own="$(stat -c%u:%g "$1")"
- shift
- chown -R "${own}" "$@"
-}
-trap 'exit_code=$?; setowner /trafficcontrol /trafficcontrol/dist; exit $exit_code' EXIT;
-
-case ${ATS_VERSION:0:1} in
- 8) cp /trafficserver-8.spec /trafficserver.spec
- ;;
- 9) cp /trafficserver-9.spec /trafficserver.spec
- ;;
- *) echo "Unknown trafficserver version was specified"
- exit 1
- ;;
-esac
+# sets minimum defaults if these are undefined. These are normally defined in the
+# environment section of the GHA workflow see, '.github/workflows/cache-config-tests.yml'
+RHEL_VERSION="${RHEL_VERSION:-7}"
+ATS_VERSION="${ATS_VERSION:-8.1.x}"
+
+echo "RHEL_VERSION:${RHEL_VERSION}"
+echo "ATS_VERSION:${ATS_VERSION}"
+
+mkdir -p /opt/build
+cd /opt/build
+
+# build openssl 1.1.1 if RHEL_VERSION is not 8 or greater.
+if [[ ${RHEL_VERSION%%.*} -le 7 ]]; then
+ git clone $OPENSSL_URL --branch $OPENSSL_TAG || die "Failed to fetch the OpenSSL Source."
+ (
+ cd /opt/build/openssl && \
+ ./config --prefix=/opt/trafficserver/openssl --openssldir=/opt/trafficserver zlib && \
+ make -j && \
+ make install_sw
+ ) || die "Failed to build OpenSSL"
+ cjose_openssl='--with-openssl=/opt/trafficserver/openssl'
+ rpmbuild_openssl='--with openssl_included'
+else
+ cjose_openssl=''
+ rpmbuild_openssl='--without openssl_included'
+fi
+
+# Build jansson
+(
+ git clone $JANSSON_URL --branch $JANSSON_TAG
+ cd /opt/build/jansson && patch -p1 < /jansson.pic.patch && \
+ autoreconf -i && ./configure --enable-shared=no && make -j \
+ && make install
+) || die "Failed to install jansson from source."
+
+# Build and install cjose
+(
+ git clone $CJOSE_URL --branch $CJOSE_TAG
+ cd /opt/build/cjose && patch -p1 < /cjose.pic.patch && \
+ autoreconf -i && ./configure --enable-shared=no \
+ ${cjose_openssl} && make -j`nproc` && make install
Review comment:
This command substitution should be `$(nproc)` instead of using legacy backticks.
--
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 a change in pull request #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r652315820
##########
File path: cache-config/testing/docker/trafficserver/.dependency_license
##########
@@ -0,0 +1,23 @@
+# 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.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+# Patch files, cannot bear comments
+cjose.pic.patch, Apache-2.0
Review comment:
fixed
--
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 #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r652110854
##########
File path: cache-config/testing/docker/trafficserver/run.sh
##########
@@ -57,31 +83,43 @@ else
source scl_source enable gcc-toolset-9
fi
-initBuildArea
+cd /root
+# prep build environment
+[ -e rpmbuild ] && rm -rf rpmbuild
+[ ! -e rpmbuild ] || { echo "Failed to clean up rpm build directory 'rpmbuild': $?" >&2; exit 1; }
+mkdir -p rpmbuild/{BUILD,BUILDROOT,RPMS,SPECS,SOURCES,SRPMS} || die "Failed to create build directory '$RPMBUILD': $?"
cd /root/rpmbuild/SOURCES
# clone the trafficserver repo
-git clone https://github.com/apache/trafficserver.git
+git clone https://github.com/apache/trafficserver.git --branch $ATS_VERSION || die "Failed to fetch the ATS Source"
+cp /traffic_server_jemalloc .
+
+# patch in the astats plugin
+(cp -far /trafficcontrol/traffic_server/_tsb/src/ats/plugins/astats_over_http \
Review comment:
`-a` implies `-r`, so `cp -far` can be just `cp -fa`.
##########
File path: cache-config/testing/docker/trafficserver/run.sh
##########
@@ -57,31 +83,43 @@ else
source scl_source enable gcc-toolset-9
fi
-initBuildArea
+cd /root
+# prep build environment
+[ -e rpmbuild ] && rm -rf rpmbuild
+[ ! -e rpmbuild ] || { echo "Failed to clean up rpm build directory 'rpmbuild': $?" >&2; exit 1; }
+mkdir -p rpmbuild/{BUILD,BUILDROOT,RPMS,SPECS,SOURCES,SRPMS} || die "Failed to create build directory '$RPMBUILD': $?"
cd /root/rpmbuild/SOURCES
# clone the trafficserver repo
-git clone https://github.com/apache/trafficserver.git
+git clone https://github.com/apache/trafficserver.git --branch $ATS_VERSION || die "Failed to fetch the ATS Source"
+cp /traffic_server_jemalloc .
+
+# patch in the astats plugin
+(cp -far /trafficcontrol/traffic_server/_tsb/src/ats/plugins/astats_over_http \
+ /root/rpmbuild/SOURCES/trafficserver/plugins/astats_over_http
-# build trafficserver version 9
+ed /root/rpmbuild/SOURCES/trafficserver/plugins/Makefile.am << ED
+/stats_over_http/
+a
+include astats_over_http/Makefile.inc
+.
+wQ
+ED
+) || die "Failed to patch in astats_over_http"
+
+# build a trafficserver RPM
rm -f /root/rpmbuild/RPMS/x86_64/trafficserver-*.rpm
cd trafficserver
-git fetch --all
-git checkout $ATS_VERSION
-rpmbuild -bb /trafficserver.spec
+
+rpmbuild -bb ${rpmbuild_openssl} /trafficserver.spec || die "Failed to build the ATS RPM."
echo "Build completed"
if [[ ! -d /trafficcontrol/dist ]]; then
mkdir /trafficcontrol/dist
fi
-case ${ATS_VERSION:0:1} in
- 8) cp /root/rpmbuild/RPMS/x86_64/trafficserver-8*.rpm /trafficcontrol/dist
- ;;
- 9) cp /root/rpmbuild/RPMS/x86_64/trafficserver-8*.rpm /trafficcontrol/dist
- ;;
- *) echo "Unknown trafficserver version was specified"
- exit 1
- ;;
-esac
+cp /root/rpmbuild/RPMS/x86_64/trafficserver*.rpm /trafficcontrol/dist || \
+ die "Failed to copy the ATS RPM to the dist directory"
Review comment:
`||` implies a second line, so `\` is unnecessary.
##########
File path: cache-config/testing/docker/ort_test/Dockerfile
##########
@@ -28,15 +28,17 @@
# The recommended minimum size for each block devices is 1G.
# For example, `sudo modprobe brd rd_size=1048576 rd_nr=2`
-FROM centos:7
+FROM centos:${RHEL_VERSION:-8}
MAINTAINER dev@trafficcontrol.apache.org
+RUN yum -y install epel-release
+
+RUN yum repolist
+
RUN yum install -y \
Review comment:
Since these 3 `RUN` instructions all only perform `yum` commands, can they be condensed into a single `RUN` instruction for cache busting?
##########
File path: cache-config/testing/docker/trafficserver/run.sh
##########
@@ -57,31 +83,42 @@ else
source scl_source enable gcc-toolset-9
fi
-initBuildArea
+cd /root
+# prep build environment
+[ -e rpmbuild ] && rm -rf rpmbuild
+[ ! -e rpmbuild ] || { echo "Failed to clean up rpm build directory 'rpmbuild': $?" >&2; exit 1; }
+mkdir -p rpmbuild/{BUILD,BUILDROOT,RPMS,SPECS,SOURCES,SRPMS} || die "Failed to create build directory '$RPMBUILD': $?"
cd /root/rpmbuild/SOURCES
# clone the trafficserver repo
-git clone https://github.com/apache/trafficserver.git
+git clone https://github.com/apache/trafficserver.git --branch $ATS_VERSION || die "Failed to fetch the ATS Source"
+cp /traffic_server_jemalloc .
+
+# patch in the astats plugin
+(cp -far /astats_over_http /root/rpmbuild/SOURCES/trafficserver/plugins/astats_over_http
-# build trafficserver version 9
+ed /root/rpmbuild/SOURCES/trafficserver/plugins/Makefile.am << ED
Review comment:
The stock CentOS 7 and CentOS 8 Docker images do not include `ed`.
##########
File path: cache-config/testing/docker/trafficserver/run.sh
##########
@@ -18,32 +18,58 @@
# under the License.
#
-function initBuildArea() {
- cd /root
-
- # prep build environment
- [ -e rpmbuild ] && rm -rf rpmbuild
- [ ! -e rpmbuild ] || { echo "Failed to clean up rpm build directory 'rpmbuild': $?" >&2; exit 1; }
- mkdir -p rpmbuild/{BUILD,BUILDROOT,RPMS,SPECS,SOURCES,SRPMS} || { echo "Failed to create build directory '$RPMBUILD': $?" >&2;
- exit 1; }
+function die() {
+ { test -n "$@" && echo "$@"; exit 1; } >&2
}
-setowner() {
- own="$(stat -c%u:%g "$1")"
- shift
- chown -R "${own}" "$@"
-}
-trap 'exit_code=$?; setowner /trafficcontrol /trafficcontrol/dist; exit $exit_code' EXIT;
-
-case ${ATS_VERSION:0:1} in
- 8) cp /trafficserver-8.spec /trafficserver.spec
- ;;
- 9) cp /trafficserver-9.spec /trafficserver.spec
- ;;
- *) echo "Unknown trafficserver version was specified"
- exit 1
- ;;
-esac
+# sets minimum defaults if these are undefined. These are normally defined in the
+# environment section of the GHA workflow see, '.github/workflows/cache-config-tests.yml'
+RHEL_VERSION="${RHEL_VERSION:-8}"
+ATS_VERSION="${ATS_VERSION:-8.1.x}"
+
+echo "RHEL_VERSION:${RHEL_VERSION}"
+echo "ATS_VERSION:${ATS_VERSION}"
+
+mkdir -p /opt/build
+cd /opt/build
+
+# build openssl 1.1.1 if RHEL_VERSION is not 8 or greater.
+if [[ ${RHEL_VERSION%%.*} -le 7 ]]; then
+ git clone $OPENSSL_URL --branch $OPENSSL_TAG || die "Failed to fetch the OpenSSL Source."
+ (
+ cd /opt/build/openssl && \
+ ./config --prefix=/opt/trafficserver/openssl --openssldir=/opt/trafficserver zlib && \
+ make -j$(nproc) && \
Review comment:
No need to include `\` after `&&` outside of Dockerfiles.
##########
File path: cache-config/testing/docker/ort_test/Dockerfile
##########
@@ -28,15 +28,17 @@
# The recommended minimum size for each block devices is 1G.
# For example, `sudo modprobe brd rd_size=1048576 rd_nr=2`
-FROM centos:7
+FROM centos:${RHEL_VERSION:-8}
MAINTAINER dev@trafficcontrol.apache.org
+RUN yum -y install epel-release
+
+RUN yum repolist
+
RUN yum install -y \
- https://download.postgresql.org/pub/repos/yum/reporpms/EL-7-x86_64/pgdg-redhat-repo-latest.noarch.rpm \
- epel-release initscripts postgresql13.x86_64 git gcc lua-5.1.4-15.el7 lua-devel-5.1.4-15.el7 \
- ImageMagick-c++-devel && \
- # jq is used in run.sh to update tc-fixtures.json with the ATS RPM version
- yum install -y jq
+ initscripts git jq \
+ git gcc jq \
Review comment:
`jq` is listed twice
##########
File path: cache-config/testing/docker/docker-compose-ats-build.yml
##########
@@ -27,11 +27,14 @@ services:
trafficserver_build:
environment:
- - ATS_VERSION=8.1.x
+ - CJOSE_URL=https://github.com/cisco/cjose
+ - CJOSE_TAG=latest
+ - JANSSON_URL=https://github.com/akheron/jansson
+ - JANSSON_TAG=v2.11
+ - OPENSSL_URL=https://github.com/openssl/openssl
+ - OPENSSL_TAG=OpenSSL_1_1_1
build:
context: ../../..
dockerfile: cache-config/testing/docker/trafficserver/Dockerfile
- args:
- RHEL_VERSION: ${RHEL_VERSION:-7}
Review comment:
Why remove the `RHEL_VERSION` build arg? Without it, `RHEL_VERSION` is not inherited from the environment, which is how the CDN in a Box Makefile passes the `RHEL_VERSION` build arg to Dockerfiles.
##########
File path: cache-config/testing/docker/ort_test/Dockerfile
##########
@@ -28,15 +28,17 @@
# The recommended minimum size for each block devices is 1G.
# For example, `sudo modprobe brd rd_size=1048576 rd_nr=2`
-FROM centos:7
+FROM centos:${RHEL_VERSION:-8}
Review comment:
Because there is no [`ARG`](https://docs.docker.com/engine/reference/builder/#arg) instruction for `RHEL_VERSION`, there is no way to set `RHEL_VERSION` to `7` at build time.
##########
File path: cache-config/testing/docker/trafficserver/Dockerfile
##########
@@ -17,54 +17,106 @@
###############################################################
# Dockerfile to build Traffic Server RPM
-# Based on CentOS 7 for ATS RPM ot match the ort_test
-# container running CentOS 7.
+# Defaults to CentOS 8, see RHEL_VERSION
###############################################################
-ARG RHEL_VERSION=7
+# This sets the minimum RHEL_VERSION to 8
+# You may override and use RHEL_VERSION 7 using:
+# 'docker-compose build --build-args RHEL_VERSION=7 ats_build'
+# and followed by
+# 'docker-compose run -e RHEL_VERSION=7 ats_build'
+#
+# For the GHA workflow, override this in .github/workflows/cache-config-tests.yml
+ARG RHEL_VERSION=${RHEL_VERSION:-8}
Review comment:
The `$RHEL_VERSION` build arg is always undefined before this line. Because `${RHEL_VERSION:-8}` always defaults to `8`, it can be just `RHEL_VERSION=8`.
##########
File path: .github/workflows/ciab.yaml
##########
@@ -45,6 +45,7 @@ on:
- 'NOTICE'
- 'traffic_control/java/**'
- 'cache-config/testing/**'
+ 'cache-config/testing/docker/trafficserver/**'
Review comment:
This line should be prefixed with `-` to make it a separate YAML array element.
##########
File path: cache-config/testing/docker/ort_test/Dockerfile
##########
@@ -28,15 +28,17 @@
# The recommended minimum size for each block devices is 1G.
# For example, `sudo modprobe brd rd_size=1048576 rd_nr=2`
-FROM centos:7
+FROM centos:${RHEL_VERSION:-8}
MAINTAINER dev@trafficcontrol.apache.org
+RUN yum -y install epel-release
+
+RUN yum repolist
+
RUN yum install -y \
- https://download.postgresql.org/pub/repos/yum/reporpms/EL-7-x86_64/pgdg-redhat-repo-latest.noarch.rpm \
- epel-release initscripts postgresql13.x86_64 git gcc lua-5.1.4-15.el7 lua-devel-5.1.4-15.el7 \
- ImageMagick-c++-devel && \
- # jq is used in run.sh to update tc-fixtures.json with the ATS RPM version
- yum install -y jq
+ initscripts git jq \
+ git gcc jq \
Review comment:
`git` is listed twice
##########
File path: cache-config/testing/docker/trafficserver/Dockerfile
##########
@@ -17,54 +17,106 @@
###############################################################
# Dockerfile to build Traffic Server RPM
-# Based on CentOS 7 for ATS RPM ot match the ort_test
-# container running CentOS 7.
+# Defaults to CentOS 8, see RHEL_VERSION
###############################################################
-ARG RHEL_VERSION=7
+# This sets the minimum RHEL_VERSION to 8
+# You may override and use RHEL_VERSION 7 using:
+# 'docker-compose build --build-args RHEL_VERSION=7 ats_build'
+# and followed by
+# 'docker-compose run -e RHEL_VERSION=7 ats_build'
+#
+# For the GHA workflow, override this in .github/workflows/cache-config-tests.yml
+ARG RHEL_VERSION=${RHEL_VERSION:-8}
FROM centos:${RHEL_VERSION}
-ARG RHEL_VERSION=7
+ARG RHEL_VERSION=${RHEL_VERSION:-8}
Review comment:
Same here. The `$RHEL_VERSION` build arg is always undefined before this line. Because `${RHEL_VERSION:-8}` always defaults to `8`, it can be just `RHEL_VERSION=8`.
##########
File path: .github/workflows/ciab.yaml
##########
@@ -69,6 +70,7 @@ on:
- 'NOTICE'
- 'traffic_control/java/**'
- 'cache-config/testing/**'
+ - 'cache-config/testing/docker/trafficserver**'
Review comment:
Since this section is `on.pull_request.paths-ignore`, `cache-config/testing/docker/trafficserver/**` should be prefixed with `!` to register the exception.
##########
File path: cache-config/testing/docker/trafficserver/.dependency_license
##########
@@ -0,0 +1,23 @@
+# 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.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+# Patch files, cannot bear comments
+cjose.pic.patch, Apache-2.0
Review comment:
CJOSE is MIT-licensed.
##########
File path: .github/workflows/ciab.yaml
##########
@@ -45,6 +45,7 @@ on:
- 'NOTICE'
- 'traffic_control/java/**'
- 'cache-config/testing/**'
+ 'cache-config/testing/docker/trafficserver/**'
Review comment:
Since this section is `on.push.paths-ignore`, `cache-config/testing/docker/trafficserver/**` should be prefixed with `!` to register the exception.
--
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 a change in pull request #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r652316771
##########
File path: cache-config/testing/docker/trafficserver/.dependency_license
##########
@@ -0,0 +1,23 @@
+# 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.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+# Patch files, cannot bear comments
+cjose.pic.patch, Apache-2.0
Review comment:
updated
##########
File path: cache-config/testing/docker/trafficserver/.dependency_license
##########
@@ -0,0 +1,23 @@
+# 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.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+# Patch files, cannot bear comments
+cjose.pic.patch, Apache-2.0
Review comment:
fixed
--
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 a change in pull request #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r652314143
##########
File path: cache-config/testing/docker/ort_test/Dockerfile
##########
@@ -28,15 +28,17 @@
# The recommended minimum size for each block devices is 1G.
# For example, `sudo modprobe brd rd_size=1048576 rd_nr=2`
-FROM centos:7
+FROM centos:${RHEL_VERSION:-8}
MAINTAINER dev@trafficcontrol.apache.org
+RUN yum -y install epel-release
+
+RUN yum repolist
+
RUN yum install -y \
Review comment:
done
--
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 #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r652268683
##########
File path: cache-config/testing/docker/docker-compose-ats-build.yml
##########
@@ -27,11 +27,14 @@ services:
trafficserver_build:
environment:
- - ATS_VERSION=8.1.x
+ - CJOSE_URL=https://github.com/cisco/cjose
+ - CJOSE_TAG=latest
+ - JANSSON_URL=https://github.com/akheron/jansson
+ - JANSSON_TAG=v2.11
+ - OPENSSL_URL=https://github.com/openssl/openssl
+ - OPENSSL_TAG=OpenSSL_1_1_1
build:
context: ../../..
dockerfile: cache-config/testing/docker/trafficserver/Dockerfile
- args:
- RHEL_VERSION: ${RHEL_VERSION:-7}
Review comment:
@jrushford Variable interpolation is supported in [`docker-compose.yml` version 2.1 and up](https://docs.docker.com/compose/release-notes/#new-features-15) (and `docker-compose` version 1.9.0 and up).
Increasing the `docker-compose.yml` version to `'2.1'` should make it work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] jrushford commented on a change in pull request #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r652323393
##########
File path: cache-config/testing/docker/trafficserver/.dependency_license
##########
@@ -0,0 +1,23 @@
+# 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.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+# Patch files, cannot bear comments
+cjose.pic.patch, Apache-2.0
+jansson.pic.patch, Apache-2.0
Review comment:
The package may be MIT-licensed but the pic patch files are from from us. Chris Lemmons wrote the patches to add -fpic when building those packages for his ATS uri signing plugin. See Chris's traffic_server/_tsb/.dependency_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] zrhoffman merged pull request #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
zrhoffman merged pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938
--
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 pull request #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#issuecomment-862028450
I was about to restart the CiaB job, but it's a real failure:
```
Executing(%clean): /bin/sh -e /var/tmp/rpm-tmp.ThI6Uy
+ umask 022
+ cd /root/rpmbuild/BUILD
+ cd trafficserver-8.1.1
+ rm -rf /root/rpmbuild/BUILDROOT/trafficserver-8.1.1-45.55bbe4a64.el8.x86_64
+ exit 0
Build completed
trafficserver RPM has been copied
cp -f "../../dist/trafficserver-.el8.x86_64.rpm" "cache/trafficserver.rpm" || (rm ".//cache/ATS_VERSION"; false)
cp: cannot stat '../../dist/trafficserver-.el8.x86_64.rpm': No such file or directory
rm: cannot remove './/cache/ATS_VERSION': No such file or directory
make: *** [Makefile:147: cache/trafficserver.rpm] Error 1
Child process make exited with status code 2 !
```
Thought it was worth calling out explicitly, since my first instinct was to just restart 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] jrushford commented on a change in pull request #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r652324114
##########
File path: cache-config/testing/docker/trafficserver/.dependency_license
##########
@@ -0,0 +1,23 @@
+# 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.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+# Patch files, cannot bear comments
+cjose.pic.patch, Apache-2.0
Review comment:
The package may be MIT-licensed but the pic patch files are from from us. Chris Lemmons wrote the patches to add -fpic when building those packages for his ATS uri signing plugin. See Chris's traffic_server/_tsb/.dependency_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] jrushford commented on a change in pull request #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r652316771
##########
File path: cache-config/testing/docker/trafficserver/.dependency_license
##########
@@ -0,0 +1,23 @@
+# 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.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+# Patch files, cannot bear comments
+cjose.pic.patch, Apache-2.0
Review comment:
updated
--
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 a change in pull request #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r652320142
##########
File path: cache-config/testing/docker/trafficserver/run.sh
##########
@@ -57,31 +83,42 @@ else
source scl_source enable gcc-toolset-9
fi
-initBuildArea
+cd /root
+# prep build environment
+[ -e rpmbuild ] && rm -rf rpmbuild
+[ ! -e rpmbuild ] || { echo "Failed to clean up rpm build directory 'rpmbuild': $?" >&2; exit 1; }
+mkdir -p rpmbuild/{BUILD,BUILDROOT,RPMS,SPECS,SOURCES,SRPMS} || die "Failed to create build directory '$RPMBUILD': $?"
cd /root/rpmbuild/SOURCES
# clone the trafficserver repo
-git clone https://github.com/apache/trafficserver.git
+git clone https://github.com/apache/trafficserver.git --branch $ATS_VERSION || die "Failed to fetch the ATS Source"
+cp /traffic_server_jemalloc .
+
+# patch in the astats plugin
+(cp -far /astats_over_http /root/rpmbuild/SOURCES/trafficserver/plugins/astats_over_http
-# build trafficserver version 9
+ed /root/rpmbuild/SOURCES/trafficserver/plugins/Makefile.am << ED
Review comment:
ed was and is installed, see the trafficserver/Dockerfile
--
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 #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r652343055
##########
File path: cache-config/testing/docker/trafficserver/run.sh
##########
@@ -57,31 +83,42 @@ else
source scl_source enable gcc-toolset-9
fi
-initBuildArea
+cd /root
+# prep build environment
+[ -e rpmbuild ] && rm -rf rpmbuild
+[ ! -e rpmbuild ] || { echo "Failed to clean up rpm build directory 'rpmbuild': $?" >&2; exit 1; }
+mkdir -p rpmbuild/{BUILD,BUILDROOT,RPMS,SPECS,SOURCES,SRPMS} || die "Failed to create build directory '$RPMBUILD': $?"
cd /root/rpmbuild/SOURCES
# clone the trafficserver repo
-git clone https://github.com/apache/trafficserver.git
+git clone https://github.com/apache/trafficserver.git --branch $ATS_VERSION || die "Failed to fetch the ATS Source"
+cp /traffic_server_jemalloc .
+
+# patch in the astats plugin
+(cp -far /astats_over_http /root/rpmbuild/SOURCES/trafficserver/plugins/astats_over_http
-# build trafficserver version 9
+ed /root/rpmbuild/SOURCES/trafficserver/plugins/Makefile.am << ED
Review comment:
Although using `ed` works, if `wQ` is changed to `wq` in the edscript, you can use `ex`, which CentOS does include. Because lowercase `wq` works in `ed`, this is not a BC break.
--
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 #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r652338255
##########
File path: .github/workflows/ciab.yaml
##########
@@ -45,6 +45,7 @@ on:
- 'NOTICE'
- 'traffic_control/java/**'
- 'cache-config/testing/**'
+ 'cache-config/testing/docker/trafficserver/**'
Review comment:
Just making that line
```yaml
'!cache-config/testing/docker/trafficserver/**'
```
to trigger the *CDN in a Box CI* workflow when that path is modified. It isn't explicitly related to this PR, so we can just as easily add it in a separate PR after (or before) #5938 is merged.
--
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 #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r652139299
##########
File path: cache-config/testing/docker/trafficserver/.dependency_license
##########
@@ -0,0 +1,23 @@
+# 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.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+# Patch files, cannot bear comments
+cjose.pic.patch, Apache-2.0
+jansson.pic.patch, Apache-2.0
Review comment:
Jansson is MIT-licensed.
--
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 a change in pull request #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r653929402
##########
File path: cache-config/testing/docker/docker-compose-ats-build.yml
##########
@@ -27,11 +27,19 @@ services:
trafficserver_build:
environment:
+ - RHEL_VERSION=8
Review comment:
Made the changes to the ort_test/Dockerfile.
##########
File path: cache-config/testing/docker/docker-compose-ats-build.yml
##########
@@ -27,11 +27,19 @@ services:
trafficserver_build:
environment:
+ - RHEL_VERSION=8
- ATS_VERSION=8.1.x
+ - CJOSE_URL=https://github.com/cisco/cjose
+ - CJOSE_TAG=latest
+ - JANSSON_URL=https://github.com/akheron/jansson
+ - JANSSON_TAG=v2.11
+ - OPENSSL_URL=https://github.com/openssl/openssl
+ - OPENSSL_TAG=OpenSSL_1_1_1
+ - RUN_ATS_UNIT_TESTS=true
Review comment:
done
##########
File path: cache-config/testing/docker/docker-compose-ats-build.yml
##########
@@ -27,11 +27,19 @@ services:
trafficserver_build:
environment:
+ - RHEL_VERSION=8
Review comment:
done
--
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 a change in pull request #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r652159850
##########
File path: .github/workflows/ciab.yaml
##########
@@ -45,6 +45,7 @@ on:
- 'NOTICE'
- 'traffic_control/java/**'
- 'cache-config/testing/**'
+ 'cache-config/testing/docker/trafficserver/**'
Review comment:
oops
--
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 #5938: Updates ATS builds for cache-config integration tests.
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5938:
URL: https://github.com/apache/trafficcontrol/pull/5938#discussion_r652138717
##########
File path: cache-config/testing/docker/trafficserver/run.sh
##########
@@ -57,31 +83,42 @@ else
source scl_source enable gcc-toolset-9
fi
-initBuildArea
+cd /root
+# prep build environment
+[ -e rpmbuild ] && rm -rf rpmbuild
+[ ! -e rpmbuild ] || { echo "Failed to clean up rpm build directory 'rpmbuild': $?" >&2; exit 1; }
+mkdir -p rpmbuild/{BUILD,BUILDROOT,RPMS,SPECS,SOURCES,SRPMS} || die "Failed to create build directory '$RPMBUILD': $?"
cd /root/rpmbuild/SOURCES
# clone the trafficserver repo
-git clone https://github.com/apache/trafficserver.git
+git clone https://github.com/apache/trafficserver.git --branch $ATS_VERSION || die "Failed to fetch the ATS Source"
+cp /traffic_server_jemalloc .
+
+# patch in the astats plugin
+(cp -far /astats_over_http /root/rpmbuild/SOURCES/trafficserver/plugins/astats_over_http
Review comment:
`-a` implies `-r`, so `cp -far` can be just `cp -fa`.
--
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