From 9dd14462f09b508bc4eab1f86bbf3b3da958fbcf Mon Sep 17 00:00:00 2001 From: Sushanta Das Date: Tue, 12 Nov 2024 11:01:11 +0530 Subject: [PATCH] Address review comments --- .github/workflows/run-task-tests.yaml | 17 +--- .github/workflows/task-lint.yaml | 53 +++++++++++ README.md | 27 ++++++ hack/run-test.sh | 59 ------------ .../0.1/tests/pre-apply-taskrun-hook.sh | 2 +- test/common.sh | 92 ++++++++----------- test/run-test.sh | 2 +- test/test-tasks.sh | 15 +-- 8 files changed, 128 insertions(+), 139 deletions(-) create mode 100644 .github/workflows/task-lint.yaml delete mode 100644 hack/run-test.sh diff --git a/.github/workflows/run-task-tests.yaml b/.github/workflows/run-task-tests.yaml index a0855c263e..8888a83c93 100644 --- a/.github/workflows/run-task-tests.yaml +++ b/.github/workflows/run-task-tests.yaml @@ -23,16 +23,6 @@ jobs: tool-cache: false docker-images: false - - name: Setup Go - uses: actions/setup-go@v5 - with: - go-version: '1.22' - - - name: Install tektor - if: steps.changed-files.outputs.any_changed == 'true' - run: | - go install github.com/lcarva/tektor@latest - - name: Checkout build-defintions Repository if: steps.changed-files.outputs.any_changed == 'true' uses: actions/checkout@v3 @@ -46,6 +36,7 @@ jobs: with: repository: 'konflux-ci/konflux-ci' path: konflux-ci + ref: f6d9d0fe8f34199eb118febcbf7f7944ae7772a9 - name: Create k8s Kind Cluster if: steps.changed-files.outputs.any_changed == 'true' @@ -68,13 +59,13 @@ jobs: - name: Wait for the dependencies to be ready if: steps.changed-files.outputs.any_changed == 'true' run: | - #cd $GITHUB_WORKSPACE/konflux-ci + cd $GITHUB_WORKSPACE/konflux-ci ./wait-for-all.sh - name: Deploying Konflux if: steps.changed-files.outputs.any_changed == 'true' run: | - #cd $GITHUB_WORKSPACE/konflux-ci + cd $GITHUB_WORKSPACE/konflux-ci ./deploy-konflux.sh - name: List namespaces @@ -85,7 +76,7 @@ jobs: - name: Deploy test resources if: steps.changed-files.outputs.any_changed == 'true' run: | - #cd $GITHUB_WORKSPACE/konflux-ci + cd $GITHUB_WORKSPACE/konflux-ci ./deploy-test-resources.sh - name: Run the task tests diff --git a/.github/workflows/task-lint.yaml b/.github/workflows/task-lint.yaml new file mode 100644 index 0000000000..f220c1e8b6 --- /dev/null +++ b/.github/workflows/task-lint.yaml @@ -0,0 +1,53 @@ +--- +name: tasklint +"on": + pull_request: + branches: [main] +jobs: + tasklint: + runs-on: ubuntu-latest + steps: + - name: Get all changed files in the PR from task directories + id: changed-files + uses: tj-actions/changed-files@v45 + with: + # Avoid using single or double quotes for multiline patterns + files: | + task/** + + - name: Setup Go + if: steps.changed-files.outputs.any_changed == 'true' + uses: actions/setup-go@v5 + with: + go-version: '1.22' + + - name: Install tektor + if: steps.changed-files.outputs.any_changed == 'true' + run: | + go install github.com/lcarva/tektor@4e9385885cc638fe337bcb9cc5f3d64cc8c262c5 + + - name: Run tektor linter + if: steps.changed-files.outputs.any_changed == 'true' + env: + CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }} + run: | + #!/bin/bash + echo "Files changed in PR: ${CHANGED_FILES}" + function print_changed_tasks_to_console() { + echo ${CHANGED_FILES} | grep -o 'task/[^\/]*/[^\/]*/*[^/]*.yaml' || true + } + function validate_task_yaml_using_tektor() { + all_tasks="$*" + for task in ${all_tasks}; do + # Skip if it is kustomize related yaml files + if [[ ${task} == *"kustomization.yaml" || ${task} == *"patch.yaml" || ${task} == *"recipe.yaml" ]]; then + continue + fi + tektor validate ${task} + done + } + # Validate task yamls using tektor linter + tasks_changed=$(print_changed_tasks_to_console) + [[ ! -z ${tasks_changed} ]] && { + validate_task_yaml_using_tektor "${tasks_changed}" + } diff --git a/README.md b/README.md index 78c0ca61c9..94acc1f0d1 100644 --- a/README.md +++ b/README.md @@ -132,6 +132,33 @@ Specify the Quay repository using the `QUAY_NAMESPACE` environment variable in t ./hack/test-shellspec.sh` ``` +## Testing Tasks + +Tasks can be validated in the GithubActions CI setup with this repository. + +If you have a directory called `tests/` inside the task directory, CI will create a random `Namespace`, apply the task first in the namespace and then all the test yaml files (containing TaskRun or PipelineRun) inside the `tests` directory. + +We can use the functionality of running scripts before applying the tested task or the other yaml files. There are two different scripts that are automatically applied if present. These are applied using the `source` bash script: + +- `pre-apply-task-hook.sh`: Script to run before applying the task, it can include `kubectl` commands to apply any kubernetes resources in the cluster +- `pre-apply-taskrun-hook.sh`: Script to run before applying the task run or pipeline run yaml files, it can also include `kubectl` commands to apply any kubernetes resources in the cluster + +For example refer: Git Clone task [tests](./task/git-clone/0.1/tests/run.yaml) + +CI also validates the task yamls using [tektor](https://github.com/lcarva/tektor) + +### Running Task tests locally + +Prerequisites: Deploy [upstream konflux](https://github.com/konflux-ci/konflux-ci?tab=readme-ov-file#bootstrapping-the-cluster) locally. + +A helper script called `run-test.sh` is present inside `test` directory to help the developer run the test. Just specify resource type as first argument, resource name as second argument and the version as third argument i.e: + +``` +./test/run-test.sh task git-clone 0.1 +``` + +and it will use your current kubernetes context to run the test and show you the outputs similar to the CI. + ### Compliance Task definitions must comply with the [Enterprise Contract](https://enterprisecontract.dev/) policies. diff --git a/hack/run-test.sh b/hack/run-test.sh deleted file mode 100644 index 91ccd3d206..0000000000 --- a/hack/run-test.sh +++ /dev/null @@ -1,59 +0,0 @@ -#!/bin/bash - -set -ex - -cd $(git rev-parse --show-toplevel) -source test/common.sh - -if [[ -z ${@} || ${1} == "-h" ]];then - cat </dev/null 2>/dev/null || : - -${KUBECTL_CMD} create namespace ${tns} - -#create appstudio-pipeline SA in the test namespace if not already -if ! ${KUBECTL_CMD} get sa appstudio-pipeline -n ${tns} | grep 'appstudio-pipeline'; then - $KUBECTL_CMD create sa appstudio-pipeline -n ${tns} -fi - -test_resource_creation ${RESOURCE}/${NAME}/${VERSION}/tests \ No newline at end of file diff --git a/task/git-clone/0.1/tests/pre-apply-taskrun-hook.sh b/task/git-clone/0.1/tests/pre-apply-taskrun-hook.sh index 232ece36d3..d01d25bb63 100644 --- a/task/git-clone/0.1/tests/pre-apply-taskrun-hook.sh +++ b/task/git-clone/0.1/tests/pre-apply-taskrun-hook.sh @@ -1,3 +1,3 @@ #!/bin/bash -echo "an example of pre-apply-taskrun-hook script" \ No newline at end of file +echo "an example of pre-apply-taskrun-hook script" diff --git a/test/common.sh b/test/common.sh index 6aa8455254..d68cf69a78 100644 --- a/test/common.sh +++ b/test/common.sh @@ -26,18 +26,14 @@ function set_test_return_code() { function detect_changed_e2e_test() { # check if any file from test/ directory is changed - echo ${CHANGED_FILES} |grep "test/" || true + echo ${CHANGED_FILES} | grep "test/" || true } -function detect_new_changed_resources() { +function print_changed_resources_to_console() { # detect for changes in tests dir of the task - echo ${CHANGED_FILES} |grep -o 'task/[^\/]*/[^\/]*/tests/[^/]*'|xargs -I {} dirname {}|sed 's/\(tests\).*/\1/g' + echo ${CHANGED_FILES} | grep -o 'task/[^\/]*/[^\/]*/tests/[^/]*'|xargs -I {} dirname {}|sed 's/\(tests\).*/\1/g' # detect for changes in the task manifest - echo ${CHANGED_FILES} |grep -o 'task/[^\/]*/[^\/]*/*[^/]*.yaml'|xargs -I {} dirname {}|awk '{print $1"/tests"}' -} - -function get_new_changed_tasks() { - echo ${CHANGED_FILES} |grep -o 'task/[^\/]*/[^\/]*/*[^/]*.yaml' || true + echo ${CHANGED_FILES} | grep -o 'task/[^\/]*/[^\/]*/*[^/]*.yaml'|xargs -I {} dirname {}|awk '{print $1"/tests"}' } # Signal (as return code and in the logs) that all E2E tests passed. @@ -50,35 +46,34 @@ function success() { } function show_failure() { - local testname=$1 tns=$2 + local testname=$1 test_namespace=$2 echo "FAILED: ${testname} task has failed to comeback properly" ; ${KUBECTL_CMD} api-resources - echo "Namespace: ${tns}" + echo "Namespace: ${test_namespace}" echo "--- TaskRun Dump" - ${KUBECTL_CMD} get --ignore-not-found=true -n ${tns} taskrun -o yaml + ${KUBECTL_CMD} get --ignore-not-found=true -n ${test_namespace} taskrun -o yaml echo "--- Task Dump" - ${KUBECTL_CMD} get --ignore-not-found=true -n ${tns} task -o yaml + ${KUBECTL_CMD} get --ignore-not-found=true -n ${test_namespace} task -o yaml echo "--- PipelineRun Dump" - ${KUBECTL_CMD} get --ignore-not-found=true -n ${tns} pipelinerun -o yaml + ${KUBECTL_CMD} get --ignore-not-found=true -n ${test_namespace} pipelinerun -o yaml echo "--- Pipeline Dump" - ${KUBECTL_CMD} get --ignore-not-found=true -n ${tns} pipeline -o yaml + ${KUBECTL_CMD} get --ignore-not-found=true -n ${test_namespace} pipeline -o yaml echo "--- StepAction Dump" - ${KUBECTL_CMD} get --ignore-not-found=true -n ${tns} stepaction -o yaml + ${KUBECTL_CMD} get --ignore-not-found=true -n ${test_namespace} stepaction -o yaml echo "--- Container Logs" - for pod in $(${KUBECTL_CMD} get pod -o name -n ${tns}); do + for pod in $(${KUBECTL_CMD} get pod -o name -n ${test_namespace}); do echo "----POD_NAME: ${pod}---" - ${KUBECTL_CMD} logs --all-containers -n ${tns} ${pod} || true + ${KUBECTL_CMD} logs --all-containers -n ${test_namespace} ${pod} || true done exit 1 - } function test_yaml_can_install() { # Validate that all the Task CRDs in this repo are valid by creating them in a NS. ns="test-yaml-ns" all_tasks="$*" - ${KUBECTL_CMD} create ns "${ns}" || true + ${KUBECTL_CMD} create namespace "${ns}" || true local runtest for runtest in ${all_tasks}; do @@ -94,7 +89,7 @@ function test_yaml_can_install() { runtest="${runtest}${testname}.yaml" skipit= - for ignore in ${TEST_YAML_IGNORES};do + for ignore in ${TEST_YAML_IGNORES}; do [[ ${ignore} == "${testname}" ]] && skipit=True done @@ -110,22 +105,11 @@ function test_yaml_can_install() { done } -function validate_task_yaml_using_tektor() { - all_tasks="$*" - for task in ${all_tasks}; do - # Skip if it is kustomize related yaml files - if [[ ${task} == *"kustomization.yaml" || ${task} == *"patch.yaml" || ${task} == *"recipe.yaml" ]]; then - continue - fi - tektor validate ${task} - done -} - -function test_resource_creation() { +function test_resource_creation_and_validation() { local runtest declare -A resource_to_wait_for - for runtest in $@;do + for runtest in $@; do # remove task/ from beginning local runtestdir=${runtest#*/} # remove //tests from end @@ -136,10 +120,10 @@ function test_resource_creation() { # replace . with - in version as not supported in namespace name version="$( echo $version | tr '.' '-' )" - local tns="${testname}-${version}" + local test_namespace="${testname}-${version}" local skipit= - for ignore in ${TEST_TASKRUN_IGNORES};do + for ignore in ${TEST_TASKRUN_IGNORES}; do [[ ${ignore} == ${testname} ]] && skipit=True done @@ -159,14 +143,14 @@ function test_resource_creation() { [[ -n ${skipit} ]] && continue - # Delete the tns if already exists - ${KUBECTL_CMD} delete ns ${tns} >/dev/null 2>/dev/null || : + # Delete the test_namespace if already exists + ${KUBECTL_CMD} delete namespace ${test_namespace} >/dev/null 2>/dev/null || : # create the test namespace - ${KUBECTL_CMD} create namespace ${tns} >/dev/null 2>/dev/null + ${KUBECTL_CMD} create namespace ${test_namespace} >/dev/null 2>/dev/null # create the service account appstudio-pipeline (konflux spedific requirement) - $KUBECTL_CMD create sa appstudio-pipeline -n ${tns} + $KUBECTL_CMD create sa appstudio-pipeline -n ${test_namespace} # Install the task itself first. We can only have one YAML file yaml=$(printf ${resourcedir}/*.yaml) @@ -175,7 +159,7 @@ function test_resource_creation() { # dry-run this YAML to validate and also get formatting side-effects. # TODO: need to add tekton linter here - ${KUBECTL_CMD} -n ${tns} create -f ${yaml} --dry-run=client -o yaml >${TMPF} + ${KUBECTL_CMD} -n ${test_namespace} create -f ${yaml} --dry-run=client -o yaml >${TMPF} [[ -f ${resourcedir}/tests/pre-apply-task-hook.sh ]] && source ${resourcedir}/tests/pre-apply-task-hook.sh function_exists pre-apply-task-hook && pre-apply-task-hook @@ -183,11 +167,11 @@ function test_resource_creation() { # Make sure we have deleted the content, this is in case of rerun # and namespace hasn't been cleaned up or there is some Cluster* # stuff, which really should not be allowed. - ${KUBECTL_CMD} -n ${tns} delete -f ${TMPF} >/dev/null 2>/dev/null || true - ${KUBECTL_CMD} -n ${tns} create -f ${TMPF} + ${KUBECTL_CMD} -n ${test_namespace} delete -f ${TMPF} >/dev/null 2>/dev/null || true + ${KUBECTL_CMD} -n ${test_namespace} create -f ${TMPF} # Install resource and run - for yaml in ${runtest}/*.yaml;do + for yaml in ${runtest}/*.yaml; do cp ${yaml} ${TMPF} [[ -f ${resourcedir}/tests/pre-apply-taskrun-hook.sh ]] && source ${resourcedir}/tests/pre-apply-taskrun-hook.sh function_exists pre-apply-taskrun-hook && pre-apply-taskrun-hook @@ -195,11 +179,11 @@ function test_resource_creation() { # Make sure we have deleted the content, this is in case of rerun # and namespace hasn't been cleaned up or there is some Cluster* # stuff, which really should not be allowed. - ${KUBECTL_CMD} -n ${tns} delete -f ${TMPF} >/dev/null 2>/dev/null || true - ${KUBECTL_CMD} -n ${tns} create -f ${TMPF} + ${KUBECTL_CMD} -n ${test_namespace} delete -f ${TMPF} >/dev/null 2>/dev/null || true + ${KUBECTL_CMD} -n ${test_namespace} create -f ${TMPF} done - resource_to_wait_for["$testname/${version}"]="${tns}|$started" + resource_to_wait_for["$testname/${version}"]="${test_namespace}|$started" done @@ -211,11 +195,11 @@ function test_resource_creation() { local maxloop=60 # 10 minutes max set +x - while true;do + while true; do # If we have timed out then show failures of what's remaining in # resource_to_wait_for we assume only first one fails this [[ ${cnt} == "${maxloop}" ]] && { - for testname in "${!resource_to_wait_for[@]}";do + for testname in "${!resource_to_wait_for[@]}"; do target_ns=${resource_to_wait_for[$testname]} show_failure "${testname}" "${target_ns}" done @@ -224,12 +208,12 @@ function test_resource_creation() { break } - for testname in "${!resource_to_wait_for[@]}";do + for testname in "${!resource_to_wait_for[@]}"; do target_ns=${resource_to_wait_for[$testname]%|*} started=${resource_to_wait_for[$testname]#*|} # sometimes we don't get all_status and reason in one go so # wait until we get the reason and all_status for 5 iterations - for tektontype in pipelinerun taskrun;do + for tektontype in pipelinerun taskrun; do for _ in {1..10}; do all_status=$(${KUBECTL_CMD} get -n ${target_ns} ${tektontype} --output=jsonpath='{.items[*].status.conditions[*].status}') reason=$(${KUBECTL_CMD} get -n ${target_ns} ${tektontype} --output=jsonpath='{.items[*].status.conditions[*].reason}') @@ -247,11 +231,11 @@ function test_resource_creation() { fi breakit=True - for status in ${all_status};do + for status in ${all_status}; do [[ ${status} == *ERROR || ${reason} == *Fail* || ${reason} == Couldnt* ]] && show_failure ${testname} ${target_ns} if [[ ${status} != True ]];then - breakit= + breakit=False fi done @@ -263,7 +247,7 @@ function test_resource_creation() { if [[ ${breakit} == True ]];then unset resource_to_wait_for[$testname] - [[ -z ${CATALOG_TEST_SKIP_CLEANUP} ]] && ${KUBECTL_CMD} delete ns ${target_ns} >/dev/null + [[ -z ${CATALOG_TEST_SKIP_CLEANUP} ]] && ${KUBECTL_CMD} delete namespace ${target_ns} >/dev/null echo "${started}::$(date '+%Hh%M:%S') SUCCESS: ${testname} testrun has successfully executed" ; fi @@ -273,4 +257,4 @@ function test_resource_creation() { cnt=$((cnt+1)) done set -x -} \ No newline at end of file +} diff --git a/test/run-test.sh b/test/run-test.sh index 921b8a3747..4b943b8e4d 100755 --- a/test/run-test.sh +++ b/test/run-test.sh @@ -44,4 +44,4 @@ if [[ ! -d ${resourcedir}/tests ]];then exit 1 fi -test_resource_creation ${RESOURCE}/${NAME}/${VERSION}/tests \ No newline at end of file +test_resource_creation_and_validation ${RESOURCE}/${NAME}/${VERSION}/tests diff --git a/test/test-tasks.sh b/test/test-tasks.sh index 893aa4208d..43df98a244 100755 --- a/test/test-tasks.sh +++ b/test/test-tasks.sh @@ -35,20 +35,13 @@ all_tests=$(echo task/*/*/tests) # Run only the tests related to the task modified in the PR if [[ -z ${TEST_RUN_ALL_TESTS} ]];then - all_tests=$(detect_new_changed_resources|sort -u || true) + all_tests=$(print_changed_resources_to_console|sort -u || true) [[ -z ${all_tests} ]] && { echo "No tests has been detected in this PR. exiting." success } fi -# Validate task yamls using tektor linter -tasks_changed=$(get_new_changed_tasks) -[[ ! -z ${tasks_changed} ]] && { - validate_task_yaml_using_tektor "${tasks_changed}" -} - - # Validate task yamls can be installed test_yaml_can_install "${all_tests}" @@ -59,7 +52,7 @@ function test_resources { for runtest in $@;do resource_to_tests="${resource_to_tests} ${runtest}" if [[ ${cnt} == "${MAX_NUMBERS_OF_PARALLEL_TASKS}" ]];then - test_resource_creation "${resource_to_tests}" + test_resource_creation_and_validation "${resource_to_tests}" cnt=0 resource_to_tests="" continue @@ -69,10 +62,10 @@ function test_resources { # in case if there are some remaining resources if [[ -n ${resource_to_tests} ]];then - test_resource_creation "${resource_to_tests}" + test_resource_creation_and_validation "${resource_to_tests}" fi } test_resources "${all_tests}" -success \ No newline at end of file +success