From ffed3674fd8232353c02b6674febc60f542f950c Mon Sep 17 00:00:00 2001 From: marun Date: Mon, 1 Jul 2024 14:04:31 -0700 Subject: [PATCH 1/2] [ci] Add actionlint job (#3160) --- .github/actionlint.yml | 5 +++++ .github/workflows/build-linux-binaries.yml | 8 +++---- .github/workflows/build-macos-release.yml | 8 +++---- .github/workflows/build-public-ami.yml | 8 +++---- .../workflows/build-ubuntu-amd64-release.yml | 8 +++---- .../workflows/build-ubuntu-arm64-release.yml | 8 +++---- .github/workflows/build-win-release.yml | 4 ++-- .github/workflows/ci.yml | 5 ++++- .../workflows/publish_antithesis_images.yml | 4 ++-- .github/workflows/trigger-antithesis-runs.yml | 22 ++++++++++--------- scripts/actionlint.sh | 7 ++++++ 11 files changed, 52 insertions(+), 35 deletions(-) create mode 100644 .github/actionlint.yml create mode 100755 scripts/actionlint.sh diff --git a/.github/actionlint.yml b/.github/actionlint.yml new file mode 100644 index 000000000000..2e6d753c2282 --- /dev/null +++ b/.github/actionlint.yml @@ -0,0 +1,5 @@ +self-hosted-runner: + labels: + - custom-arm64-focal + - custom-arm64-jammy + - net-outage-sim diff --git a/.github/workflows/build-linux-binaries.yml b/.github/workflows/build-linux-binaries.yml index 9f5cdfe97475..bcaf374f3935 100644 --- a/.github/workflows/build-linux-binaries.yml +++ b/.github/workflows/build-linux-binaries.yml @@ -40,14 +40,14 @@ jobs: if: "${{ github.event.inputs.tag == '' }}" id: get_tag_from_git run: | - echo "TAG=${GITHUB_REF/refs\/tags\//}" >> $GITHUB_ENV + echo "TAG=${GITHUB_REF/refs\/tags\//}" >> "$GITHUB_ENV" shell: bash - name: Try to get tag from workflow dispatch if: "${{ github.event.inputs.tag != '' }}" id: get_tag_from_workflow run: | - echo "TAG=${{ github.event.inputs.tag }}" >> $GITHUB_ENV + echo "TAG=${{ github.event.inputs.tag }}" >> "$GITHUB_ENV" shell: bash - name: Create tgz package structure and upload to S3 @@ -101,14 +101,14 @@ jobs: if: "${{ github.event.inputs.tag == '' }}" id: get_tag_from_git run: | - echo "TAG=${GITHUB_REF/refs\/tags\//}" >> $GITHUB_ENV + echo "TAG=${GITHUB_REF/refs\/tags\//}" >> "$GITHUB_ENV" shell: bash - name: Try to get tag from workflow dispatch if: "${{ github.event.inputs.tag != '' }}" id: get_tag_from_workflow run: | - echo "TAG=${{ github.event.inputs.tag }}" >> $GITHUB_ENV + echo "TAG=${{ github.event.inputs.tag }}" >> "$GITHUB_ENV" shell: bash - name: Create tgz package structure and upload to S3 diff --git a/.github/workflows/build-macos-release.yml b/.github/workflows/build-macos-release.yml index 2a4bfb1c45d1..8a7f641ed3f7 100644 --- a/.github/workflows/build-macos-release.yml +++ b/.github/workflows/build-macos-release.yml @@ -35,18 +35,18 @@ jobs: if: "${{ github.event.inputs.tag == '' }}" id: get_tag_from_git run: | - echo "TAG=${GITHUB_REF/refs\/tags\//}" >> $GITHUB_ENV + echo "TAG=${GITHUB_REF/refs\/tags\//}" >> "$GITHUB_ENV" shell: bash - name: Try to get tag from workflow dispatch if: "${{ github.event.inputs.tag != '' }}" id: get_tag_from_workflow run: | - echo "TAG=${{ github.event.inputs.tag }}" >> $GITHUB_ENV + echo "TAG=${{ github.event.inputs.tag }}" >> "$GITHUB_ENV" shell: bash - name: Create zip file - run: 7z a avalanchego-macos-${TAG}.zip build/avalanchego + run: 7z a "avalanchego-macos-${TAG}.zip" build/avalanchego env: TAG: ${{ env.TAG }} @@ -63,7 +63,7 @@ jobs: aws-region: us-east-1 - name: Upload file to S3 - run: aws s3 cp avalanchego-macos-${{ env.TAG }}.zip s3://${BUCKET}/macos/ + run: aws s3 cp avalanchego-macos-${{ env.TAG }}.zip "s3://${BUCKET}/macos/" env: BUCKET: ${{ secrets.BUCKET }} diff --git a/.github/workflows/build-public-ami.yml b/.github/workflows/build-public-ami.yml index cc9082ab3e3c..314b110865a1 100644 --- a/.github/workflows/build-public-ami.yml +++ b/.github/workflows/build-public-ami.yml @@ -27,16 +27,16 @@ jobs: - name: Install aws cli run: | sudo apt update - sudo apt-get -y install python3-boto3=${PYTHON3_BOTO3_VERSION} + sudo apt-get -y install python3-boto3="${PYTHON3_BOTO3_VERSION}" - name: Get the tag id: get_tag run: | if [[ ${{ github.event_name }} == 'push' ]]; then - echo "TAG=${GITHUB_REF/refs\/tags\//}" >> $GITHUB_ENV + echo "TAG=${GITHUB_REF/refs\/tags\//}" >> "$GITHUB_ENV" else - echo "TAG=${{ inputs.tag }}" >> $GITHUB_ENV + echo "TAG=${{ inputs.tag }}" >> "$GITHUB_ENV" fi shell: bash @@ -44,7 +44,7 @@ jobs: run: | if [ "${{ github.event_name }}" == "workflow_dispatch" ]; then echo "Setting SKIP_CREATE_AMI to False" - echo "SKIP_CREATE_AMI=False" >> $GITHUB_ENV + echo "SKIP_CREATE_AMI=False" >> "$GITHUB_ENV" fi - name: Configure AWS credentials diff --git a/.github/workflows/build-ubuntu-amd64-release.yml b/.github/workflows/build-ubuntu-amd64-release.yml index 7c00b56d1224..ff26569570c2 100644 --- a/.github/workflows/build-ubuntu-amd64-release.yml +++ b/.github/workflows/build-ubuntu-amd64-release.yml @@ -38,14 +38,14 @@ jobs: if: "${{ github.event.inputs.tag == '' }}" id: get_tag_from_git run: | - echo "TAG=${GITHUB_REF/refs\/tags\//}" >> $GITHUB_ENV + echo "TAG=${GITHUB_REF/refs\/tags\//}" >> "$GITHUB_ENV" shell: bash - name: Try to get tag from workflow dispatch if: "${{ github.event.inputs.tag != '' }}" id: get_tag_from_workflow run: | - echo "TAG=${{ github.event.inputs.tag }}" >> $GITHUB_ENV + echo "TAG=${{ github.event.inputs.tag }}" >> "$GITHUB_ENV" shell: bash - name: Create debian package @@ -88,14 +88,14 @@ jobs: if: "${{ github.event.inputs.tag == '' }}" id: get_tag_from_git run: | - echo "TAG=${GITHUB_REF/refs\/tags\//}" >> $GITHUB_ENV + echo "TAG=${GITHUB_REF/refs\/tags\//}" >> "$GITHUB_ENV" shell: bash - name: Try to get tag from workflow dispatch if: "${{ github.event.inputs.tag != '' }}" id: get_tag_from_workflow run: | - echo "TAG=${{ github.event.inputs.tag }}" >> $GITHUB_ENV + echo "TAG=${{ github.event.inputs.tag }}" >> "$GITHUB_ENV" shell: bash - name: Configure AWS credentials diff --git a/.github/workflows/build-ubuntu-arm64-release.yml b/.github/workflows/build-ubuntu-arm64-release.yml index 096137b1a2ef..4d8fa841cbd8 100644 --- a/.github/workflows/build-ubuntu-arm64-release.yml +++ b/.github/workflows/build-ubuntu-arm64-release.yml @@ -38,14 +38,14 @@ jobs: if: "${{ github.event.inputs.tag == '' }}" id: get_tag_from_git run: | - echo "TAG=${GITHUB_REF/refs\/tags\//}" >> $GITHUB_ENV + echo "TAG=${GITHUB_REF/refs\/tags\//}" >> "$GITHUB_ENV" shell: bash - name: Try to get tag from workflow dispatch if: "${{ github.event.inputs.tag != '' }}" id: get_tag_from_workflow run: | - echo "TAG=${{ github.event.inputs.tag }}" >> $GITHUB_ENV + echo "TAG=${{ github.event.inputs.tag }}" >> "$GITHUB_ENV" shell: bash - name: Create debian package @@ -96,14 +96,14 @@ jobs: if: "${{ github.event.inputs.tag == '' }}" id: get_tag_from_git run: | - echo "TAG=${GITHUB_REF/refs\/tags\//}" >> $GITHUB_ENV + echo "TAG=${GITHUB_REF/refs\/tags\//}" >> "$GITHUB_ENV" shell: bash - name: Try to get tag from workflow dispatch if: "${{ github.event.inputs.tag != '' }}" id: get_tag_from_workflow run: | - echo "TAG=${{ github.event.inputs.tag }}" >> $GITHUB_ENV + echo "TAG=${{ github.event.inputs.tag }}" >> "$GITHUB_ENV" shell: bash - name: Create debian package diff --git a/.github/workflows/build-win-release.yml b/.github/workflows/build-win-release.yml index 9d04e036b7e8..15502e003223 100644 --- a/.github/workflows/build-win-release.yml +++ b/.github/workflows/build-win-release.yml @@ -44,14 +44,14 @@ jobs: if: "${{ github.event.inputs.tag == '' }}" id: get_tag_from_git run: | - echo "TAG=${GITHUB_REF/refs\/tags\//}" >> $GITHUB_ENV + echo "TAG=${GITHUB_REF/refs\/tags\//}" >> "$GITHUB_ENV" shell: bash - name: Try to get tag from workflow dispatch if: "${{ github.event.inputs.tag != '' }}" id: get_tag_from_workflow run: | - echo "TAG=${{ github.event.inputs.tag }}" >> $GITHUB_ENV + echo "TAG=${{ github.event.inputs.tag }}" >> "$GITHUB_ENV" shell: bash # Runs a single command using the runners shell diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7fc80756daed..68ee2585a3b7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,7 +37,7 @@ jobs: - name: Set timeout on Windows # Windows UT run slower and need a longer timeout shell: bash if: matrix.os == 'windows-2022' - run: echo "TIMEOUT=240s" >> $GITHUB_ENV + run: echo "TIMEOUT=240s" >> "$GITHUB_ENV" - name: build_test shell: bash run: ./scripts/build_test.sh @@ -213,6 +213,9 @@ jobs: - name: Run shellcheck shell: bash run: scripts/shellcheck.sh + - name: Run actionlint + shell: bash + run: scripts/actionlint.sh buf-lint: name: Protobuf Lint runs-on: ubuntu-latest diff --git a/.github/workflows/publish_antithesis_images.yml b/.github/workflows/publish_antithesis_images.yml index 8363ad73e975..8dc9d426f942 100644 --- a/.github/workflows/publish_antithesis_images.yml +++ b/.github/workflows/publish_antithesis_images.yml @@ -35,12 +35,12 @@ jobs: run: bash -x ./scripts/build_antithesis_images.sh env: IMAGE_PREFIX: ${{ env.REGISTRY }}/${{ env.REPOSITORY }} - TAG: ${{ github.events.inputs.image_tag || 'latest' }} + TAG: ${{ github.event.inputs.image_tag || 'latest' }} TEST_SETUP: avalanchego - name: Build and push images for xsvm test setup run: bash -x ./scripts/build_antithesis_images.sh env: IMAGE_PREFIX: ${{ env.REGISTRY }}/${{ env.REPOSITORY }} - TAG: ${{ github.events.inputs.image_tag || 'latest' }} + TAG: ${{ github.event.inputs.image_tag || 'latest' }} TEST_SETUP: xsvm diff --git a/.github/workflows/trigger-antithesis-runs.yml b/.github/workflows/trigger-antithesis-runs.yml index 0521b0770d79..f893b88da9ec 100644 --- a/.github/workflows/trigger-antithesis-runs.yml +++ b/.github/workflows/trigger-antithesis-runs.yml @@ -21,7 +21,8 @@ on: type: string jobs: - Run Antithesis Avalanchego Test Setup: + antithesis_avalanchego: + name: Run Antithesis Avalanchego Test Setup runs-on: ubuntu-latest steps: - uses: antithesishq/antithesis-trigger-action@v0.5 @@ -31,12 +32,13 @@ jobs: username: ${{ secrets.ANTITHESIS_USERNAME }} password: ${{ secrets.ANTITHESIS_PASSWORD }} github_token: ${{ secrets.ANTITHESIS_GH_PAT }} - config_image: antithesis-avalanchego-config@${{ github.events.inputs.image_tag }} - images: antithesis-avalanchego-workload@${{ github.events.inputs.image_tag }};antithesis-avalanchego-node@${{ github.events.inputs.image_tag }} - email_recipients: ${{ github.events.inputs.recipients }} + config_image: antithesis-avalanchego-config@${{ github.event.inputs.image_tag || 'latest' }} + images: antithesis-avalanchego-workload@${{ github.event.inputs.image_tag || 'latest' }};antithesis-avalanchego-node@${{ github.event.inputs.image_tag || 'latest' }} + email_recipients: ${{ github.event.inputs.recipients || secrets.ANTITHESIS_RECIPIENTS }} additional_parameters: |- - custom.duration=${{ github.events.inputs.duration }} - Run Antithesis XSVM Test Setup: + custom.duration=${{ github.event.inputs.duration || '0.5' }} + antithesis_xsvm: + name: Run Antithesis XSVM Test Setup runs-on: ubuntu-latest steps: - uses: antithesishq/antithesis-trigger-action@v0.5 @@ -46,8 +48,8 @@ jobs: username: ${{ secrets.ANTITHESIS_USERNAME }} password: ${{ secrets.ANTITHESIS_PASSWORD }} github_token: ${{ secrets.ANTITHESIS_GH_PAT }} - config_image: antithesis-xsvm-config@${{ github.events.inputs.image_tag }} - images: antithesis-xsvm-workload@${{ github.events.inputs.image_tag }};antithesis-xsvm-node@${{ github.events.inputs.image_tag }} - email_recipients: ${{ github.events.inputs.recipients }} + config_image: antithesis-xsvm-config@${{ github.event.inputs.image_tag || 'latest' }} + images: antithesis-xsvm-workload@${{ github.event.inputs.image_tag || 'latest' }};antithesis-xsvm-node@${{ github.event.inputs.image_tag || 'latest' }} + email_recipients: ${{ github.event.inputs.recipients || secrets.ANTITHESIS_RECIPIENTS }} additional_parameters: |- - custom.duration=${{ github.events.inputs.duration }} + custom.duration=${{ github.event.inputs.duration || '0.5' }} diff --git a/scripts/actionlint.sh b/scripts/actionlint.sh new file mode 100755 index 000000000000..bdc3083e6b65 --- /dev/null +++ b/scripts/actionlint.sh @@ -0,0 +1,7 @@ +#!/usr/bin/env bash + +set -euo pipefail + +go install github.com/rhysd/actionlint/cmd/actionlint@v1.7.1 + +actionlint From ae35eeb61f6d7b86a13c4e1770ba1e4130a9fd78 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Mon, 1 Jul 2024 23:19:25 +0200 Subject: [PATCH 2/2] check router is closing in requests (#3157) Co-authored-by: Stephen Buttolph --- snow/networking/router/chain_router.go | 67 ++++++++++++ snow/networking/router/chain_router_test.go | 111 ++++++++++++++++++++ 2 files changed, 178 insertions(+) diff --git a/snow/networking/router/chain_router.go b/snow/networking/router/chain_router.go index 27bf891ab4f9..6af0984afc3f 100644 --- a/snow/networking/router/chain_router.go +++ b/snow/networking/router/chain_router.go @@ -31,6 +31,7 @@ import ( var ( errUnknownChain = errors.New("received message for unknown chain") errUnallowedNode = errors.New("received message from non-allowed node") + errClosing = errors.New("router is closing") _ Router = (*ChainRouter)(nil) _ benchlist.Benchable = (*ChainRouter)(nil) @@ -63,6 +64,7 @@ type ChainRouter struct { clock mockable.Clock log logging.Logger lock sync.Mutex + closing bool chainHandlers map[ids.ID]handler.Handler // It is only safe to call [RegisterResponse] with the router lock held. Any @@ -154,6 +156,18 @@ func (cr *ChainRouter) RegisterRequest( engineType p2p.EngineType, ) { cr.lock.Lock() + if cr.closing { + cr.log.Debug("dropping request", + zap.Stringer("nodeID", nodeID), + zap.Stringer("requestingChainID", requestingChainID), + zap.Stringer("respondingChainID", respondingChainID), + zap.Uint32("requestID", requestID), + zap.Stringer("messageOp", op), + zap.Error(errClosing), + ) + cr.lock.Unlock() + return + } // When we receive a response message type (Chits, Put, Accepted, etc.) // we validate that we actually sent the corresponding request. // Give this request a unique ID so we can do that validation. @@ -244,6 +258,17 @@ func (cr *ChainRouter) HandleInbound(ctx context.Context, msg message.InboundMes cr.lock.Lock() defer cr.lock.Unlock() + if cr.closing { + cr.log.Debug("dropping message", + zap.Stringer("messageOp", op), + zap.Stringer("nodeID", nodeID), + zap.Stringer("chainID", destinationChainID), + zap.Error(errClosing), + ) + msg.OnFinishedHandling() + return + } + // Get the chain, if it exists chain, exists := cr.chainHandlers[destinationChainID] if !exists { @@ -356,6 +381,7 @@ func (cr *ChainRouter) Shutdown(ctx context.Context) { cr.lock.Lock() prevChains := cr.chainHandlers cr.chainHandlers = map[ids.ID]handler.Handler{} + cr.closing = true cr.lock.Unlock() for _, chain := range prevChains { @@ -388,6 +414,13 @@ func (cr *ChainRouter) AddChain(ctx context.Context, chain handler.Handler) { defer cr.lock.Unlock() chainID := chain.Context().ChainID + if cr.closing { + cr.log.Debug("dropping add chain request", + zap.Stringer("chainID", chainID), + zap.Error(errClosing), + ) + return + } cr.log.Debug("registering chain with chain router", zap.Stringer("chainID", chainID), ) @@ -446,6 +479,14 @@ func (cr *ChainRouter) Connected(nodeID ids.NodeID, nodeVersion *version.Applica cr.lock.Lock() defer cr.lock.Unlock() + if cr.closing { + cr.log.Debug("dropping connected message", + zap.Stringer("nodeID", nodeID), + zap.Error(errClosing), + ) + return + } + connectedPeer, exists := cr.peers[nodeID] if !exists { connectedPeer = &peer{ @@ -493,6 +534,14 @@ func (cr *ChainRouter) Disconnected(nodeID ids.NodeID) { cr.lock.Lock() defer cr.lock.Unlock() + if cr.closing { + cr.log.Debug("dropping disconnected message", + zap.Stringer("nodeID", nodeID), + zap.Error(errClosing), + ) + return + } + peer := cr.peers[nodeID] delete(cr.peers, nodeID) if _, benched := cr.benched[nodeID]; benched { @@ -522,6 +571,15 @@ func (cr *ChainRouter) Benched(chainID ids.ID, nodeID ids.NodeID) { cr.lock.Lock() defer cr.lock.Unlock() + if cr.closing { + cr.log.Debug("dropping benched message", + zap.Stringer("nodeID", nodeID), + zap.Stringer("chainID", chainID), + zap.Error(errClosing), + ) + return + } + benchedChains, exists := cr.benched[nodeID] benchedChains.Add(chainID) cr.benched[nodeID] = benchedChains @@ -554,6 +612,15 @@ func (cr *ChainRouter) Unbenched(chainID ids.ID, nodeID ids.NodeID) { cr.lock.Lock() defer cr.lock.Unlock() + if cr.closing { + cr.log.Debug("dropping unbenched message", + zap.Stringer("nodeID", nodeID), + zap.Stringer("chainID", chainID), + zap.Error(errClosing), + ) + return + } + benchedChains := cr.benched[nodeID] benchedChains.Remove(chainID) if benchedChains.Len() != 0 { diff --git a/snow/networking/router/chain_router_test.go b/snow/networking/router/chain_router_test.go index 19b889cd2d94..9eaae3071e15 100644 --- a/snow/networking/router/chain_router_test.go +++ b/snow/networking/router/chain_router_test.go @@ -191,6 +191,117 @@ func TestShutdown(t *testing.T) { require.Less(shutdownDuration, 250*time.Millisecond) } +func TestConnectedAfterShutdownErrorLogRegression(t *testing.T) { + require := require.New(t) + + snowCtx := snowtest.Context(t, snowtest.PChainID) + chainCtx := snowtest.ConsensusContext(snowCtx) + + chainRouter := ChainRouter{} + require.NoError(chainRouter.Initialize( + ids.EmptyNodeID, + logging.NoWarn{}, // If an error log is emitted, the test will fail + nil, + time.Second, + set.Set[ids.ID]{}, + true, + set.Set[ids.ID]{}, + nil, + HealthConfig{}, + prometheus.NewRegistry(), + )) + + resourceTracker, err := tracker.NewResourceTracker( + prometheus.NewRegistry(), + resource.NoUsage, + meter.ContinuousFactory{}, + time.Second, + ) + require.NoError(err) + + p2pTracker, err := p2p.NewPeerTracker( + logging.NoLog{}, + "", + prometheus.NewRegistry(), + nil, + version.CurrentApp, + ) + require.NoError(err) + + h, err := handler.New( + chainCtx, + nil, + nil, + time.Second, + testThreadPoolSize, + resourceTracker, + validators.UnhandledSubnetConnector, + subnets.New(chainCtx.NodeID, subnets.Config{}), + commontracker.NewPeers(), + p2pTracker, + prometheus.NewRegistry(), + ) + require.NoError(err) + + engine := common.EngineTest{ + T: t, + StartF: func(context.Context, uint32) error { + return nil + }, + ContextF: func() *snow.ConsensusContext { + return chainCtx + }, + HaltF: func(context.Context) {}, + ShutdownF: func(context.Context) error { + return nil + }, + ConnectedF: func(context.Context, ids.NodeID, *version.Application) error { + return nil + }, + } + engine.Default(true) + engine.CantGossip = false + + bootstrapper := &common.BootstrapperTest{ + EngineTest: engine, + CantClear: true, + } + + h.SetEngineManager(&handler.EngineManager{ + Avalanche: &handler.Engine{ + StateSyncer: nil, + Bootstrapper: bootstrapper, + Consensus: &engine, + }, + Snowman: &handler.Engine{ + StateSyncer: nil, + Bootstrapper: bootstrapper, + Consensus: &engine, + }, + }) + chainCtx.State.Set(snow.EngineState{ + Type: engineType, + State: snow.NormalOp, // assumed bootstrapping is done + }) + + chainRouter.AddChain(context.Background(), h) + + h.Start(context.Background(), false) + + chainRouter.Shutdown(context.Background()) + + shutdownDuration, err := h.AwaitStopped(context.Background()) + require.NoError(err) + require.GreaterOrEqual(shutdownDuration, time.Duration(0)) + + // Calling connected after shutdown should result in an error log. + chainRouter.Connected( + ids.GenerateTestNodeID(), + version.CurrentApp, + ids.GenerateTestID(), + ) +} + func TestShutdownTimesOut(t *testing.T) { require := require.New(t)