From 0febfbffcab323c0011c7ab0a26da817b94f1108 Mon Sep 17 00:00:00 2001 From: Tit Petric Date: Mon, 23 Dec 2024 16:59:08 +0100 Subject: [PATCH 01/11] GHA updates for 1.23 --- .github/workflows/ci-tests.yml | 2 +- .github/workflows/plugin-compiler-build.yml | 2 +- .github/workflows/release.yml | 22 ++++++++++----------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index a6a209cd13d..f6aa4d6374c 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -94,7 +94,7 @@ jobs: fail-fast: false matrix: redis-version: [7] - go-version: [1.22.x] + go-version: [1.23.x] env: REDIS_IMAGE: redis:${{ matrix.redis-version }} diff --git a/.github/workflows/plugin-compiler-build.yml b/.github/workflows/plugin-compiler-build.yml index aa4939fd603..30d9f22e344 100644 --- a/.github/workflows/plugin-compiler-build.yml +++ b/.github/workflows/plugin-compiler-build.yml @@ -11,7 +11,7 @@ on: - "v*" env: - GOLANG_CROSS: 1.22-bullseye + GOLANG_CROSS: 1.23-bullseye concurrency: group: ${{ github.workflow }}-${{ github.ref }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e611642b870..3032690a57f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -41,9 +41,9 @@ jobs: fail-fast: false matrix: golang_cross: - - 1.22-bullseye + - 1.23-bullseye include: - - golang_cross: 1.22-bullseye + - golang_cross: 1.23-bullseye goreleaser: 'ci/goreleaser/goreleaser.yml' cgo: 1 rpmvers: 'el/7 el/8 el/9 amazon/2 amazon/2023' @@ -127,12 +127,12 @@ jobs: mask-aws-account-id: false - uses: aws-actions/amazon-ecr-login@v2 id: ecr - if: ${{ matrix.golang_cross == '1.22-bullseye' }} + if: ${{ matrix.golang_cross == '1.23-bullseye' }} with: mask-password: 'true' - name: Docker metadata for CI id: ci_metadata_ - if: ${{ matrix.golang_cross == '1.22-bullseye' }} + if: ${{ matrix.golang_cross == '1.23-bullseye' }} uses: docker/metadata-action@v5 with: images: ${{ steps.ecr.outputs.registry }}/tyk @@ -146,7 +146,7 @@ jobs: type=semver,pattern={{major}}.{{minor}},prefix=v type=semver,pattern={{version}},prefix=v - name: push image to CI - if: ${{ matrix.golang_cross == '1.22-bullseye' }} + if: ${{ matrix.golang_cross == '1.23-bullseye' }} uses: docker/build-push-action@v6 with: context: "dist" @@ -163,7 +163,7 @@ jobs: EDITION= - name: Docker metadata for CI ee id: ci_metadata_ee - if: ${{ matrix.golang_cross == '1.22-bullseye' }} + if: ${{ matrix.golang_cross == '1.23-bullseye' }} uses: docker/metadata-action@v5 with: images: ${{ steps.ecr.outputs.registry }}/tyk-ee @@ -177,7 +177,7 @@ jobs: type=semver,pattern={{major}}.{{minor}},prefix=v type=semver,pattern={{version}},prefix=v - name: push image to CI ee - if: ${{ matrix.golang_cross == '1.22-bullseye' }} + if: ${{ matrix.golang_cross == '1.23-bullseye' }} uses: docker/build-push-action@v6 with: context: "dist" @@ -207,7 +207,7 @@ jobs: type=semver,pattern={{version}} labels: "org.opencontainers.image.title=tyk-gateway (distroless) \norg.opencontainers.image.description=Tyk Open Source API Gateway written in Go, supporting REST, GraphQL, TCP and gRPC protocols\norg.opencontainers.image.vendor=tyk.io\norg.opencontainers.image.version=${{ github.ref_name }}\n" - name: push image to prod - if: ${{ matrix.golang_cross == '1.22-bullseye' }} + if: ${{ matrix.golang_cross == '1.23-bullseye' }} uses: docker/build-push-action@v6 with: context: "dist" @@ -236,7 +236,7 @@ jobs: type=semver,pattern={{version}} labels: "org.opencontainers.image.title=tyk-gateway Enterprise Edition (distroless) \norg.opencontainers.image.description=Tyk Open Source API Gateway written in Go, supporting REST, GraphQL, TCP and gRPC protocols\norg.opencontainers.image.vendor=tyk.io\norg.opencontainers.image.version=${{ github.ref_name }}\n" - name: push image to prod ee - if: ${{ matrix.golang_cross == '1.22-bullseye' }} + if: ${{ matrix.golang_cross == '1.23-bullseye' }} uses: docker/build-push-action@v6 with: context: "dist" @@ -253,7 +253,7 @@ jobs: EDITION=-ee - name: save deb uses: actions/upload-artifact@v4 - if: ${{ matrix.golang_cross == '1.22-bullseye' }} + if: ${{ matrix.golang_cross == '1.23-bullseye' }} with: name: deb retention-days: 1 @@ -263,7 +263,7 @@ jobs: !dist/*fips*.deb - name: save rpm uses: actions/upload-artifact@v4 - if: ${{ matrix.golang_cross == '1.22-bullseye' }} + if: ${{ matrix.golang_cross == '1.23-bullseye' }} with: name: rpm retention-days: 1 From 816e1cb1620fd48f0a2cd14b38583ee4238be729 Mon Sep 17 00:00:00 2001 From: Tit Petric Date: Mon, 23 Dec 2024 17:00:01 +0100 Subject: [PATCH 02/11] Clean up dev dockerfile and taskfile --- Dockerfile | 62 ++++++++++------------------------------------------ Taskfile.yml | 2 +- 2 files changed, 12 insertions(+), 52 deletions(-) diff --git a/Dockerfile b/Dockerfile index c094b71ccd2..0b9d631f9f1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,68 +1,28 @@ -FROM debian:bookworm AS assets - -# This Dockerfile facilitates bleeding edge development docker image builds -# directly from source. To build a development image, run `make docker`. -# If you need to tweak the environment for testing, you can override the -# `GO_VERSION` and `PYTHON_VERSION` as docker build arguments. - -ARG GO_VERSION=1.22.6 -ARG PYTHON_VERSION=3.11.6 - -WORKDIR /assets - -RUN apt update && apt install wget -y && \ - wget -q https://dl.google.com/go/go${GO_VERSION}.linux-amd64.tar.gz && \ - wget -q https://www.python.org/ftp/python/${PYTHON_VERSION}/Python-${PYTHON_VERSION}.tar.xz - -FROM debian:bookworm - -ARG GO_VERSION=1.22.6 -ARG PYTHON_VERSION=3.11.6 - -COPY --from=assets /assets/ /tmp/ -WORKDIR /tmp - -# Install Go - -ENV PATH=$PATH:/usr/local/go/bin - -RUN tar -C /usr/local -xzf go${GO_VERSION}.linux-amd64.tar.gz && \ - go version +ARG GO_VERSION=1.23 +FROM golang:${GO_VERSION}-bullseye # Build essentials RUN apt update && apt install build-essential zlib1g-dev libncurses5-dev libgdbm-dev libnss3-dev libssl-dev libsqlite3-dev libreadline-dev libffi-dev curl wget libbz2-dev -y -# Install $PYTHON_VERSION - ## This just installs whatever is is bullseye, makes docker build (fast/small)-(er) RUN apt install python3 -y -## This runs python code slower, but the process finishes quicker -# RUN tar -xf Python-${PYTHON_VERSION}.tar.xz && ls -la && \ -# cd Python-${PYTHON_VERSION}/ && \ -# ./configure --enable-shared && make build_all && \ -# make altinstall && \ -# ldconfig $PWD - -## This runs python code faster, but is expensive to build and runs regression tests -# RUN tar -xf Python-${PYTHON_VERSION}.tar.xz && ls -la && \ -# cd Python-${PYTHON_VERSION}/ && \ -# ./configure --enable-shared --enable-optimizations && make -j 2 && \ -# make altinstall && \ -# ldconfig $PWD - -# Clean up build assets -RUN find /tmp -type f -delete - # Build gateway RUN mkdir /opt/tyk-gateway WORKDIR /opt/tyk-gateway + ADD go.mod go.sum /opt/tyk-gateway/ -RUN go mod download + +RUN --mount=type=cache,mode=0755,target=/go/pkg/mod \ + --mount=type=cache,mode=0755,target=/root/.cache/go-build \ + go mod download + ADD . /opt/tyk-gateway -RUN make build +RUN --mount=type=cache,mode=0755,target=/go/pkg/mod \ + --mount=type=cache,mode=0755,target=/root/.cache/go-build \ + make build COPY tyk.conf.example tyk.conf diff --git a/Taskfile.yml b/Taskfile.yml index 33c8bd1a8a2..8ce0c1e23ca 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -25,7 +25,7 @@ tasks: docker: desc: "build Tyk gateway internal/tyk-gateway" cmds: - - docker build --platform "linux/amd64" --rm -t internal/tyk-gateway . + - docker build --build-arg GO_VERSION="$(go mod edit -json | jq .Go -r)" --platform "linux/amd64" --rm -t internal/tyk-gateway . sources: - go.mod - go.sum From 02341e4630e78aaf0000957a8135a26dd3b0255a Mon Sep 17 00:00:00 2001 From: Tit Petric Date: Mon, 23 Dec 2024 17:01:00 +0100 Subject: [PATCH 03/11] Update plugin compiler default BASE_IMAGE --- ci/images/plugin-compiler/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/images/plugin-compiler/Dockerfile b/ci/images/plugin-compiler/Dockerfile index f045ce5a784..5f0219afa9a 100644 --- a/ci/images/plugin-compiler/Dockerfile +++ b/ci/images/plugin-compiler/Dockerfile @@ -1,4 +1,4 @@ -ARG BASE_IMAGE=tykio/golang-cross:1.22-bullseye +ARG BASE_IMAGE=tykio/golang-cross:1.23-bullseye FROM ${BASE_IMAGE} LABEL description="Image for plugin development" From 9007d2bffb9fb2e8c0af066304bf7dcce45f3ceb Mon Sep 17 00:00:00 2001 From: Tit Petric Date: Mon, 23 Dec 2024 17:02:31 +0100 Subject: [PATCH 04/11] Fix BASE_IMAGE argument --- .github/workflows/plugin-compiler-build.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/plugin-compiler-build.yml b/.github/workflows/plugin-compiler-build.yml index 30d9f22e344..0a51ef68fd1 100644 --- a/.github/workflows/plugin-compiler-build.yml +++ b/.github/workflows/plugin-compiler-build.yml @@ -77,7 +77,7 @@ jobs: labels: ${{ steps.set-metadata.outputs.labels }} tags: ${{ steps.set-metadata.outputs.tags }} build-args: | - BASE-IMAGE=tykio/golang-cross:${{ env.GOLANG_CROSS }} + BASE_IMAGE=tykio/golang-cross:${{ env.GOLANG_CROSS }} GITHUB_SHA=${{ github.sha }} GITHUB_TAG=${{ github.ref_name }} @@ -108,7 +108,7 @@ jobs: labels: ${{ steps.set-metadata-ee.outputs.labels }} tags: ${{ steps.set-metadata-ee.outputs.tags }} build-args: | - BASE-IMAGE=tykio/golang-cross:${{ env.GOLANG_CROSS }} + BASE_IMAGE=tykio/golang-cross:${{ env.GOLANG_CROSS }} GITHUB_SHA=${{ github.sha }} GITHUB_TAG=${{ github.ref_name }} BUILD_TAG=ee From 33dad32dcaf046725db9c37392f7fb668af2d821 Mon Sep 17 00:00:00 2001 From: Tit Petric Date: Mon, 23 Dec 2024 17:02:48 +0100 Subject: [PATCH 05/11] Bump go version required --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 7fafc85b413..603d1ed5a22 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/TykTechnologies/tyk -go 1.22.6 +go 1.23.4 require ( github.com/Jeffail/tunny v0.1.4 From bdc48dce49919cdbf2f47ca299a36ed56e5cc794 Mon Sep 17 00:00:00 2001 From: Tit Petric Date: Mon, 23 Dec 2024 20:08:35 +0100 Subject: [PATCH 06/11] It would seem TestReloadGoroutineLeak detects leaks --- gateway/gateway_test.go | 52 +++++++++---- internal/debug2/goroutine.go | 124 ++++++++++++++++++++++++++++++ internal/debug2/goroutine_test.go | 102 ++++++++++++++++++++++++ 3 files changed, 263 insertions(+), 15 deletions(-) create mode 100644 internal/debug2/goroutine.go create mode 100644 internal/debug2/goroutine_test.go diff --git a/gateway/gateway_test.go b/gateway/gateway_test.go index e018caa87b0..5fbd66f2b2a 100644 --- a/gateway/gateway_test.go +++ b/gateway/gateway_test.go @@ -26,6 +26,7 @@ import ( "github.com/TykTechnologies/tyk-pump/analytics" "github.com/TykTechnologies/tyk/apidef" "github.com/TykTechnologies/tyk/config" + "github.com/TykTechnologies/tyk/internal/debug2" "github.com/TykTechnologies/tyk/storage" "github.com/TykTechnologies/tyk/test" "github.com/TykTechnologies/tyk/user" @@ -643,30 +644,55 @@ func TestListenPathTykPrefix(t *testing.T) { } func TestReloadGoroutineLeakWithTest(t *testing.T) { - test.Flaky(t) + newRecord := func() *debug2.Record { + result := debug2.NewRecord() + result.SetIgnores([]string{ + "runtime/pprof.writeRuntimeProfile", + "/root/go/pkg/mod/github.com/!tyk!technologies/leakybucket@v0.0.0-20170301023702-71692c943e3c/memorycache/cache.go:69", + "/root/go/pkg/mod/github.com/pmylund/go-cache@v2.1.0+incompatible/cache.go:1079", + "/root/tyk/tyk/gateway/distributed_rate_limiter.go:31", + "/root/tyk/tyk/gateway/redis_signals.go:68", + }) + + return result + } - before := runtime.NumGoroutine() + before := newRecord() ts := StartTest(nil) ts.Close() - time.Sleep(time.Second) - - after := runtime.NumGoroutine() + time.Sleep(100 * time.Millisecond) + runtime.GC() - if before < after { - t.Errorf("Goroutine leak, was: %d, after reload: %d", before, after) - } + final := newRecord().Since(before) + assert.Equal(t, 0, final.Count(), "final count not zero: %s", final) } func TestReloadGoroutineLeakWithCircuitBreaker(t *testing.T) { ts := StartTest(nil) t.Cleanup(ts.Close) + newRecord := func() *debug2.Record { + result := debug2.NewRecord() + result.SetIgnores([]string{ + "runtime/pprof.writeRuntimeProfile", + "/root/tyk/tyk/gateway/reverse_proxy.go:223", + "/root/tyk/tyk/gateway/api_definition.go:1025", + "/root/tyk/tyk/gateway/distributed_rate_limiter.go:31", + "/root/go/pkg/mod/github.com/pmylund/go-cache@v2.1.0+incompatible/cache.go:1079", + "/root/go/pkg/mod/github.com/!tyk!technologies/circuitbreaker@v2.2.2+incompatible/circuitbreaker.go:202", + }) + + return result + } + globalConf := ts.Gw.GetConfig() globalConf.EnableJSVM = false ts.Gw.SetConfig(globalConf) + stage1 := newRecord() + specs := ts.Gw.BuildAndLoadAPI(func(spec *APISpec) { spec.Proxy.ListenPath = "/" UpdateAPIVersion(spec, "v1", func(version *apidef.VersionInfo) { @@ -684,17 +710,13 @@ func TestReloadGoroutineLeakWithCircuitBreaker(t *testing.T) { }) }) - before := runtime.NumGoroutine() - ts.Gw.LoadAPI(specs...) // just doing globalGateway.DoReload() doesn't load anything as BuildAndLoadAPI cleans up folder with API specs time.Sleep(100 * time.Millisecond) + runtime.GC() - after := runtime.NumGoroutine() - - if before < after { - t.Errorf("Goroutine leak, was: %d, after reload: %d", before, after) - } + final := newRecord().Since(stage1) + assert.Equal(t, 0, final.Count(), "final count not zero: %s", final) } func listenProxyProto(ls net.Listener) error { diff --git a/internal/debug2/goroutine.go b/internal/debug2/goroutine.go new file mode 100644 index 00000000000..115975597c1 --- /dev/null +++ b/internal/debug2/goroutine.go @@ -0,0 +1,124 @@ +package debug2 + +import ( + "bytes" + "fmt" + "regexp" + "runtime/pprof" + "strings" +) + +// Record captures goroutine states +type Record struct { + buffer *bytes.Buffer + ignores []string +} + +// NewRecord creates a new Record and populates it with the current goroutine dump. +func NewRecord() *Record { + result := &Record{ + buffer: bytes.NewBuffer([]byte{}), + } + + pprof.Lookup("goroutine").WriteTo(result.buffer, 1) + + result.SetIgnores([]string{ + "runtime/pprof.writeRuntimeProfile", + }) + return result +} + +func (r *Record) SetIgnores(ignores []string) { + r.ignores = ignores +} + +var headerMatchRe = regexp.MustCompile(`^[0-9]+ @ 0x.*`) + +// parseGoroutines parses goroutines from the buffer into a map where each key is a +// goroutine header and the value is its stack trace as a slice of strings. +func (r *Record) parseGoroutines() map[string][]string { + goroutines := make(map[string][]string) + var currentHeader string + var currentStack []string + toDelete := []string{} + lines := strings.Split(r.buffer.String(), "\n") + + for _, line := range lines { + var skip bool + for _, ign := range r.ignores { + if strings.Contains(line, ign) { + skip = true + break + } + } + + if skip { + toDelete = append(toDelete, currentHeader) + } + + if headerMatchRe.MatchString(line) { + // Save the previous goroutine and reset + if currentHeader != "" { + goroutines[currentHeader] = currentStack + } + currentHeader = line + currentStack = []string{line} + } else if currentHeader != "" { + // Add stack trace lines to the current goroutine + currentStack = append(currentStack, line) + } + } + + // Save the last goroutine + if currentHeader != "" { + goroutines[currentHeader] = currentStack + } + + for _, key := range toDelete { + delete(goroutines, key) + } + + return goroutines +} + +// Since compares the current Record with another Record and returns a new Record +// containing only the goroutines found in the current Record but not in the last. +func (r *Record) Since(last *Record) *Record { + currentGoroutines := r.parseGoroutines() + lastGoroutines := last.parseGoroutines() + + diffBuffer := bytes.NewBuffer([]byte{}) + for header, stack := range currentGoroutines { + if _, exists := lastGoroutines[header]; !exists { + diffBuffer.WriteString(header + "\n") + for _, line := range stack { + diffBuffer.WriteString(line + "\n") + } + } + } + + return &Record{ + buffer: diffBuffer, + } +} + +// Count returns the number of unique goroutines in the Record. +func (r *Record) Count() int { + return len(r.parseGoroutines()) +} + +// String implements the fmt.Stringer interface, providing a formatted view +// of the goroutines in the Record. +func (r *Record) String() string { + goroutines := r.parseGoroutines() + var builder strings.Builder + builder.WriteString(fmt.Sprintf("Number of goroutines: %d\n", len(goroutines))) + for header, stack := range goroutines { + builder.WriteString("--- Goroutine ---\n") + builder.WriteString(header + "\n") + for _, line := range stack { + builder.WriteString(line + "\n") + } + } + return builder.String() +} diff --git a/internal/debug2/goroutine_test.go b/internal/debug2/goroutine_test.go new file mode 100644 index 00000000000..aea6be8f361 --- /dev/null +++ b/internal/debug2/goroutine_test.go @@ -0,0 +1,102 @@ +package debug2_test + +import ( + "context" + "fmt" + "runtime" + "sync" + "testing" + "time" + + "github.com/TykTechnologies/tyk/internal/debug2" + "github.com/stretchr/testify/assert" +) + +func TestNewRecordWithGoroutines(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + time.Sleep(100 * time.Millisecond) + + // Capture the initial state of goroutines + initialRecord := debug2.NewRecord() + + // Create and start a new goroutine + go func() { + time.Sleep(100 * time.Millisecond) + }() + go func() { + time.Sleep(100 * time.Millisecond) + }() + + // Capture the state after starting the goroutine + intermediateRecord := debug2.NewRecord() + // t.Log("The intermediate goroutines:\n", intermediateRecord.String()) + + newGoroutines := intermediateRecord.Since(initialRecord) + assert.Equal(t, 2, newGoroutines.Count(), "Expected new goroutines, but found none") + + for { + // Wait for the goroutine to finish + time.Sleep(100 * time.Millisecond) + runtime.GC() + time.Sleep(10 * time.Millisecond) + + // Capture the state after the goroutine has finished + finalRecord := debug2.NewRecord() + remainingGoroutines := finalRecord.Since(initialRecord) + + // Expecting goroutines clear + if remainingGoroutines.Count() == 0 { + break + } + + if ctx.Err() != nil { + break + } + + fmt.Print(remainingGoroutines.String()) + } + + assert.NoError(t, ctx.Err(), "cancelled goroutine leak check after timeout") +} + +func BenchmarkNewRecordWithGoroutines(b *testing.B) { + // Capture the initial state of goroutines + initialRecord := debug2.NewRecord() + + // Create and start a new goroutine + + var wg sync.WaitGroup + wg.Add(b.N) + + var i int + for i = 0; i < b.N; i++ { + go func() { + defer wg.Done() + + time.Sleep(100 * time.Millisecond) + }() + } + + // Capture the state after starting the goroutine + intermediateRecord := debug2.NewRecord() + b.Logf("Started %d goroutines with sleep", b.N) + b.Log("Intermediate Record count: ", intermediateRecord.Count()) + + wg.Wait() + + runtime.GC() + + // Capture the state after the goroutine has finished + finalRecord := debug2.NewRecord() + b.Log("Finished with finalRecord count: ", finalRecord.Count()) + + // Check that the intermediate record contains the new goroutine + newGoroutines := intermediateRecord.Since(initialRecord) + assert.Greater(b, newGoroutines.Count(), 0, "Expected new goroutines, but found none") + + // Check that the final record no longer contains the new goroutine + remainingGoroutines := finalRecord.Since(initialRecord) + assert.Equal(b, 0, remainingGoroutines.Count(), "Expected no new goroutines, but found: "+remainingGoroutines.String()) +} From 15ecb032265c3f8f9aefbeef432fad2636ef7d21 Mon Sep 17 00:00:00 2001 From: Tit Petric Date: Mon, 23 Dec 2024 20:14:53 +0100 Subject: [PATCH 07/11] Fix imports --- internal/debug2/goroutine_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/debug2/goroutine_test.go b/internal/debug2/goroutine_test.go index aea6be8f361..0d55d582247 100644 --- a/internal/debug2/goroutine_test.go +++ b/internal/debug2/goroutine_test.go @@ -8,8 +8,9 @@ import ( "testing" "time" - "github.com/TykTechnologies/tyk/internal/debug2" "github.com/stretchr/testify/assert" + + "github.com/TykTechnologies/tyk/internal/debug2" ) func TestNewRecordWithGoroutines(t *testing.T) { From 61d5173866064da8e733b5f27e0c581c09bf28e5 Mon Sep 17 00:00:00 2001 From: Tit Petric Date: Fri, 27 Dec 2024 10:06:33 +0100 Subject: [PATCH 08/11] last leak assertion --- gateway/gateway_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gateway/gateway_test.go b/gateway/gateway_test.go index 5fbd66f2b2a..c85967de43d 100644 --- a/gateway/gateway_test.go +++ b/gateway/gateway_test.go @@ -21,6 +21,7 @@ import ( "github.com/gorilla/websocket" proxyproto "github.com/pires/go-proxyproto" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" msgpack "gopkg.in/vmihailenco/msgpack.v2" "github.com/TykTechnologies/tyk-pump/analytics" @@ -658,6 +659,7 @@ func TestReloadGoroutineLeakWithTest(t *testing.T) { } before := newRecord() + require.Less(t, before.Count(), 100, "before count over a 100, leak: %s", before) ts := StartTest(nil) ts.Close() From 215507d05eba9349746dd0764f0d6df59d605115 Mon Sep 17 00:00:00 2001 From: Tit Petric Date: Tue, 7 Jan 2025 14:14:49 +0100 Subject: [PATCH 09/11] Divide and conquer --- gateway/gateway_test.go | 80 ---------------------------- tests/system/README.md | 5 ++ tests/system/goroutine_test.go | 97 ++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 80 deletions(-) create mode 100644 tests/system/README.md create mode 100644 tests/system/goroutine_test.go diff --git a/gateway/gateway_test.go b/gateway/gateway_test.go index c85967de43d..e104fccee6c 100644 --- a/gateway/gateway_test.go +++ b/gateway/gateway_test.go @@ -11,7 +11,6 @@ import ( "net/http/httptest" "net/url" "os" - "runtime" "strconv" "strings" "sync/atomic" @@ -21,13 +20,11 @@ import ( "github.com/gorilla/websocket" proxyproto "github.com/pires/go-proxyproto" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" msgpack "gopkg.in/vmihailenco/msgpack.v2" "github.com/TykTechnologies/tyk-pump/analytics" "github.com/TykTechnologies/tyk/apidef" "github.com/TykTechnologies/tyk/config" - "github.com/TykTechnologies/tyk/internal/debug2" "github.com/TykTechnologies/tyk/storage" "github.com/TykTechnologies/tyk/test" "github.com/TykTechnologies/tyk/user" @@ -644,83 +641,6 @@ func TestListenPathTykPrefix(t *testing.T) { }) } -func TestReloadGoroutineLeakWithTest(t *testing.T) { - newRecord := func() *debug2.Record { - result := debug2.NewRecord() - result.SetIgnores([]string{ - "runtime/pprof.writeRuntimeProfile", - "/root/go/pkg/mod/github.com/!tyk!technologies/leakybucket@v0.0.0-20170301023702-71692c943e3c/memorycache/cache.go:69", - "/root/go/pkg/mod/github.com/pmylund/go-cache@v2.1.0+incompatible/cache.go:1079", - "/root/tyk/tyk/gateway/distributed_rate_limiter.go:31", - "/root/tyk/tyk/gateway/redis_signals.go:68", - }) - - return result - } - - before := newRecord() - require.Less(t, before.Count(), 100, "before count over a 100, leak: %s", before) - - ts := StartTest(nil) - ts.Close() - - time.Sleep(100 * time.Millisecond) - runtime.GC() - - final := newRecord().Since(before) - assert.Equal(t, 0, final.Count(), "final count not zero: %s", final) -} - -func TestReloadGoroutineLeakWithCircuitBreaker(t *testing.T) { - ts := StartTest(nil) - t.Cleanup(ts.Close) - - newRecord := func() *debug2.Record { - result := debug2.NewRecord() - result.SetIgnores([]string{ - "runtime/pprof.writeRuntimeProfile", - "/root/tyk/tyk/gateway/reverse_proxy.go:223", - "/root/tyk/tyk/gateway/api_definition.go:1025", - "/root/tyk/tyk/gateway/distributed_rate_limiter.go:31", - "/root/go/pkg/mod/github.com/pmylund/go-cache@v2.1.0+incompatible/cache.go:1079", - "/root/go/pkg/mod/github.com/!tyk!technologies/circuitbreaker@v2.2.2+incompatible/circuitbreaker.go:202", - }) - - return result - } - - globalConf := ts.Gw.GetConfig() - globalConf.EnableJSVM = false - ts.Gw.SetConfig(globalConf) - - stage1 := newRecord() - - specs := ts.Gw.BuildAndLoadAPI(func(spec *APISpec) { - spec.Proxy.ListenPath = "/" - UpdateAPIVersion(spec, "v1", func(version *apidef.VersionInfo) { - version.ExtendedPaths = apidef.ExtendedPathsSet{ - CircuitBreaker: []apidef.CircuitBreakerMeta{ - { - Path: "/", - Method: http.MethodGet, - ThresholdPercent: 0.5, - Samples: 5, - ReturnToServiceAfter: 10, - }, - }, - } - }) - }) - - ts.Gw.LoadAPI(specs...) // just doing globalGateway.DoReload() doesn't load anything as BuildAndLoadAPI cleans up folder with API specs - - time.Sleep(100 * time.Millisecond) - runtime.GC() - - final := newRecord().Since(stage1) - assert.Equal(t, 0, final.Count(), "final count not zero: %s", final) -} - func listenProxyProto(ls net.Listener) error { pl := &proxyproto.Listener{Listener: ls} for { diff --git a/tests/system/README.md b/tests/system/README.md new file mode 100644 index 00000000000..853187c45b9 --- /dev/null +++ b/tests/system/README.md @@ -0,0 +1,5 @@ +# System tests + +These tests are system level and depend on the current code structure +and go toolchain behaviour. They are flaky as code and the go toolchain +changes and drifts from what's asserted here. diff --git a/tests/system/goroutine_test.go b/tests/system/goroutine_test.go new file mode 100644 index 00000000000..5ccfbf7904d --- /dev/null +++ b/tests/system/goroutine_test.go @@ -0,0 +1,97 @@ +package system_test + +import ( + "net/http" + "runtime" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/TykTechnologies/tyk/apidef" + "github.com/TykTechnologies/tyk/gateway" + "github.com/TykTechnologies/tyk/internal/debug2" + "github.com/TykTechnologies/tyk/test" +) + +func TestReloadGoroutineLeakWithTest(t *testing.T) { + test.Flaky(t) + + newRecord := func() *debug2.Record { + result := debug2.NewRecord() + result.SetIgnores([]string{ + "runtime/pprof.writeRuntimeProfile", + "/root/go/pkg/mod/github.com/!tyk!technologies/leakybucket@v0.0.0-20170301023702-71692c943e3c/memorycache/cache.go:69", + "/root/go/pkg/mod/github.com/pmylund/go-cache@v2.1.0+incompatible/cache.go:1079", + "/root/tyk/tyk/gateway/distributed_rate_limiter.go:31", + "/root/tyk/tyk/gateway/redis_signals.go:68", + }) + + return result + } + + before := newRecord() + require.Less(t, before.Count(), 100, "before count over a 100, leak: %s", before) + + ts := gateway.StartTest(nil) + ts.Close() + + time.Sleep(100 * time.Millisecond) + runtime.GC() + + final := newRecord().Since(before) + assert.Equal(t, 0, final.Count(), "final count not zero: %s", final) +} + +func TestReloadGoroutineLeakWithCircuitBreaker(t *testing.T) { + test.Flaky(t) + + ts := gateway.StartTest(nil) + t.Cleanup(ts.Close) + + newRecord := func() *debug2.Record { + result := debug2.NewRecord() + result.SetIgnores([]string{ + "runtime/pprof.writeRuntimeProfile", + "/root/tyk/tyk/gateway/reverse_proxy.go:223", + "/root/tyk/tyk/gateway/api_definition.go:1025", + "/root/tyk/tyk/gateway/distributed_rate_limiter.go:31", + "/root/go/pkg/mod/github.com/pmylund/go-cache@v2.1.0+incompatible/cache.go:1079", + "/root/go/pkg/mod/github.com/!tyk!technologies/circuitbreaker@v2.2.2+incompatible/circuitbreaker.go:202", + }) + + return result + } + + globalConf := ts.Gw.GetConfig() + globalConf.EnableJSVM = false + ts.Gw.SetConfig(globalConf) + + stage1 := newRecord() + + specs := ts.Gw.BuildAndLoadAPI(func(spec *APISpec) { + spec.Proxy.ListenPath = "/" + UpdateAPIVersion(spec, "v1", func(version *apidef.VersionInfo) { + version.ExtendedPaths = apidef.ExtendedPathsSet{ + CircuitBreaker: []apidef.CircuitBreakerMeta{ + { + Path: "/", + Method: http.MethodGet, + ThresholdPercent: 0.5, + Samples: 5, + ReturnToServiceAfter: 10, + }, + }, + } + }) + }) + + ts.Gw.LoadAPI(specs...) // just doing globalGateway.DoReload() doesn't load anything as BuildAndLoadAPI cleans up folder with API specs + + time.Sleep(100 * time.Millisecond) + runtime.GC() + + final := newRecord().Since(stage1) + assert.Equal(t, 0, final.Count(), "final count not zero: %s", final) +} From 52d176e29bc81c0d78cf0c26a0d4f50a5d32ba22 Mon Sep 17 00:00:00 2001 From: Tit Petric Date: Tue, 7 Jan 2025 14:35:42 +0100 Subject: [PATCH 10/11] Fix taskfile structure --- .taskfiles/{deps/Taskfile.yml => deps.yml} | 0 .taskfiles/test.yml | 2 +- Taskfile.yml | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename .taskfiles/{deps/Taskfile.yml => deps.yml} (100%) diff --git a/.taskfiles/deps/Taskfile.yml b/.taskfiles/deps.yml similarity index 100% rename from .taskfiles/deps/Taskfile.yml rename to .taskfiles/deps.yml diff --git a/.taskfiles/test.yml b/.taskfiles/test.yml index d6933c4b372..368babe2f6c 100644 --- a/.taskfiles/test.yml +++ b/.taskfiles/test.yml @@ -2,7 +2,7 @@ version: "3" includes: - deps: ./deps/Taskfile.yml + deps: ./deps.yml services: taskfile: ../docker/services/Taskfile.yml dir: ../docker/services diff --git a/Taskfile.yml b/Taskfile.yml index 8ce0c1e23ca..e20dbe9e4f6 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -3,7 +3,7 @@ version: "3" includes: test: .taskfiles/test.yml - deps: .taskfiles/deps/Taskfile.yml + deps: .taskfiles/deps.yml hooks: .taskfiles/hooks.yml opentelemetry: taskfile: ci/tests/tracing/Taskfile.yml From 87de2c8004892f584e389c1a0477c0b290a1e8ca Mon Sep 17 00:00:00 2001 From: Tit Petric Date: Thu, 9 Jan 2025 09:27:53 +0100 Subject: [PATCH 11/11] Fix system test --- tests/system/goroutine_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/system/goroutine_test.go b/tests/system/goroutine_test.go index 5ccfbf7904d..99527cb7a69 100644 --- a/tests/system/goroutine_test.go +++ b/tests/system/goroutine_test.go @@ -70,9 +70,9 @@ func TestReloadGoroutineLeakWithCircuitBreaker(t *testing.T) { stage1 := newRecord() - specs := ts.Gw.BuildAndLoadAPI(func(spec *APISpec) { + specs := ts.Gw.BuildAndLoadAPI(func(spec *gateway.APISpec) { spec.Proxy.ListenPath = "/" - UpdateAPIVersion(spec, "v1", func(version *apidef.VersionInfo) { + gateway.UpdateAPIVersion(spec, "v1", func(version *apidef.VersionInfo) { version.ExtendedPaths = apidef.ExtendedPathsSet{ CircuitBreaker: []apidef.CircuitBreakerMeta{ {