Skip to content
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

Mark some storage tests as LinuxOnly #96644

Merged
merged 1 commit into from Nov 17, 2020

Conversation

jingxu97
Copy link
Contributor

@jingxu97 jingxu97 commented Nov 17, 2020

Some storage tests has commands not available in Windows. Mark them as
LinuxOnly now. Will check later to see whether equivalent windows
commands are available.

Change-Id: I41b5668c855b2754a2e332cff4e90ebf2981aca0

What type of PR is this?
/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

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


Some storage tests has commands not available in Windows. Mark them as
LinuxOnly now. Will check later to see whether equivalent windows
commands are available.

Change-Id: I41b5668c855b2754a2e332cff4e90ebf2981aca0
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Nov 17, 2020
@k8s-ci-robot
Copy link
Contributor

@jingxu97: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

@k8s-ci-robot k8s-ci-robot added needs-priority Indicates a PR lacks a `priority/foo` label and requires one. release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Nov 17, 2020
@jingxu97 jingxu97 added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. sig/storage Categorizes an issue or PR as relevant to SIG Storage. labels Nov 17, 2020
@k8s-ci-robot k8s-ci-robot removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Nov 17, 2020
@jingxu97 jingxu97 added the kind/bug Categorizes issue or PR as related to a bug. label Nov 17, 2020
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. label Nov 17, 2020
@k8s-ci-robot k8s-ci-robot added area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 17, 2020
@@ -127,7 +127,7 @@ func (t *multiVolumeTestSuite) DefineTests(driver TestDriver, pattern testpatter
// [ node1 ] ==> [ node1 ]
// / \ <- same volume mode / \
// [volume1] [volume2] [volume1] [volume2]
ginkgo.It("should access to two volumes with the same volume mode and retain data across pod recreation on the same node", func() {
ginkgo.It("should access to two volumes with the same volume mode and retain data across pod recreation on the same node [LinuxOnly]", func() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this failing just for block volmode or also filesystem volmode?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

block test will not be triggered because pd driver for windows does not support block.
Only one negative test for blockvolume will be triggered since it is trying to test if block is not supported.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test case runs for both filesystem volmode and block volmode. By adding the Linux tag here, then the filesystem volmode test case will no longer run on Windows. Is that what we want?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the command used here is not available for windows no matter vol type. I need to work on finding similar commands for windows before enabling them.

@@ -157,7 +157,7 @@ func (t *multiVolumeTestSuite) DefineTests(driver TestDriver, pattern testpatter
// [ node1 ] ==> [ node2 ]
// / \ <- same volume mode / \
// [volume1] [volume2] [volume1] [volume2]
ginkgo.It("should access to two volumes with the same volume mode and retain data across pod recreation on different node", func() {
ginkgo.It("should access to two volumes with the same volume mode and retain data across pod recreation on different node [LinuxOnly]", func() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, we are inconsistent with how we skip tests based on OS. Sometimes, we check node OS, and sometimes we use [LinuxOnly] tag. Can you clarify when do we use which method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the tests can only work on linux, we mark it as [linuxonly]

for some tests, the test can work on Windows but with certain fstypes (not all fstypes), there is a check here

if pattern.FsType == "xfs" && framework.NodeOSDistroIs("windows") {

@msau42
Copy link
Member

msau42 commented Nov 17, 2020

can you open a bug to track investigating these tests?

@jingxu97
Copy link
Contributor Author

can you open a bug to track investigating these tests?

y, I have one opened #96481

@msau42
Copy link
Member

msau42 commented Nov 17, 2020

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 17, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jingxu97, 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:

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

@jingxu97 jingxu97 added this to the v1.20 milestone Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. release-note-none Denotes a PR that doesn't merit a release note. sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants