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

Tests: Add check for refreshing additional snapshots #138

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

roosterfish
Copy link
Contributor

With this change we actually test if a delta of snapshots is transferred accordingly for both local and remote VM copies.

Before the refresh was only testing if the volume itself can be refreshed as the number of snapshots on both source and target volume was identical due to the copy operation.

Part of canonical/lxd#13305.

@roosterfish roosterfish force-pushed the snap_refresh branch 2 times, most recently from f18e9d7 to f937448 Compare April 11, 2024 09:05
@roosterfish
Copy link
Contributor Author

The storage-vm tests on zfs (e.g. https://github.com/canonical/lxd-ci/actions/runs/8644234403/job/23698965959?pr=138) are expected to fail due to canonical/lxd#13304.

tomponline
tomponline previously approved these changes Apr 11, 2024
Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

Thanks!

Can we add similar tests for the storage volume CI tests too (as a separate PR)?

With this change we actually test if a delta of snapshots is transferred accordingly
for both local and remote VM copies.

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>
@roosterfish
Copy link
Contributor Author

Can we add similar tests for the storage volume CI tests too (as a separate PR)?

Absolutely. I'll also add the cross pool migration checks. Thanks for the hint, looks like snapshots of custom volumes are completely missing there until now.

@roosterfish
Copy link
Contributor Author

I have force pushed once more as the LVM thin pool test was exceeding the disk space. So the test now removes the second snapshot after every check to save disk space.

Pipeline looks clean now.

@tomponline tomponline merged commit b751e35 into canonical:main Apr 11, 2024
59 of 63 checks passed
@roosterfish roosterfish deleted the snap_refresh branch April 11, 2024 12:14
@simondeziel
Copy link
Member

The v1/snap1 is apparently identical to v1/snap0 since v1 is stopped the whole time. Should we power up v1 in between to have different snap content to refresh? Or maybe we could grow v1's rootfs in between and see if v2 has the new size after the refresh?

@roosterfish what do you think of ^? I'd be happy to add it if that makes sense to you (I know very little about snapshot refreshes).

@roosterfish
Copy link
Contributor Author

The v1/snap1 is apparently identical to v1/snap0 since v1 is stopped the whole time.

We start v1 here https://github.com/canonical/lxd-ci/blob/main/tests/storage-vm#L52 and leave it running throughout the copy/refresh tests. Is it actually stopped somewhere in the middle?

I'd be happy to add it if that makes sense to you (I know very little about snapshot refreshes).

I like the idea. Maybe just putting files would be easier as we wouldn't need exceptions for the PowerFlex driver in regards to volume size :)

@tomponline
Copy link
Member

I like the idea. Maybe just putting files would be easier as we wouldn't need exceptions for the PowerFlex driver in regards to volume size :)

Sounds good to me!

This pull request was closed.
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.

3 participants