Fail upgrade tests when test script fails #6871
Conversation
Prior to this commit, if any lines in the upgrade testing script that didn't have special error handling failed, the script would continue to run, making it very hard to debug. This commit fails the script if any lines fail, and doesn't prevent lines with special error handling (i.e. `fail_test`) from performing their own cleanup steps. In addition, prior to this commit, lines deleting Tekton CRDs were run after the pipelines webhook and CRD definitions were deleted. This commit moves deletion of resources to happen before Tekton is uninstalled, so deletion can happen successfully. Lastly, this commit handles cases where the variable PIPELINE_FEATURE_GATE is not set, and causes the script to fail if any unbound variables are referenced.
tekton-robot
added
kind/bug
tekton-robot
added
the
size/S
| # Make sure that everything is cleaned up in the current namespace. | ||
| delete_pipeline_resources |
There was a problem hiding this comment.
I wonder if it is the same if we move this line before line 144 instead of moving out of this function?
There was a problem hiding this comment.
I'd prefer to decouple deleting the CRDs from uninstalling pipelines. It's doesn't make sense to me that uninstall_pipeline_crd should delete resources but uninstall_pipeline_crd_version shouldn't, but we can't have them both delete resources since we call uninstall_pipeline_crd and then uninstall_pipeline_crd_version right after (https://github.com/tektoncd/pipeline/blob/main/test/e2e-tests-upgrade.sh#L61-L62). IMO it's better to just let the calling script handle deletion since the calling script is what created the resources in the first place.
| } | ||
|
|
||
| function uninstall_pipeline_crd_version() { | ||
| echo ">> Uninstalling Tekton Pipelines of version $1" | ||
| kubectl delete --ignore-not-found=true -f "https://github.com/tektoncd/pipeline/releases/download/$1/release.yaml" | ||
|
|
||
| if [ "${PIPELINE_FEATURE_GATE}" == "alpha" ]; then | ||
| if [ -v ${PIPELINE_FEATURE_GATE+x} && "${PIPELINE_FEATURE_GATE}" == "alpha" ]; then |
There was a problem hiding this comment.
can this be instead? it seems "+x" is not needed in bash 4.3+?
| if [ -v ${PIPELINE_FEATURE_GATE+x} && "${PIPELINE_FEATURE_GATE}" == "alpha" ]; then | |
| if [ -v ${PIPELINE_FEATURE_GATE} && "${PIPELINE_FEATURE_GATE}" == "alpha" ]; then |
There was a problem hiding this comment.
good question; can you point me to where in that link you're seeing this? (I'm looking at this section.) I poked through our test runner dockerfile but there's not really a good way for me to tell what version of bash we are using, so I'd prefer to keep it. (Not sure how to go from hash-> dockerfile.)
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dibyom, Yongxuanzhang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
approved
Prior to this commit, if any lines in the upgrade testing script that didn't have special error handling failed, the script would continue to run, making it very hard to debug.
This commit fails the script if any lines fail, and doesn't prevent lines with special error handling (i.e.
fail_test) from performing their own cleanup steps.In addition, prior to this commit, lines deleting Tekton CRDs were run after the pipelines webhook and CRD definitions were deleted. This commit moves deletion of resources to happen before Tekton is uninstalled, so deletion can happen successfully.
Lastly, this commit handles cases where the variable PIPELINE_FEATURE_GATE is not set, and causes the script to fail if any unbound variables are referenced.
/kind bug
closes #6869
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes