PWX-33827: Avoid deleting the earliest clusterpair for a pair (src ->dest).#1510
PWX-33827: Avoid deleting the earliest clusterpair for a pair (src ->dest).#1510diptiranjanpx merged 2 commits intomasterfrom
Conversation
|
Can one of the admins verify this patch? |
pp511
left a comment
pp511
left a comment
There was a problem hiding this comment.
One comment. Rest looks ok.
| if !strings.Contains(err.Error(), "not found") { | ||
| // for not found errors return nil with no error | ||
| // for everything else return the error back to the caller | ||
| return nil, nil | ||
| } | ||
| return nil, err |
There was a problem hiding this comment.
The comments are not in line with the conditional statement here.
For not found errors -> It is returning nil with an err.
I think you'd want to remote ! from if.
There was a problem hiding this comment.
Right, modified it.
| @@ -185,6 +186,8 @@ type ClusterPairPluginInterface interface { | |||
| CreatePair(*storkapi.ClusterPair) (string, error) | |||
| // Deletes a paring with a remote cluster | |||
There was a problem hiding this comment.
nit.. Not your change but can you fix this typo "paring" while you are at it.
diptiranjanpx
force-pushed
the
PWX-33827
branch
from
1b305d0 to
7555dcf
Compare
|
Added the referencing clusterpairs in describe event message like Warning Deleting 2m38s (x594 over 102m) stork Cluster Pair delete failed: other clusterpairs [test1/test11 test2/test2 test2/test22] are dependent on this cluster pair and the objectstore location objects created by it` |
|
|
||
| func (c *ClusterPairController) cleanup(clusterPair *stork_api.ClusterPair) error { | ||
| skipDelete := false | ||
| referencingClusterPairs := make([]string, 0) |
There was a problem hiding this comment.
nit... I think "dependent" would be a better prefix than "referencing"
diptiranjanpx
force-pushed
the
PWX-33827
branch
from
7555dcf to
282d64b
Compare
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## master #1510 +/- ##
==========================================
- Coverage 65.19% 65.13% -0.06%
==========================================
Files 43 43
Lines 5183 5183
==========================================
- Hits 3379 3376 -3
- Misses 1479 1481 +2
- Partials 325 326 +1 ☔ View full report in Codecov by Sentry. |
3 participants
Signed-Off-By: Diptiranjan
What type of PR is this?
What this PR does / why we need it:
The earliest clusterpair is used as reference by the later clusterpairs for each specific pair (src->dest).
Deleting that clusterpair should be blocked so that migrations using other clusterpairs can work normally and once the referencing clusterpairs get deleted, the referenced one can be deleted.
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) ✗ ./storkctl create clusterpair cp1 -n test1 --src-kube-file /tmp/dipti5.config --dest-kube-file /tmp/dipti7.config --s3-access-key **** --s3-secret-key **** --s3-endpoint **** --s3-region us-east1 -p s3
Source portworx endpoint is 10.13.192.154:9001
Destination portworx endpoint is 10.13.194.128:9001
Creating Secret and ObjectstoreLocation in source cluster in namespace test1...
ObjectstoreLocation cp1 created on source cluster in namespace test1
Creating Secret and ObjectstoreLocation in destination cluster in namespace test1...
ObjectstoreLocation cp1 created on destination cluster in namespace test1
Creating a cluster pair. Direction: Source -> Destination
ClusterPair cp1 created successfully. Direction Source -> Destination
Creating a cluster pair. Direction: Destination -> Source
Cluster pair cp1 created successfully. Direction: Destination -> Source
➜ stork git:(23.7.3) ✗ ./storkctl create clusterpair cp2 -n test2 --src-kube-file /tmp/dipti5.config --dest-kube-file /tmp/dipti7.config --s3-access-key **** --s3-secret-key ***** --s3-endpoint ***** --s3-region us-east1 -p s3
Source portworx endpoint is 10.13.192.154:9001
Destination portworx endpoint is 10.13.194.128:9001
Creating Secret and ObjectstoreLocation in source cluster in namespace test2...
ObjectstoreLocation cp2 created on source cluster in namespace test2
Creating Secret and ObjectstoreLocation in destination cluster in namespace test2...
ObjectstoreLocation cp2 created on destination cluster in namespace test2
Creating a cluster pair. Direction: Source -> Destination
ClusterPair cp2 created successfully. Direction Source -> Destination
Creating a cluster pair. Direction: Destination -> Source
Cluster pair cp2 created successfully. Direction: Destination -> Source
➜ ~ docker run --rm -e ETCDCTL_API=3 sens/etcdctl --endpoints http://10.13.193.170:9019 get --prefix=true pwx/dipti5/cluster/pair
pwx/dipti5/cluster/pair/4fdd2125-a731-472f-99f7-cce4147bf5ed
{"id":"4fdd2125-a731-472f-99f7-cce4147bf5ed","name":"dipti7","endpoint":"http://10.13.194.128:9001/","current_endpoints":["http://10.13.194.128:9001","http://10.13.194.228:9001","http://10.13.195.68:9001"],"token":"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJlbWFpbCI6InN1cHBvcnRAb3BlbnN0b3JhZ2UuaW8iLCJleHAiOjE4NDQ3ODM4NzYsImdyb3VwcyI6WyIqIl0sImlhdCI6MTY4NzEwMzg3NiwiaXNzIjoiNGZkZDIxMjUtYTczMS00NzJmLTk5ZjctY2NlNDE0N2JmNWVkIiwibmFtZSI6IkludGVybmFsIGNsdXN0ZXIgY29tbXVuaWNhdGlvbiIsInJvbGVzIjpbInN5c3RlbS5hZG1pbiJdLCJzdWIiOiIwMzEyODRkMS01OTA3LTQ1MTgtODY1OC04ZGM5MDM0ZTIzYjAifQ.PM9xZ0rDJ6_EP5usCFwcVV8py3qe8PdS2llVySgEfoA","options":{"CredName":"k8s/test1/cp1","CredUUID":"k8s/test1/cp1","RemoteCredUUID":"k8s/test1/cp1"},"mode":1}
pwx/dipti5/cluster/pair/default
4fdd2125-a731-472f-99f7-cce4147bf5ed
Delete of first clusterpair cp1 is hung as finalizer is set.
➜ stork git:(23.7.3) ✗ kubectl -n test1 delete clusterpair cp1
clusterpair.stork.libopenstorage.org "cp1" deleted
Describe output:
Options:
Backuplocation: cp1
Ip: 10.13.194.128
Port: 9001
Token: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJlbWFpbCI6InN1cHBvcnRAb3BlbnN0b3JhZ2UuaW8iLCJleHAiOjE4NDQ3ODM4NzYsImdyb3VwcyI6WyIqIl0sImlhdCI6MTY4NzEwMzg3NiwiaXNzIjoiNGZkZDIxMjUtYTczMS00NzJmLTk5ZjctY2NlNDE0N2JmNWVkIiwibmFtZSI6IkludGVybmFsIGNsdXN0ZXIgY29tbXVuaWNhdGlvbiIsInJvbGVzIjpbInN5c3RlbS5hZG1pbiJdLCJzdWIiOiIwMzEyODRkMS01OTA3LTQ1MTgtODY1OC04ZGM5MDM0ZTIzYjAifQ.PM9xZ0rDJ6_EP5usCFwcVV8py3qe8PdS2llVySgEfoA
Platform Options:
Status:
Remote Storage Id: 4fdd2125-a731-472f-99f7-cce4147bf5ed
Scheduler Status: Ready
Storage Status: Ready
Events:
Type Reason Age From Message
Normal Ready 66s stork Storage successfully paired
Normal Ready 66s stork Scheduler successfully paired
Warning Deleting (x2 over ) stork Cluster Pair delete failed: multiple clusterpairs found for same destination px cluster, the other clusterpairs need to be deleted first
Deleted cp2
➜ ~ kubectl -n test2 delete clusterpair cp2
clusterpair.stork.libopenstorage.org "cp2" deleted
Then cp1 got deleted automatically.
➜ stork git:(23.7.3) ✗ kubectl get clusterpair -A
No resources found
➜ stork git:(23.7.3) ✗ kubectl get backuplocations -n test1
No resources found in test1 namespace.
➜ stork git:(23.7.3) ✗ kubectl get backuplocations -n test2
No resources found in test2 namespace.
➜ stork git:(23.7.3) ✗ kubectl get secret -n test2
NAME TYPE DATA AGE
default-token-2q9r7 kubernetes.io/service-account-token 3 22h
➜ stork git:(23.7.3) ✗ kubectl get secret -n test1
NAME TYPE DATA AGE
default-token-h2bm8 kubernetes.io/service-account-token 3 22h
`