Don't rely on contents of optional Condition fields in CSI mock test

[bertinatto]

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

In order to promote the volume limits e2e test (from CSI Mock driver) to Conformance, we can't rely on specific output of optional Condition fields (e.g., Reason and Message). Thus, this PR changes the test to only check the presence of the right condition and verify that the optional fields are not empty.

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[bertinatto]

/assign @gnufied

[bertinatto]

/assign @msau42

[bertinatto]

/sig storage

[bertinatto]

From https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md#conformance-test-requirements:

anything that checks optional Condition fields, such as Reason or Message, as these may change over time (however it is reasonable to verify these fields exist or are non-empty) 

[bertinatto]

/retest

[msau42]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bertinatto, msau42

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/e2e/storage/OWNERS [msau42]

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

[gnufied]

/hold

[gnufied]

This will cause false positives where test would appear to pass but may be for some other reason. Can we have 2 versions of this test?

[msau42]

This will cause false positives where test would appear to pass but may be for some other reason. Can we have 2 versions of this test?

We can have another version of the test that's not part of conformance

[johnbelamaric]

/retest

[bertinatto]

This will cause false positives where test would appear to pass but may be for some other reason. Can we have 2 versions of this test?

We can have another version of the test that's not part of conformance

The volumeLimits test suite has a test that covers that. That test is even more complete and it uses the csi-hostpath driver instead of the mock driver:

kubernetes/test/e2e/storage/testsuites/volumelimits.go

Lines 199 to 210 in b6b494b

	if pod.Status.Phase == v1.PodPending {
	reg, err := regexp.Compile(`max.+volume.+count`)
	if err != nil {
	return false, err
	}
	for _, cond := range pod.Status.Conditions {
	matched := reg.MatchString(cond.Message)
	if cond.Type == v1.PodScheduled && cond.Status == v1.ConditionFalse && cond.Reason == "Unschedulable" && matched {
	return true, nil
	}
	}
	}

IMO it makes sense to have the less strict mock driver test graduated to Conformance and the stricter volumeLimits testsuite test running in the regular presubmit job.

The only downside I see is that the latter is currently running in a non-required job (pull-kubernetes-e2e-gce-csi-serial), but I don't know if that's a big deal.

[bertinatto]

/retest

[msau42]

@gnufied what do you think of the hostpath test being more strict than the mock one used for conformance?

[gnufied]

yeah that is fine.

/hold cancel
/lgtm