fix(reset): add etcd backup directory cleanup to prevent redeployment failures

[Irshu786]

What type of PR is this?
/kind fix

What this PR does / why we need it:

This PR removes the etcd backup directory in the reset role.

The reset playbook did not remove the etcd backup directory (/var/backups/etcd by default). As a result, stale backup data from the previous deployment remained on the node, causing subsequent cluster deployments to fail or hang during the etcd backup job.

This can lead to increased operational toil, with users spending additional time troubleshooting failed or stuck deployments.

Special notes for your reviewer:
The etcd_backup_prefix variable already exists in Kubespray and points to the etcd backup directory (e.g., /var/backups/etcd)

Fixed reset playbook to clean up etcd backup directory, preventing cluster redeployment failures

Does this PR introduce a user-facing change?:
NONE

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: Irshu786 / name: Ali Irshad (277ddb7)
• ✅ login: Irshu786 / name: Irshu786 (c58831a)

[k8s-ci-robot]

Welcome @Irshu786!

It looks like this is your first PR to kubernetes-sigs/kubespray 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubespray has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @Irshu786. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Irshu786
Once this PR has been reviewed and has the lgtm label, please assign ant31 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Copilot]

Pull request overview

Adds cleanup logic to the reset role so that etcd backup directories created under /var/backups (e.g. /var/backups/etcd-*) are removed during a cluster reset, aligning reset behavior with “remove all etcd-related data”.

Changes:

• Find directories matching etcd-* under /var/backups.
• Remove any matched etcd backup directories during playbooks/reset.yml runs.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[guoard]

Thank you for your contribution.
please sign the CLA and follow pull request template.

[Irshu786]

/easycla

[guoard]

I think the CLA error is because of copilot commit!
Can you sqush all commits so you be the only author?

[guoard]

Please follow pull request template.

kind and release-note.

[guoard]

I think kind should be fix and release note should be in block like this that Prow can undrestand it:

[guoard]

/ok-to-test

[Irshu786]

I think kind should be fix and release note should be in block like this that Prow can undrestand it:

can you please have a look at failed CI checks?

[guoard]

/retest-failed

[guoard]

/retest-failed

[guoard]

/retest

[guoard]

I tested it, this will fail because that variable is undefined when reset playbook is running.

and this is not a good idea to delete everything in /var/backups dir, we need to delete {{ etcd_backup_prefix }}/etcd-* files. but i don't know how to access that variable.

[Irshu786]

can we add a task rather than env approach?

[guoard]

@yankay WDYT?

[Irshu786]

@yankay can u have a look 👁️

[yankay]

Thanks for the fix @Irshu786!

One small concern: reset wiping the etcd backup directory feels a bit risky — backups are often the last recovery option if something goes wrong, including after an accidental reset.

Also, the current diff removes the whole {{ etcd_backup_prefix }} (defaults to /var/backups), not just the etcd-* subdirectories.

If the underlying issue is that stale backup dirs break redeployment, would it make sense to harden roles/etcd/handlers/backup_cleanup.yml instead, so it tolerates leftover dirs gracefully? Something like:

- name: Remove old etcd backups
  ansible.builtin.file:
    state: absent
    path: "{{ item }}"
  loop: "{{ (_etcd_backups.files | sort(attribute='ctime', reverse=True))[etcd_backup_retention_count:] | map(attribute='path') }}"
  when: etcd_backup_retention_count >= 0
  ignore_errors: true  # noqa ignore-errors - don't block etcd restart on stale backup cleanup

Just a thought — curious what others think.

[Irshu786]

Thanks for the fix @Irshu786!

One small concern: reset wiping the etcd backup directory feels a bit risky — backups are often the last recovery option if something goes wrong, including after an accidental reset.

Also, the current diff removes the whole {{ etcd_backup_prefix }} (defaults to /var/backups), not just the etcd-* subdirectories.

If the underlying issue is that stale backup dirs break redeployment, would it make sense to harden roles/etcd/handlers/backup_cleanup.yml instead, so it tolerates leftover dirs gracefully? Something like:

- name: Remove old etcd backups
  ansible.builtin.file:
    state: absent
    path: "{{ item }}"
  loop: "{{ (_etcd_backups.files | sort(attribute='ctime', reverse=True))[etcd_backup_retention_count:] | map(attribute='path') }}"
  when: etcd_backup_retention_count >= 0
  ignore_errors: true  # noqa ignore-errors - don't block etcd restart on stale backup cleanup

Just a thought — curious what others think.

Thanks, That makes sense !!
I think reset should ideally leave the cluster in a fully clean state, But agreed that hardening the backup cleanup handler would prevent redeploys from getting stuck on stale backup dirs even if cleanup was missed earlier.

can i update the PR with suggested changes?