Page MenuHomePhabricator

[build-bot] Persist build info between redeployments
ClosedPublic

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

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

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
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

Fixed according to feedback

This revision is now accepted and ready to land.Oct 21 2020, 18:56