stop using reader in MantleRestoreReconciler #71
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
Signed-off-by: Ryotaro Banno <[email protected]>
Signed-off-by: Ryotaro Banno <[email protected]>
Signed-off-by: Ryotaro Banno <[email protected]>
ushitora-anqou
force-pushed
the
avoid-using-reader
branch
8 times, most recently
from
0d47529 to
eb1db9d
Compare
satoru-takeuchi
requested changes
Nov 29, 2024
ushitora-anqou
force-pushed
the
avoid-using-reader
branch
2 times, most recently
from
320f305 to
333155b
Compare
|
@satoru-takeuchi I've fixed your points, so could you review this again? |
satoru-takeuchi
requested changes
Dec 2, 2024
satoru-takeuchi
requested changes
Dec 3, 2024
ushitora-anqou
force-pushed
the
avoid-using-reader
branch
from
738d6d1 to
8571fe8
Compare
|
The last commit (8571fe8) is just a kaizen and shouldn't be a functional change. |
satoru-takeuchi
requested changes
Dec 3, 2024
satoru-takeuchi
approved these changes
Dec 3, 2024
Signed-off-by: Ryotaro Banno <[email protected]>
Signed-off-by: Ryotaro Banno <[email protected]>
…V(C) Signed-off-by: Ryotaro Banno <[email protected]>
By introducing the previous commit, we cannot expect deleteRestoringPV(C) to return an error even when the PVs or PVCs have finalizers. However, some unit tests depend on the old behaviour. This commit fixes this problem as follows: - It removes all the tests that expect deleteRestoringPV(C) to return an error due to the finalizers. - It fixes the tests to call deleteRestoringPV(C) without emptying the finalizers and expects them not to return any error. Signed-off-by: Ryotaro Banno <[email protected]>
PVs and RBD images are now deleted asynchronously via the PV's finalizer, so we can't expect them to be deleted immediately after deleting a PVC, like the current e2e tests do. This commit resolves the problem above by wrapping the checks of the deletion of PVs and RBD images into Eventually. Signed-off-by: Ryotaro Banno <[email protected]>
…store removal Some of the current e2e tests expect that, when a user deletes a MantleRestore, mantle-controller tries to remove its corresponding PVs and RBD images, and, if it can't do so, it hinders deletion of the MantleRestore. However, this expectation isn't appropriate anymore, because PVs and RBD images are now removed asynchronously. This commit fixes the problem above by amending the tests. First, the test "It should wait while the PV/PVC is in used" is amended as follows: - The title is changed to "It should NOT delete an RBD image while its corresponding PV and PVC are in use". - It doesn't expect mantle-controller to prevent the ManlteRestore from being deleted anymore, even when a Pod is using the PV and PVC created by the MantleRestore. Instead, it does expect the MantleRestore to be removed, and the PV and the RBD image NOT to be removed. - After the pod stops using the PV, the PV and the RBD image should be deleted. Second, the test "It should fail to delete the MantleRestore if failed to remove rbd image" is removed totally. This is because we couldn't find any effective way to simulate a failure in deleting an RBD image. Signed-off-by: Ryotaro Banno <[email protected]>
…TestData Signed-off-by: Ryotaro Banno <[email protected]>
Signed-off-by: Ryotaro Banno <[email protected]>
Signed-off-by: Ryotaro Banno <[email protected]>
Signed-off-by: Ryotaro Banno <[email protected]>
Signed-off-by: Ryotaro Banno <[email protected]>
Signed-off-by: Ryotaro Banno <[email protected]>
ushitora-anqou
force-pushed
the
avoid-using-reader
branch
from
1d60ebc to
132a4d0
Compare
|
@satoru-takeuchi I squashed my commits and the CI has finally passed. Could you review this again? |
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.