KEP-625: Update CSI Migration to GA #3126
Conversation
Jiawei0227
commented
Jan 11, 2022
Jiawei0227
commented
- One-line PR description: Update CSI Migration to GA
- Issue link: In-tree storage plugin to CSI Driver Migration #625
- Other comments:
- With components including AWS, GCE PD, Azuredisk go to GA. We declare CSI migration as stable. And will lock the feature flag in the next release.
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
kind/kep
|
/cc @msau42 @xing-yang @jsafrane @gnufied |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
Will take a look into it later this week. |
| @@ -149,7 +149,7 @@ know what configuration it’s running in and validate the expected result. | |||
| Configurations to test: | |||
|
|
|||
| | ADC | Kubelet | Expected Result | | |||
| |-------------------|----------------------------------------------------|--------------------------------------------------------------------------| | |||
| | ----------------- | -------------------------------------------------- | ------------------------------------------------------------------------ | | |||
There was a problem hiding this comment.
Commenting here, since I can't comment on the actual lines:
- The whole design repo was archived, so the links are now outdated.
For completeness of this KEP, can you maybe just copy the design:
https://github.com/kubernetes/design-proposals-archive/blob/main/storage/csi-migration.md
to this directory (keps/sig-storage/625-csi-migration) and switch links to point to it.
[I'm not asking for any changes to that doc - just for easier discoverability of stuff and keeping stuff in a single place]
L246: it says "(...) we will have"
I'm particularly interested in upgrade tests, as the upgrade path is pretty tricky.
I'm assuming that those upgrades tests exist and hopefully are running somewhere continuously? Can we maybe add some links to testgrid or sth?
L280: it again says that metrics "will have fields". Have those been added? [if so, can you please update?]
L288: do we have continuous tests already? If so can you link there?
If not, can you add an explicit TODO and ensure that they will get linked here in the KEP (can be after feature freeze, but before we officially switch the feature gate in k/k to GA)? @msau42 - FYI
L305: again "will have migrated field" - have this been added?
L320: have those been implemented? if not, is there still a plan for doing this?
There was a problem hiding this comment.
Thanks for the comments! I moved the design proposal to this directory.
I'm particularly interested in upgrade tests, as the upgrade path is pretty tricky.
I'm assuming that those upgrades tests exist and hopefully are running somewhere continuously? Can we maybe add some links to testgrid or sth?
Yes we do have upgrade tests. The upgrade tests are managed separately by different storage plugins. For example, for GCE PD CSI driver, the upgrade test is implemented inside Google and it's one of the GKE tests running internally. So it cannot be shared here. Sorry about that. For other plugins, I believe they have their own upgrade tests running somewhere. @andyzhangx @jsafrane if you guys can help provide some links?
L280: it again says that metrics "will have fields". Have those been added
Yes, this is already implemented.
L305: again "will have migrated field" - have this been added?
Already implemented.
L320: have those been implemented? if not, is there still a plan for doing this?
Yes, it has been implemented in kubernetes/kubernetes#98979.
There was a problem hiding this comment.
OpenShift does not have upgrade tests from a version where CSI migration is disabled to a version where it's enabled. We do have tests that enable CSI migration post-install, which exercises almost the same procedure as upgrade - applies the feature gates in the right order and drains + restarts all kubelets. We do that for AWS EBS, GCP PD, Azure Disk and OpenStack Cinder. Cinder is not publicly available, the rest is in https://testgrid.k8s.io/redhat-openshift-ocp-release-4.10-broken#Summary
We plan to have a real upgrade jobs in few month.
(We use -broken suffix as a quarantine for jobs that will become more imporant in the future),
There was a problem hiding this comment.
Thanks @jsafrane
Also, by asking around, I got this link that is showing exactly the gcp migration:
https://testgrid.k8s.io/provider-gcp-compute-persistent-disk-csi-driver#Migration%20Kubernetes%20Master%20Driver%20Stable
@Jiawei0227 - can you please add both links to the KEP (with the above explanation about OpenShif tests)?
There was a problem hiding this comment.
Sure. I also add AWS ebs and Azuredisk csi migration tests link
k8s-ci-robot
added
size/XXL
| @@ -243,7 +243,7 @@ like Provision/Deletion/Attach/Detach/Mount/Unmount will not be available if CSI | |||
|
|
|||
| * **Are there any tests for feature enablement/disablement?** | |||
| We have CSI Migration e2e test for each plugin that are implemented and maintained by each driver maintainer. | |||
| Specifically, for each in-tree plugin corresponding CSI drivers, it will have | |||
| Specifically, for each in-tree plugin corresponding CSI drivers, it havs | |||
|
Thanks this looks great! /lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Jiawei0227, msau42, wojtek-t 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 |
k8s-ci-robot
added
the
approved
