You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/02/04 12:15:32 UTC

[GitHub] [apisix] Yiyiyimu opened a new pull request #3521: CI: add release check

Yiyiyimu opened a new pull request #3521:
URL: https://github.com/apache/apisix/pull/3521


   Signed-off-by: yiyiyimu <wo...@gmail.com>
   
   ### What this PR does / why we need it:
   fix #3514
   
   So the CI basically do:
   1. Whenever push to branch other than master (for us it's always release branch), CI would be triggered.
   2. Make release, remove everything except tarball, `.travis` and `t`. And untar for test
   3. Reuse CI in ubuntu for test
   4. Upload tarball as an artifact, so we could directly download it in the workflow page [Ref](https://docs.github.com/en/actions/managing-workflow-runs/downloading-workflow-artifacts)
   
   Some problems need suggestions:
   1. Github actions provide [composite](https://docs.github.com/en/actions/creating-actions/creating-a-composite-run-steps-action) run so we could reuse code in other actions. But since it does not support matrix yet, so we could not use it in this situation. I just copied `build.yml` and it's not so satisfying
   2. Using matrix, one tarball would be created and uploaded for each environment. It's not a big deal since they are all the same, but still not satisfying.
   
   **TODO**:
   - [ ] Currently for test, it would run on pull request. It should be changed to run on push before merge.
   
   ### Pre-submission checklist:
   
   * [x] Did you explain what problem does this PR solve? Or what new features have been added?
   * [ ] Have you added corresponding test cases?
   * [ ] Have you modified the corresponding document?
   * [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu commented on a change in pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on a change in pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#discussion_r571750805



##########
File path: .github/workflows/build.yml
##########
@@ -108,3 +129,10 @@ jobs:
 
       - name: Linux Script
         run: sudo ./.travis/${{ matrix.os_name }}_runner.sh script
+
+      - name: Publish Artifact
+        if: ${{ startsWith(github.ref, 'refs/heads/v') && endsWith(matrix.os_name, 'linux_openresty') }}

Review comment:
       Fixed. Thanks!




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu commented on a change in pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on a change in pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#discussion_r571620042



##########
File path: .github/workflows/build.yml
##########
@@ -53,6 +53,30 @@ jobs:
         with:
           submodules: recursive
 
+      - name: Extract branch name
+        id: branch_env
+        shell: bash
+        run: |
+          export VERSION=${GITHUB_REF##*/}
+          echo "##[set-output name=version;]$(echo ${VERSION:1})"

Review comment:
       Like currently when we run on PR, the last word of `GITHUB_REF` would be `merge`. When it runs when push, it would become the branch name. And when it runs on release branch, branch like `v1.5` would become `1.5`.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu removed a comment on pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
Yiyiyimu removed a comment on pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#issuecomment-775004549


   For record unstable test before re-run CI:
   ```
   #   Failed test 'TEST 7: check plugin configuration updating - status code ok'
   #   at /home/runner/work/apisix/apisix/test-nginx/lib/Test/Nginx/Socket.pm line 948.
   #          got: '404'
   #     expected: '200'
   
   #   Failed test 'TEST 7: check plugin configuration updating - response_body - response is expected (repeated req 0, req 0)'
   #   at /home/runner/work/apisix/apisix/test-nginx/lib/Test/Nginx/Socket.pm line 1589.
   # @@ -1,2 +1 @@
   # -passedopentracing
   # +fail
   # -passedopentracing
   
   #   Failed test 'TEST 7: check plugin configuration updating - grep_error_log_out (req 0)'
   #   at /home/runner/work/apisix/apisix/test-nginx/lib/Test/Nginx/Socket.pm line 1149.
   # @@ -1,2 +0,0 @@
   # -sending a batch logs to 127.0.0.1:5044
   # -sending a batch logs to 127.0.0.1:5045
   # Looks like you failed 3 tests of 21.
   t/plugin/tcp-logger.t ...................... 
   Dubious, test returned 3 (wstat 768, 0x300)
   ```
   ---
   
   Reported in #3560


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander merged pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #3521:
URL: https://github.com/apache/apisix/pull/3521


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on a change in pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#discussion_r570209849



##########
File path: .github/workflows/release-test.yml
##########
@@ -0,0 +1,126 @@
+name: Release-test
+on: [pull_request]
+# use pull request for test, will change to push before merge
+  #push:
+  #  branches:
+  #    - '*'
+  #    - '!master'
+
+jobs:
+  build:
+    strategy:
+      fail-fast: false
+      matrix:
+        platform:
+          - ubuntu-18.04
+        os_name:
+          - linux_openresty
+          - linux_openresty_1_17
+          - linux_openresty_1_15
+          - linux_tengine
+          - linux_apisix_master_luarocks
+          - linux_apisix_current_luarocks
+          - linux_openresty_mtls
+
+    runs-on: ${{ matrix.platform }}
+    env:
+      SERVER_NAME: ${{ matrix.os_name }}
+      OPENRESTY_VERSION: default
+
+
+    services:
+      etcd:
+        image: bitnami/etcd:3.4.0
+        ports:
+          - 2379:2379
+          - 2380:2380
+        env:
+          ALLOW_NONE_AUTHENTICATION: yes
+          ETCD_ADVERTISE_CLIENT_URLS: http://0.0.0.0:2379
+
+      old_etcd:

Review comment:
       Maybe we can add a new step in the build.yml, and remove the `apisix` in git repo, replace `./bin/apisix` with `/usr/bin/apisix`? So thev remain don't need to be changed. 

##########
File path: .github/workflows/release-test.yml
##########
@@ -0,0 +1,126 @@
+name: Release-test
+on: [pull_request]
+# use pull request for test, will change to push before merge
+  #push:
+  #  branches:
+  #    - '*'
+  #    - '!master'
+
+jobs:
+  build:
+    strategy:
+      fail-fast: false
+      matrix:
+        platform:
+          - ubuntu-18.04
+        os_name:
+          - linux_openresty
+          - linux_openresty_1_17
+          - linux_openresty_1_15
+          - linux_tengine
+          - linux_apisix_master_luarocks
+          - linux_apisix_current_luarocks
+          - linux_openresty_mtls
+
+    runs-on: ${{ matrix.platform }}
+    env:
+      SERVER_NAME: ${{ matrix.os_name }}
+      OPENRESTY_VERSION: default
+
+
+    services:
+      etcd:
+        image: bitnami/etcd:3.4.0
+        ports:
+          - 2379:2379
+          - 2380:2380
+        env:
+          ALLOW_NONE_AUTHENTICATION: yes
+          ETCD_ADVERTISE_CLIENT_URLS: http://0.0.0.0:2379
+
+      old_etcd:

Review comment:
       Maybe we can add a new step in the build.yml, and remove the `apisix` in git repo, replace `./bin/apisix` with `/usr/bin/apisix`? So the remain don't need to be changed. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on a change in pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#discussion_r571748583



##########
File path: .github/workflows/build.yml
##########
@@ -108,3 +129,10 @@ jobs:
 
       - name: Linux Script
         run: sudo ./.travis/${{ matrix.os_name }}_runner.sh script
+
+      - name: Publish Artifact
+        if: ${{ startsWith(github.ref, 'refs/heads/v') && endsWith(matrix.os_name, 'linux_openresty') }}

Review comment:
       Should be `matrix.os_name == 'linux_openresty'`?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu commented on pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#issuecomment-775126079


   Waiting for CI result triggered by `push` on branch `release/test`
   https://github.com/apache/apisix/actions/runs/547997620


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu commented on pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#issuecomment-774610699


   @spacewander PTAL


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu edited a comment on pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
Yiyiyimu edited a comment on pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#issuecomment-775126079


   Waiting for CI result triggered by `push` on branch `release/test`
   https://github.com/apache/apisix/actions/runs/547997620
   
   ---
   
   Hi @spacewander the CI passed and uploaded the artifact as we expect. You could take a look.
   BTW, do we need to rename all the past release branches?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on a change in pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#discussion_r571572164



##########
File path: .github/workflows/build.yml
##########
@@ -53,6 +53,30 @@ jobs:
         with:
           submodules: recursive
 
+      - name: Extract branch name
+        id: branch_env
+        shell: bash
+        run: |
+          export VERSION=${GITHUB_REF##*/}
+          echo "##[set-output name=version;]$(echo ${VERSION:1})"
+          echo "##[set-output name=fullname;]$(echo apache-apisix-${VERSION:1}-src.tgz)"
+
+      - name: Create tarball
+        # if: ${{ startsWith(github.ref, 'refs/heads/v') && !endsWith(matrix.os_name, 'luarocks') }}
+        run: |
+          export VERSION=${{ steps.branch_env.outputs.version }}
+          export RELEASE_SRC=${{ steps.branch_env.outputs.fullname }}
+          make compress-tar
+          # mv release/${RELEASE_SRC} ${RELEASE_SRC}
+          ls
+
+      - name: Remove everything except tarball/.travis/t

Review comment:
       Should be `Remove source code`, since we don't remove `.github` either.

##########
File path: .github/workflows/build.yml
##########
@@ -53,6 +53,30 @@ jobs:
         with:
           submodules: recursive
 
+      - name: Extract branch name
+        id: branch_env
+        shell: bash
+        run: |
+          export VERSION=${GITHUB_REF##*/}
+          echo "##[set-output name=version;]$(echo ${VERSION:1})"

Review comment:
       It seems the output is `erge`? The name should come from the name of branch. Is it expected?

##########
File path: Makefile
##########
@@ -221,25 +221,7 @@ endif
 	.travis/openwhisk-utilities/scancode/scanCode.py --config .travis/ASF-Release.cfg ./
 
 release-src:

Review comment:
       Better to use `release-src: compress-tar`

##########
File path: .github/workflows/build.yml
##########
@@ -53,6 +53,30 @@ jobs:
         with:
           submodules: recursive
 
+      - name: Extract branch name
+        id: branch_env
+        shell: bash
+        run: |
+          export VERSION=${GITHUB_REF##*/}
+          echo "##[set-output name=version;]$(echo ${VERSION:1})"
+          echo "##[set-output name=fullname;]$(echo apache-apisix-${VERSION:1}-src.tgz)"
+
+      - name: Create tarball
+        # if: ${{ startsWith(github.ref, 'refs/heads/v') && !endsWith(matrix.os_name, 'luarocks') }}
+        run: |
+          export VERSION=${{ steps.branch_env.outputs.version }}
+          export RELEASE_SRC=${{ steps.branch_env.outputs.fullname }}
+          make compress-tar
+          # mv release/${RELEASE_SRC} ${RELEASE_SRC}
+          ls
+
+      - name: Remove everything except tarball/.travis/t
+        # if: ${{ startsWith(github.ref, 'refs/heads/v') && !endsWith(matrix.os_name, 'luarocks') }}

Review comment:
       We could change the release branch to `release/x.y`, use `release/` as the prefix.
   
   BTW, is there a way to limit the source of PR to this repo? Only the member who can create a branch in this repo can trigger the packaging.

##########
File path: .github/workflows/build.yml
##########
@@ -108,3 +132,10 @@ jobs:
 
       - name: Linux Script
         run: sudo ./.travis/${{ matrix.os_name }}_runner.sh script
+
+      - name: Publish Artifact
+        # if: ${{ startsWith(github.ref, 'refs/heads/v') && !endsWith(matrix.os_name, 'luarocks') }}

Review comment:
       We can only upload artifact when `os_name` is linux_openresty.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu commented on pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#issuecomment-774611616


   The tarball compressed by Github Actions could be found at
   ![image](https://user-images.githubusercontent.com/34589752/107138382-145e7680-694f-11eb-9d4b-5626d2f7f95a.png)
   
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on a change in pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#discussion_r570198202



##########
File path: .github/workflows/release-test.yml
##########
@@ -0,0 +1,126 @@
+name: Release-test
+on: [pull_request]
+# use pull request for test, will change to push before merge
+  #push:
+  #  branches:
+  #    - '*'
+  #    - '!master'
+
+jobs:
+  build:
+    strategy:
+      fail-fast: false
+      matrix:
+        platform:
+          - ubuntu-18.04
+        os_name:
+          - linux_openresty
+          - linux_openresty_1_17
+          - linux_openresty_1_15

Review comment:
       Seems we only need the linux_openresty_xxx plus a test to run cli installed via package?

##########
File path: .github/workflows/release-test.yml
##########
@@ -0,0 +1,126 @@
+name: Release-test
+on: [pull_request]
+# use pull request for test, will change to push before merge
+  #push:
+  #  branches:
+  #    - '*'
+  #    - '!master'
+
+jobs:
+  build:
+    strategy:
+      fail-fast: false
+      matrix:
+        platform:
+          - ubuntu-18.04
+        os_name:
+          - linux_openresty
+          - linux_openresty_1_17
+          - linux_openresty_1_15
+          - linux_tengine
+          - linux_apisix_master_luarocks
+          - linux_apisix_current_luarocks
+          - linux_openresty_mtls
+
+    runs-on: ${{ matrix.platform }}
+    env:
+      SERVER_NAME: ${{ matrix.os_name }}
+      OPENRESTY_VERSION: default
+
+
+    services:
+      etcd:
+        image: bitnami/etcd:3.4.0
+        ports:
+          - 2379:2379
+          - 2380:2380
+        env:
+          ALLOW_NONE_AUTHENTICATION: yes
+          ETCD_ADVERTISE_CLIENT_URLS: http://0.0.0.0:2379
+
+      old_etcd:

Review comment:
       There is too much repeated code. Can we reduce them? Or can we just modify the `linux_openresty_common_runner.sh`?

##########
File path: .github/workflows/release-test.yml
##########
@@ -0,0 +1,126 @@
+name: Release-test
+on: [pull_request]
+# use pull request for test, will change to push before merge
+  #push:
+  #  branches:
+  #    - '*'
+  #    - '!master'
+
+jobs:
+  build:
+    strategy:
+      fail-fast: false
+      matrix:
+        platform:
+          - ubuntu-18.04
+        os_name:
+          - linux_openresty
+          - linux_openresty_1_17
+          - linux_openresty_1_15
+          - linux_tengine
+          - linux_apisix_master_luarocks
+          - linux_apisix_current_luarocks
+          - linux_openresty_mtls
+
+    runs-on: ${{ matrix.platform }}
+    env:
+      SERVER_NAME: ${{ matrix.os_name }}
+      OPENRESTY_VERSION: default
+
+
+    services:
+      etcd:
+        image: bitnami/etcd:3.4.0
+        ports:
+          - 2379:2379
+          - 2380:2380
+        env:
+          ALLOW_NONE_AUTHENTICATION: yes
+          ETCD_ADVERTISE_CLIENT_URLS: http://0.0.0.0:2379
+
+      old_etcd:

Review comment:
       Maybe we can add a new step in the build.yml, and remove the `apisix` in git repo, replace `./bin/apisix` with `/usr/bin/apisix`? So the remain don't need to be changed. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu commented on a change in pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on a change in pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#discussion_r571622327



##########
File path: .github/workflows/build.yml
##########
@@ -108,3 +132,10 @@ jobs:
 
       - name: Linux Script
         run: sudo ./.travis/${{ matrix.os_name }}_runner.sh script
+
+      - name: Publish Artifact
+        # if: ${{ startsWith(github.ref, 'refs/heads/v') && !endsWith(matrix.os_name, 'luarocks') }}

Review comment:
       Cool that could fix the problem. Thanks!




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu commented on pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#issuecomment-781760478


   @spacewander Do we need to change the branch titles of previous releases?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu commented on a change in pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on a change in pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#discussion_r571620984



##########
File path: .github/workflows/build.yml
##########
@@ -53,6 +53,30 @@ jobs:
         with:
           submodules: recursive
 
+      - name: Extract branch name
+        id: branch_env
+        shell: bash
+        run: |
+          export VERSION=${GITHUB_REF##*/}
+          echo "##[set-output name=version;]$(echo ${VERSION:1})"
+          echo "##[set-output name=fullname;]$(echo apache-apisix-${VERSION:1}-src.tgz)"
+
+      - name: Create tarball
+        # if: ${{ startsWith(github.ref, 'refs/heads/v') && !endsWith(matrix.os_name, 'luarocks') }}
+        run: |
+          export VERSION=${{ steps.branch_env.outputs.version }}
+          export RELEASE_SRC=${{ steps.branch_env.outputs.fullname }}
+          make compress-tar
+          # mv release/${RELEASE_SRC} ${RELEASE_SRC}
+          ls
+
+      - name: Remove everything except tarball/.travis/t
+        # if: ${{ startsWith(github.ref, 'refs/heads/v') && !endsWith(matrix.os_name, 'luarocks') }}

Review comment:
       Are we changing the release name these days? Yeah it would definitely be of help, so people won't trigger this action when push to their own branch say `verbose`.
   
   And to the last problem, since it would only be triggered when push to release branches, maintainers could control who would trigger this CI.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu edited a comment on pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
Yiyiyimu edited a comment on pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#issuecomment-775004549


   For record unstable test before re-run CI:
   ```
   #   Failed test 'TEST 7: check plugin configuration updating - status code ok'
   #   at /home/runner/work/apisix/apisix/test-nginx/lib/Test/Nginx/Socket.pm line 948.
   #          got: '404'
   #     expected: '200'
   
   #   Failed test 'TEST 7: check plugin configuration updating - response_body - response is expected (repeated req 0, req 0)'
   #   at /home/runner/work/apisix/apisix/test-nginx/lib/Test/Nginx/Socket.pm line 1589.
   # @@ -1,2 +1 @@
   # -passedopentracing
   # +fail
   # -passedopentracing
   
   #   Failed test 'TEST 7: check plugin configuration updating - grep_error_log_out (req 0)'
   #   at /home/runner/work/apisix/apisix/test-nginx/lib/Test/Nginx/Socket.pm line 1149.
   # @@ -1,2 +0,0 @@
   # -sending a batch logs to 127.0.0.1:5044
   # -sending a batch logs to 127.0.0.1:5045
   # Looks like you failed 3 tests of 21.
   t/plugin/tcp-logger.t ...................... 
   Dubious, test returned 3 (wstat 768, 0x300)
   ```
   ---
   
   Reported in #3560


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on a change in pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#discussion_r571747906



##########
File path: .github/workflows/build.yml
##########
@@ -53,6 +53,30 @@ jobs:
         with:
           submodules: recursive
 
+      - name: Extract branch name
+        id: branch_env
+        shell: bash
+        run: |
+          export VERSION=${GITHUB_REF##*/}
+          echo "##[set-output name=version;]$(echo ${VERSION:1})"
+          echo "##[set-output name=fullname;]$(echo apache-apisix-${VERSION:1}-src.tgz)"
+
+      - name: Create tarball
+        # if: ${{ startsWith(github.ref, 'refs/heads/v') && !endsWith(matrix.os_name, 'luarocks') }}
+        run: |
+          export VERSION=${{ steps.branch_env.outputs.version }}
+          export RELEASE_SRC=${{ steps.branch_env.outputs.fullname }}
+          make compress-tar
+          # mv release/${RELEASE_SRC} ${RELEASE_SRC}
+          ls
+
+      - name: Remove everything except tarball/.travis/t
+        # if: ${{ startsWith(github.ref, 'refs/heads/v') && !endsWith(matrix.os_name, 'luarocks') }}

Review comment:
       We don't need to change the release name. We only need to change the branch name.
   
   Will you submit a final commit to `release/test` branch and show us the final result after changing this? Thanks!




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#issuecomment-781762918


   Let the previous releases keep frozen.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on a change in pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#discussion_r570199706



##########
File path: .github/workflows/release-test.yml
##########
@@ -0,0 +1,126 @@
+name: Release-test
+on: [pull_request]
+# use pull request for test, will change to push before merge
+  #push:
+  #  branches:
+  #    - '*'
+  #    - '!master'
+
+jobs:
+  build:
+    strategy:
+      fail-fast: false
+      matrix:
+        platform:
+          - ubuntu-18.04
+        os_name:
+          - linux_openresty
+          - linux_openresty_1_17
+          - linux_openresty_1_15
+          - linux_tengine
+          - linux_apisix_master_luarocks
+          - linux_apisix_current_luarocks
+          - linux_openresty_mtls
+
+    runs-on: ${{ matrix.platform }}
+    env:
+      SERVER_NAME: ${{ matrix.os_name }}
+      OPENRESTY_VERSION: default
+
+
+    services:
+      etcd:
+        image: bitnami/etcd:3.4.0
+        ports:
+          - 2379:2379
+          - 2380:2380
+        env:
+          ALLOW_NONE_AUTHENTICATION: yes
+          ETCD_ADVERTISE_CLIENT_URLS: http://0.0.0.0:2379
+
+      old_etcd:

Review comment:
       There is too much repeated code. Can we reduce them? Or can we just modify the `linux_openresty_common_runner.sh`?

##########
File path: .github/workflows/release-test.yml
##########
@@ -0,0 +1,126 @@
+name: Release-test
+on: [pull_request]
+# use pull request for test, will change to push before merge
+  #push:
+  #  branches:
+  #    - '*'
+  #    - '!master'
+
+jobs:
+  build:
+    strategy:
+      fail-fast: false
+      matrix:
+        platform:
+          - ubuntu-18.04
+        os_name:
+          - linux_openresty
+          - linux_openresty_1_17
+          - linux_openresty_1_15

Review comment:
       Seems we only need the linux_openresty_xxx plus a test to run cli installed via package?

##########
File path: .github/workflows/release-test.yml
##########
@@ -0,0 +1,126 @@
+name: Release-test
+on: [pull_request]
+# use pull request for test, will change to push before merge
+  #push:
+  #  branches:
+  #    - '*'
+  #    - '!master'
+
+jobs:
+  build:
+    strategy:
+      fail-fast: false
+      matrix:
+        platform:
+          - ubuntu-18.04
+        os_name:
+          - linux_openresty
+          - linux_openresty_1_17
+          - linux_openresty_1_15
+          - linux_tengine
+          - linux_apisix_master_luarocks
+          - linux_apisix_current_luarocks
+          - linux_openresty_mtls
+
+    runs-on: ${{ matrix.platform }}
+    env:
+      SERVER_NAME: ${{ matrix.os_name }}
+      OPENRESTY_VERSION: default
+
+
+    services:
+      etcd:
+        image: bitnami/etcd:3.4.0
+        ports:
+          - 2379:2379
+          - 2380:2380
+        env:
+          ALLOW_NONE_AUTHENTICATION: yes
+          ETCD_ADVERTISE_CLIENT_URLS: http://0.0.0.0:2379
+
+      old_etcd:

Review comment:
       Maybe we can add a new step in the build.yml, and remove the `apisix` in git repo, replace `./bin/apisix` with `/usr/bin/apisix`? So thev remain don't need to be changed. 

##########
File path: .github/workflows/release-test.yml
##########
@@ -0,0 +1,126 @@
+name: Release-test
+on: [pull_request]
+# use pull request for test, will change to push before merge
+  #push:
+  #  branches:
+  #    - '*'
+  #    - '!master'
+
+jobs:
+  build:
+    strategy:
+      fail-fast: false
+      matrix:
+        platform:
+          - ubuntu-18.04
+        os_name:
+          - linux_openresty
+          - linux_openresty_1_17
+          - linux_openresty_1_15
+          - linux_tengine
+          - linux_apisix_master_luarocks
+          - linux_apisix_current_luarocks
+          - linux_openresty_mtls
+
+    runs-on: ${{ matrix.platform }}
+    env:
+      SERVER_NAME: ${{ matrix.os_name }}
+      OPENRESTY_VERSION: default
+
+
+    services:
+      etcd:
+        image: bitnami/etcd:3.4.0
+        ports:
+          - 2379:2379
+          - 2380:2380
+        env:
+          ALLOW_NONE_AUTHENTICATION: yes
+          ETCD_ADVERTISE_CLIENT_URLS: http://0.0.0.0:2379
+
+      old_etcd:

Review comment:
       Maybe we can add a new step in the build.yml, and remove the `apisix` in git repo, replace `./bin/apisix` with `/usr/bin/apisix`? So the remain don't need to be changed. 

##########
File path: .github/workflows/release-test.yml
##########
@@ -0,0 +1,126 @@
+name: Release-test
+on: [pull_request]
+# use pull request for test, will change to push before merge
+  #push:
+  #  branches:
+  #    - '*'
+  #    - '!master'
+
+jobs:
+  build:
+    strategy:
+      fail-fast: false
+      matrix:
+        platform:
+          - ubuntu-18.04
+        os_name:
+          - linux_openresty
+          - linux_openresty_1_17
+          - linux_openresty_1_15

Review comment:
       Seems we only need the linux_openresty_xxx plus a test to run cli installed via package?

##########
File path: .github/workflows/release-test.yml
##########
@@ -0,0 +1,126 @@
+name: Release-test
+on: [pull_request]
+# use pull request for test, will change to push before merge
+  #push:
+  #  branches:
+  #    - '*'
+  #    - '!master'
+
+jobs:
+  build:
+    strategy:
+      fail-fast: false
+      matrix:
+        platform:
+          - ubuntu-18.04
+        os_name:
+          - linux_openresty
+          - linux_openresty_1_17
+          - linux_openresty_1_15
+          - linux_tengine
+          - linux_apisix_master_luarocks
+          - linux_apisix_current_luarocks
+          - linux_openresty_mtls
+
+    runs-on: ${{ matrix.platform }}
+    env:
+      SERVER_NAME: ${{ matrix.os_name }}
+      OPENRESTY_VERSION: default
+
+
+    services:
+      etcd:
+        image: bitnami/etcd:3.4.0
+        ports:
+          - 2379:2379
+          - 2380:2380
+        env:
+          ALLOW_NONE_AUTHENTICATION: yes
+          ETCD_ADVERTISE_CLIENT_URLS: http://0.0.0.0:2379
+
+      old_etcd:

Review comment:
       There is too much repeated code. Can we reduce them? Or can we just modify the `linux_openresty_common_runner.sh`?

##########
File path: .github/workflows/release-test.yml
##########
@@ -0,0 +1,126 @@
+name: Release-test
+on: [pull_request]
+# use pull request for test, will change to push before merge
+  #push:
+  #  branches:
+  #    - '*'
+  #    - '!master'
+
+jobs:
+  build:
+    strategy:
+      fail-fast: false
+      matrix:
+        platform:
+          - ubuntu-18.04
+        os_name:
+          - linux_openresty
+          - linux_openresty_1_17
+          - linux_openresty_1_15
+          - linux_tengine
+          - linux_apisix_master_luarocks
+          - linux_apisix_current_luarocks
+          - linux_openresty_mtls
+
+    runs-on: ${{ matrix.platform }}
+    env:
+      SERVER_NAME: ${{ matrix.os_name }}
+      OPENRESTY_VERSION: default
+
+
+    services:
+      etcd:
+        image: bitnami/etcd:3.4.0
+        ports:
+          - 2379:2379
+          - 2380:2380
+        env:
+          ALLOW_NONE_AUTHENTICATION: yes
+          ETCD_ADVERTISE_CLIENT_URLS: http://0.0.0.0:2379
+
+      old_etcd:

Review comment:
       Maybe we can add a new step in the build.yml, and remove the `apisix` in git repo, replace `./bin/apisix` with `/usr/bin/apisix`? So the remain don't need to be changed. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu edited a comment on pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
Yiyiyimu edited a comment on pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#issuecomment-775126079


   Waiting for CI result triggered by `push` on branch `release/test`
   https://github.com/apache/apisix/actions/runs/547997620
   
   ---
   
   Hi @spacewander the CI passed and uploaded the artifact as we expect. You could take a look.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] starsz commented on pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
starsz commented on pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#issuecomment-781434320


   LGTM. I think the same things can be done in the dashboard.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on a change in pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#discussion_r570199706



##########
File path: .github/workflows/release-test.yml
##########
@@ -0,0 +1,126 @@
+name: Release-test
+on: [pull_request]
+# use pull request for test, will change to push before merge
+  #push:
+  #  branches:
+  #    - '*'
+  #    - '!master'
+
+jobs:
+  build:
+    strategy:
+      fail-fast: false
+      matrix:
+        platform:
+          - ubuntu-18.04
+        os_name:
+          - linux_openresty
+          - linux_openresty_1_17
+          - linux_openresty_1_15
+          - linux_tengine
+          - linux_apisix_master_luarocks
+          - linux_apisix_current_luarocks
+          - linux_openresty_mtls
+
+    runs-on: ${{ matrix.platform }}
+    env:
+      SERVER_NAME: ${{ matrix.os_name }}
+      OPENRESTY_VERSION: default
+
+
+    services:
+      etcd:
+        image: bitnami/etcd:3.4.0
+        ports:
+          - 2379:2379
+          - 2380:2380
+        env:
+          ALLOW_NONE_AUTHENTICATION: yes
+          ETCD_ADVERTISE_CLIENT_URLS: http://0.0.0.0:2379
+
+      old_etcd:

Review comment:
       There is too much repeated code. Can we reduce them? Or can we just modify the `linux_openresty_common_runner.sh`?

##########
File path: .github/workflows/release-test.yml
##########
@@ -0,0 +1,126 @@
+name: Release-test
+on: [pull_request]
+# use pull request for test, will change to push before merge
+  #push:
+  #  branches:
+  #    - '*'
+  #    - '!master'
+
+jobs:
+  build:
+    strategy:
+      fail-fast: false
+      matrix:
+        platform:
+          - ubuntu-18.04
+        os_name:
+          - linux_openresty
+          - linux_openresty_1_17
+          - linux_openresty_1_15

Review comment:
       Seems we only need the linux_openresty_xxx plus a test to run cli installed via package?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu commented on pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#issuecomment-775004549


   For record unstable test before re-run CI:
   ```
   #   Failed test 'TEST 7: check plugin configuration updating - status code ok'
   #   at /home/runner/work/apisix/apisix/test-nginx/lib/Test/Nginx/Socket.pm line 948.
   #          got: '404'
   #     expected: '200'
   
   #   Failed test 'TEST 7: check plugin configuration updating - response_body - response is expected (repeated req 0, req 0)'
   #   at /home/runner/work/apisix/apisix/test-nginx/lib/Test/Nginx/Socket.pm line 1589.
   # @@ -1,2 +1 @@
   # -passedopentracing
   # +fail
   # -passedopentracing
   
   #   Failed test 'TEST 7: check plugin configuration updating - grep_error_log_out (req 0)'
   #   at /home/runner/work/apisix/apisix/test-nginx/lib/Test/Nginx/Socket.pm line 1149.
   # @@ -1,2 +0,0 @@
   # -sending a batch logs to 127.0.0.1:5044
   # -sending a batch logs to 127.0.0.1:5045
   # Looks like you failed 3 tests of 21.
   t/plugin/tcp-logger.t ...................... 
   Dubious, test returned 3 (wstat 768, 0x300)
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu commented on a change in pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on a change in pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#discussion_r571622356



##########
File path: .github/workflows/build.yml
##########
@@ -53,6 +53,30 @@ jobs:
         with:
           submodules: recursive
 
+      - name: Extract branch name
+        id: branch_env
+        shell: bash
+        run: |
+          export VERSION=${GITHUB_REF##*/}
+          echo "##[set-output name=version;]$(echo ${VERSION:1})"
+          echo "##[set-output name=fullname;]$(echo apache-apisix-${VERSION:1}-src.tgz)"
+
+      - name: Create tarball
+        # if: ${{ startsWith(github.ref, 'refs/heads/v') && !endsWith(matrix.os_name, 'luarocks') }}
+        run: |
+          export VERSION=${{ steps.branch_env.outputs.version }}
+          export RELEASE_SRC=${{ steps.branch_env.outputs.fullname }}
+          make compress-tar
+          # mv release/${RELEASE_SRC} ${RELEASE_SRC}
+          ls
+
+      - name: Remove everything except tarball/.travis/t

Review comment:
       That would be better, thx!

##########
File path: Makefile
##########
@@ -221,25 +221,7 @@ endif
 	.travis/openwhisk-utilities/scancode/scanCode.py --config .travis/ASF-Release.cfg ./
 
 release-src:

Review comment:
       Thanks! Fixed




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu commented on pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#issuecomment-774592809


   One problem is which test should we use. Currently we only ignore luarocks check, should we ignore more?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu commented on pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#issuecomment-776444335


   @spacewander PTAL


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] Yiyiyimu commented on a change in pull request #3521: ci: add release check

Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on a change in pull request #3521:
URL: https://github.com/apache/apisix/pull/3521#discussion_r571558102



##########
File path: .github/workflows/build.yml
##########
@@ -53,6 +53,30 @@ jobs:
         with:
           submodules: recursive
 
+      - name: Extract branch name
+        id: branch_env
+        shell: bash
+        run: |
+          export VERSION=${GITHUB_REF##*/}
+          echo "##[set-output name=version;]$(echo ${VERSION:1})"
+          echo "##[set-output name=fullname;]$(echo apache-apisix-${VERSION:1}-src.tgz)"
+
+      - name: Create tarball
+        # if: ${{ startsWith(github.ref, 'refs/heads/v') && !endsWith(matrix.os_name, 'luarocks') }}

Review comment:
       need to uncomment this line before merge




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org