aws-efs-csi-driver

aws-efs-csi-driver copied to clipboard

Is this a bug fix or adding new feature? New Feature, now that ReadWriteOncePod has entered Alpha testing we should add support for it in the driver.

What is this PR about? / Why do we need it? This PR enables us to serve users that want to use ReadWriteOncePod in their manifests. It adds new E2E tests to cover the use cases the ReadWriteOncePod covers.

What testing is done? More E2E testing in Kops only, as EKS has no way to set the feature gates required. The first draft of this may not be exactly what's needed from E2E tests but I need to use the CI framework to run them and then debug.

fixes #540

@chrishenzie tagging for visibility and assistance.

Hi @jonathanrainer. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@Ashley-wenyizha would I able to get this approved for testing so I can start getting the E2E tests sorted out?

/ok-to-test

@Ashley-wenyizha would I able to get this approved for testing so I can start getting the E2E tests sorted out?

sure, thanks for working on this, tag added

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: chrishenzie, jonathanrainer Once this PR has been reviewed and has the lgtm label, please assign justinsb for approval by writing /assign @justinsb in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

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

/remove-lifecycle stale

Hi! Just a heads up, an e2e test for ReadWriteOncePod was added here.

I think in order to make this work we'll need to:

• Update our k8s dependency (to pull in the capability + test suite), v1.26 should work
• Update InitEFSCSIDriver() to include storageframework.CapReadWriteOncePod
• Update csiTestSuites to include testsuites.InitReadWriteOncePodTestSuite()
• Verify the e2e test uses at least these sidecar versions:
  • csi-provisioner:v3.0.0+
  • csi-attacher:v3.3.0+
  • csi-resizer:v1.3.0+

And I think these tests will be pulled in and ran.

Note that for this to succeed the ReadWriteOncePod feature flag will need to be enabled in the k8s cluster. In v1.27 this feature will be moved to beta and will be enabled by default. There are additional changes being made to the e2e test in v1.27 so this dependency will need an additional update once we start testing against that k8s version.

Hi @chrishenzie,

Sorry it's taken a long time for me to reply, I've had a look at this but I think that updating the Kubernetes version throughout the driver, even if it's just to access new E2E tests is something that should be done in a separate PR, not this one as it's likely to introduce breaking changes etc. Adding the test is definitely something we could do in the future though, might be best to raise a separate issue for it so when we do upgrade to later Kubernetes versions we can sort it then :)

ReadWriteOncePod is now graduated to beta in K8s v.127 and enabled by default. See this comment on the Github issue for more details.

The only thing required of CSI driver users is to update their sidecars.

There is also an E2E test suite for the ReadWriteOncePod feature. See the steps in the above comment for what's required to pull in these tests and it should start working with the AWS EFS CSI driver. Please let me know if you encounter any issues and I can help.

/lgtm

Hi Jonathan, this PR lgtm, could you give a fix of the conflicts? I will merge this in

thank you!

New changes are detected. LGTM label has been removed.

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: chrishenzie, jonathanrainer Once this PR has been reviewed and has the lgtm label, please assign justinsb for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

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

@Ashley-wenyizha I've fixed this up now, had some weird errors with the new E2E tests but I think that was a result of a bad merge conflict/adding things I didn't need because we're now using 1.27 in our E2E tests.

PR needs rebase.

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.

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Mark this PR as fresh with /remove-lifecycle stale
• Close this PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

Update -- ReadWriteOncePod is now GA/Stable as of K8s v1.29. So long as this driver is deployed with these minimum CSI sidecar versions, ReadWriteOncePod will work without any driver changes here.

Additionally, the K8s E2E test suite includes tests for this access mode that should work for this driver. In order to pull in and use these tests you will need to first update your K8s dependencies to at least v1.27.

An important thing this PR is tackling is the migration to the new CSI access modes. Since support for the K8s access mode and E2E testing is handled outside of this repo, I would consider refocusing this PR on the CSI access mode migration.

The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Mark this PR as fresh with /remove-lifecycle rotten
• Close this PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Reopen this PR with /reopen
• Mark this PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Reopen this PR with /reopen
• Mark this PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

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.