Skip to content

Commit 23996e8

Browse files
committed
fix(ci): eliminate race conditions for parallel deployments
Address 3 critical race conditions identified by Qodo code review: **1. Shared temp files in helm::merge_values (Bug #1)** - BEFORE: Used fixed paths /tmp/step-without-plugins.yaml and /tmp/step-only-plugins.yaml - AFTER: Generate unique temp files per invocation via mktemp - WHY: Concurrent helm::merge_values calls would overwrite each other's intermediate files, corrupting merged values **2. Kubeconfig global state mutation (Bug #2)** - BEFORE: namespace::configure and apply_yaml_files both ran `oc config set-context --current --namespace=...` - AFTER: Removed all oc config set-context calls; added explicit --namespace flag to all oc/kubectl apply commands that lacked it - WHY: Parallel deployments racing on current context would apply resources to nondeterministic namespaces **3. In-place YAML file edits (Bug #3)** - BEFORE: apply_yaml_files used sed -i to modify shared YAML templates directly in the repo - AFTER: Copy templates to per-deployment tmpdir, patch copies, apply from tmpdir - WHY: Concurrent sed -i edits would clobber each other and apply manifests with wrong namespace fields All three bugs would have caused nondeterministic deployment failures when base_deployment and rbac_deployment run in parallel. Changes: - .ci/pipelines/lib/helm.sh: mktemp + trap cleanup for intermediate files - .ci/pipelines/lib/namespace.sh: Remove oc config set-context - .ci/pipelines/utils.sh: Tmpdir for YAML patches, explicit --namespace on all oc apply calls
1 parent f631cd9 commit 23996e8

3 files changed

Lines changed: 27 additions & 17 deletions

File tree

.ci/pipelines/lib/helm.sh

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,19 @@ helm::merge_values() {
3232
local base_file=$2
3333
local diff_file=$3
3434
local final_file=$4
35-
local step_1_file="/tmp/step-without-plugins.yaml"
36-
local step_2_file="/tmp/step-only-plugins.yaml"
3735

3836
if [[ -z "$plugin_operation" || -z "$base_file" || -z "$diff_file" || -z "$final_file" ]]; then
3937
log::error "Missing required parameters"
4038
log::info "Usage: helm::merge_values <operation> <base_file> <diff_file> <output_file>"
4139
return 1
4240
fi
4341

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+
4448
if [[ "$plugin_operation" == "merge" ]]; then
4549
# Step 1: Merge files, excluding the .global.dynamic.plugins key
4650
# Values from `diff_file` override those in `base_file`

.ci/pipelines/lib/namespace.sh

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,15 @@ namespace::setup_image_pull_secret() {
8383
# ==============================================================================
8484

8585
# Function: namespace::configure
86-
# Description: Deletes and recreates a namespace, then sets it as current context
86+
# Description: Deletes and recreates a namespace
8787
# Arguments:
8888
# $1 - project: The namespace/project name
8989
# Returns:
9090
# 0 - Success
9191
# Exits on failure
92+
# Notes:
93+
# Does not set kubeconfig current context to support parallel deployments.
94+
# All downstream oc/kubectl commands must use explicit --namespace flag.
9295
namespace::configure() {
9396
local project=$1
9497
log::warn "Deleting and recreating namespace: $project"
@@ -98,10 +101,6 @@ namespace::configure() {
98101
log::error "Error: Failed to create namespace ${project}" >&2
99102
exit 1
100103
fi
101-
if ! oc config set-context --current --namespace="${project}"; then
102-
log::error "Error: Failed to set context for namespace ${project}" >&2
103-
exit 1
104-
fi
105104

106105
echo "Namespace ${project} is ready."
107106
return 0

.ci/pipelines/utils.sh

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -300,28 +300,35 @@ apply_yaml_files() {
300300
local rhdh_base_url=$3
301301
log::info "Applying YAML files to namespace ${project}"
302302

303-
oc config set-context --current --namespace="${project}"
303+
# Create temporary directory for namespace-patched YAMLs (parallel-deployment safe)
304+
local tmpdir
305+
tmpdir=$(mktemp -d "${TMPDIR:-/tmp}/apply-yaml-${project}-XXXXXX")
306+
trap 'rm -rf "${tmpdir}"' RETURN
304307

308+
# Copy and patch YAML files that need namespace substitution
305309
local files=(
306310
"$dir/resources/service_account/service-account-rhdh.yaml"
307311
"$dir/resources/cluster_role_binding/cluster-role-binding-k8s.yaml"
308312
"$dir/resources/cluster_role/cluster-role-k8s.yaml"
309313
)
310314

311315
for file in "${files[@]}"; do
312-
common::sed_inplace "s/namespace:.*/namespace: ${project}/g" "$file"
316+
local basename
317+
basename=$(basename "$file")
318+
sed "s/namespace:.*/namespace: ${project}/g" "$file" > "${tmpdir}/${basename}"
313319
done
314320

315321
DH_TARGET_URL=$(common::base64_encode "test-backstage-customization-provider-${project}.${K8S_CLUSTER_ROUTER_BASE}")
316322
RHDH_BASE_URL=$(common::base64_encode "$rhdh_base_url")
317323
RHDH_BASE_URL_HTTP=$(common::base64_encode "${rhdh_base_url/https/http}")
318324
export DH_TARGET_URL RHDH_BASE_URL RHDH_BASE_URL_HTTP
319325

320-
oc apply -f "$dir/resources/service_account/service-account-rhdh.yaml" --namespace="${project}"
326+
# Apply YAMLs from tmpdir (patched) or original location (no patch needed)
327+
oc apply -f "${tmpdir}/service-account-rhdh.yaml" --namespace="${project}"
321328
oc apply -f "$dir/auth/service-account-rhdh-secret.yaml" --namespace="${project}"
322329

323-
oc apply -f "$dir/resources/cluster_role/cluster-role-k8s.yaml" --namespace="${project}"
324-
oc apply -f "$dir/resources/cluster_role_binding/cluster-role-binding-k8s.yaml" --namespace="${project}"
330+
oc apply -f "${tmpdir}/cluster-role-k8s.yaml" --namespace="${project}"
331+
oc apply -f "${tmpdir}/cluster-role-binding-k8s.yaml" --namespace="${project}"
325332

326333
envsubst < "${DIR}/auth/secrets-rhdh-secrets.yaml" | oc apply --namespace="${project}" -f -
327334

@@ -349,15 +356,15 @@ apply_yaml_files() {
349356
# Tekton tests are not executed in showcase-k8s or showcase-rbac-k8s projects
350357
if [[ "$JOB_NAME" != *"aks"* && "$JOB_NAME" != *"eks"* && "$JOB_NAME" != *"gke"* ]]; then
351358
# Create Pipeline run for tekton test case.
352-
oc apply -f "$dir/resources/pipeline-run/hello-world-pipeline.yaml"
353-
oc apply -f "$dir/resources/pipeline-run/hello-world-pipeline-run.yaml"
359+
oc apply -f "$dir/resources/pipeline-run/hello-world-pipeline.yaml" --namespace="${project}"
360+
oc apply -f "$dir/resources/pipeline-run/hello-world-pipeline-run.yaml" --namespace="${project}"
354361

355362
# Create Deployment and Pipeline for Topology test.
356-
oc apply -f "$dir/resources/topology_test/topology-test.yaml"
363+
oc apply -f "$dir/resources/topology_test/topology-test.yaml" --namespace="${project}"
357364
if [[ -z "${IS_OPENSHIFT}" || "${IS_OPENSHIFT}" == "false" ]]; then
358-
kubectl apply -f "$dir/resources/topology_test/topology-test-ingress.yaml"
365+
kubectl apply -f "$dir/resources/topology_test/topology-test-ingress.yaml" --namespace="${project}"
359366
else
360-
oc apply -f "$dir/resources/topology_test/topology-test-route.yaml"
367+
oc apply -f "$dir/resources/topology_test/topology-test-route.yaml" --namespace="${project}"
361368
fi
362369
else
363370
log::info "Skipping Tekton Pipeline and Topology resources for K8s deployment (${JOB_NAME})"

0 commit comments

Comments
 (0)