-
Notifications
You must be signed in to change notification settings - Fork 226
Fix: Prevent duplicate sidecars by copying to temp before sed modifications #621
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
base: master
Are you sure you want to change the base?
Fix: Prevent duplicate sidecars by copying to temp before sed modifications #621
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: kaovilai The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Welcome @kaovilai! |
|
Hi @kaovilai. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
When SNAPSHOT_METADATA_TESTS=true or VOLUME_MODE_CONVERSION_TESTS=true, the deploy script was modifying git-tracked YAML files in-place using sed -i. This caused issues on subsequent runs: - Run 1: Works fine, adds 1 snapshot-metadata sidecar - Run 2: File already modified, adds another sidecar → duplicate containers - Run 3+: More duplicates → Kubernetes rejects deployment Root cause: - Lines 257, 263-265 used sed -i to modify files directly - Modified files were committed to git or left dirty - Not idempotent - each run added more modifications Solution: - Copy files to TEMP_DIR before modifications - Apply all sed operations to temporary copies - Keep original git-tracked files pristine - Script is now fully idempotent Fixes the bug across all kubernetes-* deployments that use deploy/util/deploy-hostpath.sh as a symlink. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
d185fc6 to
822fe02
Compare
|
/ok-to-test |
Description
Fixes #620
This PR prevents the duplicate container bug that occurs when running
SNAPSHOT_METADATA_TESTS=true ./deploy.shorVOLUME_MODE_CONVERSION_TESTS=true ./deploy.shmultiple times.Problem
The deployment script was using
sed -ito modify git-tracked YAML files in-place. Each run would add more sidecars, causing:Solution
Copy files to
${TEMP_DIR}before applying sed modifications:This keeps original files pristine and makes the script fully idempotent.
Changes
deploy/util/deploy-hostpath.shlines 257-270deploy/kubernetes-*/deploy.shsymlinks benefit from this fixTesting
The script can now be run multiple times without creating duplicate containers. Original git-tracked files remain unchanged.
What type of PR is this?
/kind bug
What this PR does / why we need it:
Fixes deployment script to copy YAML files to temp directory before modifications, preventing duplicate sidecars on subsequent runs.
Which issue(s) this PR fixes:
Fixes #620
Special notes for your reviewer:
The fix is simple: copy files to
${TEMP_DIR}before runningsed -i, keeping the same sed syntax but operating on temporary copies instead of git-tracked files.Does this PR introduce a user-facing change?:
Note
Responses generated with Claude