Skip to content

ETCD-704: cluster-restore.sh: move extra /var/lib/etcd files to backup #1628

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.

Sign up for GitHub

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

Merged
openshift-merge-bot merged 4 commits into from
Jun 15, 2026
+24 −8
Merged

ETCD-704: cluster-restore.sh: move extra /var/lib/etcd files to backup #1628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cf13962
ETCD-704 — cluster-restore.sh: move extra /var/lib/etcd files to backup
Jun 7, 2026
5f56b3a
ETCD-704: move extra etcd data dir files to backup subdirectory
Jun 9, 2026
4b715f1
ETCD-704: move backup helper to etcd-common-tools
Jun 11, 2026
38bade7
ETCD-704: backup extra data dir contents in cluster-restore-tnf.sh
Jun 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions bindata/etcd/cluster-restore-tnf.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,9 @@ if [ -d "${ETCD_DATA_DIR}/member" ]; then
mv "${ETCD_DATA_DIR}"/member "${ETCD_DATA_DIR_BACKUP}"/
fi

# Move any remaining files out of the data dir before wiping it.
backup_remaining_etcd_data_dir_contents

echo "removing etcd data dir..."
rm -rf "${ETCD_DATA_DIR}"
mkdir -p "${ETCD_DATA_DIR}"
Expand Down
11 changes: 3 additions & 8 deletions bindata/etcd/cluster-restore.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,9 @@ if [ -z "${ETCD_ETCDCTL_RESTORE}" ]; then
cp -p "${SNAPSHOT_FILE}" "${ETCD_DATA_DIR_BACKUP}"/snapshot.db
# Move the revision.json when it exists
[ ! -f "${ETCD_REV_JSON}" ] || mv -f "${ETCD_REV_JSON}" "${ETCD_DATA_DIR_BACKUP}"/revision.json
# removing any fio perf files left behind that could be deleted without problems
rm -f "${ETCD_DATA_DIR}"/etcd_perf*

# ensure the folder is really empty, otherwise the restore pod will crash loop
if [ -n "$(ls -A "${ETCD_DATA_DIR}")" ]; then
echo "folder ${ETCD_DATA_DIR} is not empty, please review and remove all files in it"
exit 1
fi
# Move any remaining files (fio perf artifacts, stray snapshots, etc.) out of the data dir.
# The restore pod requires /var/lib/etcd to be empty before it runs.
backup_remaining_etcd_data_dir_contents
Comment on lines +113 to +115

@coderabbitai coderabbitai Bot Jun 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Back up leftovers in the ETCD_ETCDCTL_RESTORE branch too.

Right now the new helper only runs in the restore-pod path. When ETCD_ETCDCTL_RESTORE is set, Line 120-122 still wipes ${ETCD_DATA_DIR} directly, so any remaining top-level files are silently deleted instead of being preserved in backup. That leaves one supported restore mode outside the new contract.

Suggested fix 
 else
+  backup_remaining_etcd_data_dir_contents
   echo "removing etcd data dir..."
   rm -rf "${ETCD_DATA_DIR}"
   mkdir -p "${ETCD_DATA_DIR}"

Also applies to: 120-122

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bindata/etcd/cluster-restore.sh` around lines 113 - 115, The
ETCD_ETCDCTL_RESTORE branch currently wipes ${ETCD_DATA_DIR} without preserving
top-level leftovers; call the existing backup helper instead of (or before)
direct deletion so remaining files are preserved. Specifically, in the branch
gated by ETCD_ETCDCTL_RESTORE add an invocation of
backup_remaining_etcd_data_dir_contents (the same helper used in the restore-pod
path) prior to removing or emptying ETCD_DATA_DIR, ensuring the helper runs for
both restore paths and uses the same backup behavior.


echo "starting restore-etcd static pod"
cp -p "${RESTORE_ETCD_POD_YAML}" "${MANIFEST_DIR}/etcd-pod.yaml"
Expand Down
18 changes: 18 additions & 0 deletions bindata/etcd/etcd-common-tools
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,24 @@ function wait_for_containers_to_stop() {
done
}

function backup_remaining_etcd_data_dir_contents() {

@tjungblu tjungblu Jun 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so where do you call it in the other restore script?

@apurvanisal5 apurvanisal5 Jun 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cluster-restore-tnf.sh does not call it yet. After moving member/ to backup,
that script still wipes the data dir and recreates it:

  echo "removing etcd data dir..."
  rm -rf "${ETCD_DATA_DIR}"
  mkdir -p "${ETCD_DATA_DIR}"

Then it runs etcdctl snapshot restore. Extra files in /var/lib/etcd are
deleted by rm -rf, not moved to backup.

cluster-restore.sh is the one that needs an empty dir for the restore pod
(ETCD-704). It calls:

backup_remaining_etcd_data_dir_contents

Happy to add the same call in cluster-restore-tnf.sh before rm -rf if you
want extras backed up there too, e.g.:

  backup_remaining_etcd_data_dir_contents
  echo "removing etcd data dir..."
  rm -rf "${ETCD_DATA_DIR}"
  mkdir -p "${ETCD_DATA_DIR}"

local entry base extras_dir="${ETCD_DATA_DIR_BACKUP}/extra-data-dir-contents"

mkdir -p "${extras_dir}"

shopt -s nullglob dotglob
for entry in "${ETCD_DATA_DIR}"/*; do
base=$(basename "${entry}")
if [ -e "${extras_dir}/${base}" ]; then
echo "removing previous backup ${extras_dir}/${base}"
rm -rf "${extras_dir:?}/${base}"
fi
echo "Moving ${entry} to ${extras_dir}/"
mv "${entry}" "${extras_dir}/"
done
shopt -u nullglob dotglob
}

function mv_static_pods() {
local containers=("$@")

Expand Down