Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Added transitive source build for ATS.#5004

Merged
zrhoffman merged 7 commits intoapache:masterfrom
alficles:ats-transitive-build
Dec 4, 2020
Merged

Added transitive source build for ATS.#5004
zrhoffman merged 7 commits intoapache:masterfrom
alficles:ats-transitive-build

Conversation

@alficles
Copy link
Contributor

@alficles alficles commented Sep 2, 2020

This provides a basic Apache Traffic Server compile, complete with relevant
libraries, that can be used along with Traffic Control.

What does this PR (Pull Request) do?

Adds a transitive source build recipe for producing ATS RPMs that work nicely with ATC, for either testing or production purposes.

  • This PR fixes is not related to any Issue

Which Traffic Control components are affected by this PR?

  • Apache Traffic Server

What is the best way to verify this PR?

Build the RPMs with ./pkg -o and ./pkg -a and ensure that the correct RPMs are produced successfully. Additionally, test ./pkg to ensure that it does not produce these RPMs by default.

The following criteria are ALL met by this PR

  • This PR includes tests OR I have explained why tests are unnecessary
  • 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
  • This PR includes any and all required license headers
  • This PR ensures that database migration sequence is correct OR this PR does not include a database migration
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

Additional Information

It might be helpful to review the documentation for tsb here: https://comcast.github.io/tsb/

@alficles alficles force-pushed the ats-transitive-build branch 3 times, most recently from b23787a to 02b7144 Compare September 2, 2020 17:45
Copy link
Member

@ezelkow1 ezelkow1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in general, built all/optional here

The one other thing Id be uncertain of is including luacrypto in this build. We use it for old antiquated reasons and its horribly out of date and unsupported. Our only use case does have an ATS plugin to serve that purpose anyway so Id be inclined to say it should be removed in general to not give people the idea that its good to be using it

@mitchell852 mitchell852 added Traffic Server related to Apache Traffic Server build related to the build process labels Sep 2, 2020
@alficles
Copy link
Contributor Author

I'll investigate removing luacrypto and update.

@alficles alficles force-pushed the ats-transitive-build branch from 02b7144 to bc6e855 Compare September 14, 2020 17:00
@zrhoffman
Copy link
Member

TR failed to build in the CI for this PR due to a rate limiting issue that #5036 fixed. Would you please rebase onto master so it can run again?

@alficles alficles force-pushed the ats-transitive-build branch from bc6e855 to 40adb4b Compare December 4, 2020 07:10
This provides a basic Apache Traffic Server compile, complete with relevant
libraries, that can be used along with Traffic Control.
@alficles alficles force-pushed the ats-transitive-build branch from 40adb4b to 0b3b101 Compare December 4, 2020 07:17
@zrhoffman
Copy link
Member

zrhoffman commented Dec 4, 2020

Force-pushing to rebase is fine if the diffs of the commits themselves don't change in the process (other than to resolve any merge conflicts).

For the sake of reviewers: For future changes, please keep separate your original commit(s) from commits containing changes made in response to PR reviews.

@alficles
Copy link
Contributor Author

alficles commented Dec 4, 2020

I did have to rebase this on to master in order to handle the GO_VERSION stuff, because none of that was in the version I had built off of.

The lines had spaces where the file intended to use tabs. This has been
corrected.
These were introduced erroneously as the result of a merge conflict resolution.
@zrhoffman
Copy link
Member

zrhoffman commented Dec 4, 2020

Fails to build.

./pkg -o -v
Building ats.
Pulling ats ...
Pulling ats ... Native build is an experimental feature and could change at any time
Building ats
#2 [internal] load build definition from Dockerfile-tsb
#2 transferring dockerfile: 42B done
#2 DONE 0.3s

#1 [internal] load .dockerignore
#1 transferring context: 2B done
#1 DONE 0.4s

#3 [internal] load metadata for docker.io/library/centos:8
#3 DONE 0.0s

#4 [tsb-build 1/6] FROM docker.io/library/centos:8
#4 CACHED

#9 [internal] load build context
#9 transferring context: 31B done
#9 DONE 0.2s

#5 [stage-1 2/7] RUN	YUM -y install git sudo
#5 3.478 CentOS-8 - AppStream                            2.4 MB/s | 5.8 MB     00:02    
#5 CANCELED

#8 [tsb-build 2/6] RUN yum clean all && yum -y install git
#8 0.877 0 files removed
#8 4.068 CentOS-8 - AppStream                            1.9 MB/s | 5.8 MB     00:02    
#8 5.622 CentOS-8 - Base                                  10 kB/s | 6.4 kB     00:00    
#8 5.625 Error: Failed to download metadata for repo 'BaseOS': repomd.xml parser error: Parse error at line: 28 (Opening and ending tag mismatch: meta line 0 and head
#8 5.625 )
#8 ERROR: executor failed running [/bin/sh -c yum clean all && yum -y install git]: runc did not terminate sucessfully
------
 > [tsb-build 2/6] RUN yum clean all && yum -y install git:
------
failed to solve with frontend dockerfile.v0: failed to build LLB: executor failed running [/bin/sh -c yum clean all && yum -y install git]: runc did not terminate sucessfully
Service 'ats' failed to build
Failed to build ats.
Results in 'dist':
total 0

@zrhoffman
Copy link
Member

Fails to build.

Scratch that, I just needed to restart Docker

pkg Outdated
fi

# Update the ownership of the artefacts to match the main repo.
chown -R "$(stat -c '%u:%g' .)" dist
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line causes the build to "fail" (even though the RPMs are successfully built) when pkg is not run using sudo:

Requires(rpmlib): rpmlib(FileDigests) <= 4.6.0-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rpmlib(CompressedFileNames) <= 3.0.4-1
Checking for unpackaged file(s): /usr/lib/rpm/check-files /rpmbuilddir/BUILDROOT/trafficserver-8.0.8-11.3a53261.el7.x86_64
Wrote: /rpmbuilddir/RPMS/x86_64/trafficserver-8.0.8-11.3a53261.el7.x86_64.rpm
Wrote: /rpmbuilddir/RPMS/x86_64/trafficserver-debuginfo-8.0.8-11.3a53261.el7.x86_64.rpm
Executing(%clean): /bin/sh -e /var/tmp/rpm-tmp.lnPh60
+ umask 022
+ cd /rpmbuilddir/BUILD
+ cd trafficserver-8.0.8
+ rm -rf /rpmbuilddir/BUILDROOT/trafficserver-8.0.8-11.3a53261.el7.x86_64
+ exit 0
chown: changing ownership of 'dist/trafficserver-debuginfo-8.0.8-11.3a53261.el7.x86_64.rpm': Operation not permitted
chown: changing ownership of 'dist/build.log': Operation not permitted
chown: changing ownership of 'dist/trafficserver-8.0.8-11.3a53261.el7.x86_64.rpm': Operation not permitted
Failed to build ats_build.
Results in 'dist':
total 58376
-rw-r--r-- 1 root      root        849202 Dec  4 12:22 build.log
-rw-r--r-- 1 root      root      51845204 Dec  4 12:22 trafficserver-debuginfo-8.0.8-11.3a53261.el7.x86_64.rpm
-rw-r--r-- 1 root      root       7066360 Dec  4 12:22 trafficserver-8.0.8-11.3a53261.el7.x86_64.rpm
lrwxrwxrwx 1 zhoffm468 zhoffm468       60 Dec  4 12:13 dist -> /home/zhoffm468/go/src/github.com/apache/trafficcontrol/dist
Failed to build ats.
Results in 'dist':
total 58376
-rw-r--r-- 1 root      root        849202 Dec  4 12:22 build.log
-rw-r--r-- 1 root      root      51845204 Dec  4 12:22 trafficserver-debuginfo-8.0.8-11.3a53261.el7.x86_64.rpm
-rw-r--r-- 1 root      root       7066360 Dec  4 12:22 trafficserver-8.0.8-11.3a53261.el7.x86_64.rpm
lrwxrwxrwx 1 zhoffm468 zhoffm468       60 Dec  4 12:13 dist -> /home/zhoffm468/go/src/github.com/apache/trafficcontrol/dist

All of our Docker builders use clean_build.sh, which set artifact ownership on exist:

setowner() {
own=$(stat -c%u:%g "$1" 2>/dev/null || stat -f%u:%g "$1")
shift
[ -n "$*" ] && chown -R "${own}" "$@"
}

Ownership should be set within Docker so that root privileges is not a requirement for running pkg.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This permissions issue will affect people using the optional docker-compose, so people building other components won't be affected. We can address this later.

…ed up.

It was being left in place when the build did not succeed.
This only works when pkg is run with enough permissions to modify the files it
creates.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

build related to the build process Traffic Server related to Apache Traffic Server

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants