Skip to content

Commit a7771f4

Browse files
committed
fix(ci): address code review findings for parallel deployments
1. postgres-cred.yaml in-place edit race: configure_external_postgres_db now patches a temp copy instead of editing the shared template via sed_inplace. Same class of bug fixed in apply_yaml_files. 2. Unquoted ${NAME_SPACE} in namespace::configure calls: quote both occurrences (lines 553, 721) to prevent word-splitting. 3. mktemp in helm::merge_values overwrite path: move mktemp + trap inside the "merge" branch so the "overwrite" path doesn't create and immediately discard two unused temp files. 4. Remove redundant "Waiting for parallel deployments" log::section that fires right before the blocking wait calls — the bookend sections are sufficient.
1 parent 23996e8 commit a7771f4

2 files changed

Lines changed: 17 additions & 12 deletions

File tree

.ci/pipelines/lib/helm.sh

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,13 @@ helm::merge_values() {
3939
return 1
4040
fi
4141

42-
# Use unique temp files per invocation to support parallel deployments
43-
local step_1_file step_2_file
44-
step_1_file=$(mktemp "${TMPDIR:-/tmp}/helm-merge-step1-XXXXXX.yaml")
45-
step_2_file=$(mktemp "${TMPDIR:-/tmp}/helm-merge-step2-XXXXXX.yaml")
46-
trap 'rm -f "${step_1_file}" "${step_2_file}"' RETURN
47-
4842
if [[ "$plugin_operation" == "merge" ]]; then
43+
# Temp files only needed for the merge path; overwrite writes directly to final_file
44+
local step_1_file step_2_file
45+
step_1_file=$(mktemp "${TMPDIR:-/tmp}/helm-merge-step1-XXXXXX.yaml")
46+
step_2_file=$(mktemp "${TMPDIR:-/tmp}/helm-merge-step2-XXXXXX.yaml")
47+
trap 'rm -f "${step_1_file}" "${step_2_file}"' RETURN
48+
4949
# Step 1: Merge files, excluding the .global.dynamic.plugins key
5050
# Values from `diff_file` override those in `base_file`
5151
yq eval-all '

.ci/pipelines/utils.sh

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -281,15 +281,21 @@ configure_external_postgres_db() {
281281

282282
# Now we can safely get the password
283283
POSTGRES_PASSWORD=$(oc get secret/postgress-external-db-pguser-janus-idp -n "${NAME_SPACE_POSTGRES_DB}" -o jsonpath='{.data.password}')
284-
common::sed_inplace "s|POSTGRES_PASSWORD:.*|POSTGRES_PASSWORD: ${POSTGRES_PASSWORD}|g" "${DIR}/resources/postgres-db/postgres-cred.yaml"
285284
POSTGRES_HOST=$(common::base64_encode "postgress-external-db-primary.$NAME_SPACE_POSTGRES_DB.svc.cluster.local")
286-
common::sed_inplace "s|POSTGRES_HOST:.*|POSTGRES_HOST: ${POSTGRES_HOST}|g" "${DIR}/resources/postgres-db/postgres-cred.yaml"
285+
286+
# Patch a temp copy instead of editing the shared template in-place (parallel-deployment safe)
287+
local pg_cred_file
288+
pg_cred_file=$(mktemp "${TMPDIR:-/tmp}/postgres-cred-${project}-XXXXXX.yaml")
289+
sed "s|POSTGRES_PASSWORD:.*|POSTGRES_PASSWORD: ${POSTGRES_PASSWORD}|g;s|POSTGRES_HOST:.*|POSTGRES_HOST: ${POSTGRES_HOST}|g" \
290+
"${DIR}/resources/postgres-db/postgres-cred.yaml" > "${pg_cred_file}"
287291

288292
# Validate final configuration apply
289-
if ! oc apply -f "${DIR}/resources/postgres-db/postgres-cred.yaml" --namespace="${project}"; then
293+
if ! oc apply -f "${pg_cred_file}" --namespace="${project}"; then
294+
rm -f "${pg_cred_file}"
290295
log::error "Failed to apply PostgreSQL credentials"
291296
return 1
292297
fi
298+
rm -f "${pg_cred_file}"
293299

294300
log::success "External PostgreSQL database configured successfully!"
295301
}
@@ -550,7 +556,7 @@ base_deployment() {
550556
common::require_vars "RELEASE_NAME" "TAG_NAME" "IMAGE_REGISTRY" "IMAGE_REPO" "K8S_CLUSTER_ROUTER_BASE" || return 1
551557
local artifacts_subdir=$1
552558

553-
namespace::configure ${NAME_SPACE}
559+
namespace::configure "${NAME_SPACE}"
554560

555561
deploy_redis_cache "${NAME_SPACE}"
556562

@@ -680,7 +686,6 @@ _run_parallel_deployments() {
680686
local rbac_pid=$!
681687
log::info "RBAC deployment started in background (PID: ${rbac_pid})"
682688

683-
log::section "Waiting for parallel deployments to complete..."
684689
local base_rc=0 rbac_rc=0
685690
wait "${base_pid}" || base_rc=$?
686691
wait "${rbac_pid}" || rbac_rc=$?
@@ -718,7 +723,7 @@ base_deployment_osd_gcp() {
718723
common::require_vars "RELEASE_NAME" "TAG_NAME" "IMAGE_REGISTRY" "IMAGE_REPO" "K8S_CLUSTER_ROUTER_BASE" || return 1
719724
local artifacts_subdir=$1
720725

721-
namespace::configure ${NAME_SPACE}
726+
namespace::configure "${NAME_SPACE}"
722727

723728
deploy_redis_cache "${NAME_SPACE}"
724729

0 commit comments

Comments
 (0)