ci: run USDT interface tests in the CI

0xB10C commented at 12:34 pm on July 2, 2022: contributor

Changes a CI task that runs test the previously not run test/functional/interface_usdt_*.py functional tests (added in #24358).

This task is run as CirussCI compute_engine_instance VM as hooking into the tracepoints is not possible in CirrusCI docker containers (https://github.com/bitcoin/bitcoin/issues/23296#issuecomment-1024920845). We use an unoffical PPA and untrusted bpfcc-tools package in the CI as the Ubuntu jammy and Debian bullseye packages are outdated. We hope use an official package when new Ubuntu/Debian releases are available for the use with Google Compute Engine.

We make sure to hook into bitcoind binaries in USDT interface tests via their PID, instead of their path. This makes sure multiple functional tests running in parallel don’t interfere with each other.

The utxocache USDT interface tests is adopted to a change of the functional test framework that wasn’t detected as the tests weren’t run in the CI. As the tracepoints expose internals, it can happen that we need to adopt the interface test when internals change. This is a bit awkward, and if it happens to frequently, we should consider generalizing the tests a bit more. For now it’s fine, I think.

See the individual commit messages for more details on the changes.

Fixes #24782 Fixes #23296

I’d like to hear from reviewers:

Are we OK with using the hadret/bpfcc PPA for now? There is a clear plan when to drop it and as is currently, it could only impact the newly added VM task.
~~Adding a new task increases CI runtime and costs. Should an existing container CI task be ported to a VM and reused instead?~~ Yes, see #25528 (comment)

fanquake added the label Tests on Jul 2, 2022

test: hook into PID in tracing tests

This makes sure to NOT hook into other bitcoind binaries run in
paralell in the test framework. We only want to trace the intended
binary.

In interface_usdt_utxocache.py:
While testing the utxocache flush with pruning, bitcoind is
restarted and we need to hook into the new PID again.

220a5a2841

test: adopt USDT utxocache interface tests

The USDT interface exposes process internals via the tracepoints. This
means, the USDT interface tests somewhat awardly depend on these
internals. If internals change, the tests have to adopt to that change.
Previously, the USDT interface tests weren't run in the CI so changes
could break the USDT interface tests without being noticed (e.g.
https://github.com/bitcoin/bitcoin/pull/25486).

In fa13375aa3fcb4fd5b9e0d4c69ac31cf66c3209a a 'self.rescan_utxos()' call
was added in the 'generate()' function of the test framework.
'rescan_utxos()' causes the UTXO cache to be flushed. In the USDT
interface tests for the 'utxocache:flush' trancepoint, 'generate()' is
used. As the utxo cache is now flushed more often, the number of flushes
the tests expectes need to be adopted. Also, the utxo cache has now a
different size when being flushed.

The utxocache tracepoint is tested by shutting the node down and
pruning blocks, to test the 'for_prune' argument.

Changes:
- A list 'expected_flushes' is now used which contains 'mode',
  'for_prune', and 'size' for each expected flush.
- When a flush happens, the expected-flush is removed from the list.
  This list is checked to be empty (unchanged).
- Previously, shutting down caused these two flushes:
    UTXOCacheFlush(duration=*, mode=ALWAYS, size=104, memory=*, for_prune=False)
    UTXOCacheFlush(duration=*, mode=ALWAYS, size=0, memory=*, for_prune=False)
  now it causes these flushes:
    UTXOCacheFlush(duration=*, mode=ALWAYS, size=2, memory=*, for_prune=False)
    UTXOCacheFlush(duration=*, mode=ALWAYS, size=0, memory=*, for_prune=False)
  The 104 UTXOs flushed previously were mainly coinbase UTXOs generated
  in previous tests and the test setup. These are now already flushed.
- In the 'for_prune' test we previously hooked into the tracepoint
  before mining blocks. This changed to only get notified about the
  tracepoint being triggered for the prune. Here, the utxo cache is
  empty already as it has just been flushed in 'generate()'.
  old:
    UTXOCacheFlush(duration=*, mode=NONE, size=350, memory=*, for_prune=True)
  new:
    UTXOCacheFlush(duration=*, mode=NONE, size=0, memory=*, for_prune=True)

dba6f82342

0xB10C force-pushed on Jul 2, 2022

maflcko commented at 1:02 pm on July 4, 2022: member

I tried to experiment with this, but I ran into: net.cpp:2980:5: error: must specify at least one argument for '...' parameter of variadic macro [-Werror,-Wgnu-zero-variadic-macro-arguments]

See https://cirrus-ci.com/task/6300898480619520?logs=ci#L2025

With the only change being to switch to debian:bookworm, see https://github.com/MarcoFalke/bitcoin-core-with-ci/commit/803f365f47d2f40a88d8969f0e84a6779dc5a5cb#diff-b37d362747ecdb36dbfcbe2b8aab779f06404eee471ad835b58355e353d4fe79R9

Any ideas?

0xB10C commented at 3:28 pm on July 4, 2022: contributor

With the only change being to switch to debian:bookworm, see MarcoFalke/bitcoin-core-with-ci@803f365#diff-b37d362747ecdb36dbfcbe2b8aab779f06404eee471ad835b58355e353d4fe79R9

Any ideas?

Interesting. This seems to be unrelated to the tests. More the tracepoint macros in general. Probably a problem with the Systemtap package (systemtap-sdt-dev) in bookworm? Per https://packages.debian.org/bookworm/systemtap-sdt-dev it’s version 4.7, which is also what we use in depends since #25360. Does a depends build on bookworm fail too?

I’ll also do some poking.

edit: the variadic marco arg was indeed not present in 4.5 (which we used previously): https://sourceware.org/git/?p=systemtap.git;a=commitdiff;h=ecab2afea46099b4e7dfd551462689224afdbe3a. I think it also has something to do with clang, -pedantic and C++20. See https://stackoverflow.com/questions/67912343/how-to-resolve-must-specify-at-least-one-argument-for-parameter-of-variad

0xB10C commented at 8:51 pm on July 4, 2022: contributor

Tried running a depends build on bookworm - but no success so far: https://cirrus-ci.com/task/5255417889554432?logs=ci#L570

https://github.com/0xB10C/bitcoin/commit/5641883b3a148f01e1ef910705fd82a4b5132dde

maflcko commented at 3:52 pm on July 5, 2022: member

In any case it would fail because it wouldn’t be possible to install the kernel headers in docker: https://cirrus-ci.com/task/5345925198512128?logs=ci#L3567 ?

0xB10C commented at 12:19 pm on July 7, 2022: contributor

One way to get the hosts kernel headers into the docker container is to mount them. I’ve mounted /lib/modules/5.15.0-1010-gcp/build however, there are quite a few relative symlinks which cause problems with mounts. To make them available, I’ve mounted /usr/src/linux-gcp-headers-5.15.0-1010 and /usr/src/linux-headers-5.15.0-1010-gcp but this doesn’t seem to be enough. Not sure if it’s worth the effort. See https://github.com/0xB10C/bitcoin/commit/6ac8e7effc8d01c34c83ab8f2ce70cd9e0712ef8 and https://cirrus-ci.com/task/6730748215427072?logs=ci#L2762.

0<built-in>:1:10: fatal error: './include/linux/kconfig.h' file not found
1#include "./include/linux/kconfig.h"
2         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
31 error generated.

which is in /lib/modules/5.15.0-1010-gcp/build/include/linux/kconfig.h.

maflcko commented at 1:14 pm on July 7, 2022: member

Makes sense. That doesn’t seem to be worth the effort.

An alternative to using a private ppa would be to bump the OS version in /etc/apt/sources.list before installing the package (See for example https://github.com/google/oss-fuzz/commit/0da6949a7bc3586cbf160738e8f09b209ea97a06 ), though that’d just seem too brittle.

To preserve CI resources, I am thinking about combining this with another task. Since a wallet would be good to also cover the coinselection traces, and a recent-ish OS is required, we could re-use one of the sanitizer builds or the i686 build?

maflcko commented at 5:17 pm on July 7, 2022: member

It worked. This commit should be passing: https://cirrus-ci.com/build/4705943798677504

maflcko commented at 10:01 am on July 8, 2022: member

Sorry, I lied. This one should pass: https://cirrus-ci.com/build/4898814673813504

0xB10C force-pushed on Jul 8, 2022

ci: run USDT interface test in a VM

Our CI tasks are run by CirrusCI in Docker containers in a Google
Compute Engine based Kubernetes environment. These containers have
limited capabilities - especially CAP_SYS_ADMIN is missing. See
https://github.com/bitcoin/bitcoin/issues/23296#issuecomment-1024920845

We need elevated privileges to hook into the USDT tracepoints. We use a
CirrusCI "compute_engine_instance" (a VM, not a container) where we have
the required privileges. The ubunut-mininmal-2204-lts was choosen with
debian-11 being an alternative. Both pack an outdated 'bpfcc-tools'
package (v0.18.0) from 2020. This version prints warnings to stderr
during BPF bytecode compilation, which causes our functional test runner
to fail. This is fixed in newer verison.

Until debian-12 or a newer Ubuntu release is avaliable as image in GCE
(https://cloud.google.com/compute/docs/images/os-details), we use a
third-party and untrusted PPA that releases up-to-date versions of the
package.

The official iovisor (authors of BCC) PPA is outdated too. An
alternative would be to compile BCC from source in the CI.

Co-authored-by: MacroFake <falke.marco@gmail.com>

cc7335edc8

0xB10C force-pushed on Jul 8, 2022

0xB10C commented at 9:06 am on July 9, 2022: contributor

As discussed offline with MarcoFake, we’re using the [ASan + LSan + UBSan + integer, no depends, USDT] [jammy] task to run the USDT tests. The i686 didn’t work - probably because the tracing isn’t really supported on 32bit.

maflcko commented at 7:56 am on July 11, 2022: member

cr ACK cc7335edc87c6ef34429b4df94f53973db520aac

in .cirrus.yml:270 in cc7335edc8

268+  # containers don't have privileges to hook into bitcoind. CirrusCI uses
269+  # Google Compute Engine instances: https://cirrus-ci.org/guide/custom-vms/
270+  # Images can be found here: https://cloud.google.com/compute/docs/images/os-details
271+  compute_engine_instance:
272+    image_project: ubuntu-os-cloud
273+    image: family/ubuntu-2204-lts # when upgrading, check if we can drop "ADD_UNTRUSTED_BPFCC_PPA"

maflcko commented at 2:38 pm on July 11, 2022:

note to the future: I was able to change this to family/ubuntu-2110, but the gce docs only include lts version. So if 2210 has the updated package, we can probably switch to that: https://packages.ubuntu.com/kinetic/bpfcc-tools

theStack commented at 8:46 am on July 21, 2022: contributor

Concept ACK

maflcko merged this on Aug 1, 2022

maflcko closed this on Aug 1, 2022

sidhujag referenced this in commit c35b2bfab1 on Aug 1, 2022

0xB10C deleted the branch on Sep 14, 2022

in ci/test/04_install.sh:77 in cc7335edc8

72+    # Ubuntu 22.04 LTS and Debian 11 both have an outdated bpfcc-tools packages.
73+    # The iovisor PPA is outdated as well. The next Ubuntu and Debian releases will contain updated
74+    # packages. Meanwhile, use an untrusted PPA to install an up-to-date version of the bpfcc-tools
75+    # package.
76+    # TODO: drop this once we can use newer images in GCE
77+    CI_EXEC add-apt-repository ppa:hadret/bpfcc

hebasto commented at 10:15 am on November 25, 2022:

Using of add-apt-repository requires some packages been installed.

0xB10C referenced this in commit 2811f40f30 on Nov 28, 2022

maflcko referenced this in commit 9e59d21fbe on Dec 2, 2022

Fabcien referenced this in commit 7415dd1f91 on Dec 5, 2022

in ci/test/00_setup_env_native_tidy.sh:18 in cc7335edc8

14@@ -15,5 +15,5 @@ export RUN_FUNCTIONAL_TESTS=false
15 export RUN_FUZZ_TESTS=false
16 export RUN_TIDY=true
17 export GOAL="install"
18-export BITCOIN_CONFIG="CC=clang CXX=clang++ --with-incompatible-bdb --disable-hardening CFLAGS='-O0 -g0' CXXFLAGS='-O0 -g0'"
19+export BITCOIN_CONFIG="CC=clang CXX=clang++ --enable-c++20 --with-incompatible-bdb --disable-hardening CFLAGS='-O0 -g0' CXXFLAGS='-O0 -g0'"

hebasto commented at 11:47 am on December 29, 2022:

@0xB10C @MarcoFalke

What was the point to add --enable-c++20 here?

Asking in context of #26763 (review).

maflcko commented at 11:55 am on December 29, 2022:

It wasn’t added in this pull, see the diff in ci/test/00_setup_env_native_asan.sh.

It was added to check that compiling with it works.

hebasto commented at 12:38 pm on December 29, 2022:

Could another CI task use it instead of the “tidy” one? (fwiw, we have a native Windows build using C++20)

maflcko commented at 1:32 pm on December 29, 2022:

Sure, any Linux task that builds all targets (including fuzz and gui) should do. For example, multiprocess, asan, or tsan.

hebasto commented at 6:51 pm on December 29, 2022:

It wasn’t added in this pull, see the diff in ci/test/00_setup_env_native_asan.sh.

Suggesting to revert the move of --enable-c++20 between CI tasks in #26770.

maflcko referenced this in commit 65de8eeeca on Dec 29, 2022

bitcoin locked this on Dec 29, 2023

ci: run USDT interface tests in the CI #25528