K8SPS-535: fix more `delete-backup` finalizer issues #1113

pooknull · 2025-10-03T12:16:29Z

https://perconadev.atlassian.net/browse/K8SPS-535

DESCRIPTION

This PR addresses two additional issues related to the percona.com/delete-backup finalizer:

The backupExists function in the backup sidecar lists an MD5 file of the backup. xbcloud does not count this file as part of the backup, so the finalizer fails to delete when only this md5 file exists. This function should only check files in the backup directory.
During reconcile, the controller can receive a cached version of a PerconaServerMySQLBackup object that still contains finalizers even though the backup has already been deleted. This caused the operator to rerun the finalizer. After that, it attempts to update the backup resource which resulted in errors like: Operation cannot be fulfilled on perconaservermysqlbackups.... To fix this, we should check if the backup object actually exists before updaring.

CHECKLIST

Jira

Is the Jira ticket created and referenced properly?
Does the Jira ticket have the proper statuses for documentation (Needs Doc) and QA (Needs QA)?
Does the Jira ticket link to the proper milestone (Fix Version field)?

Tests

Is an E2E test/test case added for the new feature/change?
Are unit tests added where appropriate?

Config/Logging/Testability

Are all needed new/changed options added to default YAML files?
Are all needed new/changed options added to the Helm Chart?
Did we add proper logging messages for operator actions?
Did we ensure compatibility with the previous version or cluster upgrade process?
Does the change support oldest and newest supported PS version?
Does the change support oldest and newest supported Kubernetes version?

https://perconadev.atlassian.net/browse/K8SPS-535

Copilot

Pull Request Overview

This PR fixes finalizer-related issues in the backup deletion process. The changes improve the handling of backup deletion finalizers and address object listing behavior.

Simplifies finalizer logic by removing unnecessary state checks and improving early return conditions
Implements retry mechanism with fresh object retrieval when updating finalizers
Fixes object listing path by adding trailing slash to destination

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
pkg/controller/psbackup/controller.go	Updates finalizer handling logic and adds debug print statements
cmd/sidecar/handler/backup/handler.go	Fixes object listing path by adding trailing slash

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

pkg/controller/psbackup/controller.go

Copilot

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

pkg/controller/psbackup/controller.go

Copilot

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-03T14:44:57Z

pkg/controller/psbackup/controller_test.go

+			require.NoError(t, r.Delete(t.Context(), cr))
+
+			// Getting backup with deletion timestamp
+			if err := r.Get(t.Context(), client.ObjectKeyFromObject(cr), cr); err != nil {
+				if k8serrors.IsNotFound(err) && len(tt.expectedFinalizers) == 0 {
+					return
+				}
 				t.Fatal(err)
 			}
-			cr := new(apiv1.PerconaServerMySQLBackup)
-			if err := r.Get(ctx, types.NamespacedName{Name: tt.cr.Name, Namespace: tt.cr.Namespace}, cr); err != nil {
-				if k8serrors.IsNotFound(err) && len(cr.Finalizers) == 0 {
+
+			assert.NoError(t, r.checkFinalizers(t.Context(), cr))


The method t.Context() is not a standard Go testing method. This should be context.Background() or define a proper context variable.

Copilot · 2025-10-03T14:44:58Z

pkg/controller/psbackup/controller_test.go

+			require.NoError(t, r.Delete(t.Context(), cr))
+
+			// Getting backup with deletion timestamp
+			if err := r.Get(t.Context(), client.ObjectKeyFromObject(cr), cr); err != nil {
+				if k8serrors.IsNotFound(err) && len(tt.expectedFinalizers) == 0 {
+					return
+				}
 				t.Fatal(err)
 			}
-			cr := new(apiv1.PerconaServerMySQLBackup)
-			if err := r.Get(ctx, types.NamespacedName{Name: tt.cr.Name, Namespace: tt.cr.Namespace}, cr); err != nil {
-				if k8serrors.IsNotFound(err) && len(cr.Finalizers) == 0 {
+
+			assert.NoError(t, r.checkFinalizers(t.Context(), cr))
+
+			// Getting backup with updated finalizers
+			if err := r.Get(t.Context(), client.ObjectKeyFromObject(cr), cr); err != nil {


The method t.Context() is not a standard Go testing method. These should use context.Background() or a properly defined context variable.

Copilot · 2025-10-03T14:44:58Z

pkg/controller/psbackup/controller_test.go

+			require.NoError(t, r.Delete(t.Context(), cr))
+
+			// Getting backup with deletion timestamp
+			if err := r.Get(t.Context(), client.ObjectKeyFromObject(cr), cr); err != nil {
+				if k8serrors.IsNotFound(err) && len(tt.expectedFinalizers) == 0 {
+					return
+				}
 				t.Fatal(err)
 			}
-			cr := new(apiv1.PerconaServerMySQLBackup)
-			if err := r.Get(ctx, types.NamespacedName{Name: tt.cr.Name, Namespace: tt.cr.Namespace}, cr); err != nil {
-				if k8serrors.IsNotFound(err) && len(cr.Finalizers) == 0 {
+
+			assert.NoError(t, r.checkFinalizers(t.Context(), cr))
+
+			// Getting backup with updated finalizers
+			if err := r.Get(t.Context(), client.ObjectKeyFromObject(cr), cr); err != nil {


The method t.Context() is not a standard Go testing method. These should use context.Background() or a properly defined context variable.

JNKPercona · 2025-10-03T16:42:27Z

Test Name	Result	Time
version-service-8-4	passed	00:12:37
async-ignore-annotations-8-4	passed	00:06:30
async-global-metadata-8-4	passed	00:14:12
auto-config-8-4	passed	00:24:02
config-8-4	passed	00:16:51
config-router-8-0	passed	00:07:12
config-router-8-4	passed	00:07:11
demand-backup-minio-8-0	passed	00:19:21
demand-backup-minio-8-4	passed	00:19:51
demand-backup-cloud-8-4	passed	00:21:50
async-data-at-rest-encryption-8-0	passed	00:14:48
async-data-at-rest-encryption-8-4	passed	00:14:08
gr-global-metadata-8-4	passed	00:16:31
gr-data-at-rest-encryption-8-0	passed	00:14:02
gr-data-at-rest-encryption-8-4	passed	00:14:07
gr-demand-backup-minio-8-4	passed	00:12:18
gr-demand-backup-cloud-8-4	passed	00:22:28
gr-demand-backup-haproxy-8-4	passed	00:09:39
gr-finalizer-8-4	passed	00:05:42
gr-haproxy-8-0	passed	00:04:05
gr-haproxy-8-4	passed	00:04:10
gr-ignore-annotations-8-4	passed	00:05:42
gr-init-deploy-8-0	passed	00:09:15
gr-init-deploy-8-4	passed	00:08:57
gr-one-pod-8-4	passed	00:05:53
gr-recreate-8-4	passed	00:17:27
gr-scaling-8-4	passed	00:08:24
gr-scheduled-backup-8-4	passed	00:16:35
gr-security-context-8-4	passed	00:09:45
gr-self-healing-8-4	passed	00:23:59
gr-tls-cert-manager-8-4	passed	00:09:46
gr-users-8-4	passed	00:05:28
haproxy-8-0	passed	00:08:48
haproxy-8-4	passed	00:08:29
init-deploy-8-0	passed	00:06:22
init-deploy-8-4	passed	00:05:44
limits-8-4	passed	00:06:37
monitoring-8-4	passed	00:14:05
one-pod-8-0	passed	00:06:59
one-pod-8-4	passed	00:05:51
operator-self-healing-8-4	passed	00:11:37
pvc-resize-8-4	passed	00:07:23
recreate-8-4	passed	00:12:25
scaling-8-4	passed	00:11:08
scheduled-backup-8-0	passed	00:18:04
scheduled-backup-8-4	passed	00:19:32
service-per-pod-8-4	passed	00:06:35
sidecars-8-4	passed	00:04:46
smart-update-8-4	passed	00:09:28
storage-8-4	passed	00:03:52
telemetry-8-4	passed	00:06:17
tls-cert-manager-8-4	passed	00:10:18
users-8-0	passed	00:07:47
users-8-4	passed	00:08:00

Summary	Value
Tests Run	54/54
Job Duration	01:53:57
Total Test Time	10:03:18

commit: 42af5a8
image: perconalab/percona-server-mysql-operator:PR-1113-42af5a85

K8SPS-535: fix more delete-backup finalizer issues

30786ee

https://perconadev.atlassian.net/browse/K8SPS-535

Copilot AI review requested due to automatic review settings October 3, 2025 12:16

pull-request-size bot added the size/M 30-99 lines label Oct 3, 2025

remove debug log

3d424a7

Copilot AI reviewed Oct 3, 2025

View reviewed changes

hors added this to the v1.0.0 milestone Oct 3, 2025

pooknull added 2 commits October 3, 2025 16:36

fix unit-test

ee03389

fix lint

d28352f

Copilot AI review requested due to automatic review settings October 3, 2025 14:05

Copilot AI reviewed Oct 3, 2025

View reviewed changes

pkg/controller/psbackup/controller.go Show resolved Hide resolved

fix unit-test

032e2bb

pooknull marked this pull request as ready for review October 3, 2025 14:43

pooknull requested review from hors and egegunes as code owners October 3, 2025 14:43

Copilot AI review requested due to automatic review settings October 3, 2025 14:43

pooknull requested review from nmarukovich and gkech as code owners October 3, 2025 14:43

Merge branch 'main' into K8SPS-535-fix

42af5a8

Copilot AI reviewed Oct 3, 2025

View reviewed changes

egegunes approved these changes Oct 6, 2025

View reviewed changes

hors approved these changes Oct 6, 2025

View reviewed changes

hors merged commit eb35850 into main Oct 6, 2025
16 checks passed

hors deleted the K8SPS-535-fix branch October 6, 2025 12:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

K8SPS-535: fix more `delete-backup` finalizer issues #1113

K8SPS-535: fix more `delete-backup` finalizer issues #1113

Uh oh!

pooknull commented Oct 3, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 3, 2025

Uh oh!

Copilot AI Oct 3, 2025

Uh oh!

Copilot AI Oct 3, 2025

Uh oh!

JNKPercona commented Oct 3, 2025

Uh oh!

Uh oh!

Uh oh!

K8SPS-535: fix more delete-backup finalizer issues #1113

K8SPS-535: fix more delete-backup finalizer issues #1113

Uh oh!

Conversation

pooknull commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DESCRIPTION

CHECKLIST

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

JNKPercona commented Oct 3, 2025

Uh oh!

Uh oh!

Uh oh!

K8SPS-535: fix more `delete-backup` finalizer issues #1113

K8SPS-535: fix more `delete-backup` finalizer issues #1113

pooknull commented Oct 3, 2025 •

edited

Loading