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/03/10 02:08:31 UTC
[GitHub] [trafficcontrol] jhg03a opened a new pull request #5620: Convert the fakeOrigin tool to leverage pkg and standard ATC version
jhg03a opened a new pull request #5620:
URL: https://github.com/apache/trafficcontrol/pull/5620
<!--
************ STOP!! ************
If this Pull Request is intended to fix a security vulnerability, DO NOT submit it! Instead, contact
the Apache Software Foundation Security Team at security@trafficcontrol.apache.org and follow the
guidelines at https://www.apache.org/security/ regarding vulnerability disclosure.
-->
## What does this PR (Pull Request) do?
<!-- Explain the changes you made here. If this fixes an Issue, identify it by
replacing the text in the checkbox item with the Issue number e.g.
- [x] This PR fixes #9001 OR is not related to any Issue
^ This will automatically close Issue number 9001 when the Pull Request is
merged (The '#' is important).
Be sure you check the box properly, see the "The following criteria are ALL
met by this PR" section for details.
-->
This PR aligns the fakeOrigin testing tool in the repo with the norm for other ATC components both in terms of using pkg to build it and the version number following that of the main ATC project.
- [x] This PR is not related to any Issue <!-- You can check for an issue here: https://github.com/apache/trafficcontrol/issues -->
## 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. -->
- fakeOrigin
## 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. -->
`./pkg -o fakeorigin_build`
After build or install the binary version may be checked with `fakeOrigin --version`
## If this is a bug fix, what versions of Traffic Control are affected?
<!-- If this PR fixes a bug, please list here all of the affected versions - to
the best of your knowledge. It's also pretty helpful to include a commit hash
of where 'master' is at the time this PR is opened (if it affects master),
because what 'master' means will change over time. For example, if this PR
fixes a bug that's present in master (at commit hash '1df853c8'), in v4.0.0,
and in the current 4.0.1 Release candidate (e.g. RC1), then this list would
look like:
- master (e2fecc1997c)
If you don't know what other versions might have this bug, AND don't know how
to find the commit hash of 'master', then feel free to leave this section
blank (or, preferably, delete it entirely).
-->
## The following criteria are ALL met by this PR
<!-- Check the boxes to signify that the associated statement is true. To
"check a box", replace the space inside of the square brackets with an 'x'.
e.g.
- [ x] <- Wrong
- [x ] <- Wrong
- [] <- Wrong
- [*] <- Wrong
- [x] <- Correct!
-->
- [x] This PR is only to adjust the build method and versioning and as such no tests are required
- [x] This PR includes documentation
- [x] This PR does not include a changelog entry as it is an ancillary test tool
- [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)
----------------------------------------------------------------
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] jhg03a commented on a change in pull request #5620: Convert the fakeOrigin tool to leverage pkg and standard ATC version
Posted by GitBox <gi...@apache.org>.
jhg03a commented on a change in pull request #5620:
URL: https://github.com/apache/trafficcontrol/pull/5620#discussion_r592596948
##########
File path: infrastructure/docker/build/docker-compose-opt.yml
##########
@@ -26,3 +26,12 @@ services:
RHEL_VERSION: ${RHEL_VERSION:-8}
volumes:
- ../../../traffic_server/tsb:/opt/tsb-ats:z
+ fakeorigin_build:
+ image: apache/fakeorigin_builder:master
Review comment:
If a consistent naming convention for all ATC images is desired, it should be `apache/atc_.*_builder` so we also don't conflict with ATS accidentally. Regardless, I think making that kind of a sweeping change is outside the scope of a single addition.
----------------------------------------------------------------
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 #5620: Convert the fakeOrigin tool to leverage pkg and standard ATC version
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5620:
URL: https://github.com/apache/trafficcontrol/pull/5620#discussion_r592544430
##########
File path: infrastructure/docker/build/docker-compose-opt.yml
##########
@@ -26,3 +26,12 @@ services:
RHEL_VERSION: ${RHEL_VERSION:-8}
volumes:
- ../../../traffic_server/tsb:/opt/tsb-ats:z
+ fakeorigin_build:
+ image: apache/fakeorigin_builder:master
Review comment:
Fair enough, since `fakeorigin` is a pretty specific name, leaving `traffic_` out should be okay.
----------------------------------------------------------------
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 #5620: Convert the fakeOrigin tool to leverage pkg and standard ATC version
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5620:
URL: https://github.com/apache/trafficcontrol/pull/5620#discussion_r592546751
##########
File path: infrastructure/docker/build/docker-compose-opt.yml
##########
@@ -26,3 +26,12 @@ services:
RHEL_VERSION: ${RHEL_VERSION:-8}
volumes:
- ../../../traffic_server/tsb:/opt/tsb-ats:z
+ fakeorigin_build:
+ image: apache/fakeorigin_builder:master
Review comment:
> Also, that convention is already broken with grove.
That convention is not already broken with grove.
https://github.com/apache/trafficcontrol/blob/41669a17927a1d11e8e7f5cbbf930bec2ac216d1/infrastructure/docker/build/docker-compose.yml#L100
https://github.com/apache/trafficcontrol/blob/41669a17927a1d11e8e7f5cbbf930bec2ac216d1/infrastructure/docker/build/docker-compose.yml#L110
----------------------------------------------------------------
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 #5620: Convert the fakeOrigin tool to leverage pkg and standard ATC version
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5620:
URL: https://github.com/apache/trafficcontrol/pull/5620#discussion_r591725746
##########
File path: test/fakeOrigin/build/fakeOrigin.spec
##########
@@ -35,16 +33,22 @@ A fake HTTP CDN Origin for testing
%prep
%build
-set -o nounset
# copy license
-cp "${DIR}/../../../LICENSE" %{_builddir}
+cp "$TC_DIR/LICENSE" %{_builddir}
-tar -xvzf %{_sourcedir}/%{name}-%{_version}-%{_release}.tgz --directory %{_builddir}
+# copy fakeOrigin binary
+godir=src/github.com/apache/trafficcontrol/test/%{name}
+( mkdir -p "$godir" && \
+ cd "$godir" && \
+ cp -r "$TC_DIR"/test/%{name}/* .
+) || { echo "Could not copy go program at $(pwd): $!"; exit 1; }
%install
+cd src/github.com/apache/trafficcontrol/test/%{name}
rm -rf %{buildroot}/opt/%{name}
mkdir -p %{buildroot}/opt/%{name}/example
cp -p %{name} %{buildroot}/opt/%{name}
+#cp -p LICENSE %{buildroot}/opt/LICENSE
Review comment:
This comment can be removed, right?
----------------------------------------------------------------
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 #5620: Convert the fakeOrigin tool to leverage pkg and standard ATC version
Posted by GitBox <gi...@apache.org>.
zrhoffman merged pull request #5620:
URL: https://github.com/apache/trafficcontrol/pull/5620
----------------------------------------------------------------
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 #5620: Convert the fakeOrigin tool to leverage pkg and standard ATC version
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5620:
URL: https://github.com/apache/trafficcontrol/pull/5620#discussion_r591722121
##########
File path: infrastructure/docker/build/Dockerfile-fakeOrigin
##########
@@ -0,0 +1,63 @@
+# 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.
+ARG RHEL_VERSION=8
+FROM centos:${RHEL_VERSION} as common-dependencies
+ARG RHEL_VERSION=8
+
+MAINTAINER dev@trafficcontrol.apache.org
+
+# top level of trafficcontrol directory must be mounted as a volume:
+# docker run --volume /trafficcontrol:$(pwd) ...
+VOLUME /trafficcontrol
+
+### Common for all sub-component builds
+RUN if [[ ${RHEL_VERSION%%.*} -ge 8 ]]; then \
+ rpm_gpg_key=RPM-GPG-KEY-centosofficial; \
+ else \
+ rpm_gpg_key="RPM-GPG-KEY-CentOS-${RHEL_VERSION%%.*}"; \
+ fi && \
+ rpm --import "/etc/pki/rpm-gpg/${rpm_gpg_key}" && \
+ rpm --import "https://dl.fedoraproject.org/pub/epel/RPM-GPG-KEY-EPEL-${RHEL_VERSION%%.*}" && \
+ yum -y update ca-certificates && \
+ yum -y install \
+ git \
+ rpm-build \
+ rsync \
+ epel-release && \
+ yum -y clean all
+
+### fakeOrigin specific requirements
+FROM common-dependencies AS fakeOrigin
+
+RUN yum -y install \
+ # gcc is necessary in case -gcflags 'all=-N -l' is passed to go build
+ gcc \
+ && \
+ yum -y clean all
+
+COPY GO_VERSION /
+RUN go_version=$(cat /GO_VERSION) && \
+ curl -Lo go.tar.gz https://dl.google.com/go/go${go_version}.linux-amd64.tar.gz && \
+ tar -C /usr/local -xvzf go.tar.gz && \
+ ln -s /usr/local/go/bin/go /usr/bin/go && \
+ rm go.tar.gz
+
+###
+
+CMD /trafficcontrol/build/clean_build.sh fakeOrigin
Review comment:
Getting `fakeOrigin/build/build_rpm.sh not found`:
```shell
+ for project in "$@"
+ ./build/build.sh fakeOrigin
+ tee dist/build-fakeOrigin.log
/usr/bin/realpath
fakeOrigin/build/build_rpm.sh not found
The following subdirectories had errors:
fakeOrigin
Error on line 1 of ./build/build.sh
```
Since `fakeOrigin` is in the `test` directory, this should be:
```dockerfile
CMD /trafficcontrol/build/clean_build.sh test/fakeOrigin
```
For an example of another component that requires this, see `grovetccfg`:
https://github.com/apache/trafficcontrol/blob/74c07997b905fbc6c5feace5f7c849d032bdbc7f/infrastructure/docker/build/Dockerfile-grovetccfg#L57
##########
File path: infrastructure/docker/build/docker-compose-opt.yml
##########
@@ -26,3 +26,12 @@ services:
RHEL_VERSION: ${RHEL_VERSION:-8}
volumes:
- ../../../traffic_server/tsb:/opt/tsb-ats:z
+ fakeorigin_build:
+ image: apache/fakeorigin_builder:master
Review comment:
Can this be `apache/traffic_fakeorigin_builder:master` so that all of the ATC images start with `apache/traffic*_builder`?
----------------------------------------------------------------
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] jhg03a commented on a change in pull request #5620: Convert the fakeOrigin tool to leverage pkg and standard ATC version
Posted by GitBox <gi...@apache.org>.
jhg03a commented on a change in pull request #5620:
URL: https://github.com/apache/trafficcontrol/pull/5620#discussion_r592542814
##########
File path: infrastructure/docker/build/docker-compose-opt.yml
##########
@@ -26,3 +26,12 @@ services:
RHEL_VERSION: ${RHEL_VERSION:-8}
volumes:
- ../../../traffic_server/tsb:/opt/tsb-ats:z
+ fakeorigin_build:
+ image: apache/fakeorigin_builder:master
Review comment:
That's not its name and would be confusing.
----------------------------------------------------------------
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] jhg03a commented on a change in pull request #5620: Convert the fakeOrigin tool to leverage pkg and standard ATC version
Posted by GitBox <gi...@apache.org>.
jhg03a commented on a change in pull request #5620:
URL: https://github.com/apache/trafficcontrol/pull/5620#discussion_r590994652
##########
File path: pkg
##########
@@ -99,8 +99,8 @@ while getopts :?78abdf:lopqv opt; do
PROJECTS=`$SELF -l | sed "s/^/ - /"`
<<-HELP_TEXT cat
Usage: $SELF [options] [projects]
- -7 Build RPMs targeting CentOS 7 (default)
- -8 Build RPMs targeting CentOS 8
+ -7 Build RPMs targeting CentOS 7
+ -8 Build RPMs targeting CentOS 8 (default)
Review comment:
Yes
----------------------------------------------------------------
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] jhg03a commented on a change in pull request #5620: Convert the fakeOrigin tool to leverage pkg and standard ATC version
Posted by GitBox <gi...@apache.org>.
jhg03a commented on a change in pull request #5620:
URL: https://github.com/apache/trafficcontrol/pull/5620#discussion_r592544288
##########
File path: infrastructure/docker/build/docker-compose-opt.yml
##########
@@ -26,3 +26,12 @@ services:
RHEL_VERSION: ${RHEL_VERSION:-8}
volumes:
- ../../../traffic_server/tsb:/opt/tsb-ats:z
+ fakeorigin_build:
+ image: apache/fakeorigin_builder:master
Review comment:
Also, that convention is already broken with grove.
----------------------------------------------------------------
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] jhg03a commented on a change in pull request #5620: Convert the fakeOrigin tool to leverage pkg and standard ATC version
Posted by GitBox <gi...@apache.org>.
jhg03a commented on a change in pull request #5620:
URL: https://github.com/apache/trafficcontrol/pull/5620#discussion_r592614072
##########
File path: infrastructure/docker/build/docker-compose-opt.yml
##########
@@ -26,3 +26,12 @@ services:
RHEL_VERSION: ${RHEL_VERSION:-8}
volumes:
- ../../../traffic_server/tsb:/opt/tsb-ats:z
+ fakeorigin_build:
+ image: apache/fakeorigin_builder:master
Review comment:
Ah, you're right. I was looking in the wrong place. I've updated it since everything else is consistent, but stand by that we should have used a different prefix for 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] jhg03a commented on a change in pull request #5620: Convert the fakeOrigin tool to leverage pkg and standard ATC version
Posted by GitBox <gi...@apache.org>.
jhg03a commented on a change in pull request #5620:
URL: https://github.com/apache/trafficcontrol/pull/5620#discussion_r592590636
##########
File path: test/fakeOrigin/build/fakeOrigin.spec
##########
@@ -35,16 +33,22 @@ A fake HTTP CDN Origin for testing
%prep
%build
-set -o nounset
# copy license
-cp "${DIR}/../../../LICENSE" %{_builddir}
+cp "$TC_DIR/LICENSE" %{_builddir}
-tar -xvzf %{_sourcedir}/%{name}-%{_version}-%{_release}.tgz --directory %{_builddir}
+# copy fakeOrigin binary
+godir=src/github.com/apache/trafficcontrol/test/%{name}
+( mkdir -p "$godir" && \
+ cd "$godir" && \
+ cp -r "$TC_DIR"/test/%{name}/* .
+) || { echo "Could not copy go program at $(pwd): $!"; exit 1; }
%install
+cd src/github.com/apache/trafficcontrol/test/%{name}
rm -rf %{buildroot}/opt/%{name}
mkdir -p %{buildroot}/opt/%{name}/example
cp -p %{name} %{buildroot}/opt/%{name}
+#cp -p LICENSE %{buildroot}/opt/LICENSE
Review comment:
Addressed with commit
----------------------------------------------------------------
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 #5620: Convert the fakeOrigin tool to leverage pkg and standard ATC version
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5620:
URL: https://github.com/apache/trafficcontrol/pull/5620#discussion_r590985665
##########
File path: pkg
##########
@@ -99,8 +99,8 @@ while getopts :?78abdf:lopqv opt; do
PROJECTS=`$SELF -l | sed "s/^/ - /"`
<<-HELP_TEXT cat
Usage: $SELF [options] [projects]
- -7 Build RPMs targeting CentOS 7 (default)
- -8 Build RPMs targeting CentOS 8
+ -7 Build RPMs targeting CentOS 7
+ -8 Build RPMs targeting CentOS 8 (default)
Review comment:
Is this true, builds use CentOS8 builds by default?
----------------------------------------------------------------
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] jhg03a commented on a change in pull request #5620: Convert the fakeOrigin tool to leverage pkg and standard ATC version
Posted by GitBox <gi...@apache.org>.
jhg03a commented on a change in pull request #5620:
URL: https://github.com/apache/trafficcontrol/pull/5620#discussion_r592590475
##########
File path: infrastructure/docker/build/Dockerfile-fakeOrigin
##########
@@ -0,0 +1,63 @@
+# 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.
+ARG RHEL_VERSION=8
+FROM centos:${RHEL_VERSION} as common-dependencies
+ARG RHEL_VERSION=8
+
+MAINTAINER dev@trafficcontrol.apache.org
+
+# top level of trafficcontrol directory must be mounted as a volume:
+# docker run --volume /trafficcontrol:$(pwd) ...
+VOLUME /trafficcontrol
+
+### Common for all sub-component builds
+RUN if [[ ${RHEL_VERSION%%.*} -ge 8 ]]; then \
+ rpm_gpg_key=RPM-GPG-KEY-centosofficial; \
+ else \
+ rpm_gpg_key="RPM-GPG-KEY-CentOS-${RHEL_VERSION%%.*}"; \
+ fi && \
+ rpm --import "/etc/pki/rpm-gpg/${rpm_gpg_key}" && \
+ rpm --import "https://dl.fedoraproject.org/pub/epel/RPM-GPG-KEY-EPEL-${RHEL_VERSION%%.*}" && \
+ yum -y update ca-certificates && \
+ yum -y install \
+ git \
+ rpm-build \
+ rsync \
+ epel-release && \
+ yum -y clean all
+
+### fakeOrigin specific requirements
+FROM common-dependencies AS fakeOrigin
+
+RUN yum -y install \
+ # gcc is necessary in case -gcflags 'all=-N -l' is passed to go build
+ gcc \
+ && \
+ yum -y clean all
+
+COPY GO_VERSION /
+RUN go_version=$(cat /GO_VERSION) && \
+ curl -Lo go.tar.gz https://dl.google.com/go/go${go_version}.linux-amd64.tar.gz && \
+ tar -C /usr/local -xvzf go.tar.gz && \
+ ln -s /usr/local/go/bin/go /usr/bin/go && \
+ rm go.tar.gz
+
+###
+
+CMD /trafficcontrol/build/clean_build.sh fakeOrigin
Review comment:
Addressed with commit. I'd already dealt with most of the issues, but I think I must have had a stale docker image or cache.
----------------------------------------------------------------
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