Skip to content

Conversation

@diptiranjanpx
Copy link
Contributor

Signed-Off-By: Diptiranjan

What type of PR is this?

Uncomment only one and also add the corresponding label in the PR:
bug

What this PR does / why we need it:
Deleting a namespace can make backuplocation to delete first and it should not block the clusterpair deletion from px.

Does this PR change a user-facing CRD or CLI?:

Is a release note needed?:

Does this change need to be cherry-picked to a release branch?:

Test
➜ stork git:(23.7.3) ✗ kubectl delete ns test1
namespace "test1" deleted
➜ stork git:(23.7.3) ✗ kubectl get clusterpair -A
No resources found
➜ stork git:(23.7.3) ✗ kubectl get backuplocations -ntest1
No resources found in test1 namespace.
➜ stork git:(23.7.3) ✗ kubectl get secret -ntest1
No resources found in test1 namespace.

[root@dipti5-k8s-pxc-8d21-0 ~]# docker run --rm -e ETCDCTL_API=3 sens/etcdctl --endpoints http://*******:9019 get --prefix=true pwx/dipti5/cluster/pair
[root@dipti5-k8s-pxc-8d21-0 ~]#

Complete result has been updated in PWX-33787,

@cnbu-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

Copy link
Contributor

@pp511 pp511 left a comment

Choose a reason for hiding this comment

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

Along the same line of thought..Is it not possible that Secret still exists and doesn't get cleaned up?

@diptiranjanpx
Copy link
Contributor Author

Along the same line of thought..Is it not possible that Secret still exists and doesn't get cleaned up?

Earlier issue was , there was a return statement if backuplocation is not present which was causing clusterpair not to be deleted. The fix is around that to ignore that error and continue to delete the clusterpair from px.

Coming back to if secret can still lie around, unless manually the backuplocation is deleted it should not happen.

  1. If backuplocation gets manually deleted, we will not find any ref to the secret, so it will still exists as part of clusterpair deletion flow.
  2. If ns gets deleted, secret may not get deleted as part of clusterpair deletion flow but as part of ns deletion, it will get deleted anyway.
  3. In a regular clusterpair deletion workflow, backuplocation deletion is after secret deletion so as long as all exists, deletion happens in order
    a. secret
    b. backuplocation
    c. clusterpair entry in px.

Copy link
Contributor

@pp511 pp511 left a comment

Choose a reason for hiding this comment

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

If backuplocation gets manually deleted, we will not find any ref to the secret, so it will still exists as part of clusterpair deletion flow.

If I understand correctly this is still an issue we can address. Can we file a ticket to track this? We will discuss about the actual implementation when we get to it.

@diptiranjanpx
Copy link
Contributor Author

If backuplocation gets manually deleted, we will not find any ref to the secret, so it will still exists as part of clusterpair deletion flow.

If I understand correctly this is still an issue we can address. Can we file a ticket to track this? We will discuss about the actual implementation when we get to it.

done

@codecov
Copy link

codecov bot commented Sep 19, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.05% 🎉

Comparison is base (25361e3) 65.13% compared to head (3f13ef3) 65.19%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1507      +/-   ##
==========================================
+ Coverage   65.13%   65.19%   +0.05%     
==========================================
  Files          43       43              
  Lines        5183     5183              
==========================================
+ Hits         3376     3379       +3     
+ Misses       1481     1479       -2     
+ Partials      326      325       -1     

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@diptiranjanpx diptiranjanpx merged commit c90292a into master Sep 19, 2023
@diptiranjanpx diptiranjanpx deleted the PWX-33787 branch May 27, 2024 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants