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

e2e storage testsuites: avoid timeout in "volumeMode should fail in binding dynamic provisioned PV to PVC" #71748

Open
pohly opened this Issue Dec 5, 2018 · 4 comments

Comments

Projects
None yet
4 participants
@pohly
Contributor

pohly commented Dec 5, 2018

What would you like to be added:

Here's how the test is written at the moment:

err = framework.WaitForPersistentVolumeClaimPhase(v1.ClaimBound, cs, input.pvc.Namespace, input.pvc.Name, framework.Poll, framework.ClaimProvisionTimeout)
Expect(err).To(HaveOccurred())

During a normal test run, this framework.WaitForPersistentVolumeClaimPhase will run for five minutes and then time out. The timeout error then is considered a test success.

Instead, the test should wait for the expected error condition and fail if that isn't reached within the timeout period.

Why is this needed:

Faster test execution, more precise testing (no false positives when the operation fails for some other reason).

/kind feature

/sig storage
/cc @mkimuram

@mkimuram

This comment has been minimized.

Contributor

mkimuram commented Dec 5, 2018

@pohly

Thank you for pointing it out.

I think that this will be achieved by the similar way to #71428, it checks that proper events happen instead of waiting for not success.

However, because events are not reliable, #71570 will change it to waiting for not success.

Are there any good ideas to check proper failure instead of waiting not fail or relying on events?

/cc @msau42 @cofyc @saad-ali

@msau42

This comment has been minimized.

Member

msau42 commented Dec 5, 2018

For PVC errors, I'm not sure if there's a great way. For the Pod subpath error, the error is propagated to Pod conditions. But PVC does not have a similar field.

@pohly

This comment has been minimized.

Contributor

pohly commented Dec 6, 2018

@msau42

This comment has been minimized.

Member

msau42 commented Dec 10, 2018

I mean that today PVC.status.conditions are only used by the resizing controller. So we would have to consider if it makes sense to report binding errors in the PVC conditions as well.

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