-
Notifications
You must be signed in to change notification settings - Fork 221
chore(ci): fix Operator tests, artifact handling, pod selectors #4414
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d1c17cc
dcfd168
7d2c984
429058e
882a27b
fa41e70
2e9192c
221cffb
8b4c611
92d923e
d6613b5
004ba27
dee327c
4576353
35e5c39
78ff1fe
70970ed
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,18 +87,16 @@ testing::run_tests() { | |
| pkill Xvfb || true | ||
|
|
||
| # Use artifacts_subdir for artifact directory to keep artifacts organized | ||
| mkdir -p "${ARTIFACT_DIR}/${artifacts_subdir}/test-results" | ||
| mkdir -p "${ARTIFACT_DIR}/${artifacts_subdir}/attachments/screenshots" | ||
| rsync -a "${e2e_tests_dir}/test-results/" "${ARTIFACT_DIR}/${artifacts_subdir}/test-results/" || true | ||
| rsync -a "${e2e_tests_dir}/${JUNIT_RESULTS}" "${ARTIFACT_DIR}/${artifacts_subdir}/${JUNIT_RESULTS}" || true | ||
| common::save_artifact "${artifacts_subdir}" "${e2e_tests_dir}/test-results/" "test-results" || true | ||
| common::save_artifact "${artifacts_subdir}" "${e2e_tests_dir}/${JUNIT_RESULTS}" || true | ||
| if [[ "${CI}" == "true" ]]; then | ||
| rsync "${ARTIFACT_DIR}/${artifacts_subdir}/${JUNIT_RESULTS}" "${SHARED_DIR}/junit-results-${artifacts_subdir}.xml" || true | ||
| fi | ||
|
|
||
| rsync -a "${e2e_tests_dir}/screenshots/" "${ARTIFACT_DIR}/${artifacts_subdir}/attachments/screenshots/" || true | ||
| common::save_artifact "${artifacts_subdir}" "${e2e_tests_dir}/screenshots/" "attachments/screenshots" || true | ||
| ansi2html < "/tmp/${LOGFILE}" > "/tmp/${LOGFILE}.html" | ||
| rsync -a "/tmp/${LOGFILE}.html" "${ARTIFACT_DIR}/${artifacts_subdir}/" || true | ||
| rsync -a "${e2e_tests_dir}/playwright-report/" "${ARTIFACT_DIR}/${artifacts_subdir}/" || true | ||
| common::save_artifact "${artifacts_subdir}" "/tmp/${LOGFILE}.html" || true | ||
| common::save_artifact "${artifacts_subdir}" "${e2e_tests_dir}/playwright-report/" || true | ||
|
|
||
| echo "Playwright project '${playwright_project}' in namespace '${namespace}' (artifacts: ${artifacts_subdir}) RESULT: ${test_result}" | ||
| local test_passed="true" | ||
|
|
@@ -134,26 +132,23 @@ testing::run_tests() { | |
| # $1 - release_name: The Helm release name | ||
| # $2 - namespace: The namespace where Backstage is deployed | ||
| # $3 - url: The URL to check | ||
| # $4 - max_attempts: (optional) Maximum number of attempts (default: 30) | ||
| # $5 - wait_seconds: (optional) Seconds to wait between attempts (default: 30) | ||
| # $4 - artifacts_subdir: (optional) Subdirectory for artifacts (defaults to namespace) | ||
| # $5 - max_attempts: (optional) Maximum number of attempts (default: 30) | ||
| # $6 - wait_seconds: (optional) Seconds to wait between attempts (default: 30) | ||
| # Returns: | ||
| # 0 - Backstage is running | ||
| # 1 - Backstage is not running or crashed | ||
| testing::check_backstage_running() { | ||
| local release_name=$1 | ||
| local namespace=$2 | ||
| local url=$3 | ||
| local max_attempts=${4:-30} | ||
| local wait_seconds=${5:-30} | ||
| local artifacts_subdir=$4 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a breaking positional parameter change — old
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's called only once (in |
||
| local max_attempts=${5:-30} | ||
| local wait_seconds=${6:-30} | ||
|
|
||
| if [[ -z "$release_name" || -z "$namespace" ]]; then | ||
| if [[ -z "$release_name" || -z "$namespace" || -z "$url" || -z "$artifacts_subdir" ]]; then | ||
| log::error "${_TESTING_ERR_MISSING_PARAMS}" | ||
| log::info "Usage: testing::check_backstage_running <release_name> <namespace> <url> [max_attempts] [wait_seconds]" | ||
| return 1 | ||
| fi | ||
|
|
||
| if [[ -z "${url}" ]]; then | ||
| log::error "Error: URL is not set. Please provide a valid URL." | ||
| log::info "Usage: testing::check_backstage_running <release_name> <namespace> <url> <artifacts_subdir> [max_attempts] [wait_seconds]" | ||
| return 1 | ||
| fi | ||
|
|
||
|
|
@@ -190,8 +185,7 @@ testing::check_backstage_running() { | |
| || oc logs deployment/${release_name} -n "${namespace}" --tail=100 --all-containers=true 2> /dev/null || true | ||
| log::error "Recent events:" | ||
| oc get events -n "${namespace}" --sort-by='.lastTimestamp' | tail -20 | ||
| mkdir -p "${ARTIFACT_DIR}/${namespace}" | ||
| rsync -a "/tmp/${LOGFILE}" "${ARTIFACT_DIR}/${namespace}/" || true | ||
| common::save_artifact "${artifacts_subdir}" "/tmp/${LOGFILE}" || true | ||
| return 1 | ||
| fi | ||
|
|
||
|
|
@@ -201,8 +195,7 @@ testing::check_backstage_running() { | |
|
|
||
| log::error "Failed to reach Backstage at ${url} after ${max_attempts} attempts." | ||
| oc get events -n "${namespace}" --sort-by='.lastTimestamp' | tail -10 | ||
| mkdir -p "${ARTIFACT_DIR}/${namespace}" | ||
| rsync -a "/tmp/${LOGFILE}" "${ARTIFACT_DIR}/${namespace}/" || true | ||
| common::save_artifact "${artifacts_subdir}" "/tmp/${LOGFILE}" || true | ||
| return 1 | ||
| } | ||
|
|
||
|
|
@@ -235,7 +228,7 @@ testing::check_and_test() { | |
| return 1 | ||
| fi | ||
|
|
||
| if testing::check_backstage_running "${release_name}" "${namespace}" "${url}" "${max_attempts}" "${wait_seconds}"; then | ||
| if testing::check_backstage_running "${release_name}" "${namespace}" "${url}" "${artifacts_subdir}" "${max_attempts}" "${wait_seconds}"; then | ||
| echo "Display pods for verification..." | ||
| oc get pods -n "${namespace}" | ||
| if [[ "${SKIP_TESTS:-false}" == "true" ]]; then | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -154,8 +154,7 @@ save_all_pod_logs() { | |
| wait "$pid" 2> /dev/null || true | ||
| done | ||
|
|
||
| mkdir -p "${ARTIFACT_DIR}/${artifacts_subdir}/pod_logs" | ||
| rsync -a pod_logs/ "${ARTIFACT_DIR}/${artifacts_subdir}/pod_logs/" || true | ||
| common::save_artifact "${artifacts_subdir}" "pod_logs/" "pod_logs" || true | ||
| set -e | ||
| } | ||
|
|
||
|
|
@@ -528,6 +527,8 @@ cluster_setup_k8s_helm() { | |
| # ============================================================================== | ||
|
|
||
| base_deployment() { | ||
| local artifacts_subdir=$1 | ||
|
|
||
| namespace::configure ${NAME_SPACE} | ||
|
|
||
| deploy_redis_cache "${NAME_SPACE}" | ||
|
|
@@ -542,8 +543,7 @@ base_deployment() { | |
| helm::merge_values "merge" "${DIR}/value_files/${HELM_CHART_VALUE_FILE_NAME}" "${DIR}/value_files/diff-values_showcase_PR.yaml" "${merged_pr_value_file}" | ||
| disable_orchestrator_plugins_in_values "${merged_pr_value_file}" | ||
|
|
||
| mkdir -p "${ARTIFACT_DIR}/${NAME_SPACE}" | ||
| rsync -a "${merged_pr_value_file}" "${ARTIFACT_DIR}/${NAME_SPACE}/" || true | ||
| common::save_artifact "${artifacts_subdir}" "${merged_pr_value_file}" || true | ||
| # shellcheck disable=SC2046 | ||
| helm upgrade -i "${RELEASE_NAME}" -n "${NAME_SPACE}" \ | ||
| "${HELM_CHART_URL}" --version "${CHART_VERSION}" \ | ||
|
|
@@ -562,6 +562,8 @@ base_deployment() { | |
| } | ||
|
|
||
| rbac_deployment() { | ||
| local artifacts_subdir=$1 | ||
|
|
||
| namespace::configure "${NAME_SPACE_POSTGRES_DB}" | ||
| namespace::configure "${NAME_SPACE_RBAC}" | ||
| configure_external_postgres_db "${NAME_SPACE_RBAC}" | ||
|
|
@@ -583,8 +585,7 @@ rbac_deployment() { | |
| helm::merge_values "merge" "${DIR}/value_files/${HELM_CHART_RBAC_VALUE_FILE_NAME}" "${DIR}/value_files/diff-values_showcase-rbac_PR.yaml" "${merged_pr_rbac_value_file}" | ||
| disable_orchestrator_plugins_in_values "${merged_pr_rbac_value_file}" | ||
|
|
||
| mkdir -p "${ARTIFACT_DIR}/${NAME_SPACE_RBAC}" | ||
| rsync -a "${merged_pr_rbac_value_file}" "${ARTIFACT_DIR}/${NAME_SPACE_RBAC}/" || true | ||
| common::save_artifact "${artifacts_subdir}" "${merged_pr_rbac_value_file}" || true | ||
| # shellcheck disable=SC2046 | ||
| helm upgrade -i "${RELEASE_NAME_RBAC}" -n "${NAME_SPACE_RBAC}" \ | ||
| "${HELM_CHART_URL}" --version "${CHART_VERSION}" \ | ||
|
|
@@ -618,13 +619,18 @@ rbac_deployment() { | |
| } | ||
|
|
||
| initiate_deployments() { | ||
| local base_artifacts_subdir=$1 | ||
| local rbac_artifacts_subdir=$2 | ||
|
|
||
| cd "${DIR}" | ||
| base_deployment | ||
| rbac_deployment | ||
| base_deployment "${base_artifacts_subdir}" | ||
| rbac_deployment "${rbac_artifacts_subdir}" | ||
| } | ||
|
|
||
| # OSD-GCP specific deployment functions that merge diff files and skip orchestrator workflows | ||
| base_deployment_osd_gcp() { | ||
| local artifacts_subdir=$1 | ||
|
|
||
| namespace::configure ${NAME_SPACE} | ||
|
|
||
| deploy_redis_cache "${NAME_SPACE}" | ||
|
|
@@ -635,8 +641,7 @@ base_deployment_osd_gcp() { | |
|
|
||
| # Merge base values with OSD-GCP diff file | ||
| helm::merge_values "merge" "${DIR}/value_files/${HELM_CHART_VALUE_FILE_NAME}" "${DIR}/value_files/${HELM_CHART_OSD_GCP_DIFF_VALUE_FILE_NAME}" "/tmp/merged-values_showcase_OSD-GCP.yaml" | ||
| mkdir -p "${ARTIFACT_DIR}/${NAME_SPACE}" | ||
| rsync -a "/tmp/merged-values_showcase_OSD-GCP.yaml" "${ARTIFACT_DIR}/${NAME_SPACE}/" # Save the final value-file into the artifacts directory. | ||
| common::save_artifact "${artifacts_subdir}" "/tmp/merged-values_showcase_OSD-GCP.yaml" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most other
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the codebase the collection of merged yaml value files files (like this one) lack the |
||
|
|
||
| log::info "Deploying image from repository: ${QUAY_REPO}, TAG_NAME: ${TAG_NAME}, in NAME_SPACE: ${NAME_SPACE}" | ||
|
|
||
|
|
@@ -652,6 +657,8 @@ base_deployment_osd_gcp() { | |
| } | ||
|
|
||
| rbac_deployment_osd_gcp() { | ||
| local artifacts_subdir=$1 | ||
|
|
||
| namespace::configure "${NAME_SPACE_POSTGRES_DB}" | ||
| namespace::configure "${NAME_SPACE_RBAC}" | ||
| configure_external_postgres_db "${NAME_SPACE_RBAC}" | ||
|
|
@@ -662,8 +669,7 @@ rbac_deployment_osd_gcp() { | |
|
|
||
| # Merge RBAC values with OSD-GCP diff file | ||
| helm::merge_values "merge" "${DIR}/value_files/${HELM_CHART_RBAC_VALUE_FILE_NAME}" "${DIR}/value_files/${HELM_CHART_RBAC_OSD_GCP_DIFF_VALUE_FILE_NAME}" "/tmp/merged-values_showcase-rbac_OSD-GCP.yaml" | ||
| mkdir -p "${ARTIFACT_DIR}/${NAME_SPACE_RBAC}" | ||
| rsync -a "/tmp/merged-values_showcase-rbac_OSD-GCP.yaml" "${ARTIFACT_DIR}/${NAME_SPACE_RBAC}/" # Save the final value-file into the artifacts directory. | ||
| common::save_artifact "${artifacts_subdir}" "/tmp/merged-values_showcase-rbac_OSD-GCP.yaml" | ||
|
|
||
| log::info "Deploying image from repository: ${QUAY_REPO}, TAG_NAME: ${TAG_NAME}, in NAME_SPACE: ${RELEASE_NAME_RBAC}" | ||
|
|
||
|
|
@@ -679,9 +685,12 @@ rbac_deployment_osd_gcp() { | |
| } | ||
|
|
||
| initiate_deployments_osd_gcp() { | ||
| local base_artifacts_subdir=$1 | ||
| local rbac_artifacts_subdir=$2 | ||
|
|
||
| cd "${DIR}" | ||
| base_deployment_osd_gcp | ||
| rbac_deployment_osd_gcp | ||
| base_deployment_osd_gcp "${base_artifacts_subdir}" | ||
| rbac_deployment_osd_gcp "${rbac_artifacts_subdir}" | ||
| } | ||
|
|
||
| # install base RHDH deployment before upgrade | ||
|
|
@@ -761,14 +770,14 @@ initiate_sanity_plugin_checks_deployment() { | |
| local release_name=$1 | ||
| local name_space_sanity_plugins_check=$2 | ||
| local sanity_plugins_url=$3 | ||
| local artifacts_subdir=$4 | ||
|
|
||
| namespace::configure "${name_space_sanity_plugins_check}" | ||
| helm::uninstall "${name_space_sanity_plugins_check}" "${release_name}" | ||
| deploy_redis_cache "${name_space_sanity_plugins_check}" | ||
| apply_yaml_files "${DIR}" "${name_space_sanity_plugins_check}" "${sanity_plugins_url}" | ||
| helm::merge_values "overwrite" "${DIR}/value_files/${HELM_CHART_VALUE_FILE_NAME}" "${DIR}/value_files/${HELM_CHART_SANITY_PLUGINS_DIFF_VALUE_FILE_NAME}" "/tmp/${HELM_CHART_SANITY_PLUGINS_MERGED_VALUE_FILE_NAME}" | ||
| mkdir -p "${ARTIFACT_DIR}/${name_space_sanity_plugins_check}" | ||
| rsync -a "/tmp/${HELM_CHART_SANITY_PLUGINS_MERGED_VALUE_FILE_NAME}" "${ARTIFACT_DIR}/${name_space_sanity_plugins_check}/" || true # Save the final value-file into the artifacts directory. | ||
| common::save_artifact "${artifacts_subdir}" "/tmp/${HELM_CHART_SANITY_PLUGINS_MERGED_VALUE_FILE_NAME}" || true | ||
| # shellcheck disable=SC2046 | ||
| helm upgrade -i "${release_name}" -n "${name_space_sanity_plugins_check}" \ | ||
| "${HELM_CHART_URL}" --version "${CHART_VERSION}" \ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment says this is only relevant "on OpenShift" but the function itself has no guard — it'll fail if ever called on a non-OCP cluster. The protection comes from call sites being in OCP-specific scripts, which is fine, but maybe add a brief note like
# Caller must ensure this runs on OCP onlyto make the contract explicit?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's actually an AI comment from the codebase which wasn't correct. I'll update the description of the function to be more accurate