From 842669dd3a12d1927e2799e20f2b24a52b0ba7d8 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 25 Mar 2022 14:22:29 +0100 Subject: [PATCH 1/3] travis+github: move from Travis to GitHub Actions --- .github/workflows/main.yml | 89 ++++++++++++++++++++++++++++++++++++++ .travis.yml | 16 ------- 2 files changed, 89 insertions(+), 16 deletions(-) create mode 100644 .github/workflows/main.yml delete mode 100644 .travis.yml diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml new file mode 100644 index 0000000..2f6ab30 --- /dev/null +++ b/.github/workflows/main.yml @@ -0,0 +1,89 @@ +name: CI + +on: + push: + branches: + - "master" + pull_request: + branches: + - "*" + +defaults: + run: + shell: bash + +env: + # go needs absolute directories, using the $HOME variable doesn't work here. + GOCACHE: /home/runner/work/go/pkg/build + GOPATH: /home/runner/work/go + + # If you change this value, please change it in the following files as well: + # /Dockerfile + GO_VERSION: 1.17.8 + +jobs: + ######################## + # lint code + ######################## + lint: + name: lint code + runs-on: ubuntu-latest + steps: + - name: git checkout + uses: actions/checkout@v2 + with: + fetch-depth: 0 + + - name: go cache + uses: actions/cache@v1 + with: + path: /home/runner/work/go + key: subasta-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}-${{ hashFiles('**/go.sum') }} + restore-keys: | + aperture-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}-${{ hashFiles('**/go.sum') }} + aperture-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}- + aperture-${{ runner.os }}-go-${{ env.GO_VERSION }}- + aperture-${{ runner.os }}-go- + + - name: setup go ${{ env.GO_VERSION }} + uses: actions/setup-go@v2 + with: + go-version: '~${{ env.GO_VERSION }}' + + - name: lint + run: make lint + + ######################## + # run unit tests + ######################## + unit-test: + name: run unit tests + runs-on: ubuntu-latest + strategy: + # Allow other tests in the matrix to continue if one fails. + fail-fast: false + matrix: + unit_type: + - unit-race + steps: + - name: git checkout + uses: actions/checkout@v2 + + - name: go cache + uses: actions/cache@v1 + with: + path: /home/runner/work/go + key: subasta-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}-${{ hashFiles('**/go.sum') }} + restore-keys: | + aperture-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}-${{ hashFiles('**/go.sum') }} + aperture-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}- + aperture-${{ runner.os }}-go-${{ env.GO_VERSION }}- + aperture-${{ runner.os }}-go- + + - name: setup go ${{ env.GO_VERSION }} + uses: actions/setup-go@v2 + with: + go-version: '~${{ env.GO_VERSION }}' + + - name: run ${{ matrix.unit_type }} + run: make ${{ matrix.unit_type }} diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 30c6f49..0000000 --- a/.travis.yml +++ /dev/null @@ -1,16 +0,0 @@ -language: go - -go: - - "1.16.x" - -cache: - directories: - - $GOCACHE - - $GOPATH/pkg/mod - -env: - global: - - GOCACHE=$HOME/.go-build - -script: - - make travis-race From d86e49706fe17fd684ebc4e72350ff82235de225 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 25 Mar 2022 14:22:31 +0100 Subject: [PATCH 2/3] aperture: only register prometheus metrics when enabled To avoid running into an issue in the race unit test with the Prometheus histogram metrics that aren't concurrency safe, we don't register any of them if Prometheus isn't enabled in the first place. This shouldn't be an issue in production, since we don't start multiple instances of Aperture _within the same process_ at the same time. --- aperture.go | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/aperture.go b/aperture.go index e06641b..e885936 100644 --- a/aperture.go +++ b/aperture.go @@ -680,22 +680,27 @@ func createProxy(cfg *Config, challenger *LndChallenger, func createHashMailServer(cfg *Config) ([]proxy.LocalService, func(), error) { var localServices []proxy.LocalService - // Before we register both servers, we'll also ensure that the - // collector will export latency metrics for the histogram. - grpc_prometheus.EnableHandlingTimeHistogram() - serverOpts := []grpc.ServerOption{ - grpc.ChainUnaryInterceptor( - grpc_prometheus.UnaryServerInterceptor, - ), - grpc.ChainStreamInterceptor( - grpc_prometheus.StreamServerInterceptor, - ), grpc.KeepaliveEnforcementPolicy(keepalive.EnforcementPolicy{ MinTime: time.Minute, }), } + // Before we register both servers, we'll also ensure that the collector + // will export latency metrics for the histogram. + if cfg.Prometheus != nil && cfg.Prometheus.Enabled { + grpc_prometheus.EnableHandlingTimeHistogram() + serverOpts = append( + serverOpts, + grpc.ChainUnaryInterceptor( + grpc_prometheus.UnaryServerInterceptor, + ), + grpc.ChainStreamInterceptor( + grpc_prometheus.StreamServerInterceptor, + ), + ) + } + // Create a gRPC server for the hashmail server. hashMailServer := newHashMailServer(hashMailServerConfig{ msgRate: cfg.HashMail.MessageRate, @@ -710,7 +715,9 @@ func createHashMailServer(cfg *Config) ([]proxy.LocalService, func(), error) { ) // Export the gRPC information for the public gRPC server. - grpc_prometheus.Register(hashMailGRPC) + if cfg.Prometheus != nil && cfg.Prometheus.Enabled { + grpc_prometheus.Register(hashMailGRPC) + } // And a REST proxy for it as well. // The default JSON marshaler of the REST proxy only sets OrigName to From 2475dd1a8b200acdf65a5145960c821aa20f125a Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 25 Mar 2022 14:22:32 +0100 Subject: [PATCH 3/3] github+Docker: bump and fix go version to 1.16.9 There's a race condition in the h2_bundle.go of go 1.16.10 and later. The issue https://github.com/golang/go/issues/51799 mentions that this might be fixed in go 1.19, so we'll need to wait for that. We make sure we build our docker images with go 1.16.9 to not run into the issue in the wild. --- .github/workflows/main.yml | 11 ++++++++--- Dockerfile | 6 +++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 2f6ab30..1408800 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -19,7 +19,12 @@ env: # If you change this value, please change it in the following files as well: # /Dockerfile - GO_VERSION: 1.17.8 + # + # Don't bump this until go 1.19 is out (which should include a fix for + # https://github.com/golang/go/issues/51799). There was a race condition + # introduced with go 1.16.10 that causes the unit tests to fail (could also + # happen in production). + GO_VERSION: 1.16.9 jobs: ######################## @@ -48,7 +53,7 @@ jobs: - name: setup go ${{ env.GO_VERSION }} uses: actions/setup-go@v2 with: - go-version: '~${{ env.GO_VERSION }}' + go-version: '${{ env.GO_VERSION }}' - name: lint run: make lint @@ -83,7 +88,7 @@ jobs: - name: setup go ${{ env.GO_VERSION }} uses: actions/setup-go@v2 with: - go-version: '~${{ env.GO_VERSION }}' + go-version: '${{ env.GO_VERSION }}' - name: run ${{ matrix.unit_type }} run: make ${{ matrix.unit_type }} diff --git a/Dockerfile b/Dockerfile index c796429..ba65a6c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,8 @@ -FROM golang:1.15.5-alpine as builder +# Don't bump this until go 1.19 is out (which should include a fix for +# https://github.com/golang/go/issues/51799). There was a race condition +# introduced with go 1.16.10 that causes the unit tests to fail (could also +# happen in production). +FROM golang:1.16.9-alpine as builder # Force Go to use the cgo based DNS resolver. This is required to ensure DNS # queries required to connect to linked containers succeed.