[GitHub] [trafficcontrol] zrhoffman commented on a diff in pull request #7357: Add TPv2 RPM Spec and add to `pkg`

["zrhoffman (via GitHub)" <gi...@apache.org>]

zrhoffman commented on code in PR #7357:
URL: https://github.com/apache/trafficcontrol/pull/7357#discussion_r1117573239


##########
infrastructure/docker/build/Dockerfile-traffic-portal:
##########
@@ -0,0 +1,49 @@
+# Licensed to the Apache Software Foundation (ASF) under one

Review Comment:
   `Dockerfile-traffic-portal` is basically the same as `Dockerfile-traffic_portal`, except it includes `jq` and does not include `grunt-cli` or `sass` (and has a different `CMD`).
   
   Instead of making a brand new image, how about using Dockerfile build targets? Example:
   
   https://github.com/apache/trafficcontrol/blob/a167f1b04447fdf6f60f752e7ff42ec2e7c0c922/infrastructure/cdn-in-a-box/cache/Dockerfile#L129-L131
   
   https://github.com/apache/trafficcontrol/blob/a167f1b04447fdf6f60f752e7ff42ec2e7c0c922/infrastructure/cdn-in-a-box/docker-compose.yml#L200-L203



##########
infrastructure/docker/build/docker-compose.yml:
##########
@@ -117,6 +117,23 @@ services:
       - ../../..:/trafficcontrol:z
       - ../../../.npm:/root/.npm:z
 
+  traffic-portal_build:

Review Comment:
   This is confusing to have both `traffic-portal_build` and `traffic_portal_build`, especially since they show up in different places in the `./pkg -h` text:
   
   ```shell
   If no projects are listed, all projects will be packaged.
   Valid projects:
     - weasel
     - cache-config_build
     - docs
     - grove_build
     - grovetccfg_build
     - source
     - tc-health-client_build
     - traffic-portal_build
     - traffic_monitor_build
     - traffic_ops_build
     - traffic_portal_build
     - traffic_router_build
     - traffic_stats_build
   ```
   
   So one of them should be renamed.



##########
infrastructure/docker/build/Dockerfile-traffic-portal:
##########
@@ -0,0 +1,49 @@
+# 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.
+    # Change BASE_IMAGE to centos when RHEL_VERSION=7
+ARG BASE_IMAGE=rockylinux \
+    RHEL_VERSION=8
+FROM ${BASE_IMAGE}:${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 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 \
+		jq \

Review Comment:
   `jq` is required?



##########
infrastructure/docker/build/docker-compose.yml:
##########
@@ -117,6 +117,23 @@ services:
       - ../../..:/trafficcontrol:z
       - ../../../.npm:/root/.npm:z
 
+  traffic-portal_build:

Review Comment:
   The build log is named `build-experimental-traffic-portal.log`. I'm not particular about what name is chosen, but the service and log should match.



##########
experimental/traffic-portal/build/traffic-portal.spec:
##########
@@ -0,0 +1,96 @@
+#
+#
+# Licensed 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.
+#
+#
+# RPM spec file for the Traffic Portal
+#
+%define   debug_package %{nil}
+Name:     traffic-portal
+Version:  %{traffic_control_version}
+Release:  %{build_number}
+Summary:  Traffic Portal v2
+Group:    Applications/Communications
+License:  Apache License, Version 2.0
+URL:      https://github.com/apache/trafficcontrol/
+Source:   %{_sourcedir}/traffic-portal-%{traffic_control_version}.tgz
+AutoReqProv: no
+Requires: nodejs >= 2:16.0.0
+Requires(pre): /usr/sbin/useradd, /usr/bin/getent
+
+%define traffic_portal_home /opt/traffic-portal
+%define traffic_portal_log /var/log/traffic-portal
+%define traffic_portal_conf /etc/traffic-portal
+%define traffic_portal traffic-portal-%{version}
+%define traffic_portal_user trafficportal
+%description
+Installs Traffic Portal
+
+Built: @BUILT@
+
+%prep
+%__rm -rf $RPM_BUILD_DIR/%{traffic_portal}
+tar -xzvf $RPM_SOURCE_DIR/%{traffic_portal}.tgz
+
+%pre
+/usr/bin/getent group %{traffic_portal_user} || /usr/sbin/groupadd -r %{traffic_portal_user}
+/usr/bin/getent passwd %{traffic_portal_user} || /usr/sbin/useradd -r -d %{traffic_portal_home} -s /sbin/nologin %{traffic_portal_user} -g %{traffic_portal_user}
+
+%build
+cd ${RPM_BUILD_DIR}/%{traffic_portal}
+npm run build:ssr
+
+%install
+%__mkdir -p ${RPM_BUILD_ROOT}/etc/init.d
+%__mkdir -p ${RPM_BUILD_ROOT}/etc/logrotate.d
+%__mkdir -p ${RPM_BUILD_ROOT}%{traffic_portal_conf}
+%__mkdir -p ${RPM_BUILD_ROOT}%{traffic_portal_home}/browser
+%__mkdir -p ${RPM_BUILD_ROOT}%{traffic_portal_home}/server
+%__mkdir -p ${RPM_BUILD_ROOT}%{traffic_portal_home}/node_modules
+%__mkdir -p ${RPM_BUILD_ROOT}%{traffic_portal_log}
+
+%__cp ${RPM_BUILD_DIR}/%{traffic_portal}/build/config.json ${RPM_BUILD_ROOT}%{traffic_portal_conf}/.
+%__cp -r ${RPM_BUILD_DIR}/%{traffic_portal}/dist/traffic-portal/* ${RPM_BUILD_ROOT}%{traffic_portal_home}/.
+%__cp -r ${RPM_BUILD_DIR}/%{traffic_portal}/build/node_modules ${RPM_BUILD_ROOT}%{traffic_portal_home}/.
+%__cp ${RPM_BUILD_DIR}/%{traffic_portal}/build/etc/init.d/traffic-portal ${RPM_BUILD_ROOT}/etc/init.d/.
+%__cp ${RPM_BUILD_DIR}/%{traffic_portal}/build/etc/logrotate.d/traffic-portal ${RPM_BUILD_ROOT}/etc/logrotate.d/.
+%__cp ${RPM_BUILD_DIR}/%{traffic_portal}/LICENSE ${RPM_BUILD_DIR}/.
+
+# creates dynamic json file needed at runtime for traffic portal to display release info
+echo "{
+	\"date\": \"$(date +'%Y-%m-%d %H:%M')\",
+	\"elRelease\": \"%{rhel_vers}\",
+	\"hash\": \"%{build_number}\",
+	\"version\": \"%{version}\"
+}" > ${RPM_BUILD_ROOT}%{traffic_portal_conf}/version.json
+
+%post
+%__chmod +x %{traffic_portal_home}/node_modules/pm2/bin/pm2
+echo "Successfully installed traffic-portal to " %{traffic_portal_home}
+/sbin/chkconfig traffic-portal on
+echo ""
+echo "Start with 'systemctl start traffic-portal' or by running '%{traffic_portal_conf}/traffic-portal'"
+

Review Comment:
   When installing the RPM with 
   
   ```shell
   rpm -e traffic-portal
   ```
   , a `/opt/traffic-portal` still exists (but is empty). That should be removed too, 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.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org