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.

Sign up for GitHub

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

Jump to bottom

Add tests for trim reducing space usage on filesystem and block storage #5622

Merged
merged 4 commits into from
May 28, 2021
Merged

Add tests for trim reducing space usage on filesystem and block storage #5622

merged 4 commits into from
May 28, 2021

Conversation

maya-r
Copy link
Contributor

@maya-r maya-r commented May 10, 2021

Test flow:
Boot Fedora
Write large file
Write down space usage
Write small file
Delete large file, trim /

Write down space usage again

If preallocated, we expect no change in disk usage.
If trim is expected to work (default) we expect a decrease.
If trim isn't expected to work (thick provisioned) we expect an increase, due to the small file.

Both OCS and local storage are tested.

Depends on #5578 to be merged first.

What this PR does / why we need it:
Testing actual end-to-end functionality.

Release note:

NONE

@kubevirt-bot kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels May 10, 2021
@maya-r maya-r marked this pull request as draft May 10, 2021 12:17
@kubevirt-bot kubevirt-bot added size/L do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels May 10, 2021
@maya-r
Copy link
Contributor Author

maya-r commented May 10, 2021

Draft until #5578 is merged.

@maya-r maya-r marked this pull request as ready for review May 10, 2021 13:56
@kubevirt-bot kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 10, 2021
@alicefr
Copy link
Member

alicefr commented May 11, 2021

Looks good :)

kgoldbla
kgoldbla suggested changes May 12, 2021
View reviewed changes
Copy link

@kgoldbla kgoldbla left a comment •
edited

Choose a reason for hiding this comment

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

Looks ok. Please add the requirement CNV-5750 to the test:

@kubevirt-bot
Copy link
Contributor

@kgoldbla: changing LGTM is restricted to collaborators

In response to this:

Looks good. Please add the requirement to the test:
CNV-5750

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@mhenriks
Copy link
Member

/lgtm
/approve

@kubevirt-bot kubevirt-bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 13, 2021
@mhenriks
Copy link
Member

/approve cancel

@kubevirt-bot kubevirt-bot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 13, 2021
@mhenriks
Copy link
Member

@maya-r I think the toolbox should be baked into kubevirtci for rook-ceph-default

@mhenriks
Copy link
Member

/lgtm cancel

@kubevirt-bot kubevirt-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lgtm Indicates that a PR is ready to be merged. labels May 13, 2021
kgoldbla
kgoldbla reviewed May 18, 2021
View reviewed changes
Copy link

@kgoldbla kgoldbla left a comment

Choose a reason for hiding this comment

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

lgtm

@maya-r maya-r mentioned this pull request May 18, 2021
Merged
@kubevirt-bot kubevirt-bot added dco-signoff: no Indicates the PR's author has not DCO signed all their commits. and removed dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels May 24, 2021
@kubevirt-bot kubevirt-bot removed the dco-signoff: no Indicates the PR's author has not DCO signed all their commits. label May 24, 2021
@kubevirt-bot kubevirt-bot added the dco-signoff: yes Indicates the PR's author has DCO signed all their commits. label May 24, 2021
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 24, 2021
@kubevirt-bot kubevirt-bot added size/XL kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API and removed size/L labels May 25, 2021
@maya-r
Bump kubevirtci to include ceph toolbox
b0e8103 
Signed-off-by: Maya Rashish <mrashish@redhat.com>
@maya-r
Add function for accessing ceph toolbox
5b283d0 
Signed-off-by: Maya Rashish <mrashish@redhat.com>
@maya-r
Add the option to HTTP import a Fedora image.
5b1eb73 
Signed-off-by: Maya Rashish <mrashish@redhat.com>
@maya-r
Add tests for trim/discard actually reducing space usage
a367e7c 
On both block (OCS) and filesystem (local storage)

Signed-off-by: Maya Rashish <mrashish@redhat.com>
@maya-r
Copy link
Contributor Author

maya-r commented May 25, 2021

the goveralls failure seems to originate from #5694, rather than from this PR.

@maya-r
Copy link
Contributor Author

maya-r commented May 25, 2021

@maya-r I think the toolbox should be baked into kubevirtci for rook-ceph-default

Done :)

@mhenriks
Copy link
Member

/retest

@mhenriks
Copy link
Member

/lgtm
/approve

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label May 27, 2021
@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mhenriks

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 27, 2021
@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

1 similar comment
@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-bot kubevirt-bot merged commit 02714cd into kubevirt:master May 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kgoldbla kgoldbla kgoldbla requested changes

@jean-edouard jean-edouard Awaiting requested review from jean-edouard

@rnetser rnetser Awaiting requested review from rnetser

Assignees

@mhenriks mhenriks

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@maya-r @alicefr @kubevirt-bot @mhenriks @kubevirt-commenter-bot @kgoldbla