paul-rogers commented on code in PR #13681: URL: https://github.com/apache/druid/pull/13681#discussion_r1090983204
########## .github/workflows/reusable-revised-its.yml: ########## @@ -0,0 +1,73 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: Revised Integration Tests shared workflow +on: + workflow_call: + inputs: + script: + description: 'Which IT script to run' + required: true + type: string + build_jdk: + description: 'Which jdk version was used to build' + required: true + type: string + runtime_jdk: + description: 'Which JDK version to use at runtime' + required: true + type: string + use_indexer: + description: 'Which indexer to use' + required: true + type: string + it: + description: 'IT test Category' + required: true + type: string + +env: + SEGMENT_DOWNLOAD_TIMEOUT_MINS: 5 + +jobs: + test: # Github job that runs a given revised/new IT against retrieved cached druid docker image + name: ${{ inputs.it }} integration test (Compile=jdk${{ inputs.build_jdk }}, Run=jdk${{ inputs.runtime_jdk }}, Indexer=${{ inputs.use_indexer }}) + runs-on: ubuntu-22.04 + steps: + - name: Checkout branch + uses: actions/checkout@v3 + + - name: Setup java + uses: actions/setup-java@v3 + with: + distribution: 'zulu' + java-version: ${{ inputs.build_jdk }} + cache: maven + + - name: Retrieve cached docker image + uses: actions/cache/restore@v3 + with: + key: druid-container-jdk${{ inputs.build_jdk }}.tar.gz + path: | + ./druid-container-jdk${{ inputs.build_jdk }}.tar.gz + ./integration-tests-ex/image/target/env.sh + + - name: Load docker image + run: | + docker load --input druid-container-jdk${{ inputs.build_jdk }}.tar.gz + docker images + + - name: Run IT Review Comment: The tests run locally using locally-built artifacts for the JUnit tests and their dependencies. Is there a step above that obtains the previously-built artifacts? ########## .github/workflows/revised-its.yml: ########## @@ -0,0 +1,36 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Github workflow that runs revised/new ITs +on: + workflow_call + +jobs: + it: + strategy: + fail-fast: false + matrix: + #jdk: [8, 11, 17] Review Comment: This comment is nice: concise and tells us exactly what is legal. ########## integration-tests/docker/docker-compose.query-error-test.yml: ########## @@ -78,7 +78,7 @@ services: druid-it-net: ipv4_address: 172.172.172.14 ports: - - 8084:8083 + - 8086:8083 Review Comment: Is this an actual change in port assignments? If so, then we're changing the host port from 8084 to 8086. (The port in the container is 8083). The tests have to know to use this new port. Did we make the matching changes elsewhere? ########## .github/workflows/unit-and-integration-tests-unified.yml: ########## @@ -0,0 +1,160 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: Unit & Integration tests CI +on: + push: + branches: + - master + - /^\d+\.\d+\.\d+(-\S*)?$/ # release branches + pull_request: + branches: + - master + - /^\d+\.\d+\.\d+(-\S*)?$/ # release branches + +concurrency: + group: '${{ github.workflow }} @ ${{ github.event.pull_request.head.label || github.head_ref || github.ref }}' + cancel-in-progress: true + +jobs: + build: + strategy: + fail-fast: false + matrix: + jdk: [ '8', '11', '17' ] + runs-on: ubuntu-latest + steps: + - name: Checkout branch + uses: actions/checkout@v3 + - name: Cache Maven m2 repository + id: maven + uses: actions/cache@v3 + with: + path: ~/.m2/repository + key: maven-${{ runner.os }}-${{ matrix.jdk }}-${{ hashFiles('**/pom.xml') }} + restore-keys: | + maven-${{ runner.os }}-${{ matrix.jdk }} + - name: Cache targets + id: target + uses: actions/cache@v3 + with: + path: | + ./target + ./extendedset/target + ./hll/target + ./processing/target + ./server/target + ./indexing-hadoop/target + ./web-console/target + ./indexing-service/target + ./services/target + ./benchmarks/target + ./distribution/target + ./integration-tests/target + key: maven-${{ runner.os }}-${{ matrix.jdk }}-targets-${{ hashFiles('**/pom.xml') }} + restore-keys: | + maven-${{ runner.os }}-${{ matrix.jdk }}-targets + - name: Cache image + id: docker_container + uses: actions/cache@v3 + with: + key: druid-container-jdk${{ matrix.jdk }}.tar.gz-${{ hashFiles('**/pom.xml') }} + path: | + ./druid-container-jdk${{ matrix.jdk }}.tar.gz + ./integration-tests-ex/image/target/env.sh + restore-keys: | + druid-container-jdk${{ matrix.jdk }}.tar.gz- + - name: Load docker image + if: steps.docker_container.outputs.cache-hit == 'true' + run: | + docker load --input druid-container-jdk${{ matrix.jdk }}.tar.gz + docker images + - name: Setup java + if: (steps.docker_container.outputs.cache-hit != 'true' || steps.target.outputs.cache-hit != 'true') + uses: actions/setup-java@v3 + with: + distribution: 'zulu' + java-version: ${{ matrix.jdk }} + - name: Maven build + if: (steps.maven.outputs.cache-hit != 'true' || steps.target.outputs.cache-hit != 'true') + id: maven_build + run: | + ./it.sh ci + - name: Container build + if: (steps.docker_container.outputs.cache-hit != 'true' || steps.maven.outputs.cache-hit != 'true' || steps.target.outputs.cache-hit != 'true') + run: | + ./it.sh image + source ./integration-tests-ex/image/target/env.sh Review Comment: Like most things with ITs, this feature can be a bit confusing because it solves two distinct problems. When we run the ITs on a dev machine, we can use the same image name over and over: it is unlikely we'd run two ITs concurrently. To make that experience simple, `it.sh` will use a default name, and write that name to `env.sh` so that the test can use it. When run in GHA, we'll build the image once, then use it for multiple tests. In some environments (Jenkins, say) the image goes into a repo that is shared by multiple jobs. So, in that case, the image name for each build has to be unique. The build environment picks the name, then passes it into `it.sh` via the `DRUID_IT_IMAGE_NAME` variable. Still, `it.sh image` copies that name into `env.sh`. Just to clarify, when I say `it.sh image`, the real work is done in `druid-it-image/pom.xml` and `build-image.sh`. `it.sh` is just a convenient wrapper. When `it.sh image` runs, it gathers up all the per-test config: image name, MySQL driver, software versions, etc. It writes them to `env.sh`. Thus, `env.sh` has to be passed from the image-build step to the step which runs each test. Maybe there is a better way to do this on GHA? The script approach worked OK on Jenkins (we use a Jenkins feature to copy the file) and Travis (where we did things the slow way: built the image for each test.) Does GHA use a shared Docker repo for images? If not, we can use the default name. If the Docker repo is shared, then your GHA code should pick an image name unique to each build (include the build ID, say) so that names won't clash in the Docker repo. Set `DRUID_IT_IMAGE_NAME` to pass that name into `it.sh image`. From there, `env.sh` will pass the name to each test run. Yes, this is a bit complex, but it is the easiest thing that solved both the desktop and Jenkins/Travis requirements. We can improve it to fit GHA. I hope the above made sense. If not, please reach out and we can sort out the remaining questions. ########## .github/workflows/reusable-standard-its.yml: ########## @@ -0,0 +1,85 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: Revised Integration Tests shared workflow +on: + workflow_call: + inputs: + runtime_jdk: + description: 'Which JDK version to use at runtime' + required: true + type: string + use_indexer: + description: 'Which indexer to use' + required: true + type: string + override_config_path: Review Comment: The descriptions are very helpful. Can we add one for this input as well? ########## .github/workflows/reusable-standard-its.yml: ########## @@ -0,0 +1,91 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: Revised Integration Tests shared workflow +on: + workflow_call: + inputs: + runtime_jdk: + description: 'Which JDK version to use at runtime' + required: true + type: string + use_indexer: + description: 'Which indexer to use' + required: true + type: string + override_config_path: + required: false + type: string + testing_groups: + required: true + type: string + build_jdk: + description: 'Which jdk version was used to build' + required: true + type: string + mysql_driver: + description: MySQL driver to use + required: false + type: string + default: com.mysql.jdbc.Driver + +env: + MVN: mvn --no-snapshot-updates + MAVEN_SKIP: -P skip-static-checks -Dweb.console.skip=true -Dmaven.javadoc.skip=true + MAVEN_SKIP_TESTS: -P skip-tests + DOCKER_IP: 127.0.0.1 # for integration tests + MYSQL_DRIVER_CLASSNAME: ${{ inputs.mysql_driver }} Review Comment: `DOCKER_IP` is not really a per-test setting. It is used in the case in which the Docker engine runs on a machine other than the one running the tests. That is, to access the Druid services, we have to say `$DOCKER_IP:1234" not just `localhost:1234`. Thus, this is a for-all-tests config in GHA if Docker runs in the same machine/host/runner/container as the tests. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
