OSASINFRA-4468: add test for appendVolumeMetadata support in Cinder CSI - #318
OSASINFRA-4468: add test for appendVolumeMetadata support in Cinder CSI#318chenwng wants to merge 1 commit into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@chenwng: This pull request references OSASINFRA-4468 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.1.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| o.Expect(err).Should(o.BeNil()) | ||
|
|
||
| available, errs := constraint.Validate(latestVer) | ||
| o.Expect(len(errs)).Should(o.BeZero()) |
There was a problem hiding this comment.
This fails the test on older clusters instead of skipping it. Constraints.Validate (vendored Masterminds/semver) returns available=false AND a non-empty errs whenever the version does not satisfy >=5.1.0-0. So on exactly the clusters we intend to skip, this assertion fails before the Skipf on line 343 is ever reached — the skip branch is unreachable.
Suggest gating on the boolean only and dropping the errs assertion:
if !constraint.Check(latestVer) {
e2eskipper.Skipf("appendVolumeMetadata is only available on >= 5.1.0")
}
errs is just the why-not detail, not an error condition to assert against.
|
|
||
| o.Expect(len(cv.Status.History)).ShouldNot(o.BeZero()) | ||
|
|
||
| latestVer, err := semver.NewVersion(cv.Status.History[0].Version) |
There was a problem hiding this comment.
History[0] is the most recent entry but may be State: Partial during an in-progress upgrade, so this can read a version the cluster hasn't fully reached. Prefer cv.Status.Desired.Version (or the first History entry with State == Completed) for capability gating.
| volMetadata := vols[0].Metadata | ||
| e2e.Logf("volume metadata %q", volMetadata) | ||
| for k, v := range metadata { | ||
| v1, ok := volMetadata[k] |
There was a problem hiding this comment.
Minor: v1 shadows the v1 "k8s.io/api/core/v1" import. It compiles fine since the package isn't referenced in this block, but renaming (e.g. got) reads more clearly.
3 participants
This PR adds an e2e test case for the support of
appendVolumeMetadatain Cinder CSI. After the merge of kubernetes/cloud-provider-openstack#3162 and downstream sync, it will be avaiable on OCP version >=5.1.0.