Skip to content

Tests: Address bug in quarantined export tests#13326

Merged
kubevirt-bot merged 2 commits into
kubevirt:mainfrom
alromeros:fix-quarantined-test
Dec 10, 2024
Merged

Tests: Address bug in quarantined export tests#13326
kubevirt-bot merged 2 commits into
kubevirt:mainfrom
alromeros:fix-quarantined-test

Conversation

@alromeros
Copy link
Copy Markdown
Member

@alromeros alromeros commented Nov 21, 2024

What this PR does

When importing a fully expanded image into a PVC that matches its size, the process can fail due to insufficient usable space in the scratch PVC caused by the presence of the lost+found directory.

We should probably address this behavior in CDI by increasing the default fs overhead. Until we decide what to do, this Pull Request temporarily addresses this issue in a quarantined test by doubling the size request, which results in increased overhead space now able to contain lost+found.

More info: https://issues.redhat.com/browse/CNV-51575.

Partially fixes #13316

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. size/S sig/storage labels Nov 21, 2024
@dosubot dosubot Bot added the kind/bug label Nov 21, 2024
When importing a fully allocated image into a PVC that matches its size, the process can fail due to insufficient usable space in the scratch PVC caused by the presence of the lost+found directory.

This commit temporarily addresses this issue in a quarantined tests by doubling the requested size.

More info in https://issues.redhat.com/browse/CNV-51575.

Signed-off-by: Alvaro Romero <[email protected]>
@alromeros alromeros force-pushed the fix-quarantined-test branch from 6f6e4f6 to 241020e Compare November 21, 2024 17:24
@alromeros alromeros changed the title Tests: Address bug in quarantined export test Tests: Address bug in quarantined export tests Nov 21, 2024
Comment thread tests/storage/export.go
libdv.StorageWithStorageClass(sc),
// TODO: Rendering this VM with more size than usual as fully expanded images are likely
// to leave scratch space PVC without space if files such as lost+found exist.
// More info in https://issues.redhat.com/browse/CNV-51575.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's make a Kubevirt issue for this.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ACK: #13438

@dhiller
Copy link
Copy Markdown
Contributor

dhiller commented Dec 4, 2024

@alromeros what is the status of this PR?

@alromeros
Copy link
Copy Markdown
Member Author

@alromeros what is the status of this PR?

Created an issue to track the underlying bug in kubevirt: #13438. Other than that, this is ready for review, @akalenyu @mhenriks wdyt?

Comment thread tests/storage/export.go
// TODO: Rendering this VM with more size than usual as fully expanded images are likely
// to leave scratch space PVC without space if files such as lost+found exist.
// More info in https://issues.redhat.com/browse/CNV-51575.
libdv.StorageWithVolumeSize("1024Mi")),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1Gi?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

512Mi was enough to trigger the bug so decided to double the amount.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I mean why use mebibytes instead of gibibytes

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

hmm just cosmetics, easier to notice I doubled the original amount. I can change it if preferred.

Copy link
Copy Markdown
Contributor

@akalenyu akalenyu left a comment

Choose a reason for hiding this comment

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

I have no issue with this, but the question is, will this pass when out of quarantine?

@akalenyu
Copy link
Copy Markdown
Contributor

akalenyu commented Dec 4, 2024

I have no issue with this, but the question is, will this pass when out of quarantine?

If you remove the tests and they pass, I think I'm okay with this. Maybe run the flake lane a couple of times to make sure

@mhenriks
Copy link
Copy Markdown
Member

mhenriks commented Dec 4, 2024

Yeah, let's try getting the tests out of quarantine

@alromeros
Copy link
Copy Markdown
Member Author

/retest

@alromeros
Copy link
Copy Markdown
Member Author

/test pull-kubevirt-check-tests-for-flakes

1 similar comment
@alromeros
Copy link
Copy Markdown
Member Author

/test pull-kubevirt-check-tests-for-flakes

@akalenyu
Copy link
Copy Markdown
Contributor

akalenyu commented Dec 8, 2024

/approve
/cc @mhenriks

@kubevirt-bot kubevirt-bot requested a review from mhenriks December 8, 2024 08:39
@kubevirt-bot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akalenyu

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

The pull request process is described here

Details 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 Dec 8, 2024
@mhenriks
Copy link
Copy Markdown
Member

mhenriks commented Dec 9, 2024

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 9, 2024
@kubevirt-commenter-bot
Copy link
Copy Markdown

Required labels detected, running phase 2 presubmits:
/test pull-kubevirt-e2e-windows2016
/test pull-kubevirt-e2e-kind-1.30-vgpu
/test pull-kubevirt-e2e-kind-sriov
/test pull-kubevirt-e2e-k8s-1.30-ipv6-sig-network
/test pull-kubevirt-e2e-k8s-1.29-sig-network
/test pull-kubevirt-e2e-k8s-1.29-sig-storage
/test pull-kubevirt-e2e-k8s-1.29-sig-compute
/test pull-kubevirt-e2e-k8s-1.29-sig-operator
/test pull-kubevirt-e2e-k8s-1.30-sig-network
/test pull-kubevirt-e2e-k8s-1.30-sig-storage
/test pull-kubevirt-e2e-k8s-1.30-sig-compute
/test pull-kubevirt-e2e-k8s-1.30-sig-operator

@brianmcarey
Copy link
Copy Markdown
Member

/override pull-kubevirt-e2e-k8s-1.31-sig-performance

Triggered by status-reconciler when lane was bumped.

@kubevirt-bot
Copy link
Copy Markdown
Contributor

@brianmcarey: Overrode contexts on behalf of brianmcarey: pull-kubevirt-e2e-k8s-1.31-sig-performance

Details

In response to this:

/override pull-kubevirt-e2e-k8s-1.31-sig-performance

Triggered by status-reconciler when lane was bumped.

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-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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/bug lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. sig/storage size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

quarantined,unstable: tests in sig-storage lane wrt Export - DataVolumeTemplates and DVs

8 participants