Skip to content

Commit

Permalink
KEP-596: address review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
dobsonj committed Feb 2, 2022
1 parent fd3a765 commit 3823209
Showing 1 changed file with 12 additions and 4 deletions.
16 changes: 12 additions & 4 deletions keps/sig-storage/596-csi-inline-volumes/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ Examples where this is used by the Secrets Store CSI driver:
* CSIVolumeSource info persists in CSI json file during mount/unmount: TODO
* Ensure Kubelet skips attach/detach when `CSIDriver.VolumeLifecycleModes = Ephemeral`: TODO
* Ensure Kubelet skips inline logic when `CSIDriver.VolumeLifecycleModes = Persistent` or `CSIDriver.VolumeLifecycleModes is empty`: covered by existing tests
* Add unit tests for feature gate enablement / disablement, similar to this [network policy strategy test](https://github.com/kubernetes/kubernetes/blob/b7c82bb83c1b3933b99fbc5fdcffa59fd6441617/pkg/registry/networking/networkpolicy/strategy_test.go#L246-L281): TODO

#### E2E tests

Expand Down Expand Up @@ -361,10 +362,13 @@ Examples where this is used by the Secrets Store CSI driver:
there should be no effect. Any workloads that need to make use of this feature
would require pod spec updates, along with a CSI driver that supports it.

A rollback (or disabling the feature flag) may impact running workloads,
as any pod specs taking advantage of ephemeral inline volumes would fail
to start when the feature is disabled or the CSIDriver does not support it.
It is recommended to delete any pods using this feature before disabling it.
A rollback (or disabling the feature flag) will not impact already running
workloads, however starting new workloads using this feature (as well as
potentially restarting ones that failed for some reason) would fail.
Additionally, kubelet may fail to fully cleanup after pods that were
running and taking advantage of inline volumes.

*It is highly recommended to delete any pods using this feature before disabling it*.

###### What specific metrics should inform a rollback?

Expand Down Expand Up @@ -494,6 +498,10 @@ CSI NodePublishVolume requests:
since an external CSI driver is responsible for providing the actual volume.
Note that mount time is in the critical path for [pod startup latency](https://github.com/kubernetes/community/blob/1181fb0266a01d1dfd170ff437817eb7de24b624/sig-scalability/slos/pod_startup_latency.md) and the choice to use CSI inline volumes may affect the SLI/SLO, since this is still a type of volume that needs to be mounted.

TODO: Provide a measurements showing the time it takes to start a pod with 5 secrets,
compared to mounting those secrets via a CSI inline volume. This will be driver
dependent, but it would be useful to set some baseline expectations.

###### Will enabling / using this feature result in non-negligible increase of resource usage (CPU, RAM, disk, IO, ...) in any components?

Kubernetes itself should not see any noticeable increase in resource consumption,
Expand Down

0 comments on commit 3823209

Please sign in to comment.