Page MenuHomePhabricator

Make Teamcity agent setup easier
Changes PlannedPublic

Authored by Fabien on Tue, Jun 4, 18:03.

Details

Reviewers
deadalnix
jasonbcox
Group Reviewers
Restricted Project
Summary

This diff adds a setup-agent.sh script that turns a fresh Debian 9
server into a functional Teamcity agent.

It updates the gitian.sh script to allow for single shot gitian
installation, when the agent is setup with the setup-agent.sh script.

Test Plan

Install an agent, run builds, gitian builds and ibd.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
teamcity_setup
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6249
Build 10545: Bitcoin ABC Teamcity Staging
Build 10544: arc lint + arc unit

Event Timeline

Fabien created this revision.Tue, Jun 4, 18:03
Herald added a reviewer: Restricted Project. · View Herald TranscriptTue, Jun 4, 18:03
jasonbcox requested changes to this revision.Tue, Jun 4, 20:46
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
contrib/teamcity/setup-agent.sh
30 ↗(On Diff #9123)

why set -e here as opposed to the top of the file where it can be easily seen? It seems unlikely that cat, grep, cut, or echo will exit with non-zero status and be handled in a way that you want.

57 ↗(On Diff #9123)

This list will likely need to updated as we support more builds, but this is a good starting point.

77 ↗(On Diff #9123)

pachages -> packages

80 ↗(On Diff #9123)

It's confusing to put fail2ban in this list. Also, there is no reason to split these packages away from the install step above other than for categorization purposes. The code can be simplified by doing something like:

DEPENDENCY_PACKAGES=(
# Build dependencies
...

# TeamCity dependencies
...

# Other infra dependencies
fail2ban
)

# install all of the needed packages in one shot...

Another side effect of doing it this way is, for example: TeamCity is updated and requires a dependency that is installed in another section (git or curl maybe), then the dependencies do not need to be moved/cleaned up.

155 ↗(On Diff #9123)

ditto on merging this to the above ^^^

This revision now requires changes to proceed.Tue, Jun 4, 20:46
Fabien added inline comments.Wed, Jun 5, 07:58
contrib/teamcity/setup-agent.sh
30 ↗(On Diff #9123)

This is a reordering oversight, it should come after the NET_DEV detection, where the ping can fail and is handled manually. I moved the NET_DEV detection below because it is Gitian related but forgot about this.
I cannot be placed before the above grep either as you don't want to exit if the script is not running over Debian 9, a warning is enough.

57 ↗(On Diff #9123)

Yes, from my tests it covers everything we need at the moment.
It is not exactly a minimal package list though, as some optional libraries are included. Some are required for tests to not get skipped (ZMQ) and they are no longer default-optional when building with CMake so better include them.

77 ↗(On Diff #9123)

Good catch !

80 ↗(On Diff #9123)

I agree your solution is strictly better.
The idea behind this was to be able to split the script for different kind of agents if needed (e.g.: a build agent != a gitian agent), but having categories in the package list is just as good.

Fabien updated this revision to Diff 9147.Wed, Jun 5, 08:11

Address feedback.
Tabs => spaces, add missing newline.

deadalnix requested changes to this revision.Wed, Jun 5, 12:07
deadalnix added inline comments.
contrib/teamcity/setup-agent.sh
136 ↗(On Diff #9147)

Always verify intergrity of what you get from the internet.

177 ↗(On Diff #9147)
sudo -u teamcity command
195 ↗(On Diff #9147)

Having a machine setup with a user with a default known password that is also a sudoer is probably not the best security practice.

The machine can be presetup with pubkeys for login.

This revision now requires changes to proceed.Wed, Jun 5, 12:07
Fabien updated this revision to Diff 9291.Mon, Jun 10, 14:37

Address feedback + minor improvements.
No SSH setup yet.

Fabien planned changes to this revision.Mon, Jun 10, 14:37