From 60b447bef2af3018e92f868e5a302cd2e4ee2df3 Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 23 Oct 2023 01:40:10 +1000 Subject: [PATCH] change(ci): Remove duplicate CI jobs for getblocktemplate-rpcs (#7753) * Remove duplicate CI docker job and make features consistent * Remove duplicate OS job * Rename experimental release build * Make Rust features into GitHub repository variables * Remove redundant features in entrypoint.sh * Remove a dependency on a deleted job * Fix syntax of array * Another fix attempt * Undo some accidental merge overwrites * Add missing space * Explain how default is implemented * Fix missing --features and quoting * We can fix this later * Use vars directly in with: blocks * Use correct features for fake activation heights test --- .../workflows/ci-unit-tests-docker.patch.yml | 6 -- .github/workflows/ci-unit-tests-docker.yml | 62 ++++++++----------- .github/workflows/ci-unit-tests-os.patch.yml | 9 +-- .github/workflows/ci-unit-tests-os.yml | 23 +++---- .github/workflows/release-binaries.yml | 13 ++-- .github/workflows/sub-build-docker-image.yml | 21 ++++--- docker/entrypoint.sh | 18 +++--- 7 files changed, 64 insertions(+), 88 deletions(-) diff --git a/.github/workflows/ci-unit-tests-docker.patch.yml b/.github/workflows/ci-unit-tests-docker.patch.yml index e3e629fb4..3ddb5f516 100644 --- a/.github/workflows/ci-unit-tests-docker.patch.yml +++ b/.github/workflows/ci-unit-tests-docker.patch.yml @@ -45,12 +45,6 @@ jobs: steps: - run: 'echo "No build required"' - test-all-getblocktemplate-rpcs: - name: Test all with getblocktemplate-rpcs feature - runs-on: ubuntu-latest - steps: - - run: 'echo "No build required"' - test-fake-activation-heights: name: Test with fake activation heights runs-on: ubuntu-latest diff --git a/.github/workflows/ci-unit-tests-docker.yml b/.github/workflows/ci-unit-tests-docker.yml index bf15ce559..5f90fbf21 100644 --- a/.github/workflows/ci-unit-tests-docker.yml +++ b/.github/workflows/ci-unit-tests-docker.yml @@ -65,6 +65,16 @@ on: - '.github/workflows/sub-find-cached-disks.yml' - '.github/workflows/sub-build-docker-image.yml' +env: + # We need to combine the features manually because some tests don't use the Docker entrypoint + TEST_FEATURES: ${{ format('{0} {1}', vars.RUST_PROD_FEATURES, vars.RUST_TEST_FEATURES) }} + EXPERIMENTAL_FEATURES: ${{ format('{0} {1} {2}', vars.RUST_PROD_FEATURES, vars.RUST_TEST_FEATURES, vars.RUST_EXPERIMENTAL_FEATURES) }} + RUST_LOG: ${{ vars.RUST_LOG }} + RUST_BACKTRACE: ${{ vars.RUST_BACKTRACE }} + RUST_LIB_BACKTRACE: ${{ vars.RUST_LIB_BACKTRACE }} + COLORBT_SHOW_HIDDEN: ${{ vars.COLORBT_SHOW_HIDDEN }} + CARGO_INCREMENTAL: ${{ vars.CARGO_INCREMENTAL }} + jobs: # Build the docker image used by the tests. # @@ -85,10 +95,7 @@ jobs: # Run all the zebra tests, including tests that are ignored by default. # - # - We run all the tests behind the `getblocktemplate-rpcs` feature as a separated step. # - We activate the gRPC feature to avoid recompiling `zebrad`, but we don't actually run any gRPC tests. - # - # TODO: turn this test and the getblocktemplate test into a matrix, so the jobs use exactly the same diagnostics settings test-all: name: Test all timeout-minutes: 180 @@ -105,32 +112,17 @@ jobs: # Run unit, basic acceptance tests, and ignored tests, only showing command output if the test fails. # # If some tests hang, add "-- --nocapture" for just that test, or for all the tests. + # + # TODO: move this test command into entrypoint.sh + # add a separate experimental workflow job if this job is slow - name: Run zebrad tests run: | docker pull ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} - docker run -e NETWORK --name zebrad-tests --tty ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features "lightwalletd-grpc-tests" --workspace -- --include-ignored - env: - NETWORK: ${{ inputs.network || vars.ZCASH_NETWORK }} - - # zebrad tests without cached state with `getblocktemplate-rpcs` feature - # - # Same as above but we run all the tests behind the `getblocktemplate-rpcs` feature. - test-all-getblocktemplate-rpcs: - name: Test all with getblocktemplate-rpcs feature - runs-on: ubuntu-latest-xl - needs: build - steps: - - uses: r7kamura/rust-problem-matchers@v1.4.0 - - - name: Inject slug/short variables - uses: rlespinasse/github-slug-action@v4 - with: - short-length: 7 - - - name: Run zebrad tests - run: | - docker pull ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} - docker run -e NETWORK --name zebrad-tests --tty -e ${{ inputs.network || vars.ZCASH_NETWORK }} ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features "lightwalletd-grpc-tests getblocktemplate-rpcs" --workspace -- --include-ignored + docker run -e NETWORK --name zebrad-tests --tty ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features "${{ env.TEST_FEATURES }}" --workspace -- --include-ignored + # Currently GitHub doesn't allow empty variables + if [[ -n "${{ vars.RUST_EXPERIMENTAL_FEATURES }}" && "${{ vars.RUST_EXPERIMENTAL_FEATURES }}" != " " ]]; then + docker run -e NETWORK --name zebrad-tests-experimental --tty ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features "${{ env.EXPERIMENTAL_FEATURES }} " --workspace -- --include-ignored + fi env: NETWORK: ${{ inputs.network || vars.ZCASH_NETWORK }} @@ -142,7 +134,7 @@ jobs: # # Also, we don't want to accidentally use the fake heights in other tests. # - # (The gRPC feature is a zebrad feature, so it isn't needed here.) + # (We activate the test features to avoid recompiling dependencies, but we don't actually run any gRPC tests.) test-fake-activation-heights: name: Test with fake activation heights timeout-minutes: 60 @@ -156,17 +148,17 @@ jobs: with: short-length: 7 + # TODO: move this test command into entrypoint.sh + # make sure that at least one test runs, and that it doesn't skip itself due to the environmental variable - name: Run tests with fake activation heights run: | docker pull ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} - docker run -e NETWORK -e TEST_FAKE_ACTIVATION_HEIGHTS --name zebrad-tests -t ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --package zebra-state --lib -- --nocapture --include-ignored with_fake_activation_heights + docker run -e NETWORK -e TEST_FAKE_ACTIVATION_HEIGHTS --name zebrad-tests -t ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features "zebra-test" --package zebra-state --lib -- --nocapture --include-ignored with_fake_activation_heights env: TEST_FAKE_ACTIVATION_HEIGHTS: '1' NETWORK: ${{ inputs.network || vars.ZCASH_NETWORK }} # Test that Zebra syncs and checkpoints a few thousand blocks from an empty state. - # - # (We activate the gRPC feature to avoid recompiling `zebrad`, but we don't actually run any gRPC tests.) test-empty-sync: name: Test checkpoint sync from empty state timeout-minutes: 60 @@ -180,16 +172,15 @@ jobs: with: short-length: 7 + # TODO: move this test command into entrypoint.sh - name: Run zebrad large sync tests run: | docker pull ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} - docker run -e NETWORK --name zebrad-tests -t ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features lightwalletd-grpc-tests --package zebrad --test acceptance -- --nocapture --include-ignored sync_large_checkpoints_ + docker run -e NETWORK --name zebrad-tests -t ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features "${{ env.TEST_FEATURES }}" --package zebrad --test acceptance -- --nocapture --include-ignored sync_large_checkpoints_ env: NETWORK: ${{ inputs.network || vars.ZCASH_NETWORK }} # Test launching lightwalletd with an empty lightwalletd and Zebra state. - # - # (We activate the gRPC feature to avoid recompiling `zebrad`, but we don't actually run any gRPC tests.) test-lightwalletd-integration: name: Test integration with lightwalletd timeout-minutes: 60 @@ -203,10 +194,11 @@ jobs: with: short-length: 7 + # TODO: move this test command into entrypoint.sh - name: Run tests with empty lightwalletd launch run: | docker pull ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} - docker run -e NETWORK -e ZEBRA_TEST_LIGHTWALLETD --name lightwalletd-tests -t ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features lightwalletd-grpc-tests --package zebrad --test acceptance -- --nocapture --include-ignored lightwalletd_integration + docker run -e NETWORK -e ZEBRA_TEST_LIGHTWALLETD --name lightwalletd-tests -t ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features "${{ env.TEST_FEATURES }}" --package zebrad --test acceptance -- --nocapture --include-ignored lightwalletd_integration env: ZEBRA_TEST_LIGHTWALLETD: '1' NETWORK: ${{ inputs.network || vars.ZCASH_NETWORK }} @@ -255,7 +247,7 @@ jobs: # # This list is for reliable tests that are run on the `main` branch. # Testnet jobs are not in this list, because we expect testnet to fail occasionally. - needs: [ test-all, test-all-getblocktemplate-rpcs, test-fake-activation-heights, test-empty-sync, test-lightwalletd-integration, test-configuration-file, test-zebra-conf-path ] + needs: [ test-all, test-fake-activation-heights, test-empty-sync, test-lightwalletd-integration, test-configuration-file, test-zebra-conf-path ] # Only open tickets for failed scheduled jobs, manual workflow runs, or `main` branch merges. # (PR statuses are already reported in the PR jobs list, and checked by Mergify.) # TODO: if a job times out, we want to create a ticket. Does failure() do that? Or do we need cancelled()? diff --git a/.github/workflows/ci-unit-tests-os.patch.yml b/.github/workflows/ci-unit-tests-os.patch.yml index 2d126cba0..549f1809a 100644 --- a/.github/workflows/ci-unit-tests-os.patch.yml +++ b/.github/workflows/ci-unit-tests-os.patch.yml @@ -21,19 +21,12 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - # TODO: Windows was removed for now, see https://github.com/ZcashFoundation/zebra/issues/3801 - # TODO: macOS tests were removed for now, see https://github.com/ZcashFoundation/zebra/issues/6824 os: [ubuntu-latest] rust: [stable, beta] - features: ["", " --features getblocktemplate-rpcs"] + features: [""] exclude: - os: macos-latest rust: beta - - os: macos-latest - features: " --features getblocktemplate-rpcs" - - os: ubuntu-latest - rust: beta - features: " --features getblocktemplate-rpcs" steps: - run: 'echo "No build required"' diff --git a/.github/workflows/ci-unit-tests-os.yml b/.github/workflows/ci-unit-tests-os.yml index 642c9b811..3f5d76433 100644 --- a/.github/workflows/ci-unit-tests-os.yml +++ b/.github/workflows/ci-unit-tests-os.yml @@ -78,21 +78,18 @@ jobs: # TODO: macOS tests were removed for now, see https://github.com/ZcashFoundation/zebra/issues/6824 os: [ubuntu-latest] rust: [stable, beta] - features: ["", " --features getblocktemplate-rpcs"] + # TODO: When vars.EXPERIMENTAL_FEATURES has features in it, add it here. + # Or work out a way to trim the space from the variable: GitHub doesn't allow empty variables. + # Or use `default` for the empty feature set and EXPERIMENTAL_FEATURES, and update the branch protection rules. + #features: ${{ fromJSON(format('["", "{0}"]', vars.EXPERIMENTAL_FEATURES)) }} + features: [""] exclude: - # We're excluding macOS for the following reasons: + # We're excluding macOS beta for the following reasons: # - the concurrent macOS runner limit is much lower than the Linux limit # - macOS is slower than Linux, and shouldn't have a build or test difference with Linux # - macOS is a second-tier Zebra support platform - os: macos-latest rust: beta - - os: macos-latest - features: " --features getblocktemplate-rpcs" - # getblocktemplate-rpcs is an experimental feature, so we just need to test it on stable Rust - # beta is unlikely to fail just for this feature, and if it does, we can fix it when it reaches stable. - - os: ubuntu-latest - rust: beta - features: " --features getblocktemplate-rpcs" steps: - uses: actions/checkout@v4.1.1 @@ -119,7 +116,7 @@ jobs: #with: # workspaces: ". -> C:\\zebra-target" with: - # Split the getblocktemplate-rpcs cache from the regular cache, to avoid linker errors. + # Split the experimental features cache from the regular cache, to avoid linker errors. # (These might be "disk full" errors, or they might be dependency resolution issues.) key: ${{ matrix.features }} @@ -190,17 +187,15 @@ jobs: # If some tests hang, add "-- --nocapture" for just that test, or for all the tests. - name: Run tests${{ matrix.features }} run: | - cargo test ${{ matrix.features }} --release --verbose --workspace + cargo test --features "${{ matrix.features }}" --release --verbose --workspace # Explicitly run any tests that are usually #[ignored] - name: Run zebrad large sync tests${{ matrix.features }} # Skip the entire step on Ubuntu and Windows, because the test would be skipped anyway due to ZEBRA_SKIP_NETWORK_TESTS - # Currently, this also skips large sync with `getblocktemplate-rpcs`, - # but that is already covered by the Docker tests. if: matrix.os == 'macos-latest' run: | - cargo test ${{ matrix.features }} --release --verbose --package zebrad --test acceptance -- --nocapture --include-ignored sync_large_checkpoints_ + cargo test --features "${{ matrix.features }}" --release --verbose --package zebrad --test acceptance -- --nocapture --include-ignored sync_large_checkpoints_ # Install Zebra with lockfile dependencies, with no caching and default features install-from-lockfile-no-cache: diff --git a/.github/workflows/release-binaries.yml b/.github/workflows/release-binaries.yml index 50c28acb7..f0b60acba 100644 --- a/.github/workflows/release-binaries.yml +++ b/.github/workflows/release-binaries.yml @@ -26,28 +26,29 @@ jobs: dockerfile_path: ./docker/Dockerfile dockerfile_target: runtime image_name: zebra - rust_log: info + features: ${{ vars.RUST_PROD_FEATURES }} + rust_log: ${{ vars.RUST_LOG }} # This step needs access to Docker Hub secrets to run successfully secrets: inherit # The image will be named `zebra:.experimental` - build-mining-testnet: - name: Build Release Testnet Mining Docker + build-experimental: + name: Build Experimental Features Release Docker uses: ./.github/workflows/sub-build-docker-image.yml with: dockerfile_path: ./docker/Dockerfile dockerfile_target: runtime image_name: zebra tag_suffix: .experimental - features: "default-release-binaries getblocktemplate-rpcs" - rust_log: info + features: ${{ format('{0} {1}', vars.RUST_PROD_FEATURES, vars.RUST_EXPERIMENTAL_FEATURES) }} + rust_log: ${{ vars.RUST_LOG }} # This step needs access to Docker Hub secrets to run successfully secrets: inherit failure-issue: name: Open or update issues for release binaries failures # When a new job is added to this workflow, add it to this list. - needs: [ build, build-mining-testnet ] + needs: [ build, build-experimental ] # Open tickets for any failed build in this workflow. if: failure() || cancelled() runs-on: ubuntu-latest diff --git a/.github/workflows/sub-build-docker-image.yml b/.github/workflows/sub-build-docker-image.yml index f57e74e94..1b9293246 100644 --- a/.github/workflows/sub-build-docker-image.yml +++ b/.github/workflows/sub-build-docker-image.yml @@ -21,19 +21,17 @@ on: rust_lib_backtrace: required: false type: string + # defaults to: vars.RUST_LOG rust_log: required: false type: string - default: info - # keep these in sync with: - # https://github.com/ZcashFoundation/zebra/blob/main/docker/Dockerfile#L83 + # defaults to: vars.RUST_PROD_FEATURES features: required: false - default: "default-release-binaries" type: string + # defaults to: vars.RUST_TEST_FEATURES (and entrypoint.sh adds vars.RUST_PROD_FEATURES) test_features: required: false - default: "lightwalletd-grpc-tests zebra-checkpoints" type: string tag_suffix: required: false @@ -49,6 +47,13 @@ on: description: 'The image digest to be used on a caller workflow' value: ${{ jobs.build.outputs.image_digest }} + +env: + FEATURES: ${{ inputs.features || vars.RUST_PROD_FEATURES }} + TEST_FEATURES: ${{ inputs.test_features || vars.RUST_TEST_FEATURES }} + RUST_LOG: ${{ inputs.rust_log || vars.RUST_LOG }} + CARGO_INCREMENTAL: ${{ vars.CARGO_INCREMENTAL }} + jobs: build: name: Build images @@ -143,9 +148,9 @@ jobs: labels: ${{ steps.meta.outputs.labels }} build-args: | SHORT_SHA=${{ env.GITHUB_SHA_SHORT }} - RUST_LOG=${{ inputs.rust_log }} - FEATURES=${{ inputs.features }} - TEST_FEATURES=${{ inputs.test_features }} + RUST_LOG=${{ env.RUST_LOG }} + FEATURES=${{ env.FEATURES }} + TEST_FEATURES=${{ env.TEST_FEATURES }} push: true # Don't read from the cache if the caller disabled it. # https://docs.docker.com/engine/reference/commandline/buildx_build/#options diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index 79dd0c3bb..1ffc0c0ba 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -43,15 +43,12 @@ set -o pipefail : "${TRACING_ENDPOINT_PORT:=3000}" # [rpc] : "${RPC_LISTEN_ADDR:=0.0.0.0}" -# if ${RPC_PORT} is not set and ${FEATURES} contains getblocktemplate-rpcs, -# set ${RPC_PORT} to the default value for the current network +# if ${RPC_PORT} is not set, use the default value for the current network if [[ -z "${RPC_PORT}" ]]; then - if [[ " ${FEATURES} " =~ " getblocktemplate-rpcs " ]]; then - if [[ "${NETWORK}" = "Mainnet" ]]; then - : "${RPC_PORT:=8232}" - elif [[ "${NETWORK}" = "Testnet" ]]; then - : "${RPC_PORT:=18232}" - fi + if [[ "${NETWORK}" = "Mainnet" ]]; then + : "${RPC_PORT:=8232}" + elif [[ "${NETWORK}" = "Testnet" ]]; then + : "${RPC_PORT:=18232}" fi fi @@ -319,16 +316,15 @@ case "$1" in run_cargo_test "${ENTRYPOINT_FEATURES}" "sending_transactions_using_lightwalletd" # These tests use mining code, but don't use gRPC. - # We add the mining feature here because our other code needs to pass tests without it. elif [[ "${TEST_GET_BLOCK_TEMPLATE}" -eq "1" ]]; then # Starting with a cached Zebra tip, test getting a block template from Zebra's RPC server. check_directory_files "${ZEBRA_CACHED_STATE_DIR}" - run_cargo_test "getblocktemplate-rpcs,${ENTRYPOINT_FEATURES}" "get_block_template" + run_cargo_test "${ENTRYPOINT_FEATURES}" "get_block_template" elif [[ "${TEST_SUBMIT_BLOCK}" -eq "1" ]]; then # Starting with a cached Zebra tip, test sending a block to Zebra's RPC port. check_directory_files "${ZEBRA_CACHED_STATE_DIR}" - run_cargo_test "getblocktemplate-rpcs,${ENTRYPOINT_FEATURES}" "submit_block" + run_cargo_test "${ENTRYPOINT_FEATURES}" "submit_block" else exec "$@"