From 32c736b95008ef3c96b0dee3700fb8f9f14cda89 Mon Sep 17 00:00:00 2001 From: Gustavo Lira Date: Wed, 4 Mar 2026 16:46:17 -0300 Subject: [PATCH 1/8] refactor(ci): encapsulate deployment tracking into lib/deployment.sh Replace fragile CURRENT_DEPLOYMENT global counter with a dedicated deployment module that provides a clean API (deployment::register, deployment::mark_deploy_success, deployment::mark_deploy_failed, deployment::mark_test_result). This eliminates manual counter manipulation scattered across utils.sh and lib/testing.sh. Co-Authored-By: Claude Opus 4.6 --- .ci/pipelines/lib/deployment.sh | 59 +++++++++++++++++++++++++++++++++ .ci/pipelines/lib/testing.sh | 41 ++++++++++------------- .ci/pipelines/reporting.sh | 3 +- .ci/pipelines/utils.sh | 3 +- 4 files changed, 79 insertions(+), 27 deletions(-) create mode 100644 .ci/pipelines/lib/deployment.sh diff --git a/.ci/pipelines/lib/deployment.sh b/.ci/pipelines/lib/deployment.sh new file mode 100644 index 0000000000..df6b25ed5f --- /dev/null +++ b/.ci/pipelines/lib/deployment.sh @@ -0,0 +1,59 @@ +#!/bin/bash + +# Prevent sourcing multiple times in the same shell. +if [[ -n "${RHDH_DEPLOYMENT_LIB_SOURCED:-}" ]]; then + return 0 +fi +readonly RHDH_DEPLOYMENT_LIB_SOURCED=1 + +# Internal state +_DEPLOYMENT_COUNTER=0 + +deployment::next_id() { + _DEPLOYMENT_COUNTER=$((_DEPLOYMENT_COUNTER + 1)) + echo "${_DEPLOYMENT_COUNTER}" +} + +deployment::current_id() { + echo "${_DEPLOYMENT_COUNTER}" +} + +deployment::register() { + local namespace="$1" + deployment::next_id > /dev/null + save_status_deployment_namespace "${_DEPLOYMENT_COUNTER}" "$namespace" +} + +deployment::mark_deploy_success() { + save_status_failed_to_deploy "${_DEPLOYMENT_COUNTER}" false +} + +deployment::mark_deploy_failed() { + local namespace="$1" + deployment::register "$namespace" + save_status_failed_to_deploy "${_DEPLOYMENT_COUNTER}" true + save_status_test_failed "${_DEPLOYMENT_COUNTER}" true + save_overall_result 1 +} + +deployment::mark_test_result() { + local passed="$1" + local num_failures="${2:-}" + if [[ "$passed" == "true" ]]; then + save_status_test_failed "${_DEPLOYMENT_COUNTER}" false + else + save_status_test_failed "${_DEPLOYMENT_COUNTER}" true + fi + if [[ -n "$num_failures" ]]; then + save_status_number_of_test_failed "${_DEPLOYMENT_COUNTER}" "$num_failures" + fi +} + +# Export all functions and state for subshell compatibility +export _DEPLOYMENT_COUNTER +export -f deployment::next_id +export -f deployment::current_id +export -f deployment::register +export -f deployment::mark_deploy_success +export -f deployment::mark_deploy_failed +export -f deployment::mark_test_result diff --git a/.ci/pipelines/lib/testing.sh b/.ci/pipelines/lib/testing.sh index 47fe626248..e2c1f95464 100644 --- a/.ci/pipelines/lib/testing.sh +++ b/.ci/pipelines/lib/testing.sh @@ -33,7 +33,7 @@ readonly _TESTING_ERR_MISSING_PARAMS="Missing required parameters" # Returns: # 0 - Tests passed # Non-zero - Tests failed -# Uses globals: CURRENT_DEPLOYMENT, DIR, TAG_NAME, ARTIFACT_DIR, LOGFILE, JUNIT_RESULTS, CI, SHARED_DIR +# Uses globals: DIR, TAG_NAME, ARTIFACT_DIR, LOGFILE, JUNIT_RESULTS, CI, SHARED_DIR testing::run_tests() { local release_name=$1 local namespace=$2 @@ -47,9 +47,8 @@ testing::run_tests() { return 1 fi - CURRENT_DEPLOYMENT=$((CURRENT_DEPLOYMENT + 1)) - save_status_deployment_namespace $CURRENT_DEPLOYMENT "$artifacts_subdir" - save_status_failed_to_deploy $CURRENT_DEPLOYMENT false + deployment::register "$namespace" + deployment::mark_deploy_success BASE_URL="${url}" export BASE_URL @@ -102,23 +101,25 @@ testing::run_tests() { echo "Playwright project '${playwright_project}' in namespace '${namespace}' (artifacts: ${artifacts_subdir}) RESULT: ${test_result}" if [[ "${test_result}" -ne 0 ]]; then save_overall_result 1 - save_status_test_failed $CURRENT_DEPLOYMENT true + deployment::mark_test_result false else - save_status_test_failed $CURRENT_DEPLOYMENT false + deployment::mark_test_result true fi # Use Playwright exit code as source of truth: flaky tests (failed initially # but passed on retry) report failures in JUnit XML even though they passed. # When test_result is 0, all tests ultimately passed — report 0 failures. if [[ "${test_result}" -eq 0 ]]; then - save_status_number_of_test_failed $CURRENT_DEPLOYMENT "0" + save_status_number_of_test_failed "$(deployment::current_id)" "0" elif [[ -f "${e2e_tests_dir}/${JUNIT_RESULTS}" ]]; then local failed_tests failed_tests=$(grep -oP 'failures="\K[0-9]+' "${e2e_tests_dir}/${JUNIT_RESULTS}" | head -n 1) echo "Number of failed tests: ${failed_tests}" - save_status_number_of_test_failed $CURRENT_DEPLOYMENT "${failed_tests:-some}" + save_status_number_of_test_failed "$(deployment::current_id)" "${failed_tests}" else echo "JUnit results file not found: ${e2e_tests_dir}/${JUNIT_RESULTS}" - save_status_number_of_test_failed $CURRENT_DEPLOYMENT "some" + local failed_tests="some" + echo "Number of failed tests unknown, saving as $failed_tests." + save_status_number_of_test_failed "$(deployment::current_id)" "${failed_tests}" fi return 0 } @@ -217,7 +218,7 @@ testing::check_backstage_running() { # $5 - max_attempts: (optional) Maximum number of attempts (default: 30) # $6 - wait_seconds: (optional) Seconds to wait between attempts (default: 30) # $7 - artifacts_subdir: (optional) Subdirectory for artifacts (defaults to namespace) -# Uses globals: CURRENT_DEPLOYMENT, SKIP_TESTS +# Uses globals: SKIP_TESTS testing::check_and_test() { local release_name=$1 local namespace=$2 @@ -243,16 +244,13 @@ testing::check_and_test() { fi else echo "Backstage is not running. Marking deployment as failed and continuing..." - CURRENT_DEPLOYMENT=$((CURRENT_DEPLOYMENT + 1)) - save_status_deployment_namespace $CURRENT_DEPLOYMENT "$artifacts_subdir" - save_status_failed_to_deploy $CURRENT_DEPLOYMENT true - save_status_test_failed $CURRENT_DEPLOYMENT true - save_status_number_of_test_failed $CURRENT_DEPLOYMENT "0" - save_overall_result 1 + deployment::mark_deploy_failed "$namespace" fi # Collect pod logs only on failure to speed up successful PR runs. - if [[ "${STATUS_TEST_FAILED[$CURRENT_DEPLOYMENT]:-}" == "true" || "${STATUS_FAILED_TO_DEPLOY[$CURRENT_DEPLOYMENT]:-}" == "true" ]]; then + local _current_id + _current_id="$(deployment::current_id)" + if [[ "${STATUS_TEST_FAILED[$_current_id]:-}" == "true" || "${STATUS_FAILED_TO_DEPLOY[$_current_id]:-}" == "true" ]]; then save_all_pod_logs "$namespace" else log::info "Tests passed — skipping pod log collection for namespace: ${namespace}" @@ -302,7 +300,7 @@ testing::check_helm_upgrade() { # $4 - playwright_project: The Playwright project to run # $5 - url: The URL to test against # $6 - timeout: (optional) Timeout in seconds (default: 600) -# Uses globals: CURRENT_DEPLOYMENT +# Uses globals: none (deployment state managed by lib/deployment.sh) testing::check_upgrade_and_test() { local deployment_name="$1" local release_name="$2" @@ -321,12 +319,7 @@ testing::check_upgrade_and_test() { testing::check_and_test "${release_name}" "${namespace}" "${playwright_project}" "${url}" else log::error "Helm upgrade encountered an issue or timed out. Exiting..." - CURRENT_DEPLOYMENT=$((CURRENT_DEPLOYMENT + 1)) - save_status_deployment_namespace $CURRENT_DEPLOYMENT "$namespace" - save_status_failed_to_deploy $CURRENT_DEPLOYMENT true - save_status_test_failed $CURRENT_DEPLOYMENT true - save_status_number_of_test_failed $CURRENT_DEPLOYMENT "0" - save_overall_result 1 + deployment::mark_deploy_failed "$namespace" fi return 0 } diff --git a/.ci/pipelines/reporting.sh b/.ci/pipelines/reporting.sh index 692bc45732..df75a5b01d 100644 --- a/.ci/pipelines/reporting.sh +++ b/.ci/pipelines/reporting.sh @@ -2,9 +2,10 @@ # shellcheck source=.ci/pipelines/lib/log.sh source "$(dirname "${BASH_SOURCE[0]}")"/lib/log.sh +# shellcheck source=.ci/pipelines/lib/deployment.sh +source "$(dirname "${BASH_SOURCE[0]}")"/lib/deployment.sh # Variables for reporting -export CURRENT_DEPLOYMENT=0 # Counter for current deployment. export STATUS_DEPLOYMENT_NAMESPACE # Array that holds the namespaces of deployments. export STATUS_FAILED_TO_DEPLOY # Array that indicates if deployment failed. false = success, true = failure export STATUS_TEST_FAILED # Array that indicates if test run failed. false = success, true = failure diff --git a/.ci/pipelines/utils.sh b/.ci/pipelines/utils.sh index 9d07fe9194..26d4a3bd06 100755 --- a/.ci/pipelines/utils.sh +++ b/.ci/pipelines/utils.sh @@ -682,8 +682,7 @@ initiate_upgrade_base_deployments() { log::info "Initiating base RHDH deployment before upgrade" - CURRENT_DEPLOYMENT=$((CURRENT_DEPLOYMENT + 1)) - save_status_deployment_namespace $CURRENT_DEPLOYMENT "$namespace" + deployment::register "$namespace" namespace::configure "${namespace}" From c1b612038f540f64c8595a791b143623d3a542a9 Mon Sep 17 00:00:00 2001 From: Gustavo Lira Date: Wed, 4 Mar 2026 16:48:39 -0300 Subject: [PATCH 2/8] fix(ci): address review findings in deployment tracking module - Remove misleading `export _DEPLOYMENT_COUNTER` (subshells can't see parent counter updates anyway) - Source deployment.sh directly in testing.sh so job files that skip utils.sh/reporting.sh still have deployment::* available - Consolidate test result reporting into a single deployment::mark_test_result call instead of mixing old and new APIs - Add deployment::mark_deploy_success in initiate_upgrade_base_deployments to record deploy status consistently Co-Authored-By: Claude Opus 4.6 --- .ci/pipelines/lib/deployment.sh | 5 +++-- .ci/pipelines/lib/testing.sh | 16 ++++++++-------- .ci/pipelines/utils.sh | 1 + 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/.ci/pipelines/lib/deployment.sh b/.ci/pipelines/lib/deployment.sh index df6b25ed5f..c940161dc4 100644 --- a/.ci/pipelines/lib/deployment.sh +++ b/.ci/pipelines/lib/deployment.sh @@ -49,8 +49,9 @@ deployment::mark_test_result() { fi } -# Export all functions and state for subshell compatibility -export _DEPLOYMENT_COUNTER +# Export all functions for subshell compatibility. +# Note: _DEPLOYMENT_COUNTER is NOT exported because subshells inherit only +# the snapshot at fork time — counter updates in the parent would not propagate. export -f deployment::next_id export -f deployment::current_id export -f deployment::register diff --git a/.ci/pipelines/lib/testing.sh b/.ci/pipelines/lib/testing.sh index e2c1f95464..81a59248bb 100644 --- a/.ci/pipelines/lib/testing.sh +++ b/.ci/pipelines/lib/testing.sh @@ -12,6 +12,8 @@ readonly TESTING_LIB_SOURCED=1 # shellcheck source=.ci/pipelines/lib/log.sh source "${DIR}/lib/log.sh" +# shellcheck source=.ci/pipelines/lib/deployment.sh +source "${DIR}/lib/deployment.sh" # ============================================================================== # Constants @@ -99,28 +101,26 @@ testing::run_tests() { rsync -a "${e2e_tests_dir}/playwright-report/" "${ARTIFACT_DIR}/${artifacts_subdir}/" || true echo "Playwright project '${playwright_project}' in namespace '${namespace}' (artifacts: ${artifacts_subdir}) RESULT: ${test_result}" + local test_passed="true" if [[ "${test_result}" -ne 0 ]]; then save_overall_result 1 - deployment::mark_test_result false - else - deployment::mark_test_result true + test_passed="false" fi # Use Playwright exit code as source of truth: flaky tests (failed initially # but passed on retry) report failures in JUnit XML even though they passed. # When test_result is 0, all tests ultimately passed — report 0 failures. + local failed_tests if [[ "${test_result}" -eq 0 ]]; then - save_status_number_of_test_failed "$(deployment::current_id)" "0" + failed_tests="0" elif [[ -f "${e2e_tests_dir}/${JUNIT_RESULTS}" ]]; then - local failed_tests failed_tests=$(grep -oP 'failures="\K[0-9]+' "${e2e_tests_dir}/${JUNIT_RESULTS}" | head -n 1) echo "Number of failed tests: ${failed_tests}" - save_status_number_of_test_failed "$(deployment::current_id)" "${failed_tests}" else echo "JUnit results file not found: ${e2e_tests_dir}/${JUNIT_RESULTS}" - local failed_tests="some" + failed_tests="some" echo "Number of failed tests unknown, saving as $failed_tests." - save_status_number_of_test_failed "$(deployment::current_id)" "${failed_tests}" fi + deployment::mark_test_result "$test_passed" "${failed_tests}" return 0 } diff --git a/.ci/pipelines/utils.sh b/.ci/pipelines/utils.sh index 26d4a3bd06..66e2d60e1c 100755 --- a/.ci/pipelines/utils.sh +++ b/.ci/pipelines/utils.sh @@ -683,6 +683,7 @@ initiate_upgrade_base_deployments() { log::info "Initiating base RHDH deployment before upgrade" deployment::register "$namespace" + deployment::mark_deploy_success namespace::configure "${namespace}" From ba24a275473c5256a9cefa5ca394ad4001c80654 Mon Sep 17 00:00:00 2001 From: Gustavo Lira Date: Wed, 4 Mar 2026 17:09:23 -0300 Subject: [PATCH 3/8] fix(ci): add error handling, remove dead code, and fix quoting in pipeline scripts - Fix silent failures: cert extraction pipeline, helm chart version resolution, curl without -f flag, and http_status comparison - Add error checks to all helm upgrade commands in EKS/GKE/AKS deployments - Add common::require_vars() for envsubst variable validation - Remove dead functions: namespace::remove_finalizers, operator::install_postgres_k8s - Export common::base64_encode for subshell usage - Fix unquoted variables in kubectl commands and sleep Co-Authored-By: Claude Opus 4.6 --- .../cluster/aks/aks-helm-deployment.sh | 16 ++++++++--- .../cluster/eks/eks-helm-deployment.sh | 16 ++++++++--- .../cluster/gke/gke-helm-deployment.sh | 16 ++++++++--- .ci/pipelines/install-methods/operator.sh | 5 +++- .ci/pipelines/lib/common.sh | 16 +++++++++++ .ci/pipelines/lib/helm.sh | 9 ++++-- .ci/pipelines/lib/namespace.sh | 28 +------------------ .ci/pipelines/lib/operators.sh | 7 ----- .ci/pipelines/lib/testing.sh | 2 +- .ci/pipelines/utils.sh | 6 ++-- 10 files changed, 68 insertions(+), 53 deletions(-) diff --git a/.ci/pipelines/cluster/aks/aks-helm-deployment.sh b/.ci/pipelines/cluster/aks/aks-helm-deployment.sh index ea580ced98..da89a765ff 100644 --- a/.ci/pipelines/cluster/aks/aks-helm-deployment.sh +++ b/.ci/pipelines/cluster/aks/aks-helm-deployment.sh @@ -24,13 +24,17 @@ initiate_aks_helm_deployment() { namespace::setup_image_pull_secret "${NAME_SPACE}" "rh-pull-secret" "${REGISTRY_REDHAT_IO_SERVICE_ACCOUNT_DOCKERCONFIGJSON}" + common::require_vars "RELEASE_NAME" "TAG_NAME" "QUAY_REPO" "K8S_CLUSTER_ROUTER_BASE" || return 1 log::info "Deploying image from repository: ${QUAY_REPO}, TAG_NAME: ${TAG_NAME}, in NAME_SPACE: ${NAME_SPACE}" - helm upgrade -i "${RELEASE_NAME}" -n "${NAME_SPACE}" \ + if ! helm upgrade -i "${RELEASE_NAME}" -n "${NAME_SPACE}" \ "${HELM_CHART_URL}" --version "${CHART_VERSION}" \ -f "/tmp/${HELM_CHART_K8S_MERGED_VALUE_FILE_NAME}" \ --set global.host="${K8S_CLUSTER_ROUTER_BASE}" \ --set upstream.backstage.image.repository="${QUAY_REPO}" \ - --set upstream.backstage.image.tag="${TAG_NAME}" + --set upstream.backstage.image.tag="${TAG_NAME}"; then + log::error "Helm upgrade failed for ${RELEASE_NAME} in ${NAME_SPACE}" + return 1 + fi } initiate_rbac_aks_helm_deployment() { @@ -47,11 +51,15 @@ initiate_rbac_aks_helm_deployment() { namespace::setup_image_pull_secret "${NAME_SPACE_RBAC}" "rh-pull-secret" "${REGISTRY_REDHAT_IO_SERVICE_ACCOUNT_DOCKERCONFIGJSON}" + common::require_vars "RELEASE_NAME_RBAC" "TAG_NAME" "QUAY_REPO" "K8S_CLUSTER_ROUTER_BASE" || return 1 log::info "Deploying image from repository: ${QUAY_REPO}, TAG_NAME: ${TAG_NAME}, in NAME_SPACE: ${NAME_SPACE_RBAC}" - helm upgrade -i "${RELEASE_NAME_RBAC}" -n "${NAME_SPACE_RBAC}" \ + if ! helm upgrade -i "${RELEASE_NAME_RBAC}" -n "${NAME_SPACE_RBAC}" \ "${HELM_CHART_URL}" --version "${CHART_VERSION}" \ -f "/tmp/${HELM_CHART_RBAC_K8S_MERGED_VALUE_FILE_NAME}" \ --set global.host="${K8S_CLUSTER_ROUTER_BASE}" \ --set upstream.backstage.image.repository="${QUAY_REPO}" \ - --set upstream.backstage.image.tag="${TAG_NAME}" + --set upstream.backstage.image.tag="${TAG_NAME}"; then + log::error "Helm upgrade failed for ${RELEASE_NAME_RBAC} in ${NAME_SPACE_RBAC}" + return 1 + fi } diff --git a/.ci/pipelines/cluster/eks/eks-helm-deployment.sh b/.ci/pipelines/cluster/eks/eks-helm-deployment.sh index 88e91cdf6e..0eac83e112 100644 --- a/.ci/pipelines/cluster/eks/eks-helm-deployment.sh +++ b/.ci/pipelines/cluster/eks/eks-helm-deployment.sh @@ -20,16 +20,20 @@ initiate_eks_helm_deployment() { local rhdh_base_url="https://${K8S_CLUSTER_ROUTER_BASE}" apply_yaml_files "${DIR}" "${NAME_SPACE}" "${rhdh_base_url}" + common::require_vars "RELEASE_NAME" "TAG_NAME" "QUAY_REPO" "K8S_CLUSTER_ROUTER_BASE" || return 1 envsubst < "${DIR}/value_files/${HELM_CHART_EKS_DIFF_VALUE_FILE_NAME}" > "/tmp/${HELM_CHART_EKS_DIFF_VALUE_FILE_NAME}" helm::merge_values "merge" "${DIR}/value_files/${HELM_CHART_VALUE_FILE_NAME}" "/tmp/${HELM_CHART_EKS_DIFF_VALUE_FILE_NAME}" "/tmp/${HELM_CHART_K8S_MERGED_VALUE_FILE_NAME}" common::save_artifact "${NAME_SPACE}" "/tmp/${HELM_CHART_K8S_MERGED_VALUE_FILE_NAME}" # Save the final value-file into the artifacts directory. log::info "Deploying image from repository: ${QUAY_REPO}, TAG_NAME: ${TAG_NAME}, in NAME_SPACE: ${NAME_SPACE}" - helm upgrade -i "${RELEASE_NAME}" -n "${NAME_SPACE}" \ + if ! helm upgrade -i "${RELEASE_NAME}" -n "${NAME_SPACE}" \ "${HELM_CHART_URL}" --version "${CHART_VERSION}" \ -f "/tmp/${HELM_CHART_K8S_MERGED_VALUE_FILE_NAME}" \ --set global.host="${K8S_CLUSTER_ROUTER_BASE}" \ --set upstream.backstage.image.repository="${QUAY_REPO}" \ - --set upstream.backstage.image.tag="${TAG_NAME}" + --set upstream.backstage.image.tag="${TAG_NAME}"; then + log::error "Helm upgrade failed for ${RELEASE_NAME} in ${NAME_SPACE}" + return 1 + fi } initiate_rbac_eks_helm_deployment() { @@ -46,14 +50,18 @@ initiate_rbac_eks_helm_deployment() { local rbac_rhdh_base_url="https://${K8S_CLUSTER_ROUTER_BASE}" apply_yaml_files "${DIR}" "${NAME_SPACE_RBAC}" "${rbac_rhdh_base_url}" + common::require_vars "RELEASE_NAME_RBAC" "TAG_NAME" "QUAY_REPO" "K8S_CLUSTER_ROUTER_BASE" || return 1 envsubst < "${DIR}/value_files/${HELM_CHART_RBAC_EKS_DIFF_VALUE_FILE_NAME}" > "/tmp/${HELM_CHART_RBAC_EKS_DIFF_VALUE_FILE_NAME}" helm::merge_values "merge" "${DIR}/value_files/${HELM_CHART_RBAC_VALUE_FILE_NAME}" "/tmp/${HELM_CHART_RBAC_EKS_DIFF_VALUE_FILE_NAME}" "/tmp/${HELM_CHART_RBAC_K8S_MERGED_VALUE_FILE_NAME}" common::save_artifact "${NAME_SPACE_RBAC}" "/tmp/${HELM_CHART_RBAC_K8S_MERGED_VALUE_FILE_NAME}" # Save the final value-file into the artifacts directory. log::info "Deploying image from repository: ${QUAY_REPO}, TAG_NAME: ${TAG_NAME}, in NAME_SPACE: ${NAME_SPACE_RBAC}" - helm upgrade -i "${RELEASE_NAME_RBAC}" -n "${NAME_SPACE_RBAC}" \ + if ! helm upgrade -i "${RELEASE_NAME_RBAC}" -n "${NAME_SPACE_RBAC}" \ "${HELM_CHART_URL}" --version "${CHART_VERSION}" \ -f "/tmp/${HELM_CHART_RBAC_K8S_MERGED_VALUE_FILE_NAME}" \ --set global.host="${K8S_CLUSTER_ROUTER_BASE}" \ --set upstream.backstage.image.repository="${QUAY_REPO}" \ - --set upstream.backstage.image.tag="${TAG_NAME}" + --set upstream.backstage.image.tag="${TAG_NAME}"; then + log::error "Helm upgrade failed for ${RELEASE_NAME_RBAC} in ${NAME_SPACE_RBAC}" + return 1 + fi } diff --git a/.ci/pipelines/cluster/gke/gke-helm-deployment.sh b/.ci/pipelines/cluster/gke/gke-helm-deployment.sh index 19fe83de4e..ebf6b6ba64 100644 --- a/.ci/pipelines/cluster/gke/gke-helm-deployment.sh +++ b/.ci/pipelines/cluster/gke/gke-helm-deployment.sh @@ -26,14 +26,18 @@ initiate_gke_helm_deployment() { namespace::setup_image_pull_secret "${NAME_SPACE}" "rh-pull-secret" "${REGISTRY_REDHAT_IO_SERVICE_ACCOUNT_DOCKERCONFIGJSON}" + common::require_vars "RELEASE_NAME" "TAG_NAME" "QUAY_REPO" "K8S_CLUSTER_ROUTER_BASE" "GKE_CERT_NAME" || return 1 log::info "Deploying image from repository: ${QUAY_REPO}, TAG_NAME: ${TAG_NAME}, in NAME_SPACE: ${NAME_SPACE}" - helm upgrade -i "${RELEASE_NAME}" -n "${NAME_SPACE}" \ + if ! helm upgrade -i "${RELEASE_NAME}" -n "${NAME_SPACE}" \ "${HELM_CHART_URL}" --version "${CHART_VERSION}" \ -f "/tmp/${HELM_CHART_K8S_MERGED_VALUE_FILE_NAME}" \ --set global.host="${K8S_CLUSTER_ROUTER_BASE}" \ --set upstream.backstage.image.repository="${QUAY_REPO}" \ --set upstream.backstage.image.tag="${TAG_NAME}" \ - --set upstream.ingress.annotations."ingress\.gcp\.kubernetes\.io/pre-shared-cert"="${GKE_CERT_NAME}" + --set upstream.ingress.annotations."ingress\.gcp\.kubernetes\.io/pre-shared-cert"="${GKE_CERT_NAME}"; then + log::error "Helm upgrade failed for ${RELEASE_NAME} in ${NAME_SPACE}" + return 1 + fi } initiate_rbac_gke_helm_deployment() { @@ -50,12 +54,16 @@ initiate_rbac_gke_helm_deployment() { common::save_artifact "${NAME_SPACE_RBAC}" "/tmp/${HELM_CHART_RBAC_K8S_MERGED_VALUE_FILE_NAME}" namespace::setup_image_pull_secret "${NAME_SPACE_RBAC}" "rh-pull-secret" "${REGISTRY_REDHAT_IO_SERVICE_ACCOUNT_DOCKERCONFIGJSON}" + common::require_vars "RELEASE_NAME_RBAC" "TAG_NAME" "QUAY_REPO" "K8S_CLUSTER_ROUTER_BASE" "GKE_CERT_NAME" || return 1 log::info "Deploying image from repository: ${QUAY_REPO}, TAG_NAME: ${TAG_NAME}, in NAME_SPACE: ${NAME_SPACE_RBAC}" - helm upgrade -i "${RELEASE_NAME_RBAC}" -n "${NAME_SPACE_RBAC}" \ + if ! helm upgrade -i "${RELEASE_NAME_RBAC}" -n "${NAME_SPACE_RBAC}" \ "${HELM_CHART_URL}" --version "${CHART_VERSION}" \ -f "/tmp/${HELM_CHART_RBAC_K8S_MERGED_VALUE_FILE_NAME}" \ --set global.host="${K8S_CLUSTER_ROUTER_BASE}" \ --set upstream.backstage.image.repository="${QUAY_REPO}" \ --set upstream.backstage.image.tag="${TAG_NAME}" \ - --set upstream.ingress.annotations."ingress\.gcp\.kubernetes\.io/pre-shared-cert"="${GKE_CERT_NAME}" + --set upstream.ingress.annotations."ingress\.gcp\.kubernetes\.io/pre-shared-cert"="${GKE_CERT_NAME}"; then + log::error "Helm upgrade failed for ${RELEASE_NAME_RBAC} in ${NAME_SPACE_RBAC}" + return 1 + fi } diff --git a/.ci/pipelines/install-methods/operator.sh b/.ci/pipelines/install-methods/operator.sh index 7d5b013276..eeab4717a0 100755 --- a/.ci/pipelines/install-methods/operator.sh +++ b/.ci/pipelines/install-methods/operator.sh @@ -16,7 +16,10 @@ install_rhdh_operator() { fi # Make sure script is up to date rm -f /tmp/install-rhdh-catalog-source.sh - curl -L "https://raw.githubusercontent.com/redhat-developer/rhdh-operator/refs/heads/${RELEASE_BRANCH_NAME}/.rhdh/scripts/install-rhdh-catalog-source.sh" > /tmp/install-rhdh-catalog-source.sh + if ! curl -fL -o /tmp/install-rhdh-catalog-source.sh "https://raw.githubusercontent.com/redhat-developer/rhdh-operator/refs/heads/${RELEASE_BRANCH_NAME}/.rhdh/scripts/install-rhdh-catalog-source.sh"; then + log::error "Failed to download install-rhdh-catalog-source.sh from branch ${RELEASE_BRANCH_NAME}" + return 1 + fi chmod +x /tmp/install-rhdh-catalog-source.sh if [[ "$RELEASE_BRANCH_NAME" == "main" ]]; then diff --git a/.ci/pipelines/lib/common.sh b/.ci/pipelines/lib/common.sh index 071641e1a7..d1c7e673d9 100644 --- a/.ci/pipelines/lib/common.sh +++ b/.ci/pipelines/lib/common.sh @@ -127,6 +127,18 @@ common::create_configmap_from_files() { --dry-run=client -o yaml | oc apply -f - } +# Validate that required variables are set and non-empty +# Args: variable_names... +# Returns: 1 if any variable is unset or empty +common::require_vars() { + for var in "$@"; do + if [[ -z "${!var:-}" ]]; then + log::error "Required variable $var is not set" + return 1 + fi + done +} + # Base64 encode a string (no newlines, cross-platform) common::base64_encode() { echo -n "$1" | base64 | tr -d '\n' @@ -172,3 +184,7 @@ common::save_artifact() { mkdir -p "${ARTIFACT_DIR}/${namespace}" rsync -a "$file" "${ARTIFACT_DIR}/${namespace}/" } + +# Export functions for subshell usage (e.g., timeout bash -c "...") +export -f common::base64_encode +export -f common::require_vars diff --git a/.ci/pipelines/lib/helm.sh b/.ci/pipelines/lib/helm.sh index c2921c3c72..4c660e861c 100644 --- a/.ci/pipelines/lib/helm.sh +++ b/.ci/pipelines/lib/helm.sh @@ -171,9 +171,14 @@ helm::get_chart_version() { return 1 fi - curl -sSX GET "https://quay.io/api/v1/repository/rhdh/chart/tag/?onlyActiveTags=true&filter_tag_name=like:${chart_major_version}-" \ + local version + version=$(curl -sSfX GET "https://quay.io/api/v1/repository/rhdh/chart/tag/?onlyActiveTags=true&filter_tag_name=like:${chart_major_version}-" \ -H "Content-Type: application/json" \ - | jq -r '.tags[0].name' | grep -oE '[0-9]+\.[0-9]+-[0-9]+-CI' + | jq -r '.tags[0].name' | grep -oE '[0-9]+\.[0-9]+-[0-9]+-CI') || { + log::error "Failed to resolve chart version for ${chart_major_version}" + return 1 + } + echo "$version" } # Uninstall a Helm chart if it exists diff --git a/.ci/pipelines/lib/namespace.sh b/.ci/pipelines/lib/namespace.sh index ea8fb765b2..3acfc4da44 100644 --- a/.ci/pipelines/lib/namespace.sh +++ b/.ci/pipelines/lib/namespace.sh @@ -133,32 +133,6 @@ namespace::delete() { return 0 } -# Function: namespace::remove_finalizers -# Description: Removes finalizers from resources blocking namespace deletion -# Arguments: -# $1 - project: The namespace/project name -# Returns: -# 0 - Success -namespace::remove_finalizers() { - local project=$1 - echo "Removing finalizers from resources in namespace ${project} that are blocking deletion." - - # Remove finalizers from stuck PipelineRuns and TaskRuns - for resource_type in "pipelineruns.tekton.dev" "taskruns.tekton.dev"; do - for resource in $(oc get "$resource_type" -n "$project" -o name); do - oc patch "$resource" -n "$project" --type='merge' -p '{"metadata":{"finalizers":[]}}' || true - echo "Removed finalizers from $resource in $project." - done - done - - # Check and remove specific finalizers stuck on 'chains.tekton.dev' resources - for chain_resource in $(oc get pipelineruns.tekton.dev,taskruns.tekton.dev -n "$project" -o name); do - oc patch "$chain_resource" -n "$project" --type='json' -p='[{"op": "remove", "path": "/metadata/finalizers"}]' || true - echo "Removed Tekton finalizers from $chain_resource in $project." - done - return 0 -} - # Function: namespace::force_delete # Description: Forcibly deletes a namespace stuck in Terminating status # Arguments: @@ -181,7 +155,7 @@ namespace::force_delete() { log::warn "Timeout: Namespace '${project}' was not deleted within $timeout_seconds seconds." >&2 return 1 fi - sleep $sleep_interval + sleep "$sleep_interval" elapsed=$((elapsed + sleep_interval)) done diff --git a/.ci/pipelines/lib/operators.sh b/.ci/pipelines/lib/operators.sh index 4925b34240..8debbe5d52 100644 --- a/.ci/pipelines/lib/operators.sh +++ b/.ci/pipelines/lib/operators.sh @@ -72,13 +72,6 @@ operator::install_postgres_ocp() { return 0 } -# Install Crunchy Postgres Operator from OperatorHub.io -operator::install_postgres_k8s() { - operator::install_subscription crunchy-postgres-operator "${OPERATOR_NAMESPACE}" v5 crunchy-postgres-operator certified-operators openshift-marketplace - operator::check_status 300 "operators" "Crunchy Postgres for Kubernetes" "${OPERATOR_STATUS_SUCCEEDED}" - return $? -} - # Install OpenShift Serverless Logic Operator (SonataFlow) operator::install_serverless_logic() { operator::install_subscription logic-operator-rhel8 "${OPERATOR_NAMESPACE}" alpha logic-operator-rhel8 redhat-operators openshift-marketplace diff --git a/.ci/pipelines/lib/testing.sh b/.ci/pipelines/lib/testing.sh index 81a59248bb..c41d3c0aea 100644 --- a/.ci/pipelines/lib/testing.sh +++ b/.ci/pipelines/lib/testing.sh @@ -161,7 +161,7 @@ testing::check_backstage_running() { for ((i = 1; i <= max_attempts; i++)); do # Check HTTP status local http_status - http_status=$(curl --insecure -I -s -o /dev/null -w "%{http_code}" "${url}") + http_status=$(curl --insecure -I -s -o /dev/null -w "%{http_code}" "${url}" || echo "000") if [[ "${http_status}" -eq 200 ]]; then log::success "Backstage is up and running!" diff --git a/.ci/pipelines/utils.sh b/.ci/pipelines/utils.sh index 66e2d60e1c..5867c42439 100755 --- a/.ci/pipelines/utils.sh +++ b/.ci/pipelines/utils.sh @@ -222,9 +222,9 @@ configure_external_postgres_db() { fi # Extract cluster certificates - oc get secret postgress-external-db-cluster-cert -n "${NAME_SPACE_POSTGRES_DB}" -o jsonpath='{.data.ca\.crt}' | base64 --decode > postgres-ca - oc get secret postgress-external-db-cluster-cert -n "${NAME_SPACE_POSTGRES_DB}" -o jsonpath='{.data.tls\.crt}' | base64 --decode > postgres-tls-crt - oc get secret postgress-external-db-cluster-cert -n "${NAME_SPACE_POSTGRES_DB}" -o jsonpath='{.data.tls\.key}' | base64 --decode > postgres-tls-key + oc get secret postgress-external-db-cluster-cert -n "${NAME_SPACE_POSTGRES_DB}" -o jsonpath='{.data.ca\.crt}' | base64 --decode > postgres-ca || { log::error "Failed to extract ca.crt"; return 1; } + oc get secret postgress-external-db-cluster-cert -n "${NAME_SPACE_POSTGRES_DB}" -o jsonpath='{.data.tls\.crt}' | base64 --decode > postgres-tls-crt || { log::error "Failed to extract tls.crt"; return 1; } + oc get secret postgress-external-db-cluster-cert -n "${NAME_SPACE_POSTGRES_DB}" -o jsonpath='{.data.tls\.key}' | base64 --decode > postgres-tls-key || { log::error "Failed to extract tls.key"; return 1; } # Validate secret creation if ! oc create secret generic postgress-external-db-cluster-cert \ From 887edbfd179ccc2b2c8f3ccf83dff6e153206436 Mon Sep 17 00:00:00 2001 From: Gustavo Lira Date: Wed, 4 Mar 2026 17:13:26 -0300 Subject: [PATCH 4/8] style(ci): fix prettier formatting in utils.sh Co-Authored-By: Claude Opus 4.6 --- .ci/pipelines/utils.sh | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/.ci/pipelines/utils.sh b/.ci/pipelines/utils.sh index 5867c42439..45430c58d0 100755 --- a/.ci/pipelines/utils.sh +++ b/.ci/pipelines/utils.sh @@ -222,9 +222,18 @@ configure_external_postgres_db() { fi # Extract cluster certificates - oc get secret postgress-external-db-cluster-cert -n "${NAME_SPACE_POSTGRES_DB}" -o jsonpath='{.data.ca\.crt}' | base64 --decode > postgres-ca || { log::error "Failed to extract ca.crt"; return 1; } - oc get secret postgress-external-db-cluster-cert -n "${NAME_SPACE_POSTGRES_DB}" -o jsonpath='{.data.tls\.crt}' | base64 --decode > postgres-tls-crt || { log::error "Failed to extract tls.crt"; return 1; } - oc get secret postgress-external-db-cluster-cert -n "${NAME_SPACE_POSTGRES_DB}" -o jsonpath='{.data.tls\.key}' | base64 --decode > postgres-tls-key || { log::error "Failed to extract tls.key"; return 1; } + oc get secret postgress-external-db-cluster-cert -n "${NAME_SPACE_POSTGRES_DB}" -o jsonpath='{.data.ca\.crt}' | base64 --decode > postgres-ca || { + log::error "Failed to extract ca.crt" + return 1 + } + oc get secret postgress-external-db-cluster-cert -n "${NAME_SPACE_POSTGRES_DB}" -o jsonpath='{.data.tls\.crt}' | base64 --decode > postgres-tls-crt || { + log::error "Failed to extract tls.crt" + return 1 + } + oc get secret postgress-external-db-cluster-cert -n "${NAME_SPACE_POSTGRES_DB}" -o jsonpath='{.data.tls\.key}' | base64 --decode > postgres-tls-key || { + log::error "Failed to extract tls.key" + return 1 + } # Validate secret creation if ! oc create secret generic postgress-external-db-cluster-cert \ From b3a8427da885c938eb51bc3d82e54c2435712f29 Mon Sep 17 00:00:00 2001 From: Gustavo Lira Date: Fri, 6 Mar 2026 12:14:49 -0300 Subject: [PATCH 5/8] fix(ci): address PR review feedback for deployment tracking module - Move common::require_vars to beginning of deployment functions for early validation - Add common::require_vars to operator deployment files (AKS, EKS, GKE) - Add explicit source for common.sh and namespace.sh in K8s deployment files - Restore namespace::remove_finalizers function (was prematurely removed) - Add source for reporting.sh in deployment.sh (fixes dependency declaration) - Add re-source guard to reporting.sh and fix circular dependency - Use artifacts_subdir (playwright project name) as deployment label instead of namespace in deployment::register to fix RHDHBUGS-2726 - Simplify log collection: collect immediately on deploy failure, check STATUS_TEST_FAILED only after test execution - Remove unnecessary "Uses globals: none" comment - Update docs/e2e-tests/enhanced-ci-reporting.md to reflect deployment:: API Co-Authored-By: Claude Opus 4.6 --- .../cluster/aks/aks-helm-deployment.sh | 10 +- .../cluster/aks/aks-operator-deployment.sh | 6 ++ .../cluster/eks/eks-helm-deployment.sh | 10 +- .../cluster/eks/eks-operator-deployment.sh | 6 ++ .../cluster/gke/gke-helm-deployment.sh | 11 +- .../cluster/gke/gke-operator-deployment.sh | 6 ++ .ci/pipelines/lib/deployment.sh | 11 +- .ci/pipelines/lib/namespace.sh | 26 +++++ .ci/pipelines/lib/testing.sh | 11 +- .ci/pipelines/reporting.sh | 8 +- docs/e2e-tests/enhanced-ci-reporting.md | 101 ++++++++++-------- 11 files changed, 145 insertions(+), 61 deletions(-) diff --git a/.ci/pipelines/cluster/aks/aks-helm-deployment.sh b/.ci/pipelines/cluster/aks/aks-helm-deployment.sh index da89a765ff..e171776e17 100644 --- a/.ci/pipelines/cluster/aks/aks-helm-deployment.sh +++ b/.ci/pipelines/cluster/aks/aks-helm-deployment.sh @@ -2,12 +2,18 @@ # shellcheck source=.ci/pipelines/lib/log.sh source "$DIR"/lib/log.sh +# shellcheck source=.ci/pipelines/lib/common.sh +source "$DIR"/lib/common.sh +# shellcheck source=.ci/pipelines/lib/namespace.sh +source "$DIR"/lib/namespace.sh # shellcheck source=.ci/pipelines/utils.sh source "$DIR"/utils.sh # shellcheck source=.ci/pipelines/cluster/k8s/k8s-utils.sh source "$DIR"/cluster/k8s/k8s-utils.sh initiate_aks_helm_deployment() { + common::require_vars "RELEASE_NAME" "TAG_NAME" "QUAY_REPO" "K8S_CLUSTER_ROUTER_BASE" || return 1 + namespace::delete "${NAME_SPACE_RBAC}" namespace::configure "${NAME_SPACE}" @@ -24,7 +30,6 @@ initiate_aks_helm_deployment() { namespace::setup_image_pull_secret "${NAME_SPACE}" "rh-pull-secret" "${REGISTRY_REDHAT_IO_SERVICE_ACCOUNT_DOCKERCONFIGJSON}" - common::require_vars "RELEASE_NAME" "TAG_NAME" "QUAY_REPO" "K8S_CLUSTER_ROUTER_BASE" || return 1 log::info "Deploying image from repository: ${QUAY_REPO}, TAG_NAME: ${TAG_NAME}, in NAME_SPACE: ${NAME_SPACE}" if ! helm upgrade -i "${RELEASE_NAME}" -n "${NAME_SPACE}" \ "${HELM_CHART_URL}" --version "${CHART_VERSION}" \ @@ -38,6 +43,8 @@ initiate_aks_helm_deployment() { } initiate_rbac_aks_helm_deployment() { + common::require_vars "RELEASE_NAME_RBAC" "TAG_NAME" "QUAY_REPO" "K8S_CLUSTER_ROUTER_BASE" || return 1 + namespace::delete "${NAME_SPACE}" namespace::configure "${NAME_SPACE_RBAC}" @@ -51,7 +58,6 @@ initiate_rbac_aks_helm_deployment() { namespace::setup_image_pull_secret "${NAME_SPACE_RBAC}" "rh-pull-secret" "${REGISTRY_REDHAT_IO_SERVICE_ACCOUNT_DOCKERCONFIGJSON}" - common::require_vars "RELEASE_NAME_RBAC" "TAG_NAME" "QUAY_REPO" "K8S_CLUSTER_ROUTER_BASE" || return 1 log::info "Deploying image from repository: ${QUAY_REPO}, TAG_NAME: ${TAG_NAME}, in NAME_SPACE: ${NAME_SPACE_RBAC}" if ! helm upgrade -i "${RELEASE_NAME_RBAC}" -n "${NAME_SPACE_RBAC}" \ "${HELM_CHART_URL}" --version "${CHART_VERSION}" \ diff --git a/.ci/pipelines/cluster/aks/aks-operator-deployment.sh b/.ci/pipelines/cluster/aks/aks-operator-deployment.sh index 9232f36c61..ddcb41d509 100644 --- a/.ci/pipelines/cluster/aks/aks-operator-deployment.sh +++ b/.ci/pipelines/cluster/aks/aks-operator-deployment.sh @@ -2,6 +2,10 @@ # shellcheck source=.ci/pipelines/lib/log.sh source "$DIR"/lib/log.sh +# shellcheck source=.ci/pipelines/lib/common.sh +source "$DIR"/lib/common.sh +# shellcheck source=.ci/pipelines/lib/namespace.sh +source "$DIR"/lib/namespace.sh # shellcheck source=.ci/pipelines/utils.sh source "$DIR"/utils.sh # shellcheck source=.ci/pipelines/install-methods/operator.sh @@ -13,6 +17,7 @@ initiate_aks_operator_deployment() { local namespace=$1 local rhdh_base_url=$2 + common::require_vars "RELEASE_NAME" "REGISTRY_REDHAT_IO_SERVICE_ACCOUNT_DOCKERCONFIGJSON" || return 1 log::info "Initiating Operator-backed non-RBAC deployment on AKS" namespace::configure "${namespace}" @@ -38,6 +43,7 @@ initiate_rbac_aks_operator_deployment() { local namespace=$1 local rhdh_base_url=$2 + common::require_vars "RELEASE_NAME_RBAC" "REGISTRY_REDHAT_IO_SERVICE_ACCOUNT_DOCKERCONFIGJSON" || return 1 log::info "Initiating Operator-backed RBAC deployment on AKS" namespace::configure "${namespace}" diff --git a/.ci/pipelines/cluster/eks/eks-helm-deployment.sh b/.ci/pipelines/cluster/eks/eks-helm-deployment.sh index 0eac83e112..b340b9094e 100644 --- a/.ci/pipelines/cluster/eks/eks-helm-deployment.sh +++ b/.ci/pipelines/cluster/eks/eks-helm-deployment.sh @@ -2,10 +2,16 @@ # shellcheck source=.ci/pipelines/lib/log.sh source "$DIR"/lib/log.sh +# shellcheck source=.ci/pipelines/lib/common.sh +source "$DIR"/lib/common.sh +# shellcheck source=.ci/pipelines/lib/namespace.sh +source "$DIR"/lib/namespace.sh # shellcheck source=.ci/pipelines/utils.sh source "$DIR"/utils.sh initiate_eks_helm_deployment() { + common::require_vars "RELEASE_NAME" "TAG_NAME" "QUAY_REPO" "K8S_CLUSTER_ROUTER_BASE" || return 1 + log::info "Initiating EKS Helm deployment" namespace::delete "${NAME_SPACE_RBAC}" @@ -20,7 +26,6 @@ initiate_eks_helm_deployment() { local rhdh_base_url="https://${K8S_CLUSTER_ROUTER_BASE}" apply_yaml_files "${DIR}" "${NAME_SPACE}" "${rhdh_base_url}" - common::require_vars "RELEASE_NAME" "TAG_NAME" "QUAY_REPO" "K8S_CLUSTER_ROUTER_BASE" || return 1 envsubst < "${DIR}/value_files/${HELM_CHART_EKS_DIFF_VALUE_FILE_NAME}" > "/tmp/${HELM_CHART_EKS_DIFF_VALUE_FILE_NAME}" helm::merge_values "merge" "${DIR}/value_files/${HELM_CHART_VALUE_FILE_NAME}" "/tmp/${HELM_CHART_EKS_DIFF_VALUE_FILE_NAME}" "/tmp/${HELM_CHART_K8S_MERGED_VALUE_FILE_NAME}" common::save_artifact "${NAME_SPACE}" "/tmp/${HELM_CHART_K8S_MERGED_VALUE_FILE_NAME}" # Save the final value-file into the artifacts directory. @@ -37,6 +42,8 @@ initiate_eks_helm_deployment() { } initiate_rbac_eks_helm_deployment() { + common::require_vars "RELEASE_NAME_RBAC" "TAG_NAME" "QUAY_REPO" "K8S_CLUSTER_ROUTER_BASE" || return 1 + log::info "Initiating EKS RBAC Helm deployment" namespace::delete "${NAME_SPACE}" @@ -50,7 +57,6 @@ initiate_rbac_eks_helm_deployment() { local rbac_rhdh_base_url="https://${K8S_CLUSTER_ROUTER_BASE}" apply_yaml_files "${DIR}" "${NAME_SPACE_RBAC}" "${rbac_rhdh_base_url}" - common::require_vars "RELEASE_NAME_RBAC" "TAG_NAME" "QUAY_REPO" "K8S_CLUSTER_ROUTER_BASE" || return 1 envsubst < "${DIR}/value_files/${HELM_CHART_RBAC_EKS_DIFF_VALUE_FILE_NAME}" > "/tmp/${HELM_CHART_RBAC_EKS_DIFF_VALUE_FILE_NAME}" helm::merge_values "merge" "${DIR}/value_files/${HELM_CHART_RBAC_VALUE_FILE_NAME}" "/tmp/${HELM_CHART_RBAC_EKS_DIFF_VALUE_FILE_NAME}" "/tmp/${HELM_CHART_RBAC_K8S_MERGED_VALUE_FILE_NAME}" common::save_artifact "${NAME_SPACE_RBAC}" "/tmp/${HELM_CHART_RBAC_K8S_MERGED_VALUE_FILE_NAME}" # Save the final value-file into the artifacts directory. diff --git a/.ci/pipelines/cluster/eks/eks-operator-deployment.sh b/.ci/pipelines/cluster/eks/eks-operator-deployment.sh index ee3580ceb6..43d092f485 100644 --- a/.ci/pipelines/cluster/eks/eks-operator-deployment.sh +++ b/.ci/pipelines/cluster/eks/eks-operator-deployment.sh @@ -2,6 +2,10 @@ # shellcheck source=.ci/pipelines/lib/log.sh source "$DIR"/lib/log.sh +# shellcheck source=.ci/pipelines/lib/common.sh +source "$DIR"/lib/common.sh +# shellcheck source=.ci/pipelines/lib/namespace.sh +source "$DIR"/lib/namespace.sh # shellcheck source=.ci/pipelines/utils.sh source "$DIR"/utils.sh # shellcheck source=.ci/pipelines/install-methods/operator.sh @@ -11,6 +15,7 @@ initiate_eks_operator_deployment() { local namespace=$1 local rhdh_base_url=$2 + common::require_vars "RELEASE_NAME" "REGISTRY_REDHAT_IO_SERVICE_ACCOUNT_DOCKERCONFIGJSON" || return 1 log::info "Initiating Operator-backed non-RBAC deployment on EKS" namespace::configure "${namespace}" @@ -35,6 +40,7 @@ initiate_rbac_eks_operator_deployment() { local namespace=$1 local rhdh_base_url=$2 + common::require_vars "RELEASE_NAME_RBAC" "REGISTRY_REDHAT_IO_SERVICE_ACCOUNT_DOCKERCONFIGJSON" || return 1 log::info "Initiating Operator-backed RBAC deployment on EKS" namespace::configure "${namespace}" diff --git a/.ci/pipelines/cluster/gke/gke-helm-deployment.sh b/.ci/pipelines/cluster/gke/gke-helm-deployment.sh index ebf6b6ba64..900486e467 100644 --- a/.ci/pipelines/cluster/gke/gke-helm-deployment.sh +++ b/.ci/pipelines/cluster/gke/gke-helm-deployment.sh @@ -2,6 +2,10 @@ # shellcheck source=.ci/pipelines/lib/log.sh source "$DIR"/lib/log.sh +# shellcheck source=.ci/pipelines/lib/common.sh +source "$DIR"/lib/common.sh +# shellcheck source=.ci/pipelines/lib/namespace.sh +source "$DIR"/lib/namespace.sh # shellcheck source=.ci/pipelines/utils.sh source "$DIR"/utils.sh # shellcheck source=.ci/pipelines/cluster/gke/gcloud.sh @@ -10,6 +14,8 @@ source "$DIR"/cluster/gke/gcloud.sh source "$DIR"/cluster/gke/manifest.sh initiate_gke_helm_deployment() { + common::require_vars "RELEASE_NAME" "TAG_NAME" "QUAY_REPO" "K8S_CLUSTER_ROUTER_BASE" "GKE_CERT_NAME" || return 1 + namespace::delete "${NAME_SPACE_RBAC}" namespace::configure "${NAME_SPACE}" @@ -26,7 +32,6 @@ initiate_gke_helm_deployment() { namespace::setup_image_pull_secret "${NAME_SPACE}" "rh-pull-secret" "${REGISTRY_REDHAT_IO_SERVICE_ACCOUNT_DOCKERCONFIGJSON}" - common::require_vars "RELEASE_NAME" "TAG_NAME" "QUAY_REPO" "K8S_CLUSTER_ROUTER_BASE" "GKE_CERT_NAME" || return 1 log::info "Deploying image from repository: ${QUAY_REPO}, TAG_NAME: ${TAG_NAME}, in NAME_SPACE: ${NAME_SPACE}" if ! helm upgrade -i "${RELEASE_NAME}" -n "${NAME_SPACE}" \ "${HELM_CHART_URL}" --version "${CHART_VERSION}" \ @@ -41,6 +46,8 @@ initiate_gke_helm_deployment() { } initiate_rbac_gke_helm_deployment() { + common::require_vars "RELEASE_NAME_RBAC" "TAG_NAME" "QUAY_REPO" "K8S_CLUSTER_ROUTER_BASE" "GKE_CERT_NAME" || return 1 + namespace::delete "${NAME_SPACE}" namespace::configure "${NAME_SPACE_RBAC}" @@ -54,7 +61,7 @@ initiate_rbac_gke_helm_deployment() { common::save_artifact "${NAME_SPACE_RBAC}" "/tmp/${HELM_CHART_RBAC_K8S_MERGED_VALUE_FILE_NAME}" namespace::setup_image_pull_secret "${NAME_SPACE_RBAC}" "rh-pull-secret" "${REGISTRY_REDHAT_IO_SERVICE_ACCOUNT_DOCKERCONFIGJSON}" - common::require_vars "RELEASE_NAME_RBAC" "TAG_NAME" "QUAY_REPO" "K8S_CLUSTER_ROUTER_BASE" "GKE_CERT_NAME" || return 1 + log::info "Deploying image from repository: ${QUAY_REPO}, TAG_NAME: ${TAG_NAME}, in NAME_SPACE: ${NAME_SPACE_RBAC}" if ! helm upgrade -i "${RELEASE_NAME_RBAC}" -n "${NAME_SPACE_RBAC}" \ "${HELM_CHART_URL}" --version "${CHART_VERSION}" \ diff --git a/.ci/pipelines/cluster/gke/gke-operator-deployment.sh b/.ci/pipelines/cluster/gke/gke-operator-deployment.sh index b3ba9913e7..09a6ee63d6 100644 --- a/.ci/pipelines/cluster/gke/gke-operator-deployment.sh +++ b/.ci/pipelines/cluster/gke/gke-operator-deployment.sh @@ -2,6 +2,10 @@ # shellcheck source=.ci/pipelines/lib/log.sh source "$DIR"/lib/log.sh +# shellcheck source=.ci/pipelines/lib/common.sh +source "$DIR"/lib/common.sh +# shellcheck source=.ci/pipelines/lib/namespace.sh +source "$DIR"/lib/namespace.sh # shellcheck source=.ci/pipelines/utils.sh source "$DIR"/utils.sh # shellcheck source=.ci/pipelines/cluster/gke/gcloud.sh @@ -15,6 +19,7 @@ initiate_gke_operator_deployment() { local namespace=$1 local rhdh_base_url=$2 + common::require_vars "RELEASE_NAME" "REGISTRY_REDHAT_IO_SERVICE_ACCOUNT_DOCKERCONFIGJSON" || return 1 log::info "Initiating Operator-backed non-RBAC deployment on GKE" namespace::configure "${namespace}" @@ -40,6 +45,7 @@ initiate_rbac_gke_operator_deployment() { local namespace=$1 local rhdh_base_url=$2 + common::require_vars "RELEASE_NAME_RBAC" "REGISTRY_REDHAT_IO_SERVICE_ACCOUNT_DOCKERCONFIGJSON" || return 1 log::info "Initiating Operator-backed RBAC deployment on GKE" namespace::configure "${namespace}" diff --git a/.ci/pipelines/lib/deployment.sh b/.ci/pipelines/lib/deployment.sh index c940161dc4..e78b764854 100644 --- a/.ci/pipelines/lib/deployment.sh +++ b/.ci/pipelines/lib/deployment.sh @@ -6,6 +6,9 @@ if [[ -n "${RHDH_DEPLOYMENT_LIB_SOURCED:-}" ]]; then fi readonly RHDH_DEPLOYMENT_LIB_SOURCED=1 +# shellcheck source=.ci/pipelines/reporting.sh +source "$(dirname "${BASH_SOURCE[0]}")/../reporting.sh" + # Internal state _DEPLOYMENT_COUNTER=0 @@ -19,9 +22,9 @@ deployment::current_id() { } deployment::register() { - local namespace="$1" + local label="$1" deployment::next_id > /dev/null - save_status_deployment_namespace "${_DEPLOYMENT_COUNTER}" "$namespace" + save_status_deployment_namespace "${_DEPLOYMENT_COUNTER}" "$label" } deployment::mark_deploy_success() { @@ -29,8 +32,8 @@ deployment::mark_deploy_success() { } deployment::mark_deploy_failed() { - local namespace="$1" - deployment::register "$namespace" + local label="$1" + deployment::register "$label" save_status_failed_to_deploy "${_DEPLOYMENT_COUNTER}" true save_status_test_failed "${_DEPLOYMENT_COUNTER}" true save_overall_result 1 diff --git a/.ci/pipelines/lib/namespace.sh b/.ci/pipelines/lib/namespace.sh index 3acfc4da44..d053788720 100644 --- a/.ci/pipelines/lib/namespace.sh +++ b/.ci/pipelines/lib/namespace.sh @@ -133,6 +133,32 @@ namespace::delete() { return 0 } +# Function: namespace::remove_finalizers +# Description: Removes finalizers from resources blocking namespace deletion +# Arguments: +# $1 - project: The namespace/project name +# Returns: +# 0 - Success +namespace::remove_finalizers() { + local project=$1 + echo "Removing finalizers from resources in namespace ${project} that are blocking deletion." + + # Remove finalizers from stuck PipelineRuns and TaskRuns + for resource_type in "pipelineruns.tekton.dev" "taskruns.tekton.dev"; do + for resource in $(oc get "$resource_type" -n "$project" -o name); do + oc patch "$resource" -n "$project" --type='merge' -p '{"metadata":{"finalizers":[]}}' || true + echo "Removed finalizers from $resource in $project." + done + done + + # Check and remove specific finalizers stuck on 'chains.tekton.dev' resources + for chain_resource in $(oc get pipelineruns.tekton.dev,taskruns.tekton.dev -n "$project" -o name); do + oc patch "$chain_resource" -n "$project" --type='json' -p='[{"op": "remove", "path": "/metadata/finalizers"}]' || true + echo "Removed Tekton finalizers from $chain_resource in $project." + done + return 0 +} + # Function: namespace::force_delete # Description: Forcibly deletes a namespace stuck in Terminating status # Arguments: diff --git a/.ci/pipelines/lib/testing.sh b/.ci/pipelines/lib/testing.sh index c41d3c0aea..6a6b57ccab 100644 --- a/.ci/pipelines/lib/testing.sh +++ b/.ci/pipelines/lib/testing.sh @@ -49,7 +49,7 @@ testing::run_tests() { return 1 fi - deployment::register "$namespace" + deployment::register "$artifacts_subdir" deployment::mark_deploy_success BASE_URL="${url}" @@ -244,13 +244,15 @@ testing::check_and_test() { fi else echo "Backstage is not running. Marking deployment as failed and continuing..." - deployment::mark_deploy_failed "$namespace" + deployment::mark_deploy_failed "$artifacts_subdir" + save_all_pod_logs "$namespace" + return 0 fi - # Collect pod logs only on failure to speed up successful PR runs. + # Collect pod logs only on test failure to speed up successful PR runs. local _current_id _current_id="$(deployment::current_id)" - if [[ "${STATUS_TEST_FAILED[$_current_id]:-}" == "true" || "${STATUS_FAILED_TO_DEPLOY[$_current_id]:-}" == "true" ]]; then + if [[ "${STATUS_TEST_FAILED[$_current_id]:-}" == "true" ]]; then save_all_pod_logs "$namespace" else log::info "Tests passed — skipping pod log collection for namespace: ${namespace}" @@ -300,7 +302,6 @@ testing::check_helm_upgrade() { # $4 - playwright_project: The Playwright project to run # $5 - url: The URL to test against # $6 - timeout: (optional) Timeout in seconds (default: 600) -# Uses globals: none (deployment state managed by lib/deployment.sh) testing::check_upgrade_and_test() { local deployment_name="$1" local release_name="$2" diff --git a/.ci/pipelines/reporting.sh b/.ci/pipelines/reporting.sh index df75a5b01d..7b736159b8 100644 --- a/.ci/pipelines/reporting.sh +++ b/.ci/pipelines/reporting.sh @@ -1,9 +1,13 @@ #!/bin/bash +# Prevent re-sourcing +if [[ -n "${REPORTING_LIB_SOURCED:-}" ]]; then + return 0 +fi +readonly REPORTING_LIB_SOURCED=1 + # shellcheck source=.ci/pipelines/lib/log.sh source "$(dirname "${BASH_SOURCE[0]}")"/lib/log.sh -# shellcheck source=.ci/pipelines/lib/deployment.sh -source "$(dirname "${BASH_SOURCE[0]}")"/lib/deployment.sh # Variables for reporting export STATUS_DEPLOYMENT_NAMESPACE # Array that holds the namespaces of deployments. diff --git a/docs/e2e-tests/enhanced-ci-reporting.md b/docs/e2e-tests/enhanced-ci-reporting.md index 23b176552c..7dc6af16d3 100644 --- a/docs/e2e-tests/enhanced-ci-reporting.md +++ b/docs/e2e-tests/enhanced-ci-reporting.md @@ -8,89 +8,101 @@ The enhanced CI reporting system uses the [`.ci/pipelines/reporting.sh`](../../. **Note:** The `SHARED_DIR` can only contain files. No directories or nested structures are supported. -## Using reporting.sh Functions +## Architecture -The [`.ci/pipelines/reporting.sh`](../../.ci/pipelines/reporting.sh) script provides several functions to signal different types of results. It uses a Bash array to store statuses for multiple deployments, indexed by `CURRENT_DEPLOYMENT` (a deployment number). +### Deployment Tracking Module -### Core Reporting Functions +The [`.ci/pipelines/lib/deployment.sh`](../../.ci/pipelines/lib/deployment.sh) module encapsulates all deployment state management into a clean API. It manages an internal counter and delegates status persistence to `reporting.sh`. -#### `save_status_deployment_namespace(deployment, namespace)` -Records the namespace where a deployment was created. +#### `deployment::register(label)` +Registers a new deployment with the given label (typically the Playwright project name or artifacts subdirectory). Increments the internal counter and records the deployment label. ```bash -save_status_deployment_namespace $CURRENT_DEPLOYMENT $namespace +deployment::register "$artifacts_subdir" ``` -#### `save_status_failed_to_deploy(deployment, status)` -Records whether a deployment failed (true/false). +#### `deployment::mark_deploy_success()` +Marks the current deployment as successfully deployed. ```bash -save_status_failed_to_deploy $CURRENT_DEPLOYMENT false # Success -save_status_failed_to_deploy $CURRENT_DEPLOYMENT true # Failure +deployment::mark_deploy_success ``` -#### `save_status_test_failed(deployment, status)` -Records whether tests failed for a deployment (true/false). +#### `deployment::mark_deploy_failed(label)` +Registers a new deployment and marks it as failed (deploy failed, tests failed, overall result = 1). ```bash -save_status_test_failed $CURRENT_DEPLOYMENT false # Tests passed -save_status_test_failed $CURRENT_DEPLOYMENT true # Tests failed +deployment::mark_deploy_failed "$artifacts_subdir" ``` -#### `save_status_number_of_test_failed(deployment, number)` -Records the number of failed tests. +#### `deployment::mark_test_result(passed, num_failures)` +Records whether tests passed and the number of failures. ```bash -save_status_number_of_test_failed $CURRENT_DEPLOYMENT "3" +deployment::mark_test_result "$test_passed" "${failed_tests}" ``` -#### `save_overall_result(result)` -Records the overall test result (0 for success, 1 for failure). +#### `deployment::current_id()` +Returns the current deployment counter value. ```bash -save_overall_result 0 # Overall success -save_overall_result 1 # Overall failure +local id +id="$(deployment::current_id)" ``` +### Core Reporting Functions + +The [`.ci/pipelines/reporting.sh`](../../.ci/pipelines/reporting.sh) script provides low-level functions used internally by the deployment module. These persist status to `SHARED_DIR` files and `ARTIFACT_DIR/reporting/`. + +#### `save_status_deployment_namespace(deployment, label)` +Records the label for a deployment. + +#### `save_status_failed_to_deploy(deployment, status)` +Records whether a deployment failed (true/false). + +#### `save_status_test_failed(deployment, status)` +Records whether tests failed for a deployment (true/false). + +#### `save_status_number_of_test_failed(deployment, number)` +Records the number of failed tests. + +#### `save_overall_result(result)` +Records the overall test result (0 for success, 1 for failure). + ## SHARED_DIR Integration All status information is written to files in the `SHARED_DIR` directory, which is shared between OpenShift CI steps: -- `SHARED_DIR/STATUS_DEPLOYMENT_NAMESPACE.txt` - Bash array format -- `SHARED_DIR/STATUS_FAILED_TO_DEPLOY.txt` - Bash array format -- `SHARED_DIR/STATUS_TEST_FAILED.txt` - Bash array format -- `SHARED_DIR/STATUS_NUMBER_OF_TEST_FAILED.txt` - Bash array format -- `SHARED_DIR/STATUS_URL_REPORTPORTAL.txt` - Bash array format +- `SHARED_DIR/STATUS_DEPLOYMENT_NAMESPACE.txt` - Deployment labels (one per line) +- `SHARED_DIR/STATUS_FAILED_TO_DEPLOY.txt` - Deploy failure flags (one per line) +- `SHARED_DIR/STATUS_TEST_FAILED.txt` - Test failure flags (one per line) +- `SHARED_DIR/STATUS_NUMBER_OF_TEST_FAILED.txt` - Failure counts (one per line) +- `SHARED_DIR/STATUS_URL_REPORTPORTAL.txt` - ReportPortal URLs - `SHARED_DIR/OVERALL_RESULT.txt` - Single value -The status files use bash arrays indexed by `CURRENT_DEPLOYMENT` (deployment number), except for `OVERALL_RESULT.txt` which contains a single value. These files are also copied to `ARTIFACT_DIR/reporting/` for artifact collection. +These files are also copied to `ARTIFACT_DIR/reporting/` for artifact collection. ## Usage Examples ### In Test Scripts ```bash -# Source the reporting functions +# Source the required modules (typically done via utils.sh) source "${DIR}/reporting.sh" +source "${DIR}/lib/deployment.sh" # Initialize overall result save_overall_result 0 -# Record deployment success -save_status_deployment_namespace $CURRENT_DEPLOYMENT "showcase" -save_status_failed_to_deploy $CURRENT_DEPLOYMENT false +# Register a deployment and mark it as successful +deployment::register "showcase" +deployment::mark_deploy_success # Record test results -if [ "${RESULT}" -ne 0 ]; then - save_overall_result 1 - save_status_test_failed $CURRENT_DEPLOYMENT true -else - save_status_test_failed $CURRENT_DEPLOYMENT false -fi - -# Record number of failed tests -failed_tests=$(grep -oP 'failures="\K[0-9]+' "${JUNIT_RESULTS}" | head -n 1) -save_status_number_of_test_failed $CURRENT_DEPLOYMENT "${failed_tests}" +deployment::mark_test_result "$test_passed" "${failed_tests}" + +# Or mark a deployment as failed in one call +deployment::mark_deploy_failed "showcase-rbac" ``` ### Error Handling @@ -126,18 +138,19 @@ For nightly runs, the system automatically sends notifications to the `#rhdh-e2e - **Logs Link**: Direct link to job logs - **Triage Mention**: `@rhdh-ci-test-triage` for team notification - **Per-Deployment Status**: Each deployment shows: - - **Deployment Name**: e.g., `showcase-ci-nightly`, `showcase-rbac-nightly` + - **Deployment Label**: e.g., `showcase`, `showcase-rbac`, `showcase-runtime` - **Deployment Status**: "deployed" status - **Test Results**: "tests passed" or failure count (e.g., "2 tests failed") - **Tools**: Playwright, ReportPortal, and artifacts links ## File Locations -- **Script**: [`.ci/pipelines/reporting.sh`](../../.ci/pipelines/reporting.sh) +- **Deployment Module**: [`.ci/pipelines/lib/deployment.sh`](../../.ci/pipelines/lib/deployment.sh) +- **Reporting Script**: [`.ci/pipelines/reporting.sh`](../../.ci/pipelines/reporting.sh) - **Integration**: [`.ci/pipelines/utils.sh`](../../.ci/pipelines/utils.sh) and [`.ci/pipelines/openshift-ci-tests.sh`](../../.ci/pipelines/openshift-ci-tests.sh) ## Related Documentation - [CI Testing Overview](CI.md) - [E2E Tests Examples](examples.md) -- [Contributing to E2E Tests](CONTRIBUTING.MD) \ No newline at end of file +- [Contributing to E2E Tests](CONTRIBUTING.MD) From 462cb5c04107355be6a6baa475f1c3dc9c45e59a Mon Sep 17 00:00:00 2001 From: Gustavo Lira Date: Tue, 10 Mar 2026 09:43:18 -0300 Subject: [PATCH 6/8] fix(ci): use playwright project name for deployment labels and fix status file alignment Use playwright_project instead of namespace as the default deployment label, fixing Slack reports that showed duplicate names (RHDHBUGS-2726). Restructure testing::check_and_test to use testing::run_tests return value directly for pod log collection, simplifying the control flow. Ensure STATUS_NUMBER_OF_TEST_FAILED.txt always has an entry per deployment by defaulting num_failures to "0" and writing "N/A" for deploy failures. Co-Authored-By: Claude Opus 4.6 --- .ci/pipelines/jobs/auth-providers.sh | 2 +- .ci/pipelines/jobs/ocp-nightly.sh | 2 +- .ci/pipelines/jobs/ocp-operator.sh | 2 +- .ci/pipelines/lib/deployment.sh | 7 +++---- .ci/pipelines/lib/testing.sh | 30 ++++++++++++---------------- 5 files changed, 19 insertions(+), 24 deletions(-) diff --git a/.ci/pipelines/jobs/auth-providers.sh b/.ci/pipelines/jobs/auth-providers.sh index d5c0e953dd..240acddaba 100644 --- a/.ci/pipelines/jobs/auth-providers.sh +++ b/.ci/pipelines/jobs/auth-providers.sh @@ -31,5 +31,5 @@ handle_auth_providers() { export LOGS_FOLDER log::info "Running tests ${AUTH_PROVIDERS_RELEASE} in ${AUTH_PROVIDERS_NAMESPACE}" - testing::run_tests "${AUTH_PROVIDERS_RELEASE}" "${AUTH_PROVIDERS_NAMESPACE}" "${PW_PROJECT_SHOWCASE_AUTH_PROVIDERS}" "https://${K8S_CLUSTER_ROUTER_BASE}" + testing::run_tests "${AUTH_PROVIDERS_RELEASE}" "${AUTH_PROVIDERS_NAMESPACE}" "${PW_PROJECT_SHOWCASE_AUTH_PROVIDERS}" "https://${K8S_CLUSTER_ROUTER_BASE}" || true } diff --git a/.ci/pipelines/jobs/ocp-nightly.sh b/.ci/pipelines/jobs/ocp-nightly.sh index 2b2afbbec1..d59fc295bd 100644 --- a/.ci/pipelines/jobs/ocp-nightly.sh +++ b/.ci/pipelines/jobs/ocp-nightly.sh @@ -55,7 +55,7 @@ run_runtime_config_change_tests() { # Deploy `showcase-runtime` to run tests that require configuration changes at runtime initiate_runtime_deployment "${RELEASE_NAME}" "${NAME_SPACE_RUNTIME}" local runtime_url="https://${RELEASE_NAME}-developer-hub-${NAME_SPACE_RUNTIME}.${K8S_CLUSTER_ROUTER_BASE}" - testing::run_tests "${RELEASE_NAME}" "${NAME_SPACE_RUNTIME}" "${PW_PROJECT_SHOWCASE_RUNTIME}" "${runtime_url}" + testing::run_tests "${RELEASE_NAME}" "${NAME_SPACE_RUNTIME}" "${PW_PROJECT_SHOWCASE_RUNTIME}" "${runtime_url}" || true } run_sanity_plugins_check() { diff --git a/.ci/pipelines/jobs/ocp-operator.sh b/.ci/pipelines/jobs/ocp-operator.sh index f20ef7e04f..3815b77485 100644 --- a/.ci/pipelines/jobs/ocp-operator.sh +++ b/.ci/pipelines/jobs/ocp-operator.sh @@ -91,7 +91,7 @@ run_operator_runtime_config_change_tests() { config::create_app_config_map "$DIR/resources/postgres-db/rds-app-config.yaml" "${NAME_SPACE_RUNTIME}" deploy_rhdh_operator "${NAME_SPACE_RUNTIME}" "${DIR}/resources/rhdh-operator/rhdh-start-runtime.yaml" local runtime_url="https://backstage-${RELEASE_NAME}-${NAME_SPACE_RUNTIME}.${K8S_CLUSTER_ROUTER_BASE}" - testing::run_tests "${RELEASE_NAME}" "${NAME_SPACE_RUNTIME}" "${PW_PROJECT_SHOWCASE_RUNTIME}" "${runtime_url}" + testing::run_tests "${RELEASE_NAME}" "${NAME_SPACE_RUNTIME}" "${PW_PROJECT_SHOWCASE_RUNTIME}" "${runtime_url}" || true } handle_ocp_operator() { diff --git a/.ci/pipelines/lib/deployment.sh b/.ci/pipelines/lib/deployment.sh index e78b764854..442031455b 100644 --- a/.ci/pipelines/lib/deployment.sh +++ b/.ci/pipelines/lib/deployment.sh @@ -36,20 +36,19 @@ deployment::mark_deploy_failed() { deployment::register "$label" save_status_failed_to_deploy "${_DEPLOYMENT_COUNTER}" true save_status_test_failed "${_DEPLOYMENT_COUNTER}" true + save_status_number_of_test_failed "${_DEPLOYMENT_COUNTER}" "N/A" save_overall_result 1 } deployment::mark_test_result() { local passed="$1" - local num_failures="${2:-}" + local num_failures="${2:-0}" if [[ "$passed" == "true" ]]; then save_status_test_failed "${_DEPLOYMENT_COUNTER}" false else save_status_test_failed "${_DEPLOYMENT_COUNTER}" true fi - if [[ -n "$num_failures" ]]; then - save_status_number_of_test_failed "${_DEPLOYMENT_COUNTER}" "$num_failures" - fi + save_status_number_of_test_failed "${_DEPLOYMENT_COUNTER}" "$num_failures" } # Export all functions for subshell compatibility. diff --git a/.ci/pipelines/lib/testing.sh b/.ci/pipelines/lib/testing.sh index 6a6b57ccab..498514f22c 100644 --- a/.ci/pipelines/lib/testing.sh +++ b/.ci/pipelines/lib/testing.sh @@ -31,7 +31,7 @@ readonly _TESTING_ERR_MISSING_PARAMS="Missing required parameters" # $2 - namespace: The namespace where Backstage is deployed # $3 - playwright_project: The Playwright project to run # $4 - url: (optional) The URL to test against -# $5 - artifacts_subdir: (optional) Subdirectory for artifacts (defaults to namespace) +# $5 - artifacts_subdir: (optional) Subdirectory for artifacts (defaults to playwright_project) # Returns: # 0 - Tests passed # Non-zero - Tests failed @@ -41,7 +41,7 @@ testing::run_tests() { local namespace=$2 local playwright_project=$3 local url="${4:-}" - local artifacts_subdir="${5:-$namespace}" + local artifacts_subdir="${5:-$playwright_project}" if [[ -z "$release_name" || -z "$namespace" || -z "$playwright_project" ]]; then log::error "${_TESTING_ERR_MISSING_PARAMS}" @@ -114,6 +114,7 @@ testing::run_tests() { failed_tests="0" elif [[ -f "${e2e_tests_dir}/${JUNIT_RESULTS}" ]]; then failed_tests=$(grep -oP 'failures="\K[0-9]+' "${e2e_tests_dir}/${JUNIT_RESULTS}" | head -n 1) + failed_tests="${failed_tests:-some}" echo "Number of failed tests: ${failed_tests}" else echo "JUnit results file not found: ${e2e_tests_dir}/${JUNIT_RESULTS}" @@ -121,7 +122,7 @@ testing::run_tests() { echo "Number of failed tests unknown, saving as $failed_tests." fi deployment::mark_test_result "$test_passed" "${failed_tests}" - return 0 + return "$test_result" } # ============================================================================== @@ -217,7 +218,7 @@ testing::check_backstage_running() { # $4 - url: The URL to test against # $5 - max_attempts: (optional) Maximum number of attempts (default: 30) # $6 - wait_seconds: (optional) Seconds to wait between attempts (default: 30) -# $7 - artifacts_subdir: (optional) Subdirectory for artifacts (defaults to namespace) +# $7 - artifacts_subdir: (optional) Subdirectory for artifacts (defaults to playwright_project) # Uses globals: SKIP_TESTS testing::check_and_test() { local release_name=$1 @@ -226,7 +227,7 @@ testing::check_and_test() { local url=$4 local max_attempts=${5:-30} local wait_seconds=${6:-30} - local artifacts_subdir="${7:-$namespace}" + local artifacts_subdir="${7:-$playwright_project}" if [[ -z "$release_name" || -z "$namespace" || -z "$playwright_project" || -z "$url" ]]; then log::error "${_TESTING_ERR_MISSING_PARAMS}" @@ -240,22 +241,17 @@ testing::check_and_test() { if [[ "${SKIP_TESTS:-false}" == "true" ]]; then log::info "SKIP_TESTS=true, skipping test execution for namespace: ${namespace}" else - testing::run_tests "${release_name}" "${namespace}" "${playwright_project}" "${url}" "${artifacts_subdir}" + # Collect pod logs only on test failure to speed up successful PR runs. + if testing::run_tests "${release_name}" "${namespace}" "${playwright_project}" "${url}" "${artifacts_subdir}"; then + log::info "Tests passed — skipping pod log collection for namespace: ${namespace}" + else + save_all_pod_logs "$namespace" + fi fi else echo "Backstage is not running. Marking deployment as failed and continuing..." deployment::mark_deploy_failed "$artifacts_subdir" save_all_pod_logs "$namespace" - return 0 - fi - - # Collect pod logs only on test failure to speed up successful PR runs. - local _current_id - _current_id="$(deployment::current_id)" - if [[ "${STATUS_TEST_FAILED[$_current_id]:-}" == "true" ]]; then - save_all_pod_logs "$namespace" - else - log::info "Tests passed — skipping pod log collection for namespace: ${namespace}" fi return 0 } @@ -320,7 +316,7 @@ testing::check_upgrade_and_test() { testing::check_and_test "${release_name}" "${namespace}" "${playwright_project}" "${url}" else log::error "Helm upgrade encountered an issue or timed out. Exiting..." - deployment::mark_deploy_failed "$namespace" + deployment::mark_deploy_failed "$playwright_project" fi return 0 } From 9c5b6caefe646b73a591dd8f56032427e2fe0b56 Mon Sep 17 00:00:00 2001 From: Gustavo Lira Date: Wed, 11 Mar 2026 09:45:06 -0300 Subject: [PATCH 7/8] refactor(ci): rename deployment.sh to test-run-tracker.sh and update function prefix Rename the tracking library from `deployment` to `test_run_tracker` to better reflect its purpose: tracking test run status (deploy outcome + test results) for reporting, not managing deployments. - Rename file: lib/deployment.sh -> lib/test-run-tracker.sh - Rename function prefix: deployment:: -> test_run_tracker:: - Rename internal state: _DEPLOYMENT_COUNTER -> _TEST_RUN_COUNTER - Update all call sites in testing.sh and utils.sh Co-Authored-By: Claude Opus 4.6 --- .ci/pipelines/lib/deployment.sh | 62 --------------------------- .ci/pipelines/lib/test-run-tracker.sh | 62 +++++++++++++++++++++++++++ .ci/pipelines/lib/testing.sh | 14 +++--- .ci/pipelines/utils.sh | 4 +- 4 files changed, 71 insertions(+), 71 deletions(-) delete mode 100644 .ci/pipelines/lib/deployment.sh create mode 100644 .ci/pipelines/lib/test-run-tracker.sh diff --git a/.ci/pipelines/lib/deployment.sh b/.ci/pipelines/lib/deployment.sh deleted file mode 100644 index 442031455b..0000000000 --- a/.ci/pipelines/lib/deployment.sh +++ /dev/null @@ -1,62 +0,0 @@ -#!/bin/bash - -# Prevent sourcing multiple times in the same shell. -if [[ -n "${RHDH_DEPLOYMENT_LIB_SOURCED:-}" ]]; then - return 0 -fi -readonly RHDH_DEPLOYMENT_LIB_SOURCED=1 - -# shellcheck source=.ci/pipelines/reporting.sh -source "$(dirname "${BASH_SOURCE[0]}")/../reporting.sh" - -# Internal state -_DEPLOYMENT_COUNTER=0 - -deployment::next_id() { - _DEPLOYMENT_COUNTER=$((_DEPLOYMENT_COUNTER + 1)) - echo "${_DEPLOYMENT_COUNTER}" -} - -deployment::current_id() { - echo "${_DEPLOYMENT_COUNTER}" -} - -deployment::register() { - local label="$1" - deployment::next_id > /dev/null - save_status_deployment_namespace "${_DEPLOYMENT_COUNTER}" "$label" -} - -deployment::mark_deploy_success() { - save_status_failed_to_deploy "${_DEPLOYMENT_COUNTER}" false -} - -deployment::mark_deploy_failed() { - local label="$1" - deployment::register "$label" - save_status_failed_to_deploy "${_DEPLOYMENT_COUNTER}" true - save_status_test_failed "${_DEPLOYMENT_COUNTER}" true - save_status_number_of_test_failed "${_DEPLOYMENT_COUNTER}" "N/A" - save_overall_result 1 -} - -deployment::mark_test_result() { - local passed="$1" - local num_failures="${2:-0}" - if [[ "$passed" == "true" ]]; then - save_status_test_failed "${_DEPLOYMENT_COUNTER}" false - else - save_status_test_failed "${_DEPLOYMENT_COUNTER}" true - fi - save_status_number_of_test_failed "${_DEPLOYMENT_COUNTER}" "$num_failures" -} - -# Export all functions for subshell compatibility. -# Note: _DEPLOYMENT_COUNTER is NOT exported because subshells inherit only -# the snapshot at fork time — counter updates in the parent would not propagate. -export -f deployment::next_id -export -f deployment::current_id -export -f deployment::register -export -f deployment::mark_deploy_success -export -f deployment::mark_deploy_failed -export -f deployment::mark_test_result diff --git a/.ci/pipelines/lib/test-run-tracker.sh b/.ci/pipelines/lib/test-run-tracker.sh new file mode 100644 index 0000000000..3458a5e01e --- /dev/null +++ b/.ci/pipelines/lib/test-run-tracker.sh @@ -0,0 +1,62 @@ +#!/bin/bash + +# Prevent sourcing multiple times in the same shell. +if [[ -n "${RHDH_TEST_RUN_TRACKER_LIB_SOURCED:-}" ]]; then + return 0 +fi +readonly RHDH_TEST_RUN_TRACKER_LIB_SOURCED=1 + +# shellcheck source=.ci/pipelines/reporting.sh +source "$(dirname "${BASH_SOURCE[0]}")/../reporting.sh" + +# Internal state +_TEST_RUN_COUNTER=0 + +test_run_tracker::next_id() { + _TEST_RUN_COUNTER=$((_TEST_RUN_COUNTER + 1)) + echo "${_TEST_RUN_COUNTER}" +} + +test_run_tracker::current_id() { + echo "${_TEST_RUN_COUNTER}" +} + +test_run_tracker::register() { + local label="$1" + test_run_tracker::next_id > /dev/null + save_status_deployment_namespace "${_TEST_RUN_COUNTER}" "$label" +} + +test_run_tracker::mark_deploy_success() { + save_status_failed_to_deploy "${_TEST_RUN_COUNTER}" false +} + +test_run_tracker::mark_deploy_failed() { + local label="$1" + test_run_tracker::register "$label" + save_status_failed_to_deploy "${_TEST_RUN_COUNTER}" true + save_status_test_failed "${_TEST_RUN_COUNTER}" true + save_status_number_of_test_failed "${_TEST_RUN_COUNTER}" "N/A" + save_overall_result 1 +} + +test_run_tracker::mark_test_result() { + local passed="$1" + local num_failures="${2:-0}" + if [[ "$passed" == "true" ]]; then + save_status_test_failed "${_TEST_RUN_COUNTER}" false + else + save_status_test_failed "${_TEST_RUN_COUNTER}" true + fi + save_status_number_of_test_failed "${_TEST_RUN_COUNTER}" "$num_failures" +} + +# Export all functions for subshell compatibility. +# Note: _TEST_RUN_COUNTER is NOT exported because subshells inherit only +# the snapshot at fork time — counter updates in the parent would not propagate. +export -f test_run_tracker::next_id +export -f test_run_tracker::current_id +export -f test_run_tracker::register +export -f test_run_tracker::mark_deploy_success +export -f test_run_tracker::mark_deploy_failed +export -f test_run_tracker::mark_test_result diff --git a/.ci/pipelines/lib/testing.sh b/.ci/pipelines/lib/testing.sh index 498514f22c..c05db62f33 100644 --- a/.ci/pipelines/lib/testing.sh +++ b/.ci/pipelines/lib/testing.sh @@ -12,8 +12,8 @@ readonly TESTING_LIB_SOURCED=1 # shellcheck source=.ci/pipelines/lib/log.sh source "${DIR}/lib/log.sh" -# shellcheck source=.ci/pipelines/lib/deployment.sh -source "${DIR}/lib/deployment.sh" +# shellcheck source=.ci/pipelines/lib/test-run-tracker.sh +source "${DIR}/lib/test-run-tracker.sh" # ============================================================================== # Constants @@ -49,8 +49,8 @@ testing::run_tests() { return 1 fi - deployment::register "$artifacts_subdir" - deployment::mark_deploy_success + test_run_tracker::register "$artifacts_subdir" + test_run_tracker::mark_deploy_success BASE_URL="${url}" export BASE_URL @@ -121,7 +121,7 @@ testing::run_tests() { failed_tests="some" echo "Number of failed tests unknown, saving as $failed_tests." fi - deployment::mark_test_result "$test_passed" "${failed_tests}" + test_run_tracker::mark_test_result "$test_passed" "${failed_tests}" return "$test_result" } @@ -250,7 +250,7 @@ testing::check_and_test() { fi else echo "Backstage is not running. Marking deployment as failed and continuing..." - deployment::mark_deploy_failed "$artifacts_subdir" + test_run_tracker::mark_deploy_failed "$artifacts_subdir" save_all_pod_logs "$namespace" fi return 0 @@ -316,7 +316,7 @@ testing::check_upgrade_and_test() { testing::check_and_test "${release_name}" "${namespace}" "${playwright_project}" "${url}" else log::error "Helm upgrade encountered an issue or timed out. Exiting..." - deployment::mark_deploy_failed "$playwright_project" + test_run_tracker::mark_deploy_failed "$playwright_project" fi return 0 } diff --git a/.ci/pipelines/utils.sh b/.ci/pipelines/utils.sh index 45430c58d0..e7ce7c05cd 100755 --- a/.ci/pipelines/utils.sh +++ b/.ci/pipelines/utils.sh @@ -691,8 +691,8 @@ initiate_upgrade_base_deployments() { log::info "Initiating base RHDH deployment before upgrade" - deployment::register "$namespace" - deployment::mark_deploy_success + test_run_tracker::register "$namespace" + test_run_tracker::mark_deploy_success namespace::configure "${namespace}" From 798a2a0262b008029351577b409e73055bbb5b02 Mon Sep 17 00:00:00 2001 From: Gustavo Lira Date: Wed, 11 Mar 2026 09:47:25 -0300 Subject: [PATCH 8/8] docs(ci): update documentation and comments to reflect test-run-tracker rename - Update enhanced-ci-reporting.md to use test_run_tracker:: prefix and lib/test-run-tracker.sh file path - Clarify FUTURE MODULE comment in utils.sh to distinguish planned lib/deploy.sh from lib/test-run-tracker.sh Co-Authored-By: Claude Opus 4.6 --- .ci/pipelines/utils.sh | 2 +- docs/e2e-tests/enhanced-ci-reporting.md | 48 ++++++++++++------------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/.ci/pipelines/utils.sh b/.ci/pipelines/utils.sh index e7ce7c05cd..4714c90150 100755 --- a/.ci/pipelines/utils.sh +++ b/.ci/pipelines/utils.sh @@ -517,7 +517,7 @@ cluster_setup_k8s_helm() { } # ============================================================================== -# FUTURE MODULE: lib/deployment.sh +# FUTURE MODULE: lib/deploy.sh (not to be confused with lib/test-run-tracker.sh) # Functions: base_deployment, rbac_deployment, initiate_deployments, # base_deployment_osd_gcp, rbac_deployment_osd_gcp, initiate_deployments_osd_gcp, # initiate_upgrade_base_deployments, initiate_upgrade_deployments, diff --git a/docs/e2e-tests/enhanced-ci-reporting.md b/docs/e2e-tests/enhanced-ci-reporting.md index 7dc6af16d3..6e0c4b1486 100644 --- a/docs/e2e-tests/enhanced-ci-reporting.md +++ b/docs/e2e-tests/enhanced-ci-reporting.md @@ -10,49 +10,49 @@ The enhanced CI reporting system uses the [`.ci/pipelines/reporting.sh`](../../. ## Architecture -### Deployment Tracking Module +### Test Run Tracker Module -The [`.ci/pipelines/lib/deployment.sh`](../../.ci/pipelines/lib/deployment.sh) module encapsulates all deployment state management into a clean API. It manages an internal counter and delegates status persistence to `reporting.sh`. +The [`.ci/pipelines/lib/test-run-tracker.sh`](../../.ci/pipelines/lib/test-run-tracker.sh) module encapsulates all test run state management into a clean API. It manages an internal counter and delegates status persistence to `reporting.sh`. -#### `deployment::register(label)` -Registers a new deployment with the given label (typically the Playwright project name or artifacts subdirectory). Increments the internal counter and records the deployment label. +#### `test_run_tracker::register(label)` +Registers a new test run with the given label (typically the Playwright project name or artifacts subdirectory). Increments the internal counter and records the label. ```bash -deployment::register "$artifacts_subdir" +test_run_tracker::register "$artifacts_subdir" ``` -#### `deployment::mark_deploy_success()` -Marks the current deployment as successfully deployed. +#### `test_run_tracker::mark_deploy_success()` +Marks the current test run's deployment phase as successful. ```bash -deployment::mark_deploy_success +test_run_tracker::mark_deploy_success ``` -#### `deployment::mark_deploy_failed(label)` -Registers a new deployment and marks it as failed (deploy failed, tests failed, overall result = 1). +#### `test_run_tracker::mark_deploy_failed(label)` +Registers a new test run and marks it as failed (deploy failed, tests failed, overall result = 1). ```bash -deployment::mark_deploy_failed "$artifacts_subdir" +test_run_tracker::mark_deploy_failed "$artifacts_subdir" ``` -#### `deployment::mark_test_result(passed, num_failures)` +#### `test_run_tracker::mark_test_result(passed, num_failures)` Records whether tests passed and the number of failures. ```bash -deployment::mark_test_result "$test_passed" "${failed_tests}" +test_run_tracker::mark_test_result "$test_passed" "${failed_tests}" ``` -#### `deployment::current_id()` -Returns the current deployment counter value. +#### `test_run_tracker::current_id()` +Returns the current test run counter value. ```bash local id -id="$(deployment::current_id)" +id="$(test_run_tracker::current_id)" ``` ### Core Reporting Functions -The [`.ci/pipelines/reporting.sh`](../../.ci/pipelines/reporting.sh) script provides low-level functions used internally by the deployment module. These persist status to `SHARED_DIR` files and `ARTIFACT_DIR/reporting/`. +The [`.ci/pipelines/reporting.sh`](../../.ci/pipelines/reporting.sh) script provides low-level functions used internally by the test run tracker module. These persist status to `SHARED_DIR` files and `ARTIFACT_DIR/reporting/`. #### `save_status_deployment_namespace(deployment, label)` Records the label for a deployment. @@ -89,20 +89,20 @@ These files are also copied to `ARTIFACT_DIR/reporting/` for artifact collection ```bash # Source the required modules (typically done via utils.sh) source "${DIR}/reporting.sh" -source "${DIR}/lib/deployment.sh" +source "${DIR}/lib/test-run-tracker.sh" # Initialize overall result save_overall_result 0 -# Register a deployment and mark it as successful -deployment::register "showcase" -deployment::mark_deploy_success +# Register a test run and mark its deployment as successful +test_run_tracker::register "showcase" +test_run_tracker::mark_deploy_success # Record test results -deployment::mark_test_result "$test_passed" "${failed_tests}" +test_run_tracker::mark_test_result "$test_passed" "${failed_tests}" # Or mark a deployment as failed in one call -deployment::mark_deploy_failed "showcase-rbac" +test_run_tracker::mark_deploy_failed "showcase-rbac" ``` ### Error Handling @@ -145,7 +145,7 @@ For nightly runs, the system automatically sends notifications to the `#rhdh-e2e ## File Locations -- **Deployment Module**: [`.ci/pipelines/lib/deployment.sh`](../../.ci/pipelines/lib/deployment.sh) +- **Test Run Tracker**: [`.ci/pipelines/lib/test-run-tracker.sh`](../../.ci/pipelines/lib/test-run-tracker.sh) - **Reporting Script**: [`.ci/pipelines/reporting.sh`](../../.ci/pipelines/reporting.sh) - **Integration**: [`.ci/pipelines/utils.sh`](../../.ci/pipelines/utils.sh) and [`.ci/pipelines/openshift-ci-tests.sh`](../../.ci/pipelines/openshift-ci-tests.sh)