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

Add Container Storage Interface integration to conformance #85101

Open
msau42 opened this issue Nov 11, 2019 · 13 comments
Open

Add Container Storage Interface integration to conformance #85101

msau42 opened this issue Nov 11, 2019 · 13 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. sig/storage Categorizes an issue or PR as relevant to SIG Storage.

Comments

@msau42
Copy link
Member

msau42 commented Nov 11, 2019

What would you like to be added:
Continuing the discussion in #65155 (comment), while we can't test provider-specific CSI drivers in conformance, we can test provider-agnostic drivers like CSI hostpath, and make sure that K8s distributions are properly supporting the CSI interface.

There are some challenges that need to be solved before we can test CSI in conformance:

  • sig-storage has refactored some 30+ tests to be driver-agnostic (can run against any driver, in-tree or CSI). However, that currently makes it difficult to promote those tests to conformance using the current conformance tooling because the tests case definitions are now a library shared by many drivers. We either need to: 1) enhance the current conformance tooling to handle conditional test cases based off of other inputs (such as the driver being tested) or 2) duplicate all of the test cases to a conformance-specific suite.
  • CSI drivers need to be privileged in order to do mount propagation on kubelet. I forget if conformance tests allow privileged containers?
  • CSI hostpath needs "/dev" access on the host to support raw block (filesystem PVC is fine, the driver just writes to the container fs). It may be that we can't test raw block in conformance.
  • In addition, we have some test cases outside of our driver suite that uses the mock driver, and checks various CSI calls toggled by different settings (via K8s objects like CSIDriver, as well as CSI driver config options). Most of the tests validate that the right CSI calls are made by grepping mock driver logs for specific msgs since the mock driver isn't actually very functional (compared to the hostpath driver). Right now most of these test cases are testing beta features, but it would be a good idea to write a very basic test testing all the GA features. These test cases may be less functional than the generic driver suite since the mock driver doesn't actually write any data and most of our functional tests check for data persistence.

Why is this needed:
It is important to make sure that all K8s distributions support the CSI spec.

@kubernetes/sig-storage-feature-requests
cc @johnbelamaric

@msau42 msau42 added the kind/feature Categorizes issue or PR as related to a new feature. label Nov 11, 2019
@k8s-ci-robot k8s-ci-robot added the sig/storage Categorizes an issue or PR as relevant to SIG Storage. label Nov 11, 2019
@msau42
Copy link
Member Author

msau42 commented Nov 11, 2019

/assign

@johnbelamaric
Copy link
Member

@brahmaroutu is working on a KEP for this

@johnbelamaric
Copy link
Member

Privileged is not ideal but other things also need privileged too. Eventually maybe we will be able to segregate those but we cannot now.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 9, 2020
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 10, 2020
@msau42
Copy link
Member Author

msau42 commented Apr 2, 2020

/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. labels Apr 2, 2020
@msau42
Copy link
Member Author

msau42 commented Apr 2, 2020

@huffmanca would you be able to look into promoting the skip attach and pod info csi mock volume tests to conformance now that the features are GA?

@huffmanca
Copy link
Contributor

I can look into this!

@huffmanca
Copy link
Contributor

/assign

@msau42
Copy link
Member Author

msau42 commented Dec 7, 2020

Note our csi mock tests are dependent on kubectl exec which is not part of conformance.

Also, we plan on adding a proxy mode for csi mock driver, which would add a dependency on port forwarding, which is also not part of conformance.

@pohly
Copy link
Contributor

pohly commented Dec 7, 2020

Another technical issue: some clusters may use a non-standard kubelet data directory, in which case the current test driver deployments all fail because kubelet doesn't find the driver. We need a parameter for that and then someone running conformance tests must set that parameter if necessary - tracked in #92664

@pohly
Copy link
Contributor

pohly commented May 27, 2021

https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md#conformance-test-requirements also specifies that a test doesn't need privileges to run. We'll have to disable all mount operations when running tests.

@msau42
Copy link
Member Author

msau42 commented May 27, 2021

I spoke to @johnbelamaric earlier about the privileged requirement and concluded that we can relax it if necessary. So I don't consider requiring privileged an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. sig/storage Categorizes an issue or PR as relevant to SIG Storage.
Projects
None yet
Development

No branches or pull requests

6 participants