-
Notifications
You must be signed in to change notification settings - Fork 115
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
STOR-2285: Update group snapshot test rules #2254
STOR-2285: Update group snapshot test rules #2254
Conversation
@jsafrane: This pull request references STOR-2285 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 story to target the "4.19.0" version, but no target version was set. In 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. |
@jsafrane: the contents of this pull request could not be automatically validated. The following commits could not be validated and must be approved by a top-level approver:
Comment |
b95cf22
to
667a2c0
Compare
@jsafrane: the contents of this pull request could not be automatically validated. The following commits could not be validated and must be approved by a top-level approver:
Comment |
/payload-job periodic-ci-openshift-release-master-ci-4.19-e2e-aws-ovn-techpreview |
@jsafrane: trigger 13 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/489d4a60-0bd6-11f0-92a7-d0e3798d0d33-0 |
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
// volumegroupsnapshot in csi-hostpath tests requires changes in the test yaml files, | ||
// which are done by a script upstream. In OCP, we added a separate driver csi-hostpath-groupsnapshot, | ||
// that will not be skipped by any rule here. | ||
`\[Driver: csi-hostpath\].*\[Feature:volumegroupsnapshot\]`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the escaping?
667a2c0
to
b6f4b95
Compare
@jsafrane: the contents of this pull request could not be automatically validated. The following commits could not be validated and must be approved by a top-level approver:
Comment |
/payload-job periodic-ci-openshift-release-master-ci-4.19-e2e-aws-ovn-techpreview |
removed regexp escaping |
@jsafrane: trigger 13 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/faced480-0e40-11f0-85b3-cc0d7649b1fa-0 |
// volumegroupsnapshot in csi-hostpath tests requires changes in the test yaml files, | ||
// which are done by a script upstream. In OCP, we added a separate driver csi-hostpath-groupsnapshot, | ||
// that will not be skipped by any rule here. | ||
`[Driver: csi-hostpath].*[Feature:volumegroupsnapshot]`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The .*
wildcard here won't work. The OTE filtering mechanism doesn't use regex, it just looks for the value in the string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it does work, you can see [sig-storage] In-tree Volumes [Driver: hostPath] [Testpattern: (delete policy)] volumegroupsnapshottable [Feature:volumegroupsnapshot] VolumeGroupSnapshottable should create snapshots for multiple volumes in a pod [Suite:openshift/conformance/parallel] [Suite:k8s]
skipped in a TP job here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hold on, is the rule actually necessary? When I remove it, k8s-test-ext list
shows:
"name": "[sig-storage] CSI Volumes [Driver: csi-hostpath] [Testpattern: (delete policy)] volumegroupsnapshottable [Feature:volumegroupsnapshot] VolumeGroupSnapshottable should create snapshots for multiple volumes in a pod [Disabled:SpecialConfig] [Suite:k8s]"
"name": "[sig-storage] OCP CSI Volumes [Driver: csi-hostpath-groupsnapshot] [OCPFeatureGate:VolumeGroupSnapshot] [Testpattern: (delete policy)] volumegroupsnapshottable [Feature:volumegroupsnapshot] VolumeGroupSnapshottable should create snapshots for multiple volumes in a pod [Suite:openshift/conformance/parallel] [Suite:k8s]"
The csi-hostpath
test is probably already skipped and csi-hostpath-groupsnapshot
runs. Trying to remove the rule in this PR to see what happens.
/hold
/test periodic-ci-openshift-release-master-ci-4.19-e2e-aws-ovn-techpreview
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/payload-job periodic-ci-openshift-release-master-ci-4.19-e2e-aws-ovn-techpreview
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it seems that removal of the rule is all I need.
We want [Driver: csi-hostpath].*volumegroupsnapshottable
to be skipped and it already has `[Disabled:SpecialConfig]'.
We want [Driver: csi-hostpath-groupsnapshot].*volumegroupsnapshottable
to run in TP jobs and it already has [OCPFeatureGate:VolumeGroupSnapshot]...[Suite:openshift/conformance/parallel]
I reworked this PR to remove the rule in k8s-tests-ext.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jsafrane, we also need to translate the rule (
kubernetes/openshift-hack/e2e/annotate/rules.go
Lines 71 to 74 in d365efd
// volumegroupsnapshot in csi-hostpath tests requires changes in the test yaml files, | |
// which are done by a script upstream. In OCP, we added a separate driver csi-hostpath-groupsnapshot, | |
// that will not be skipped by any rule here. | |
`\[Driver: csi-hostpath\].*\[Feature:volumegroupsnapshot\]`, |
disabled_tests.go
in order to continue achieving the desired functionality once rules.go
is removed. Right now there is some redundancy that is allowing it to still work, but soon the annotation filtering will be gone, and it will no longer function the way you want it to. It does not allow regex though, so you will have to write a rule to select the test without using it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the following, added to the SpecialConfig
section of disabled_tests.go
, will be sufficient:
"[Driver: csi-hostpath] [Testpattern: (delete policy)] volumegroupsnapshottable [Feature:volumegroupsnapshot]"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, updated with your suggestion.
@jsafrane: the contents of this pull request could not be automatically validated. The following commits could not be validated and must be approved by a top-level approver:
Comment |
@jsafrane: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
In 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 kubernetes-sigs/prow repository. |
/payload-job periodic-ci-openshift-release-master-ci-4.19-e2e-aws-ovn-techpreview |
@jsafrane: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/77a67460-0f16-11f0-9e50-fea581e901dc-0 |
04754b8
to
e09452d
Compare
@jsafrane: the contents of this pull request could not be automatically validated. The following commits could not be validated and must be approved by a top-level approver:
Comment |
@jsafrane: This pull request references STOR-2285 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 story to target the "4.19.0" version, but no target version was set. In 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. |
/payload-job periodic-ci-openshift-release-master-ci-4.19-e2e-aws-ovn-techpreview |
@jsafrane: trigger 13 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/ee2206f0-0fb5-11f0-86d9-3d1257919d97-0 |
Update group snapshot test rules to be in sync with openshift-hack/e2e/annotate. We want group snapshot tests disabled only with csi-hostpath test driver. That one needs a special config - a chnage in the csi-hostpath CSI driver yaml files + a feature gate enabled. The others (namely csi-hostpath-groupsnapshot) should be enabled. That one has the yaml file change + [OCPFeatureGate:VolumeGroupSnapshot].
e09452d
to
ade095f
Compare
@jsafrane: the contents of this pull request could not be automatically validated. The following commits could not be validated and must be approved by a top-level approver:
Comment |
@jsafrane: This pull request references STOR-2285 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 story to target the "4.19.0" version, but no target version was set. In 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. |
/payload-job periodic-ci-openshift-release-master-ci-4.19-e2e-aws-ovn-techpreview |
@jsafrane: trigger 13 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/d29ee170-0ff4-11f0-85da-eeee344dfd92-0 |
/lgtm |
/retest |
/hold cancel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/remove-label backports/unvalidated-commits
/label backports/validated-commits
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bertinatto, jsafrane, smg247 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:
Approvers can indicate their approval by writing |
@jsafrane: all tests passed! Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
[ART PR BUILD NOTIFIER] Distgit: openshift-enterprise-pod |
[ART PR BUILD NOTIFIER] Distgit: kube-proxy |
[ART PR BUILD NOTIFIER] Distgit: ose-installer-kube-apiserver-artifacts |
[ART PR BUILD NOTIFIER] Distgit: openshift-enterprise-hyperkube |
Update group snapshot test rules to be in sync with openshift-hack/e2e/annotate.
We want group snapshot tests disabled only with csi-hostpath test driver. That one needs a special config - a change in the csi-hostpath CSI driver yaml files + a feature gate enabled.
The others (namely csi-hostpath-groupsnapshot) should be enabled. That one has the yaml file change +
[OCPFeatureGate:VolumeGroupSnapshot]
.With this PR, I can see:
Which is what I want. Before this PR, the command returns empty output and no group snapshots are tested.