-
Notifications
You must be signed in to change notification settings - Fork 204
K8SPXC-1431 fix pvc deletion with finalizer #2232
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
Conversation
| local backup_name="$1" | ||
| local finalizer_flag="off" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| local backup_name="$1" | |
| local finalizer_flag="off" | |
| local backup_name="$1" | |
| local finalizer_flag="off" |
| desc "Checking finalizers on backup $backup_name" | ||
| # Check if pxc-backup has the finalizer "percona.com/delete-backup" | ||
| if kubectl get pxc-backup "$backup_name" -o jsonpath='{.metadata.finalizers}' | grep -q "percona.com/delete-backup"; then | ||
| finalizer_flag="on" | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| desc "Checking finalizers on backup $backup_name" | |
| # Check if pxc-backup has the finalizer "percona.com/delete-backup" | |
| if kubectl get pxc-backup "$backup_name" -o jsonpath='{.metadata.finalizers}' | grep -q "percona.com/delete-backup"; then | |
| finalizer_flag="on" | |
| fi | |
| desc "Checking finalizers on backup $backup_name" | |
| # Check if pxc-backup has the finalizer "percona.com/delete-backup" | |
| if kubectl get pxc-backup "$backup_name" -o jsonpath='{.metadata.finalizers}' | grep -q "percona.com/delete-backup"; then | |
| finalizer_flag="on" | |
| fi |
| finalizer_flag="on" | ||
| fi | ||
|
|
||
| echo "Finalizer is $finalizer_flag" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| echo "Finalizer is $finalizer_flag" | |
| echo "Finalizer is $finalizer_flag" |
| echo "Looking for PVCs containing '$backup_name'" | ||
| pvc=$(kubectl get pvc --no-headers -o custom-columns=":metadata.name" | grep "$backup_name") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| echo "Looking for PVCs containing '$backup_name'" | |
| pvc=$(kubectl get pvc --no-headers -o custom-columns=":metadata.name" | grep "$backup_name") | |
| echo "Looking for PVCs containing '$backup_name'" | |
| pvc=$(kubectl get pvc --no-headers -o custom-columns=":metadata.name" | grep "$backup_name") |
| echo "Deleting backup $backup_name ..." | ||
| kubectl delete pxc-backup "$backup_name" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| echo "Deleting backup $backup_name ..." | |
| kubectl delete pxc-backup "$backup_name" | |
| echo "Deleting backup $backup_name ..." | |
| kubectl delete pxc-backup "$backup_name" |
| echo "Deleting backup $backup_name ..." | ||
| kubectl delete pxc-backup "$backup_name" | ||
|
|
||
| sleep 5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| sleep 5 | |
| sleep 5 |
| cat "$file" \ | ||
| | $sed -E "s#(claimName: xb-on-demand-backup-pvc)(-[0-9]{14}-[a-f0-9]{8})?#claimName: $pvc_name#" \ | ||
| > "${file}.patched" | ||
| mv "${file}.patched" "$file" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| cat "$file" \ | |
| | $sed -E "s#(claimName: xb-on-demand-backup-pvc)(-[0-9]{14}-[a-f0-9]{8})?#claimName: $pvc_name#" \ | |
| > "${file}.patched" | |
| mv "${file}.patched" "$file" | |
| cat "$file" \ | |
| | $sed -E "s#(claimName: xb-on-demand-backup-pvc)(-[0-9]{14}-[a-f0-9]{8})?#claimName: $pvc_name#" \ | |
| >"${file}.patched" | |
| mv "${file}.patched" "$file" |
| case naming.FinalizerDeleteBackup: | ||
| if (cr.Status.S3 == nil && cr.Status.Azure == nil) || cr.Status.Destination == "" { | ||
| storageType := cr.Status.GetStorageType(nil) | ||
| if (cr.Status.S3 == nil && cr.Status.Azure == nil && storageType != api.BackupStorageFilesystem) || cr.Status.Destination == "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't we agree on adding cr.Status.Filesystem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we didn't. I was confused what variables should we put in this structure. Let's decide here and I will fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. what confused you? what needs to be decided?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So for example for azure and aws in this structure we have
type BackupStorageAzureSpec struct {
CredentialsSecret string `json:"credentialsSecret"`
ContainerPath string `json:"container"`
Endpoint string `json:"endpointUrl"`
StorageClass string `json:"storageClass"`
BlockSize int64 `json:"blockSize"`
Concurrency int `json:"concurrency"`
}
type BackupStorageS3Spec struct {
Bucket string `json:"bucket"`
CredentialsSecret string `json:"credentialsSecret"`
Region string `json:"region,omitempty"`
EndpointURL string `json:"endpointUrl,omitempty"`
}
So in this case, what would be reasonable to add to the same structure for the fs-pvc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can put the PVC spec into status. Also the PVC name would be nice to add.
| base := naming.BackupJobName(cr.Name, false) | ||
|
|
||
| ts := cr.CreationTimestamp.UTC().Format("20060102150405") | ||
|
|
||
| uidSuffix := strings.ToLower(string(cr.UID)) | ||
| if len(uidSuffix) > 8 { | ||
| uidSuffix = uidSuffix[:8] | ||
| } | ||
|
|
||
| // Final name: xb-<name>-<YYYYMMDDhhmmss>-<uidsfx> | ||
| name := fmt.Sprintf("%s-%s-%s", base, ts, uidSuffix) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the reason behind this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that’s a good question.
Previously, the behavior was the following: when we created a backup using fs-pvc storage and then deleted that backup, the PVC was also deleted. Because of that, the finalizer never actually worked.
This PR fixes that issue.
Now the finalizer works correctly, and a new situation becomes possible:
The user turns off the finalizer, creates a backup, and later deletes it.
In this case, the backup object is deleted, but the PVC remains.
Later, the user decides to create another backup with the same name.
Since the old PVC still exists, the operator will reuse that same PVC to store the new backup data.
I think in this case it would be better to generate a unique PVC name to avoid reusing storage unintentionally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Since these PVC names are not immediately predictable for users any more, please add PVC name into backup status.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have destinatination in backup status destination: pvc/xb-on-demand-backup-pvc, but the new part with values was also added:
fsPvc:
accessModes:
- ReadWriteOnce
resources:
requests:
storage: 1Gi
storageClassName: standard-rwo
volumeMode: Filesystem
volumeName: pvc-d1c2fec7-118f-4608-becc-9dd61c198d26
eleo007
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls add description to this PR.
| kubectl delete pxc-backup "$backup_name" | ||
|
|
||
| sleep 5 | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to update the $pvc after deleting backup. Otherwise its value stays before deletion and the test does not actually check correct state.
e2e-tests/demand-backup/run
Outdated
| pvc=$(kubectl get pvc --no-headers -o custom-columns=":metadata.name" | grep "$backup_name") | ||
|
|
||
| if [[ -z "$pvc" ]]; then | ||
| echo "No PVCs found with substring '$backup_name'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extend error output to be more descriptive. Maybe smth like: echo "No PVCs found with substring '$backup_name' before the backup deletion. Smth wrong with backup"?
e2e-tests/demand-backup/run
Outdated
| if [[ -z "$pvc" && "$finalizer_flag" == "off" ]]; then | ||
| echo "Error: No PVCs found with substring '$backup_name'. PVC was deleted even if finalizer is turned off" | ||
| exit 1 | ||
| fi | ||
|
|
||
| if [[ -z "$pvc" && "$finalizer_flag" == "on" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it will be better to use if .. elif .. else in this check not to miss unexpected combinations.
hors
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nmarukovich, please add a description for this PR. We can't review it without all the needed information.
egegunes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nmarukovich please add more description to your PR. what was the problem? how did you fix it? are there any side effects of your change? for example we are changing the PVC names for backups but it's not even mentioned in description.
…percona-xtradb-cluster-operator into K8SPXC-1431_pvc_backup_deletion
| if [[ -z "$pvc" ]]; then | ||
| echo "No PVCs found with substring '$backup_name' before the backup deletion. Something wrong with backup" | ||
| return 1 | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| if [[ -z "$pvc" ]]; then | |
| echo "No PVCs found with substring '$backup_name' before the backup deletion. Something wrong with backup" | |
| return 1 | |
| fi | |
| if [[ -z $pvc ]]; then | |
| echo "No PVCs found with substring '$backup_name' before the backup deletion. Something wrong with backup" | |
| return 1 | |
| fi |
|
|
||
| sleep 5 | ||
|
|
||
| pvc=$(kubectl get pvc --no-headers -o custom-columns=":metadata.name" | grep "$backup_name" || true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| pvc=$(kubectl get pvc --no-headers -o custom-columns=":metadata.name" | grep "$backup_name" || true) | |
| pvc=$(kubectl get pvc --no-headers -o custom-columns=":metadata.name" | grep "$backup_name" || true) |
|
|
||
| pvc=$(kubectl get pvc --no-headers -o custom-columns=":metadata.name" | grep "$backup_name" || true) | ||
|
|
||
| echo "Checking PVC $pvc existence and finalizers" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| echo "Checking PVC $pvc existence and finalizers" | |
| echo "Checking PVC $pvc existence and finalizers" |
| if [[ -z "$pvc" && "$finalizer_flag" == "off" ]]; then | ||
| echo "Error: No PVCs found with substring '$backup_name'. PVC was deleted even though finalizer is OFF." | ||
| exit 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| if [[ -z "$pvc" && "$finalizer_flag" == "off" ]]; then | |
| echo "Error: No PVCs found with substring '$backup_name'. PVC was deleted even though finalizer is OFF." | |
| exit 1 | |
| if [[ -z $pvc && $finalizer_flag == "off" ]]; then | |
| echo "Error: No PVCs found with substring '$backup_name'. PVC was deleted even though finalizer is OFF." | |
| exit 1 |
| elif [[ -z "$pvc" && "$finalizer_flag" == "on" ]]; then | ||
| echo "Correct: No PVCs found with substring '$backup_name' because finalizer is ON and PVC was deleted." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| elif [[ -z "$pvc" && "$finalizer_flag" == "on" ]]; then | |
| echo "Correct: No PVCs found with substring '$backup_name' because finalizer is ON and PVC was deleted." | |
| elif [[ -z $pvc && $finalizer_flag == "on" ]]; then | |
| echo "Correct: No PVCs found with substring '$backup_name' because finalizer is ON and PVC was deleted." |
| elif [[ -n "$pvc" && "$finalizer_flag" == "on" ]]; then | ||
| echo "Error: PVC exists: $pvc (finalizer_flag=$finalizer_flag). Finalizer should delete PVC but did NOT." | ||
| exit 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| elif [[ -n "$pvc" && "$finalizer_flag" == "on" ]]; then | |
| echo "Error: PVC exists: $pvc (finalizer_flag=$finalizer_flag). Finalizer should delete PVC but did NOT." | |
| exit 1 | |
| elif [[ -n $pvc && $finalizer_flag == "on" ]]; then | |
| echo "Error: PVC exists: $pvc (finalizer_flag=$finalizer_flag). Finalizer should delete PVC but did NOT." | |
| exit 1 |
| else | ||
| echo "PVC exists: $pvc (finalizer_flag=$finalizer_flag)" | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| else | |
| echo "PVC exists: $pvc (finalizer_flag=$finalizer_flag)" | |
| fi | |
| else | |
| echo "PVC exists: $pvc (finalizer_flag=$finalizer_flag)" | |
| fi |
| check_finalizer_for_fs "on-demand-backup-pvc" | ||
|
|
||
| run_backup "$cluster" "on-demand-backup-pvc-with-finalizer" | ||
| check_finalizer_for_fs "on-demand-backup-pvc-with-finalizer" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| check_finalizer_for_fs "on-demand-backup-pvc-with-finalizer" | |
| check_finalizer_for_fs "on-demand-backup-pvc-with-finalizer" |
| base := naming.BackupJobName(cr.Name, false) | ||
|
|
||
| ts := cr.CreationTimestamp.UTC().Format("20060102150405") | ||
|
|
||
| uidSuffix := strings.ToLower(string(cr.UID)) | ||
| if len(uidSuffix) > 8 { | ||
| uidSuffix = uidSuffix[:8] | ||
| } | ||
|
|
||
| // Final name: xb-<name>-<YYYYMMDDhhmmss>-<uidsfx> | ||
| name := fmt.Sprintf("%s-%s-%s", base, ts, uidSuffix) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Since these PVC names are not immediately predictable for users any more, please add PVC name into backup status.
pkg/apis/pxc/v1/pxc_backup_types.go
Outdated
| StorageName string `json:"storageName,omitempty"` | ||
| S3 *BackupStorageS3Spec `json:"s3,omitempty"` | ||
| Azure *BackupStorageAzureSpec `json:"azure,omitempty"` | ||
| FsPvc *corev1.PersistentVolumeClaimSpec `json:"fsPvc,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better to name it just pvc
pkg/apis/pxc/v1/pxc_backup_types.go
Outdated
| S3 *BackupStorageS3Spec `json:"s3,omitempty"` | ||
| Azure *BackupStorageAzureSpec `json:"azure,omitempty"` | ||
| FsPvc *corev1.PersistentVolumeClaimSpec `json:"fsPvc,omitempty"` | ||
| Pvc *corev1.PersistentVolumeClaimSpec `json:"pvc,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
acronyms should be all upper case
| local input_file="$1" | ||
| local pvc_name="$2" | ||
|
|
||
| cat "$input_file" \ | ||
| | $sed -e "s#apiVersion: pxc.percona.com/v.*\$#apiVersion: $API#" \ | ||
| | $sed -e "s#image:.*-pxc\([0-9]*.[0-9]*\)\{0,1\}\$#image: $IMAGE_PXC#" \ | ||
| | $sed -e "s#image:.*\/percona-xtradb-cluster:.*\$#image: $IMAGE_PXC#" \ | ||
| | $sed -e "s#image:.*-init\$#image: $IMAGE#" \ | ||
| | $sed -e "s#image:.*-pmm\$#image: $IMAGE_PMM_CLIENT#" \ | ||
| | $sed -e "s#image:.*-backup\$#image: $IMAGE_BACKUP#" \ | ||
| | $sed -e "s#image:.*-proxysql\$#image: $IMAGE_PROXY#" \ | ||
| | $sed -e "s#image:.*-haproxy\$#image: $IMAGE_HAPROXY#" \ | ||
| | $sed -e "s#image:.*-logcollector\$#image: $IMAGE_LOGCOLLECTOR#" \ | ||
| | $sed -e "s~minio-service.#namespace~minio-service.$namespace~" \ | ||
| | $sed -e "s#apply:.*#apply: Never#" \ | ||
| | $sed -e "s#claimName:..*-backup-pvc\$#claimName: $pvc_name#" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| local input_file="$1" | |
| local pvc_name="$2" | |
| cat "$input_file" \ | |
| | $sed -e "s#apiVersion: pxc.percona.com/v.*\$#apiVersion: $API#" \ | |
| | $sed -e "s#image:.*-pxc\([0-9]*.[0-9]*\)\{0,1\}\$#image: $IMAGE_PXC#" \ | |
| | $sed -e "s#image:.*\/percona-xtradb-cluster:.*\$#image: $IMAGE_PXC#" \ | |
| | $sed -e "s#image:.*-init\$#image: $IMAGE#" \ | |
| | $sed -e "s#image:.*-pmm\$#image: $IMAGE_PMM_CLIENT#" \ | |
| | $sed -e "s#image:.*-backup\$#image: $IMAGE_BACKUP#" \ | |
| | $sed -e "s#image:.*-proxysql\$#image: $IMAGE_PROXY#" \ | |
| | $sed -e "s#image:.*-haproxy\$#image: $IMAGE_HAPROXY#" \ | |
| | $sed -e "s#image:.*-logcollector\$#image: $IMAGE_LOGCOLLECTOR#" \ | |
| | $sed -e "s~minio-service.#namespace~minio-service.$namespace~" \ | |
| | $sed -e "s#apply:.*#apply: Never#" \ | |
| | $sed -e "s#claimName:..*-backup-pvc\$#claimName: $pvc_name#" \ | |
| local input_file="$1" | |
| local pvc_name="$2" | |
| cat "$input_file" \ | |
| | $sed -e "s#apiVersion: pxc.percona.com/v.*\$#apiVersion: $API#" \ | |
| | $sed -e "s#image:.*-pxc\([0-9]*.[0-9]*\)\{0,1\}\$#image: $IMAGE_PXC#" \ | |
| | $sed -e "s#image:.*\/percona-xtradb-cluster:.*\$#image: $IMAGE_PXC#" \ | |
| | $sed -e "s#image:.*-init\$#image: $IMAGE#" \ | |
| | $sed -e "s#image:.*-pmm\$#image: $IMAGE_PMM_CLIENT#" \ | |
| | $sed -e "s#image:.*-backup\$#image: $IMAGE_BACKUP#" \ | |
| | $sed -e "s#image:.*-proxysql\$#image: $IMAGE_PROXY#" \ | |
| | $sed -e "s#image:.*-haproxy\$#image: $IMAGE_HAPROXY#" \ | |
| | $sed -e "s#image:.*-logcollector\$#image: $IMAGE_LOGCOLLECTOR#" \ | |
| | $sed -e "s~minio-service.#namespace~minio-service.$namespace~" \ | |
| | $sed -e "s#apply:.*#apply: Never#" \ | |
| | $sed -e "s#claimName:..*-backup-pvc\$#claimName: $pvc_name#" |
| local config_file="$1" | ||
| local pvc_name="${2:-}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| local config_file="$1" | |
| local pvc_name="${2:-}" | |
| local config_file="$1" | |
| local pvc_name="${2:-}" |
| | kubectl_bin apply -f - | ||
| else | ||
| cat_config "$1" \ | ||
| cat_config "$config_file" "$pvc_name"\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| cat_config "$config_file" "$pvc_name"\ | |
| cat_config "$config_file" "$pvc_name" \ |
| apply_config "$pxcClientFile" | ||
| if [[ $IMAGE_PXC =~ 5\.7 ]] && [[ $cluster == 'demand-backup' || $cluster == 'demand-backup-cloud' ]]; then | ||
| cat_config "$config" \ | ||
| cat_config "$config"\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| cat_config "$config"\ | |
| cat_config "$config" \ |
| } | ||
|
|
||
| check_pvc_md5() { | ||
| local backup_name="$1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| local backup_name="$1" | |
| local backup_name="$1" |
| } | ||
|
|
||
| get_pvc_name_for_backup() { | ||
| local backup_name="$1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| local backup_name="$1" | |
| local backup_name="$1" |
| local destination=$(kubectl_bin get pxc-backup "$backup_name" -o jsonpath='{.status.destination}') | ||
| if [[ -z "$destination" ]]; then | ||
| echo "No destination found in backup $backup_name" | ||
| return 1 | ||
| fi | ||
| local pvc_name="${destination#pvc/}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| local destination=$(kubectl_bin get pxc-backup "$backup_name" -o jsonpath='{.status.destination}') | |
| if [[ -z "$destination" ]]; then | |
| echo "No destination found in backup $backup_name" | |
| return 1 | |
| fi | |
| local pvc_name="${destination#pvc/}" | |
| local destination=$(kubectl_bin get pxc-backup "$backup_name" -o jsonpath='{.status.destination}') | |
| if [[ -z $destination ]]; then | |
| echo "No destination found in backup $backup_name" | |
| return 1 | |
| fi | |
| local pvc_name="${destination#pvc/}" |
| fi | ||
| local pvc_name="${destination#pvc/}" | ||
|
|
||
| echo "$pvc_name" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
| echo "$pvc_name" | |
| echo "$pvc_name" |
|
|
||
| echo "$pvc_name" | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[shfmt] reported by reviewdog 🐶
…percona-xtradb-cluster-operator into K8SPXC-1431_pvc_backup_deletion
| echo "Finalizer is $finalizer_flag" | ||
|
|
||
| echo "Looking for PVCs containing '$backup_name'" | ||
| pvc=$(kubectl get pvc --no-headers -o custom-columns=":metadata.name" | grep "$backup_name") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we have PVC name in the backup status, maybe it is better to use it to check that the PVC exists/not exists further?
commit: 31b0cef |
CHANGE DESCRIPTION
Problem:
Short explanation of the problem.
Previously, the behavior was the following: when we created a backup using fs-pvc storage and then deleted that backup, the PVC was also deleted. Because of that, the finalizer never actually worked.
This PR fixes that issue.
Cause:
pvc had ownerReference and was deleted with backup object.
Solution:
xb-backup1-20251125210833-f12d3debinstead ofxb-backup1CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
compare/*-oc.yml)?Config/Logging/Testability