Skip to content

Commit 9403307

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 fb6b330 commit 9403307

2 files changed

Lines changed: 8 additions & 9 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: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ base_deployment() {
571571
common::require_vars "RELEASE_NAME" "TAG_NAME" "IMAGE_REGISTRY" "IMAGE_REPO" "K8S_CLUSTER_ROUTER_BASE" || return 1
572572
local artifacts_subdir=$1
573573

574-
namespace::configure ${NAME_SPACE}
574+
namespace::configure "${NAME_SPACE}"
575575

576576
deploy_redis_cache "${NAME_SPACE}"
577577

@@ -701,7 +701,6 @@ _run_parallel_deployments() {
701701
local rbac_pid=$!
702702
log::info "RBAC deployment started in background (PID: ${rbac_pid})"
703703

704-
log::section "Waiting for parallel deployments to complete..."
705704
local base_rc=0 rbac_rc=0
706705
wait "${base_pid}" || base_rc=$?
707706
wait "${rbac_pid}" || rbac_rc=$?
@@ -739,7 +738,7 @@ base_deployment_osd_gcp() {
739738
common::require_vars "RELEASE_NAME" "TAG_NAME" "IMAGE_REGISTRY" "IMAGE_REPO" "K8S_CLUSTER_ROUTER_BASE" || return 1
740739
local artifacts_subdir=$1
741740

742-
namespace::configure ${NAME_SPACE}
741+
namespace::configure "${NAME_SPACE}"
743742

744743
deploy_redis_cache "${NAME_SPACE}"
745744

0 commit comments

Comments
 (0)