Skip to content
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

Implement import and cleanup #63

Merged
merged 28 commits into from
Nov 21, 2024
Merged

Conversation

ushitora-anqou
Copy link
Contributor

No description provided.

Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
@ushitora-anqou ushitora-anqou force-pushed the implement-import-and-cleanup branch 5 times, most recently from e6ec22c to cde9c4f Compare November 15, 2024 06:02
@ushitora-anqou ushitora-anqou marked this pull request as ready for review November 15, 2024 08:42
Copy link
Contributor

@satoru-takeuchi satoru-takeuchi left a comment

Choose a reason for hiding this comment

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

Review progress: I reviewd 10 commits. The last reviewed commit is below.

implement isExportDataAlreadyUploaded

internal/controller/replication.go Outdated Show resolved Hide resolved
internal/controller/replication.go Show resolved Hide resolved
internal/controller/util.go Outdated Show resolved Hide resolved
internal/controller/mantlebackup_controller.go Outdated Show resolved Hide resolved
Copy link
Contributor

@satoru-takeuchi satoru-takeuchi left a comment

Choose a reason for hiding this comment

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

reviewd 18 of 29 commits...

The last reviewed commit is below.

test/e2e: use RGW for e2e testing

internal/controller/mantlebackup_controller.go Outdated Show resolved Hide resolved
internal/controller/mantlebackup_controller.go Outdated Show resolved Hide resolved
internal/controller/mantlebackup_controller_test.go Outdated Show resolved Hide resolved
internal/controller/mantlebackup_controller_test.go Outdated Show resolved Hide resolved
internal/controller/mantlebackup_controller_test.go Outdated Show resolved Hide resolved
internal/controller/mantlebackup_controller_test.go Outdated Show resolved Hide resolved
internal/controller/mantlebackup_controller_test.go Outdated Show resolved Hide resolved
@satoru-takeuchi
Copy link
Contributor

@ushitora-anqou I guess we can remove a code snippet for testing.

Copy link
Contributor

@satoru-takeuchi satoru-takeuchi left a comment

Choose a reason for hiding this comment

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

@ushitora-anqou I have finished the review. Please reflect comments, thanks.

internal/controller/mantlebackup_controller_test.go Outdated Show resolved Hide resolved
@ushitora-anqou
Copy link
Contributor Author

@ushitora-anqou I guess we can remove a code snippet for testing.

Fixed in 2371058

@ushitora-anqou
Copy link
Contributor Author

@satoru-takeuchi I've fixed your points. Could you review this again?

Copy link
Contributor

@satoru-takeuchi satoru-takeuchi left a comment

Choose a reason for hiding this comment

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

@ushitora-anqou LGTM. Could you squash commits?

Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
We stopped setting SyncedToRemote=True in an ad hoc manner in the
previous commit, so we can't use `resMgr.WaitForbackupSyncedToRemote` to
wait for all necessary resources to be ready.

This commit resolves the problem above by packing all the tests into
`Eventually` in order to wait the resources to be ready.

Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
We need to give a StorageClass name as a command-line argument of
mantle-controller so that it can create PVCs to store its exported data.
To do so, we need to fix the StorageClass name (and the pool name used
as a parameter of the StorageClass) before starting mantle-controller.
However, the current e2e test generates the names of the StorageClass
and pool in runtime, after starting the controller.

This commit resolves the problem above, by avoiding generating the names
in runtime and fixing them as constants ("rook-ceph-block") instead.

Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
@ushitora-anqou
Copy link
Contributor Author

@satoru-takeuchi I squashed my commits. Could you review this again? The code included in the commits shouldn't have been changed: https://github.com/cybozu-go/mantle/compare/afc7d1fa1bbb7c2f8300abdea63c4a8f7f2e5a87..2da741f78b859c867c7699fae40db6a477da95f7

@satoru-takeuchi satoru-takeuchi merged commit 14279ef into main Nov 21, 2024
3 checks passed
@satoru-takeuchi satoru-takeuchi deleted the implement-import-and-cleanup branch November 21, 2024 04:20
@ushitora-anqou
Copy link
Contributor Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants