Page MenuHomePhabricator
Differential D7994

[build-bot] Persist build info between redeployments
ClosedPublic
Actions

Authored by jasonbcox on Oct 19 2020, 22:32.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC594d974a35d2: [build-bot] Persist build info between redeployments
Summary

The build bot current tracks multiple pieces of state between
requests but does not persist them to disk. Whenever the bot is redeployed,
old state is forgotten. This can result in diff builds that appear to not complete,
as well as incorrect master state messages in Slack.

While this has not been an issue in the past due to relatively infrequent changes to
the build bot, opening up the bot source code to the entire dev team means much more
frequent changes that trigger more redeployments. This issue will only become more
noticeable if not taken care of.

This patch introduces persisting state and adds some (not comprehensive) test coverage.

Test Plan
pytest
./abcbot.py  # Verify some DB-related log messages

Diff Detail

Event Timeline

jasonbcox created this revision.Oct 19 2020, 22:32
Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 19 2020, 22:32
jasonbcox requested review of this revision.Oct 19 2020, 22:32
Harbormaster completed remote builds in B13505: Diff 24805.Oct 19 2020, 22:42
teamcity added a comment.Oct 19 2020, 22:42
Build Bitcoin ABC Diffs / Diff Testing (build-debug) failed.

Tail of the build log:

  -t abc-base-image-"${HASH}" debian:buster

(cat <<EOF
results() {
  set +e
  shopt -s dotglob nullglob
  mv /work/abc-ci-builds/"build-debug"/* /results
  chown -R ${ME} /work
  chown -R ${ME} /results
  chown -R ${ME} /root/.ccache
}
trap "results" EXIT
export TEAMCITY_VERSION="2019.2.4 (build 72059)"
export BASE_CACHE="/root/abc-depends/cache"
export SDK_ARCHIVE_DIR="/root/abc-depends/osx-sdk"
export SOURCES_PATH="/root/abc-depends/sources"
mkdir -p "/root/abc-depends/cache" "/root/abc-depends/osx-sdk" "/root/abc-depends/sources"
./contrib/teamcity/build-configurations.py "build-debug"
EOF
) > run-command.sh
chmod +x run-command.sh

~/infra/docker/docker-run.sh \
  -a "-v /home/teamcity/.ccache:/root/.ccache -v /home/teamcity/.abc-depends:/root/abc-depends -v "${RESULTS_DIR}":/results" \
  -c run-command.sh /work/run-command.sh abc-base-image-"${HASH}" ./run-command.sh

[22:33:33] :	 [Step 1/1] Starting: /home/teamcity/buildAgent/temp/agentTmp/custom_script2847397302559633233
[22:33:33] :	 [Step 1/1] in directory: /home/teamcity/buildAgent/work/jailed-build
[22:33:33] :	 [Step 1/1] ~/buildAgent/work/jailed-build/bitcoin-abc ~/buildAgent/work/jailed-build
[22:33:33] :	 [Step 1/1] ~/buildAgent/work/jailed-build
[22:33:33] :	 [Step 1/1] Building base image for: dfa37720e...
[22:33:33] :	 [Step 1/1] ~/buildAgent/work/jailed-build/bitcoin-abc ~/buildAgent/work/jailed-build
[22:33:35] :	 [Step 1/1] ~/buildAgent/work/jailed-build
[22:33:35] :	 [Step 1/1] Tag name: abc-base-image-dfa37720e
[22:42:21]W:	 [Step 1/1] Traceback (most recent call last):
[22:42:21]W:	 [Step 1/1]   File "./contrib/teamcity/build-configurations.py", line 570, in <module>
[22:42:21]W:	 [Step 1/1]     main()
[22:42:21]W:	 [Step 1/1]   File "./contrib/teamcity/build-configurations.py", line 559, in main
[22:42:21]W:	 [Step 1/1]     script_dir, config_path, args.build)
[22:42:21]W:	 [Step 1/1]   File "./contrib/teamcity/build-configurations.py", line 59, in __init__
[22:42:21]W:	 [Step 1/1]     self.load(build_name)
[22:42:21]W:	 [Step 1/1]   File "./contrib/teamcity/build-configurations.py", line 82, in load
[22:42:21]W:	 [Step 1/1]     self.name, list(config.keys())
[22:42:21]W:	 [Step 1/1] AssertionError: build-debug is not a valid build identifier. Valid identifiers are ['templates', 'builds']
[22:42:21]W:	 [Step 1/1] mv: missing destination file operand after '/results'
[22:42:21]W:	 [Step 1/1] Try 'mv --help' for more information.
[22:42:25]W:	 [Step 1/1] Process exited with code 1
[22:42:25]E:	 [Step 1/1] Process exited with code 1 (Step: Command Line)
[22:42:25]E:	 [Step 1/1] Step Command Line failed
[22:42:25]E: Ant JUnit report watcher
[22:42:25]E:	 [Ant JUnit report watcher] No reports found for paths:
[22:42:25]E:	 [Ant JUnit report watcher] +:results/test_bitcoin.xml
[22:42:25]E:	 [Ant JUnit report watcher] +:results/**/junit_results*.xml
[22:42:25] : Publishing internal artifacts (2s)
[22:42:28] :	 [Publishing internal artifacts] Publishing 1 file using [WebPublisher]
[22:42:28] :	 [Publishing internal artifacts] Publishing 1 file using [ArtifactsCachePublisher]
[22:42:25]W: Publishing artifacts (3s)
[22:42:25] :	 [Publishing artifacts] Collecting files to publish: [+:results/**/junit_results*.xml]
[22:42:25]W:	 [Publishing artifacts] Artifacts path 'results/**/junit_results*.xml' not found
[22:42:29] : Build finished
jasonbcox updated this revision to Diff 24810.Oct 20 2020, 00:27

Rebase

Harbormaster completed remote builds in B13510: Diff 24810.Oct 20 2020, 00:58
Fabien requested changes to this revision.Oct 21 2020, 10:02
Fabien added a subscriber: Fabien.
Fabien added inline comments.
contrib/buildbot/abcbot.py
27 ↗(On Diff #24810)

Better default to something that don't require root privileges
Edit: looking at the code below have it None (non persistent) by default is even better

contrib/buildbot/server.py
92 ↗(On Diff #24810)

You should make it a decorator, so you won't have to call it from everywhere (untested):

def persist(func):
    def call_and_save_database(*args, **kwargs):
        # Call our decorated function
        func_ret = func(*args, **kwargs)

        # Save the database as needed
        if db_file_no_ext:
            with shelve.open(db_file_no_ext) as db:
                for key in create_server.db.keys():
                    db[key] = create_server.db[key]
                app.logger.debug("Persisted current state")
        else:
            app.logger.debug(
                "No database file specified. Persisting state is being skipped.")

        # Forward the return value
        return func_ret

    return call_and_save_database

@persist
def build_diff():

Note that I changed the logger level to debug, otherwise you can expect a ton of pollution to your logs.

contrib/buildbot/test/test_persist_database.py
69 ↗(On Diff #24810)

Use self.assertEqual(), same for the whole test file

This revision now requires changes to proceed.Oct 21 2020, 10:02
jasonbcox updated this revision to Diff 24902.Oct 21 2020, 16:55

Fixed according to feedback

Harbormaster completed remote builds in B13565: Diff 24902.Oct 21 2020, 17:19
Fabien accepted this revision.Oct 21 2020, 18:56
This revision is now accepted and ready to land.Oct 21 2020, 18:56
Closed by commit rABC594d974a35d2: [build-bot] Persist build info between redeployments (authored by jasonbcox). · Explain WhyOct 21 2020, 19:54
This revision was automatically updated to reflect the committed changes.
jasonbcox added a commit: rABC594d974a35d2: [build-bot] Persist build info between redeployments.

Revision Contents
Changeset List

PathSize
contrib/
buildbot/
11 lines
87 lines
test/
5 lines
123 lines

Diff 24805

View Options

contrib/buildbot/abcbot.py

Loading...
View Options

contrib/buildbot/server.py

Loading...
View Options

contrib/buildbot/test/abcbot_fixture.py

Loading...
View Options

contrib/buildbot/test/test_persist_database.py

Loading...