From ccea995cbe469283857e4e3a83effdb82f6fa0f0 Mon Sep 17 00:00:00 2001 From: avifenesh Date: Fri, 25 Oct 2024 21:48:43 +0000 Subject: [PATCH] Fix version comparisons and update engine versions in workflows --- .github/json_matrices/engine-matrix.json | 4 +- .github/workflows/go.yml | 7 +-- .github/workflows/install-valkey/action.yml | 27 +++++++-- .github/workflows/java-cd.yml | 2 +- .github/workflows/nightly.yml | 2 +- .github/workflows/node.yml | 2 +- .github/workflows/npm-cd.yml | 4 +- .github/workflows/pypi-cd.yml | 2 +- .github/workflows/python.yml | 49 +++-------------- node/tests/GlideClient.test.ts | 12 +--- node/tests/GlideClusterClient.test.ts | 16 ++++++ node/tests/TestUtilities.ts | 61 ++++++--------------- utils/TestUtils.ts | 2 +- 13 files changed, 76 insertions(+), 114 deletions(-) diff --git a/.github/json_matrices/engine-matrix.json b/.github/json_matrices/engine-matrix.json index 149f6f1080..06a8a27fd9 100644 --- a/.github/json_matrices/engine-matrix.json +++ b/.github/json_matrices/engine-matrix.json @@ -1,12 +1,12 @@ [ { "type": "valkey", - "version": "8.0.0", + "version": "8.0", "run": "always" }, { "type": "valkey", - "version": "7.2.5" + "version": "7.2" }, { "type": "redis", diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 4a5d11db46..41a9ec2bca 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -140,7 +140,7 @@ jobs: with: cargo-toml-folder: go github-token: ${{ secrets.GITHUB_TOKEN }} - + - name: Set up Go ${{ matrix.go }} uses: actions/setup-go@v5 with: @@ -155,9 +155,7 @@ jobs: - name: Install and run linters working-directory: go run: | - sudo apt install golang-goprotobuf-dev - LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$GITHUB_WORKSPACE/go/target/release/deps/ - make install-dev-tools install-build-tools generate-protobuf lint-ci + make install-dev-tools install-build-tools build lint-ci get-containers: runs-on: ubuntu-latest @@ -197,7 +195,6 @@ jobs: git config --global --add safe.directory "$GITHUB_WORKSPACE" echo IMAGE=amazonlinux:latest | sed -r 's/:/-/g' >> $GITHUB_ENV # Replace `:` in the variable otherwise it can't be used in `upload-artifact` - - uses: actions/checkout@v4 with: submodules: recursive diff --git a/.github/workflows/install-valkey/action.yml b/.github/workflows/install-valkey/action.yml index 36da82f93b..97c0b47c4e 100644 --- a/.github/workflows/install-valkey/action.yml +++ b/.github/workflows/install-valkey/action.yml @@ -19,7 +19,7 @@ inputs: env: CARGO_TERM_COLOR: always - VALKEY_MIN_VERSION: "7.2.5" + VALKEY_MIN_VERSION: "7.2" runs: using: "composite" @@ -27,8 +27,9 @@ runs: steps: - name: Cache Valkey - # TODO: remove the musl ARM64 limitation when https://github.com/actions/runner/issues/801 is resolved - if: ${{ inputs.target != 'aarch64-unknown-linux-musl' }} + # TODO: self-hosted runners are actually cloning the repo, using the cache from the previous run + # will not work as expected. We need to find a way to cache the valkey repo on the runner itself. + if: ${{!contains(inputs.target, 'aarch64') }} uses: actions/cache@v4 id: cache-valkey with: @@ -36,8 +37,26 @@ runs: ~/valkey key: valkey-${{ inputs.engine-version }}-${{ inputs.target }} + - name: Build Valkey for ARM + shell: bash + working-directory: ~ + run: | + echo "Building valkey ${{ inputs.engine-version }}" + # check if the valkey repo is already cloned + if [[ ! -d valkey ]]; then + git clone https://github.com/valkey-io/valkey.git + fi + cd valkey + make clean + # check if the branch=version is already checked out + if [[ $(git branch --show-current) != ${{ inputs.engine-version }} ]]; then + sudo rm -rf /usr/local/bin/redis-* /usr/local/bin/valkey-* ./valkey-* ./redis-* ./dump.rdb + git checkout ${{ inputs.engine-version }} + make BUILD_TLS=yes + fi + - name: Build Valkey - if: ${{ steps.cache-valkey.outputs.cache-hit != 'true' }} + if: ${{ steps.cache-valkey.outputs.cache-hit != 'true' && !contains(inputs.target, 'aarch64') }} shell: bash run: | echo "Building valkey ${{ inputs.engine-version }}" diff --git a/.github/workflows/java-cd.yml b/.github/workflows/java-cd.yml index 422aef97be..6f21a2a517 100644 --- a/.github/workflows/java-cd.yml +++ b/.github/workflows/java-cd.yml @@ -243,7 +243,7 @@ jobs: uses: ./.github/workflows/install-shared-dependencies with: os: ${{ matrix.host.OS }} - engine-version: "7.2.5" + engine-version: "7.2" target: ${{ matrix.host.TARGET }} github-token: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 21838c1366..ff58cd8555 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -34,7 +34,7 @@ on: default: false schedule: # running tests by cron is disabled on forks by the condition defined for each job - - cron: "0 3 * * *" # Runs at 03:00 (3 AM) UTC every day + - cron: "0 3 * * *" # Runs at 03:00 (3 AM) UTC every day concurrency: group: nightly-${{ github.head_ref || github.ref }}-${{ toJson(inputs) }} diff --git a/.github/workflows/node.yml b/.github/workflows/node.yml index 0f726ea7fe..51c70823c3 100644 --- a/.github/workflows/node.yml +++ b/.github/workflows/node.yml @@ -44,7 +44,7 @@ on: workflow_call: concurrency: - group: node-cd-${{ github.head_ref || github.ref }}-${{ toJson(inputs) }} + group: node-${{ github.head_ref || github.ref }}-${{ toJson(inputs) }} cancel-in-progress: true env: diff --git a/.github/workflows/npm-cd.yml b/.github/workflows/npm-cd.yml index 17d7827afa..936154fb7a 100644 --- a/.github/workflows/npm-cd.yml +++ b/.github/workflows/npm-cd.yml @@ -154,7 +154,7 @@ jobs: npm_scope: ${{ vars.NPM_SCOPE }} publish: "true" github-token: ${{ secrets.GITHUB_TOKEN }} - engine-version: "7.2.5" + engine-version: "7.2" - name: Check if RC and set a distribution tag for the package shell: bash @@ -258,7 +258,7 @@ jobs: os: ubuntu target: "x86_64-unknown-linux-gnu" github-token: ${{ secrets.GITHUB_TOKEN }} - engine-version: "7.2.5" + engine-version: "7.2" - name: Check if RC and set a distribution tag for the package shell: bash diff --git a/.github/workflows/pypi-cd.yml b/.github/workflows/pypi-cd.yml index 3a0b423fa0..eebf726fde 100644 --- a/.github/workflows/pypi-cd.yml +++ b/.github/workflows/pypi-cd.yml @@ -141,7 +141,7 @@ jobs: target: ${{ matrix.build.TARGET }} publish: "true" github-token: ${{ secrets.GITHUB_TOKEN }} - engine-version: "7.2.5" + engine-version: "7.2" - name: Include protobuf files in the package working-directory: ./python diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index ca2086f105..6c50d27466 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -301,39 +301,12 @@ jobs: utils/clusters/** benchmarks/results/** - start-self-hosted-runner: - if: github.event.pull_request.head.repo.owner.login == 'valkey-io' - runs-on: ubuntu-latest - environment: AWS_ACTIONS - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Start self hosted EC2 runner - uses: ./.github/workflows/start-self-hosted-runner - with: - role-to-assume: ${{ secrets.ROLE_TO_ASSUME }} - aws-region: ${{ secrets.AWS_REGION }} - ec2-instance-id: ${{ secrets.AWS_EC2_INSTANCE_ID }} - test-modules: - needs: [start-self-hosted-runner, get-matrices] + if: (github.repository_owner == 'valkey-io' && github.event_name == 'workflow_dispatch') || github.event.pull_request.head.repo.owner.login == 'valkey-io' + environment: AWS_ACTIONS name: Running Module Tests - runs-on: ${{ matrix.host.RUNNER }} - timeout-minutes: 35 - strategy: - fail-fast: false - matrix: - engine: ${{ fromJson(needs.load-engine-matrix.outputs.matrix) }} - python: - - "3.12" - host: - - { - OS: "ubuntu", - NAMED_OS: "linux", - RUNNER: ["self-hosted", "Linux", "ARM64"], - TARGET: "aarch64-unknown-linux-gnu", - } + runs-on: [self-hosted, linux, ARM64] + timeout-minutes: 15 steps: - name: Setup self-hosted runner access @@ -344,19 +317,12 @@ jobs: with: submodules: recursive - - name: Setup Python for self-hosted Ubuntu runners - run: | - sudo apt update -y - sudo apt upgrade -y - sudo apt install python3 python3-venv python3-pip -y - - name: Build Python wrapper uses: ./.github/workflows/build-python-wrapper with: - os: ${{ matrix.host.OS }} - target: ${{ matrix.host.TARGET }} + os: ubuntu + target: aarch64-unknown-linux-gnu github-token: ${{ secrets.GITHUB_TOKEN }} - engine-version: ${{ matrix.engine.version }} - name: Test with pytest working-directory: ./python @@ -364,12 +330,11 @@ jobs: source .env/bin/activate cd python/tests/ pytest --asyncio-mode=auto --tls --cluster-endpoints=${{ secrets.MEMDB_MODULES_ENDPOINT }} -k server_modules --html=pytest_report.html --self-contained-html - - name: Upload test reports if: always() continue-on-error: true uses: actions/upload-artifact@v4 with: - name: modules-report-python-${{ matrix.python }}-${{ matrix.engine.type }}-${{ matrix.engine.version }}-${{ matrix.host.RUNNER }} + name: modules-test-report path: | python/python/tests/pytest_report.html diff --git a/node/tests/GlideClient.test.ts b/node/tests/GlideClient.test.ts index 8fb0efc55a..5eb79e87fc 100644 --- a/node/tests/GlideClient.test.ts +++ b/node/tests/GlideClient.test.ts @@ -286,10 +286,7 @@ describe("GlideClient", () => { getClientConfigurationOption(cluster.getAddresses(), protocol), ); const transaction = new Transaction(); - const expectedRes = await encodedTransactionTest( - transaction, - cluster.getVersion(), - ); + const expectedRes = await encodedTransactionTest(transaction); transaction.select(0); const result = await client.exec(transaction, { decoder: Decoder.Bytes, @@ -308,10 +305,7 @@ describe("GlideClient", () => { getClientConfigurationOption(cluster.getAddresses(), protocol), ); const bytesTransaction = new Transaction(); - const expectedBytesRes = await DumpAndRestoreTest( - bytesTransaction, - Buffer.from("value"), - ); + const expectedBytesRes = await DumpAndRestoreTest(bytesTransaction); bytesTransaction.select(0); const result = await client.exec(bytesTransaction, { decoder: Decoder.Bytes, @@ -321,7 +315,7 @@ describe("GlideClient", () => { validateTransactionResponse(result, expectedBytesRes); const stringTransaction = new Transaction(); - await DumpAndRestoreTest(stringTransaction, "value"); + await DumpAndRestoreTest(stringTransaction); stringTransaction.select(0); // Since DUMP gets binary results, we cannot use the string decoder here, so we expected to get an error. diff --git a/node/tests/GlideClusterClient.test.ts b/node/tests/GlideClusterClient.test.ts index a90f3def7b..0d9a3498e5 100644 --- a/node/tests/GlideClusterClient.test.ts +++ b/node/tests/GlideClusterClient.test.ts @@ -369,6 +369,21 @@ describe("GlideClusterClient", () => { const client = await GlideClusterClient.createClient( getClientConfigurationOption(cluster.getAddresses(), protocol), ); + const lmpopArr = []; + + if (!cluster.checkIfServerVersionLessThan("7.0.0")) { + lmpopArr.push( + client.lmpop(["abc", "def"], ListDirection.LEFT, { + count: 1, + }), + ); + lmpopArr.push( + client.blmpop(["abc", "def"], ListDirection.RIGHT, 0.1, { + count: 1, + }), + ); + } + const promises: Promise[] = [ client.blpop(["abc", "zxy", "lkn"], 0.1), client.rename("abc", "zxy"), @@ -389,6 +404,7 @@ describe("GlideClusterClient", () => { client.sdiffstore("abc", ["zxy", "lkn"]), client.sortStore("abc", "zyx"), client.sortStore("abc", "zyx", { isAlpha: true }), + ...lmpopArr, client.bzpopmax(["abc", "def"], 0.5), client.bzpopmin(["abc", "def"], 0.5), client.xread({ abc: "0-0", zxy: "0-0", lkn: "0-0" }), diff --git a/node/tests/TestUtilities.ts b/node/tests/TestUtilities.ts index 54792afc8b..f846589d5e 100644 --- a/node/tests/TestUtilities.ts +++ b/node/tests/TestUtilities.ts @@ -607,51 +607,25 @@ export async function encodableTransactionTest( */ export async function encodedTransactionTest( baseTransaction: Transaction | ClusterTransaction, - version: string, ): Promise<[string, GlideReturnType][]> { - const key1 = "{key}" + uuidv4(); // string - const key2 = "{key}" + uuidv4(); // string - const key = "dumpKey"; - let dumpResult: Buffer; - - if (gte(version, "7.0.0")) { - dumpResult = Buffer.from([ - 0, 5, 118, 97, 108, 117, 101, 11, 0, 232, 41, 124, 75, 60, 53, 114, - 231, - ]); - } else { - dumpResult = Buffer.from([118, 97, 108, 117, 101]); - } - - const value = "value"; + const key = "{key}-" + uuidv4(); // string + const value = uuidv4(); const valueEncoded = Buffer.from(value); // array of tuples - first element is test name/description, second - expected return value const responseData: [string, GlideReturnType][] = []; - baseTransaction.set(key1, value); - responseData.push(["set(key1, value)", "OK"]); - baseTransaction.set(key2, value); - responseData.push(["set(key2, value)", "OK"]); - baseTransaction.get(key1); - responseData.push(["get(key1)", valueEncoded]); - baseTransaction.get(key2); - responseData.push(["get(key2)", valueEncoded]); - baseTransaction.set(key, value); responseData.push(["set(key, value)", "OK"]); + baseTransaction.get(key); + responseData.push(["get(key)", value]); baseTransaction.customCommand(["DUMP", key]); - responseData.push(['customCommand(["DUMP", key])', dumpResult]); + responseData.push(['customCommand(["DUMP", key])', valueEncoded]); baseTransaction.del([key]); responseData.push(["del(key)", 1]); + baseTransaction.customCommand(["RESTORE", key, "0", valueEncoded]); + responseData.push(['customCommand(["RESTORE", key, "0", payload])', "OK"]); baseTransaction.get(key); - responseData.push(["get(key)", null]); - baseTransaction.customCommand(["RESTORE", key, "0", dumpResult]); - responseData.push([ - 'customCommand(["RESTORE", key, "0", dumpResult])', - "OK", - ]); - baseTransaction.get(key); - responseData.push(["get(key)", valueEncoded]); + responseData.push(["get(key)", value]); return responseData; } @@ -664,31 +638,28 @@ export async function encodedTransactionTest( */ export async function DumpAndRestoreTest( baseTransaction: Transaction, - valueResponse: GlideString, ): Promise<[string, GlideReturnType][]> { - const key = "dumpKey"; - const dumpResult = Buffer.from([ - 0, 5, 118, 97, 108, 117, 101, 11, 0, 232, 41, 124, 75, 60, 53, 114, 231, - ]); - const value = "value"; + const key = "{key}-" + uuidv4(); // string + const buffKey = Buffer.from(key); + const value = uuidv4(); // array of tuples - first element is test name/description, second - expected return value const responseData: [string, GlideReturnType][] = []; baseTransaction.set(key, value); responseData.push(["set(key, value)", "OK"]); baseTransaction.customCommand(["DUMP", key]); - responseData.push(['customCommand(["DUMP", key])', dumpResult]); + responseData.push(['customCommand(["DUMP", key])', buffKey]); baseTransaction.del([key]); responseData.push(["del(key)", 1]); baseTransaction.get(key); responseData.push(["get(key)", null]); - baseTransaction.customCommand(["RESTORE", key, "0", dumpResult]); + baseTransaction.customCommand(["RESTORE", key, "0", buffKey]); responseData.push([ 'customCommand(["RESTORE", key, "0", dumpResult])', "OK", ]); baseTransaction.get(key); - responseData.push(["get(key)", valueResponse]); + responseData.push(["get(key)", value]); return responseData; } @@ -889,14 +860,14 @@ export async function transactionTest( baseTransaction.lmpop([key24], ListDirection.LEFT); responseData.push([ "lmpop([key22], ListDirection.LEFT)", - { [key24]: [field + "2"] }, + [{ key: key24, value: [field + "2"] }], ]); baseTransaction.lpush(key24, [field + "2"]); responseData.push(["lpush(key22, [2])", 2]); baseTransaction.blmpop([key24], ListDirection.LEFT, 0.1, 1); responseData.push([ "blmpop([key22], ListDirection.LEFT, 0.1, 1)", - { [key24]: [field + "2"] }, + [{ key: key24, value: [field + "2"] }], ]); } diff --git a/utils/TestUtils.ts b/utils/TestUtils.ts index 53ad2098c2..70e8f66fc7 100644 --- a/utils/TestUtils.ts +++ b/utils/TestUtils.ts @@ -120,7 +120,7 @@ export class ValkeyCluster { } public checkIfServerVersionLessThan(minVersion: string): boolean { - return lt(this.version, minVersion); + return lt(minVersion, this.version); } public async close() {