From 59003bce94fb73f18f099870ece54dc12d4ff00c Mon Sep 17 00:00:00 2001 From: Drew Sirenko <68304519+AndrewSirenko@users.noreply.github.com> Date: Wed, 5 Jun 2024 19:58:04 +0000 Subject: [PATCH 1/4] KEP-3751: Update for milestone 1.31 --- keps/prod-readiness/sig-storage/3751.yaml | 4 +- .../3751-volume-attributes-class/README.md | 72 ++++++++++++++++--- .../3751-volume-attributes-class/kep.yaml | 8 +-- 3 files changed, 68 insertions(+), 16 deletions(-) diff --git a/keps/prod-readiness/sig-storage/3751.yaml b/keps/prod-readiness/sig-storage/3751.yaml index b9ba237a660..46671672252 100644 --- a/keps/prod-readiness/sig-storage/3751.yaml +++ b/keps/prod-readiness/sig-storage/3751.yaml @@ -3,4 +3,6 @@ # of http://git.k8s.io/enhancements/OWNERS_ALIASES kep-number: 3751 alpha: - approver: "@johnbelamaric" \ No newline at end of file + approver: "@johnbelamaric" +beta: + approver: "@johnbelamaric" diff --git a/keps/sig-storage/3751-volume-attributes-class/README.md b/keps/sig-storage/3751-volume-attributes-class/README.md index 4e210760d0e..59323d0ef06 100644 --- a/keps/sig-storage/3751-volume-attributes-class/README.md +++ b/keps/sig-storage/3751-volume-attributes-class/README.md @@ -89,7 +89,7 @@ Items marked with (R) are required *prior to targeting to a milestone / release* - [ ] (R) [all GA Endpoints](https://github.com/kubernetes/community/pull/1806) must be hit by [Conformance Tests](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md) - [ ] (R) Production readiness review completed - [ ] (R) Production readiness review approved -- [ ] "Implementation History" section is up-to-date for milestone +- [X] "Implementation History" section is up-to-date for milestone - [ ] User-facing documentation has been created in [kubernetes/website], for publication to [kubernetes.io] - [ ] Supporting documentation—e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes @@ -414,9 +414,9 @@ Please see session "Kubernetes API" above. ### 5. Add new operation metrics for ModifyVolume operations -Usage metrics: +A. Count of bound/unbound PVCs per VolumeAttributesClass similar to [StorageClass](https://github.com/kubernetes/kubernetes/blob/666fc23fe4d6c84b1dde2b8d4ebf75fce466d338/pkg/controller/volume/persistentvolume/metrics/metrics.go#L98). -Count of pvcs per VolumeAttributesClass similar to [StorageClass](https://github.com/kubernetes/kubernetes/blob/666fc23fe4d6c84b1dde2b8d4ebf75fce466d338/pkg/controller/volume/persistentvolume/metrics/metrics.go#L98). +Today, we loop through all PersistentVolume objects, check if `pv.Status.Phase == v1.VolumeBound` and increment the appropriate `pv.Spec.StorageClassName` bucket. For these new metrics, when the feature flag is enabled, we also increment the appropriate `pv.Spec.VolumeAttributeClassName` if it is not empty. ``` boundPVCCountDesc = metrics.NewDesc( @@ -431,7 +431,13 @@ unboundPVCCountDesc = metrics.NewDesc( metrics.ALPHA, "") ``` -Operation metrics from [csiOperationsLatencyMetric](https://github.com/kubernetes-csi/csi-lib-utils/blob/597d128ce3a24d9e3fd5ff5b8d1ff4fd862e543a/metrics/metrics.go#LL250C6-L250C32) for the ModifyVolume operation. +B. Operation metrics for ControllerModifyVolume + +The metrics `controller_modify_volume_total` `controller_modify_volume_errors_total` can be used to issues in volume modification. + +There are operation metrics from [csiOperationsLatencyMetric](https://github.com/kubernetes-csi/csi-lib-utils/blob/597d128ce3a24d9e3fd5ff5b8d1ff4fd862e543a/metrics/metrics.go#LL250C6-L250C32) for the ModifyVolume operation to report latencies. + +Finally, CSI Driver Plugin maintainers can expose their own metrics. ## Design Details @@ -629,6 +635,8 @@ https://storage.googleapis.com/k8s-triage/index.html - The behavior with feature gate and API turned on/off and mix match - The happy path with creating and modifying volume successfully with VolumeAttributesClass + - [E2E CSI Test PR](https://github.com/kubernetes/kubernetes/pull/124151/) + - k8s-triage will be linked once test PR merged ##### e2e tests @@ -650,10 +658,12 @@ We expect no non-infra related flakes in the last month as a GA graduation crite - Give a driver that does not ControllerModifyVolume, CSI volume should not be modified. - If ControllerModifyVolume fails, PVC should have appropriate events. +[API Conformance Test PR](https://github.com/kubernetes/kubernetes/pull/121849) + ##### Stress tests -- VAC protection controller with large(define large later) lists of PVCs -- Creating a large(define large later) amount of PVCs using the same VolumeAttributesClass +- VAC protection controller with large lists of PVCs (2000) +- Creating a large amount of PVCs (2000) using the same VolumeAttributesClass ### Graduation Criteria @@ -668,13 +678,13 @@ We expect no non-infra related flakes in the last month as a GA graduation crite #### Beta -- Beta in 1.30: Since this feature is an extension of the external-resizer/external-provisioner usage flow, we are going to move this to beta with enhanced e2e and test coverage. Test cases are covered in sessions above: ``e2e tests``, ``Integration tests`` etc. +- Beta in 1.31: Since this feature is an extension of the external-resizer/external-provisioner usage flow, we are going to move this to beta with enhanced e2e and test coverage. Test cases are covered in sessions above: ``e2e tests``, ``Integration tests`` etc. - Involve 3 different CSI drivers to participate in testing - Stress test before GA #### GA -- GA in 1.31, all major issues in the issue board should be fixed before GA. +- GA in 1.3x, all major issues in the issue board should be fixed before GA. - No users complaining about the new behavior ### Upgrade / Downgrade Strategy @@ -747,7 +757,7 @@ A metric `controller_modify_volume_errors_total` will indicate a problem with th ###### Were upgrade and rollback tested? Was the upgrade->downgrade->upgrade path tested? -Upgrade and rollback will be tested when the feature gate will change to beta. +TODO Upgrade and rollback will be tested when the feature gate will change to beta. ###### Is the rollout accompanied by any deprecations and/or removals of features, APIs, fields of API types, flags, etc.? @@ -764,7 +774,7 @@ count of successful and failed ControllerModifyVolume. By inspecting a `controller_modify_volume_total` metric value. If the counter is increasing while letting PVCs being updated retroactively the feature is enabled. And at the same time if -`controller_modify_volume_total` counter does not increase the feature +`controller_modify_volume_errors_total` counter does not increase the feature works as expected. ###### What are the reasonable SLOs (Service Level Objectives) for the enhancement? @@ -784,6 +794,9 @@ These goals will help you determine what you need to measure (SLIs) in the next question. --> +- Ratio of `controller_modify_volume_errors_total`/`controller_modify_volume_total` <= 1% +- CreateVolume `csi_sidecar_operations_seconds_sum` does not increase by more than 5% when feature flags are enabled. + ###### What are the SLIs (Service Level Indicators) an operator can use to determine the health of the service? - [ ] Metrics @@ -797,6 +810,7 @@ question. Describe the metrics themselves and the reasons why they weren't added (e.g., cost, implementation difficulties, etc.). --> +No. ### Dependencies @@ -806,7 +820,7 @@ This section must be completed when targeting beta to a release. ###### Does this feature depend on any specific services running in the cluster? -external-provisioner, external-resizer. +external-provisioner, external-resizer. ### Scalability @@ -849,6 +863,11 @@ Yes, the feature may impact CreateVolume. We will measure this impact during bet ###### Will enabling / using this feature result in non-negligible increase of resource usage (CPU, RAM, disk, IO, ...) in any components? +Using this feature may result in non-negligible increase of resource usage IF customers batch modify many volumes at once and increase default external-resizer `--workers`, `-kube-api-burst`, and `-kube-api-qps` options. +- external-resizer CPU and memory will see a non-negligible increase if users increased the number of concurrent operations via the `--workers` flag. We follow the strategy of sharing that limit between `ControllerExpandVolume` and `ControllerModifyVolume` RPCs, similar to how external-provisioner functions. +- If users increased the default `-kube-api-burst` and `-kube-api-qps` of their external-resizer container, the API-Server may see a spike of CPU when processing these changes + +Stress tests will determine increase in resource usage at varying amounts of concurrent volume modifications. ### Troubleshooting @@ -865,6 +884,15 @@ details). For now, we leave it here. ###### How does this feature react if the API server and/or etcd is unavailable? +No change from today's volume provisioning workflow if API Server / etcd are unavailable. + +If API server and/or etcd is unavailable, there are two scenarios for volume modification workflow + +1. External-resizer detects volume needing modification before API Server is made unavailable. Calls ControllerModifyVolume. Cloud provider will modify volume, report success to external resizer. External-resizer will be unable to update PVC object until API Server back online. Error will be logged. + +2. External-resizer does NOT detect volume needing modification before API Server is made unavailable. Volume modification will not take place until API Server back online. + +In both cases the PVC has not been updated to reflect new VolumeAttributeClass until API Server back online. ###### What are other known failure modes? @@ -881,9 +909,21 @@ For each of them, fill in the following information by copying the below templat Not required until feature graduated to beta. - Testing: Are there any tests for failure mode? If not, describe why. --> +--> +- ControllerModifyVolume cannot modify volume to reflect new VolumeAttributeClass due to user misconfiguration or cloudprovider backend error/limits. Volume would fall back to workable default configuration but external-resizer will requeue with longer `Infeasible` interval. + - Detection: See event on PVC object. See increase in `controller_modify_volume_errors_total` + - Mitigations: No serious mitigation needed because volume would fall back to previous configuration. Can edit PVC to previous VolumeAttributeClass to prevent retry ControllerModifyVolume calls. + - Diagnostics: + - Events on PVC which include the associated [ControllerModifyVolume error](https://github.com/container-storage-interface/spec/blob/master/spec.md#controllermodifyvolume-errors) and message + - external-resizer container logs: Logs similar to "ModifyVolume failed..." (At Log Levels 2&3) + - Testing: Are there any tests for failure mode? If not, describe why. + - There are tests to that validate appropriate events/errors propagate. Otherwise + - Note: See [Modify Design](https://github.com/kubernetes/enhancements/tree/master/keps/sig-storage/3751-volume-attributes-class#modify-pvc) to see flow. + ###### What steps should be taken if SLOs are not being met to determine the problem? +When SLOs are not being met, PVC events should be observed. Debug level logging should be enabled on the appropriate containers (external-resizer for volume modifications, external-provisioner for volume creations, relevant CSI Driver plugin). If problem is not determined from PVC events, operator must look at debug logs to narrow problem to CSI Driver plugin or external sidecars. It may be helpful to see if volume was modified on storage backend. If problem is in CSI Driver plugin, must reach out to CSI Driver maintainers. Storage admin can requested for help finding root cause. ## Implementation History @@ -897,6 +937,16 @@ Major milestones might include: - the version of Kubernetes where the KEP graduated to general availability - when the KEP was retired or superseded --> +- 2023-06-15 SIG Acceptance of KEP and Agreement on proposed Volume Attributes Class design ([link](https://github.com/kubernetes/enhancements/commit/8929cf618f056e447d0b2bed562af3fc134c8cbb)) +- 2023-06-26 Original demo of VolumeAttributeClass proof-of-concept +- 2023-10-31 VolumeAttributesClass API changes merged in kubernetes/kubernetes +- 2023-10-26 Implementation merged in kubernetes-csi/external-provisioner +- 2023-11-09 Implementation merged in kubernetes-csi/external-resizer +- First available release: Alpha in Kubernetes 1.29 + +Implementations in CSI Drivers: +- [AWS EBS CSI Driver](https://github.com/kubernetes-sigs/aws-ebs-csi-driver/blob/a402256d03ab3008f642e70253acb4b41d674af0/pkg/driver/controller.go#L581) + ## Drawbacks diff --git a/keps/sig-storage/3751-volume-attributes-class/kep.yaml b/keps/sig-storage/3751-volume-attributes-class/kep.yaml index 63a7ed32969..59e282c1713 100644 --- a/keps/sig-storage/3751-volume-attributes-class/kep.yaml +++ b/keps/sig-storage/3751-volume-attributes-class/kep.yaml @@ -18,18 +18,18 @@ see-also: replaces: # The target maturity stage in the current dev cycle for this KEP. -stage: alpha +stage: beta # The most recent milestone for which work toward delivery of this KEP has been # done. This can be the current (upcoming) milestone, if it is being actively # worked on. -latest-milestone: "1.30" +latest-milestone: "1.31" # The milestone at which this feature was, or is targeted to be, at each stage. milestone: alpha: "v1.29" - beta: - stable: + beta: "v1.31" + stable: # The following PRR answers are required at alpha release # List the feature gate name and the components for which it must be enabled From 8360d8315c5c369bf2ebf9c462a41b3170022f20 Mon Sep 17 00:00:00 2001 From: Drew Sirenko <68304519+AndrewSirenko@users.noreply.github.com> Date: Mon, 10 Jun 2024 18:19:58 +0000 Subject: [PATCH 2/4] Remove mention of --- keps/sig-storage/3751-volume-attributes-class/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/keps/sig-storage/3751-volume-attributes-class/README.md b/keps/sig-storage/3751-volume-attributes-class/README.md index 59323d0ef06..78379aa437f 100644 --- a/keps/sig-storage/3751-volume-attributes-class/README.md +++ b/keps/sig-storage/3751-volume-attributes-class/README.md @@ -416,7 +416,7 @@ Please see session "Kubernetes API" above. A. Count of bound/unbound PVCs per VolumeAttributesClass similar to [StorageClass](https://github.com/kubernetes/kubernetes/blob/666fc23fe4d6c84b1dde2b8d4ebf75fce466d338/pkg/controller/volume/persistentvolume/metrics/metrics.go#L98). -Today, we loop through all PersistentVolume objects, check if `pv.Status.Phase == v1.VolumeBound` and increment the appropriate `pv.Spec.StorageClassName` bucket. For these new metrics, when the feature flag is enabled, we also increment the appropriate `pv.Spec.VolumeAttributeClassName` if it is not empty. +Prior to this enhancement, we loop through all PersistentVolume objects, check if `pv.Status.Phase == v1.VolumeBound` and increment the appropriate `pv.Spec.StorageClassName` bucket. For these new metrics, when the feature flag is enabled, we also increment the appropriate `pv.Spec.VolumeAttributeClassName` if it is not empty. ``` boundPVCCountDesc = metrics.NewDesc( @@ -863,9 +863,9 @@ Yes, the feature may impact CreateVolume. We will measure this impact during bet ###### Will enabling / using this feature result in non-negligible increase of resource usage (CPU, RAM, disk, IO, ...) in any components? -Using this feature may result in non-negligible increase of resource usage IF customers batch modify many volumes at once and increase default external-resizer `--workers`, `-kube-api-burst`, and `-kube-api-qps` options. +Using this feature may result in non-negligible increase of resource usage IF customers batch modify many volumes at once and CSI Controller Pod has high API Priority. - external-resizer CPU and memory will see a non-negligible increase if users increased the number of concurrent operations via the `--workers` flag. We follow the strategy of sharing that limit between `ControllerExpandVolume` and `ControllerModifyVolume` RPCs, similar to how external-provisioner functions. -- If users increased the default `-kube-api-burst` and `-kube-api-qps` of their external-resizer container, the API-Server may see a spike of CPU when processing these changes +- The API-Server may see a spike of CPU when processing relevant changes. Stress tests will determine increase in resource usage at varying amounts of concurrent volume modifications. From d9012fd804c242088260c2075e8b7bd501a28014 Mon Sep 17 00:00:00 2001 From: Drew Sirenko <68304519+AndrewSirenko@users.noreply.github.com> Date: Tue, 11 Jun 2024 09:25:29 -0400 Subject: [PATCH 3/4] Add e2e k8s-triage link --- keps/sig-storage/3751-volume-attributes-class/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/keps/sig-storage/3751-volume-attributes-class/README.md b/keps/sig-storage/3751-volume-attributes-class/README.md index 78379aa437f..3206e1f116b 100644 --- a/keps/sig-storage/3751-volume-attributes-class/README.md +++ b/keps/sig-storage/3751-volume-attributes-class/README.md @@ -636,7 +636,8 @@ https://storage.googleapis.com/k8s-triage/index.html - The behavior with feature gate and API turned on/off and mix match - The happy path with creating and modifying volume successfully with VolumeAttributesClass - [E2E CSI Test PR](https://github.com/kubernetes/kubernetes/pull/124151/) - - k8s-triage will be linked once test PR merged + - [k8s-triage](https://storage.googleapis.com/k8s-triage/index.html?sig=storage&test=%5C%5BFeature%3AVolumeAttributesClass%5C%5D) + - [Testgrid[(https://testgrid.k8s.io/sig-storage-kubernetes#kind-alpha-features&include-filter-by-regex=%5BFeature%3AVolumeAttributesClass%5D&include-filter-by-regex=%5BFeature%3AVolumeAttributesClass%5D&include-filter-by-regex=%5C%5BFeature%3AVolumeAttributesClass%5C%5D) ##### e2e tests From e62baf209c78738542f4734a881c889785ad88b2 Mon Sep 17 00:00:00 2001 From: Drew Sirenko <68304519+AndrewSirenko@users.noreply.github.com> Date: Wed, 12 Jun 2024 13:38:50 -0400 Subject: [PATCH 4/4] Add small edits --- keps/sig-storage/3751-volume-attributes-class/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/keps/sig-storage/3751-volume-attributes-class/README.md b/keps/sig-storage/3751-volume-attributes-class/README.md index 3206e1f116b..dfb96527e86 100644 --- a/keps/sig-storage/3751-volume-attributes-class/README.md +++ b/keps/sig-storage/3751-volume-attributes-class/README.md @@ -90,7 +90,7 @@ Items marked with (R) are required *prior to targeting to a milestone / release* - [ ] (R) Production readiness review completed - [ ] (R) Production readiness review approved - [X] "Implementation History" section is up-to-date for milestone -- [ ] User-facing documentation has been created in [kubernetes/website], for publication to [kubernetes.io] +- [X] User-facing documentation has been created in [kubernetes/website], for publication to [kubernetes.io] - [ ] Supporting documentation—e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes [kubernetes.io]: https://kubernetes.io/ @@ -679,7 +679,7 @@ We expect no non-infra related flakes in the last month as a GA graduation crite #### Beta -- Beta in 1.31: Since this feature is an extension of the external-resizer/external-provisioner usage flow, we are going to move this to beta with enhanced e2e and test coverage. Test cases are covered in sessions above: ``e2e tests``, ``Integration tests`` etc. +- Beta in 1.31: Since this feature is an extension of the external-resizer/external-provisioner usage flow, we are going to move this to beta with enhanced e2e and test coverage. Test cases are covered in sessions above: ``e2e tests``, ``Integration tests`` etc. Controllers will handle VolumeAttributeClass feature gates being on by default, but beta API itself being disabled on cluster by default. - Involve 3 different CSI drivers to participate in testing - Stress test before GA @@ -795,12 +795,12 @@ These goals will help you determine what you need to measure (SLIs) in the next question. --> -- Ratio of `controller_modify_volume_errors_total`/`controller_modify_volume_total` <= 1% +- Ratio of `controller_modify_volume_errors_total`/`controller_modify_volume_total` <= 1%. (Exclude errors with `UNAVAILABLE` code which indicate some quota has been exhausted.) - CreateVolume `csi_sidecar_operations_seconds_sum` does not increase by more than 5% when feature flags are enabled. ###### What are the SLIs (Service Level Indicators) an operator can use to determine the health of the service? -- [ ] Metrics +- [X] Metrics - Metric name: `controller_modify_volume_total` and `controller_modify_volume_errors_total` - [Optional] Aggregation method: - Components exposing the metric: external-resizer