Wait for subPath error event instead of waiting full minute

[wongma7]

What this PR does / why we need it: Currently, "should fail for new directories when readOnly specified in the volumeSource" always waits for a full minute for the error event to show up. However, the test can end earlier by watching/polling for events.

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 #

Special notes for your reviewer: this patch has the side effect of upping our wait time from 1 minute to PodStartTimeout 5 minutes...is this acceptable?

Release note:

NONE

/cc @mkimuram

[k8s-ci-robot]

@wongma7: GitHub didn't allow me to request PR reviews from the following users: mkimuram.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

What this PR does / why we need it: Currently, "should fail for new directories when readOnly specified in the volumeSource" always waits for a full minute for the error event to show up. However, the test can end earlier by watching/polling for events.

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 #

Special notes for your reviewer: this test has the side effect of upping our wait time from 1 minute to PodStartTimeout 5 minutes...is this acceptable?

Release note:

NONE

/cc @mkimuram

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.

[wongma7]

/sig storage

[wongma7]

/kind bug

[wongma7]

on second thought i shouldn't add WaitForErrorEvent and should just use the existing WaitForErrorEventOrSuccess... obviously success means a failure of the test and we can end earlier than 5 minutes.

Init container makes this possible will go back to original

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wongma7
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: marun

If they are not already assigned, you can assign the PR to them by writing /assign @marun in a comment when ready.

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:

• test/OWNERS

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

[mkimuram]

I like your idea of shortening the time of this test, because it makes sense not to wait for the pod that won't be running to be running.

I think that it would be better to also make @jsafrane and @msau42 involved. They are the original author of
testPodFailSupath and testPodFailSubpathError.

[mkimuram]

Are this list of events enough to cover all error events?
If not, won't it be better to take expected reason as an argument for WaitForErrorEvent to ensure that the expected event with the expected reason matches here?

[k8s-ci-robot]

@wongma7: PR needs rebase.

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.

[msau42]

Is this still needed? I think there were changes made to this area that fixed some flakes

[wongma7]

no, #69300 does roughly the same thing. I think waiting "WaitForPodRunning" to timeout is still redundant but we can keep it