Implement multi Rook/Ceph env e2e #27
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llamerada-jp
force-pushed
the
implement-e2e-multi-cluster
branch
17 times, most recently
from
d237b0b
to
19433d7
Compare
llamerada-jp
changed the title
satoru-takeuchi
requested changes
Jul 3, 2024
test/e2e/multi_rook_ceph_test.go
Outdated
| _, stderr, err := kubectl("delete", "sc", sc) | ||
| Expect(err).NotTo(HaveOccurred(), string(stderr)) | ||
| }) | ||
| } |
There was a problem hiding this comment.
How about deleting RBD images in the second cluster as the old code did? It's the teardown logic and it should cleanup the environment as possible.
llamerada-jp
force-pushed
the
implement-e2e-multi-cluster
branch
5 times, most recently
from
d41cd5f
to
d5729d3
Compare
satoru-takeuchi
requested changes
Jul 8, 2024
test/e2e/util.go
Outdated
| images := []string{} | ||
| clones := []string{} | ||
| snaps := []string{} | ||
| // remove RBD snapshots first |
There was a problem hiding this comment.
It's better to mention RBD clone images.
Suggested change
| // remove RBD snapshots first | |
| // remove RBD clone images of targets, RBD anspshots of targets, and target images in order | |
| // to remove them cleanly. |
Comment on lines
+63
to
+89
| It("delete resources in the namespace: "+ns, func() { | ||
| err := deleteNamespacedResource(ns, "mantlerestore") | ||
| Expect(err).NotTo(HaveOccurred()) | ||
| err = deleteNamespacedResource(ns, "mantlebackup") | ||
| Expect(err).NotTo(HaveOccurred()) | ||
| err = deleteNamespacedResource(ns, "pvc") | ||
| Expect(err).NotTo(HaveOccurred()) | ||
| }) | ||
|
|
||
| It("delete the namespace: "+ns, func() { | ||
| _, stderr, err := kubectl("delete", "namespace", ns) | ||
| Expect(err).NotTo(HaveOccurred(), string(stderr)) | ||
| }) | ||
| } | ||
|
|
||
| It("clean up the SCs and RBD pools", func() { | ||
| for _, sc := range []string{test.storageClassName1, test.storageClassName2} { | ||
| _, stderr, err := kubectl("delete", "sc", sc) | ||
| Expect(err).NotTo(HaveOccurred(), string(stderr)) | ||
| } | ||
|
|
||
| for _, ns := range []string{cephCluster1Namespace, cephCluster2Namespace} { | ||
| err := removeAllRBDImageAndSnap(ns, test.poolName) | ||
| Expect(err).NotTo(HaveOccurred()) | ||
|
|
||
| _, _, err = kubectl("delete", "-n", ns, "cephblockpool", test.poolName, "--wait=false") | ||
| Expect(err).NotTo(HaveOccurred()) |
There was a problem hiding this comment.
It's better to continue the teardown even if some deletion failed not to restore clean environment as possible.
Suggested change
| It("delete resources in the namespace: "+ns, func() { | |
| err := deleteNamespacedResource(ns, "mantlerestore") | |
| Expect(err).NotTo(HaveOccurred()) | |
| err = deleteNamespacedResource(ns, "mantlebackup") | |
| Expect(err).NotTo(HaveOccurred()) | |
| err = deleteNamespacedResource(ns, "pvc") | |
| Expect(err).NotTo(HaveOccurred()) | |
| }) | |
| It("delete the namespace: "+ns, func() { | |
| _, stderr, err := kubectl("delete", "namespace", ns) | |
| Expect(err).NotTo(HaveOccurred(), string(stderr)) | |
| }) | |
| } | |
| It("clean up the SCs and RBD pools", func() { | |
| for _, sc := range []string{test.storageClassName1, test.storageClassName2} { | |
| _, stderr, err := kubectl("delete", "sc", sc) | |
| Expect(err).NotTo(HaveOccurred(), string(stderr)) | |
| } | |
| for _, ns := range []string{cephCluster1Namespace, cephCluster2Namespace} { | |
| err := removeAllRBDImageAndSnap(ns, test.poolName) | |
| Expect(err).NotTo(HaveOccurred()) | |
| _, _, err = kubectl("delete", "-n", ns, "cephblockpool", test.poolName, "--wait=false") | |
| Expect(err).NotTo(HaveOccurred()) | |
| It("delete resources in the namespace: "+ns, func() { | |
| err := deleteNamespacedResource(ns, "mantlerestore") | |
| err = deleteNamespacedResource(ns, "mantlebackup") | |
| err = deleteNamespacedResource(ns, "pvc") | |
| }) | |
| It("delete the namespace: "+ns, func() { | |
| _, stderr, err := kubectl("delete", "namespace", ns) | |
| }) | |
| } | |
| It("clean up the SCs and RBD pools", func() { | |
| for _, sc := range []string{test.storageClassName1, test.storageClassName2} { | |
| _, stderr, err := kubectl("delete", "sc", sc) | |
| } | |
| for _, ns := range []string{cephCluster1Namespace, cephCluster2Namespace} { | |
| err := removeAllRBDImageAndSnap(ns, test.poolName) | |
| _, _, err = kubectl("delete", "-n", ns, "cephblockpool", test.poolName, "--wait=false") |
There was a problem hiding this comment.
I would like to leave these because the errors raised by tearDown sometimes made me notice cases that I was not noticed. Of course, if you know it will be an error, then it's not.
Comment on lines
+79
to
+92
| It("delete namespace: "+test.tenantNamespace, func() { | ||
| _, stderr, err := kubectl("delete", "namespace", test.tenantNamespace) | ||
| Expect(err).NotTo(HaveOccurred(), string(stderr)) | ||
| }) | ||
|
|
||
| It("clean up the SCs and RBD pools", func() { | ||
| _, stderr, err := kubectl("delete", "sc", test.storageClassName) | ||
| Expect(err).NotTo(HaveOccurred(), string(stderr)) | ||
|
|
||
| err = removeAllRBDImageAndSnap(cephCluster1Namespace, test.poolName) | ||
| Expect(err).NotTo(HaveOccurred()) | ||
|
|
||
| _, _, err = kubectl("delete", "-n", cephCluster1Namespace, "cephblockpool", test.poolName, "--wait=false") | ||
| Expect(err).NotTo(HaveOccurred()) |
There was a problem hiding this comment.
Same as multi_rook_ceph_test.go#tearDownEnv().
There was a problem hiding this comment.
It's also OK not to fix.
|
@llamerada-jp This PR will be merged after my last minor change requests. |
Signed-off-by: Yuji Ito <llamerada.jp@gmail.com>
llamerada-jp
force-pushed
the
implement-e2e-multi-cluster
branch
from
d5729d3
to
31c8679
Compare
satoru-takeuchi
approved these changes
Jul 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.