more targeted cleanup of target/scratch pvcs

[mhenriks]

What this PR does / why we need it:

Rather than cleaning up everything on target/scratch PVCs, only delete what we may have created

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #2635

Special notes for your reviewer:

Release note:

More targeted cleanup of target/scratch pvcs (only delete what we may have created)

[mhenriks]

/hold

[alromeros]

/test pull-containerized-data-importer-e2e-ceph

[mhenriks]

/unhold

[mhenriks]

/approve

[kubevirt-bot]

[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:

• OWNERS [mhenriks]

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

[mhenriks]

/cherrypick release-v1.54

[kubevirt-bot]

@mhenriks: once the present PR merges, I will cherry-pick it on top of release-v1.54 in a new PR and assign it to you.

In response to this:

/cherrypick release-v1.54

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.

[kubevirt-bot]

@mhenriks: #2676 failed to apply on top of branch "release-v1.54":

Applying: more targeted cleanup of target/scratch pvcs
Using index info to reconstruct a base tree...
M	pkg/importer/data-processor.go
M	pkg/importer/data-processor_test.go
A	pkg/importer/gcs-datasource.go
M	pkg/importer/http-datasource.go
M	pkg/importer/imageio-datasource.go
M	pkg/importer/registry-datasource.go
M	pkg/importer/util.go
M	pkg/importer/util_test.go
M	pkg/importer/vddk-datasource_amd64.go
M	pkg/importer/vddk-datasource_test.go
M	pkg/uploadserver/uploadserver.go
M	pkg/util/util.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/util/util.go
Auto-merging pkg/uploadserver/uploadserver.go
Auto-merging pkg/importer/vddk-datasource_test.go
Auto-merging pkg/importer/vddk-datasource_amd64.go
Auto-merging pkg/importer/util_test.go
Auto-merging pkg/importer/util.go
CONFLICT (content): Merge conflict in pkg/importer/util.go
Auto-merging pkg/importer/registry-datasource.go
Auto-merging pkg/importer/imageio-datasource.go
Auto-merging pkg/importer/http-datasource.go
CONFLICT (modify/delete): pkg/importer/gcs-datasource.go deleted in HEAD and modified in more targeted cleanup of target/scratch pvcs. Version more targeted cleanup of target/scratch pvcs of pkg/importer/gcs-datasource.go left in tree.
Auto-merging pkg/importer/data-processor_test.go
CONFLICT (content): Merge conflict in pkg/importer/data-processor_test.go
Auto-merging pkg/importer/data-processor.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 more targeted cleanup of target/scratch pvcs
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick release-v1.54

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.

[alromeros]

@mhenriks shouldn't we backport this to v1.55 and v1.56 too?

[awels]

/cherrypick release-v1.56

[kubevirt-bot]

@awels: #2676 failed to apply on top of branch "release-v1.56":

Applying: more targeted cleanup of target/scratch pvcs
Using index info to reconstruct a base tree...
A	pkg/importer/gcs-datasource.go
M	pkg/uploadserver/uploadserver.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/uploadserver/uploadserver.go
CONFLICT (modify/delete): pkg/importer/gcs-datasource.go deleted in HEAD and modified in more targeted cleanup of target/scratch pvcs. Version more targeted cleanup of target/scratch pvcs of pkg/importer/gcs-datasource.go left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 more targeted cleanup of target/scratch pvcs
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick release-v1.56

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.

[alromeros]

@mhenriks @awels After a couple of days investigating I’ve found that at least two of these test failures are related to this PR. The issue occurs when importing a fully allocated block PVC (e.g., the image’s actual size is 512Mi, and the PVC is also 512Mi). In such cases, we create a scratch PVC with additional overhead (542Mi).

However, using rook-ceph, there's always a lost+found directory in the scratch space that reduces the actual usable space to "capacity - overhead - lost+found size" (~505Mi in my test case), causing the import to fail. This is a bug but I’m unsure if it’s acceptable to delete lost+found in cases like these. Other solution that comes to mind is adding an arbitrary overhead to the scratch PVC to account for the lost+found size.

What are your thoughts on this? This is 100% replicable when using the vm manifest endpoint in the export API.

[alromeros]

@mhenriks @awels After a couple of days investigating I’ve found that at least two of these test failures are related to this PR. The issue occurs when importing a fully allocated block PVC (e.g., the image’s actual size is 512Mi, and the PVC is also 512Mi). In such cases, we create a scratch PVC with additional overhead (542Mi).

However, using rook-ceph, there's always a lost+found directory in the scratch space that reduces the actual usable space to "capacity - overhead - lost+found size" (~505Mi in my test case), causing the import to fail. This is a bug but I’m unsure if it’s acceptable to delete lost+found in cases like these. Other solution that comes to mind is adding an arbitrary overhead to the scratch PVC to account for the lost+found size.

What are your thoughts on this? This is 100% replicable when using the vm manifest endpoint in the export API.

Created a Jira card to track this bug https://issues.redhat.com/browse/CNV-51575