diff --git a/keps/prod-readiness/sig-storage/3762.yaml b/keps/prod-readiness/sig-storage/3762.yaml new file mode 100644 index 00000000000..3ff82dfa6b4 --- /dev/null +++ b/keps/prod-readiness/sig-storage/3762.yaml @@ -0,0 +1,3 @@ +kep-number: 3762 +alpha: + approver: "@wojtek-t" diff --git a/keps/sig-storage/3762-persistent-volume-last-phase-transition-time/README.md b/keps/sig-storage/3762-persistent-volume-last-phase-transition-time/README.md new file mode 100644 index 00000000000..437d4c742b6 --- /dev/null +++ b/keps/sig-storage/3762-persistent-volume-last-phase-transition-time/README.md @@ -0,0 +1,937 @@ + +# KEP-3762: PersistentVolume last phase transition time + + + + + + +- [Release Signoff Checklist](#release-signoff-checklist) +- [Summary](#summary) +- [Motivation](#motivation) + - [Goals](#goals) + - [Non-Goals](#non-goals) +- [Proposal](#proposal) + - [User Stories (Optional)](#user-stories-optional) + - [Story 1](#story-1) + - [Story 2](#story-2) + - [Notes/Constraints/Caveats (Optional)](#notesconstraintscaveats-optional) + - [Risks and Mitigations](#risks-and-mitigations) +- [Design Details](#design-details) + - [Test Plan](#test-plan) + - [Prerequisite testing updates](#prerequisite-testing-updates) + - [Unit tests](#unit-tests) + - [Integration tests](#integration-tests) + - [e2e tests](#e2e-tests) + - [Graduation Criteria](#graduation-criteria) + - [Alpha](#alpha) + - [Beta](#beta) + - [GA](#ga) + - [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy) + - [Version Skew Strategy](#version-skew-strategy) +- [Production Readiness Review Questionnaire](#production-readiness-review-questionnaire) + - [Feature Enablement and Rollback](#feature-enablement-and-rollback) + - [Rollout, Upgrade and Rollback Planning](#rollout-upgrade-and-rollback-planning) + - [Monitoring Requirements](#monitoring-requirements) + - [Dependencies](#dependencies) + - [Scalability](#scalability) + - [Troubleshooting](#troubleshooting) +- [Implementation History](#implementation-history) +- [Drawbacks](#drawbacks) +- [Alternatives](#alternatives) +- [Infrastructure Needed (Optional)](#infrastructure-needed-optional) + + +## Release Signoff Checklist + + + +Items marked with (R) are required *prior to targeting to a milestone / release*. + +- [ ] (R) Enhancement issue in release milestone, which links to KEP dir in [kubernetes/enhancements] (not the initial KEP PR) +- [ ] (R) KEP approvers have approved the KEP status as `implementable` +- [ ] (R) Design details are appropriately documented +- [ ] (R) Test plan is in place, giving consideration to SIG Architecture and SIG Testing input (including test refactors) + - [ ] e2e Tests for all Beta API Operations (endpoints) + - [ ] (R) Ensure GA e2e tests meet requirements for [Conformance Tests](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md) + - [ ] (R) Minimum Two Week Window for GA e2e tests to prove flake free +- [ ] (R) Graduation criteria is in place + - [ ] (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 +- [ ] 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/ +[kubernetes/enhancements]: https://git.k8s.io/enhancements +[kubernetes/kubernetes]: https://git.k8s.io/kubernetes +[kubernetes/website]: https://git.k8s.io/website + +## Summary + + + +We want to add a new PersistentVolumeStatus field, which would hold a timestamp of when a PersistentVolume last +transitioned to a different phase. + +## Motivation + +Some users have experienced data loss when using `Delete` retain policy and reverted to a safer `Retain` policy. +With `Retain` policy all volumes that are retained and left unclaimed have their phase is set to `Released`. +As the released volumes pile up over time admins want to perform manual cleanup based on the time when the volume was +last used, which is when the volume transitioned to `Released` phase. + +We can approach the solution in a more generic way and record a timestamp of when the volume transitioned to any phase, +not just to `Released` phase. This allows anyone, including our perf tests, to measure time e.g. between a PV `Pending` +and `Bound`. This can be also useful for providing metrics and SLOs. + + + +### Goals + +1) Introduce a new status field in PersistentVolumes. +2) Update the new field with a timestamp every time a volume transitions to a different phase (`pv.Status.Phase`). + + + +### Non-Goals + +1) Implement any form of volume health monitoring. +2) Kubernetes will take any new actions based on the added timestamps in PersistentVolume. + + + +## Proposal + + + +We need to update API server to support the newly proposed field and update PV controller to set a value of the new +timestamp field when a volume transitions to a different phase. Also, if the feature gate is disabled the value must be +re-set to `nil` when updating or creating a volume. + +The value of the field is not intended for use by any other Kubernetes components at this point and should be used only +as a convenience feature for cluster admins. Cluster admins should be able to list and sort PersistentVolumes based on +a timestamp which indicates when the volume transitioned to a different state. + +### User Stories (Optional) + + + +#### Story 1 + +As a cluster admin I want to use `Retain` policy for released volumes, which is safer than `Delete`, and implement a +reliable policy to delete volumes that are `Released` for more than X days. + +#### Story 2 + +As a cluster admin I want to be able to reason about volume deletion, or produce alerts, based on a volume being in +`Pending` phase for more than X hours. + +### Notes/Constraints/Caveats (Optional) + + + +The caveat of this proposal is that admins might not see the effect immediately after enabling/disabling the feature gate. +This is due to how and when the new `LastPhaseTransitionTime` field needs to be added/removed. + +Adding the field to a PV is reasonable only when the PV actually transitions its phase - only at that point we can +capture meaningful timestamp. Trying to do this at any other step than phase transition would capture a timestamp +that would semantically incorrect and misleading. + +Removing the value can be done more freely, but tradeoffs need to be considered. One alternative approach discussed was +removing the field values more aggressively, during each PV sync. As this might introduce performance issues and does +not add much value the removal should be done during PV validation instead. + +### Risks and Mitigations + + + +The new field is purely informative and should not introduce any risk. + +## Design Details + +Changes required for this KEP: + +* kube-apiserver + * extend [PersistentVolumeStatus](https://github.com/kubernetes/api/blob/a26a16a095cab454e928a95c533b8cf1b80aa2ec/core/v1/types.go#L402) type with `LastPhaseTransitionTime` field: + ``` + type PersistentVolumeStatus struct { + ... + // lastPhaseTransitionTime represents a point in time as a timestamp of when a volume last transitioned its phase. + // +optional + LastPhaseTransitionTime string `json:"lastPhaseTransitionTime,omitempty" protobuf:"bytes,4,opt,name=lastPhaseTransitionTime"` + ... + } + ``` + + * validation should check timestamp format + * validation should allow update from `nil` + * validation should allow timestamp update if the previous timestamp is older + * validation should not allow updating to a point in time before the current timestamp + +* kube-controller-manager / PV controller + * update the timestamp whenever PV controller transitions PV to a different phase (`pv.Status.Phase`) + * remove the timestamp in `LastPhaseTransitionTime` field during volume status update when feature gate is disabled + + + +### Test Plan + + + +[x] I/we understand the owners of the involved components may require updates to +existing tests to make this code solid enough prior to committing the changes necessary +to implement this enhancement. + +##### Prerequisite testing updates + +Current e2e test coverage is sufficient: [test/e2e/storage/persistent_volumes.go](https://github.com/kubernetes/kubernetes/blob/ccfac6d3200f63656575819e7b5976c12c3019a6/test/e2e/storage/persistent_volumes.go) + +New e2e tests will be added for the new timestamp feature. + + + +##### Unit tests + + + + + +Changes will be implemented in packages with sufficient unit test coverage. + +For any new or changed code we will add new unit tests. + +- `pkg/controller/volume/persistentvolume`: `2023-01-25` - `79%` +- `pkg/apis/core/validation/`: `2023-01-25` - `82%` + +##### Integration tests + + + +This feature could be covered with integration tests only, however e2e testing provides more value and might help +catch more bugs. Because these two kinds of tests would be almost identical, integration testing is not needed. + +##### e2e tests + + + +We plan to add new e2e tests which should not interfere with any other tests, and so they could run in parallel. + +While the timestamp of volume phase transition will represent an accurate point in time of when it occurred, the tests +will have to consider the time difference between user action that leads to a PV phase transition and the actual volume +phase transition done by the PV controller. +Based on exploratory testing we will define an appropriate time tolerance which will represent maximum time limit for +the volume to transition phase. + +### Graduation Criteria + + + +#### Alpha + +- Feature implemented behind a feature flag +- Unit tests completed and enabled +- Add unit tests covering feature enablement/disablement. +- Initial e2e tests completed and enabled + +#### Beta + +- Allowing time for feedback (at least 2 releases between beta and GA). +- Manually test version skew between the API server and KCM. See the expected + behavior below in Version Skew Strategy. +- Manually test upgrade->downgrade->upgrade path. + +#### GA + +- No users complaining about the new behavior. + +### Upgrade / Downgrade Strategy + + + +No change in cluster upgrade / downgrade process. + +When upgrading the new `LastPhaseTransitionTime` field and its value will be added to a PV when it transitions phase. + +When downgrading from a version that added the new timestamp field PVs we need to make sure that after downgrade the +values of the disabled field are removed. We intend to use create and update persistent volume REST strategy to remove +the values. More specifically each time the strategy invokes `PrepareForCreate` or `PrepareForUpdate` method we set the +value to `nil` if feature gate is disabled. + +This means that enabling and disabling feature gate might not have an immediate effect. See "Notes/Constraints/Caveats" +section for more details. + +### Version Skew Strategy + + + +| API server | KCM | Behavior | +|------------|-----|----------------------------------------------------------------------------------------------------------------------------| +| off | off | Existing Kubernetes behavior. | +| on | off| Existing Kubernetes behavior, only users can set `pvc.Status.LastPhaseTransitionTime`. | +| off | on | PV controller may try to change `pvc.Status.LastPhaseTransitionTime`, which will fail on the API server. | +| on | on | New behavior. + +## Production Readiness Review Questionnaire + + + +### Feature Enablement and Rollback + + + +###### How can this feature be enabled / disabled in a live cluster? + + + +- [X] Feature gate (also fill in values in `kep.yaml`) + - Feature gate name: PersistentVolumeLastPhaseTransitionTime + - Components depending on the feature gate: kube-apiserver, kube-controller-manager +- [ ] Other + - Describe the mechanism: + - Will enabling / disabling the feature require downtime of the control + plane? + - Will enabling / disabling the feature require downtime or reprovisioning + of a node? (Do not assume `Dynamic Kubelet Config` feature is enabled). + +###### Does enabling the feature change any default behavior? + + + +Yes. All PVs will start to contain the new `LastPhaseTransitionTime` field. + +###### Can the feature be disabled once it has been enabled (i.e. can we roll back the enablement)? + + + +Yes. This will result in the timestamp value being eventually set to `nil`. More details in +"Upgrade / Downgrade Strategy" section. + +###### What happens if we reenable the feature if it was previously rolled back? + +No issues expected. There are three cases that can occur for a PV: + + 1. PV did not transition its phase when feature gate was enabled - the `LastPhaseTransitionTime` field was not added + to the PV object so this is the same case as enabling the feature gate for the first time. + 2. PV did transition its phase but the `LastPhaseTransitionTime` *was not* reset to `nil` because the PV was not + validated when the feature was enabled - the timestamp value will be updated on next phase change as if the feature + gate was never disabled. + 2. PV did transition its phase but the `LastPhaseTransitionTime` *was* reset to `nil` because the PV was validated at + least once already while feature was enabled - the timestamp value will be updated on next phase change because + updates from `nil` are allowed. + +See "Upgrade / Downgrade Strategy" and "Notes/Constraints/Caveats" sections for more details. + +###### Are there any tests for feature enablement/disablement? + +Unit tests for enabling and disabling feature gate will be added when transitioning to beta. See "Graduation criteria" +section. + +The tests should focus on verifying correct handling of the new PV field in relation to feature gate state. Correct +handling means the values of the newly added field are added or updated when PV transitions its phase and cleared when +the feature is disabled. + + + +### Rollout, Upgrade and Rollback Planning + + + +###### How can a rollout or rollback fail? Can it impact already running workloads? + + + +###### What specific metrics should inform a rollback? + + + +###### Were upgrade and rollback tested? Was the upgrade->downgrade->upgrade path tested? + + + +###### Is the rollout accompanied by any deprecations and/or removals of features, APIs, fields of API types, flags, etc.? + + + +### Monitoring Requirements + + + +###### How can an operator determine if the feature is in use by workloads? + + + +###### How can someone using this feature know that it is working for their instance? + + + +- [ ] Events + - Event Reason: +- [ ] API .status + - Condition name: + - Other field: +- [ ] Other (treat as last resort) + - Details: + +###### What are the reasonable SLOs (Service Level Objectives) for the enhancement? + + + +###### What are the SLIs (Service Level Indicators) an operator can use to determine the health of the service? + + + +- [ ] Metrics + - Metric name: + - [Optional] Aggregation method: + - Components exposing the metric: +- [ ] Other (treat as last resort) + - Details: + +###### Are there any missing metrics that would be useful to have to improve observability of this feature? + + + +### Dependencies + + + +###### Does this feature depend on any specific services running in the cluster? + + + +### Scalability + + + +###### Will enabling / using this feature result in any new API calls? + + + +###### Will enabling / using this feature result in introducing new API types? + + + +###### Will enabling / using this feature result in any new calls to the cloud provider? + + + +###### Will enabling / using this feature result in increasing size or count of the existing API objects? + + + +###### Will enabling / using this feature result in increasing time taken by any operations covered by existing SLIs/SLOs? + + + +###### Will enabling / using this feature result in non-negligible increase of resource usage (CPU, RAM, disk, IO, ...) in any components? + + + +### Troubleshooting + + + +###### How does this feature react if the API server and/or etcd is unavailable? + +###### What are other known failure modes? + + + +###### What steps should be taken if SLOs are not being met to determine the problem? + +## Implementation History + + + +## Drawbacks + + + +## Alternatives + + + +## Infrastructure Needed (Optional) + + diff --git a/keps/sig-storage/3762-persistent-volume-last-phase-transition-time/kep.yaml b/keps/sig-storage/3762-persistent-volume-last-phase-transition-time/kep.yaml new file mode 100644 index 00000000000..d3254cc81c7 --- /dev/null +++ b/keps/sig-storage/3762-persistent-volume-last-phase-transition-time/kep.yaml @@ -0,0 +1,41 @@ +title: PersistentVolume last phase transition time +kep-number: 3762 +authors: + - "@RomanBednar" +owning-sig: sig-storage +participating-sigs: +status: provisional +creation-date: 2023-01-20 +reviewers: + - "@jsafrane" +approvers: + - TBD + +see-also: +replaces: + +# The target maturity stage in the current dev cycle for this KEP. +stage: alpha + +# 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: "v1.27" + +# The milestone at which this feature was, or is targeted to be, at each stage. +milestone: + alpha: "v1.27" + beta: "v1.28" + stable: "v1.30" + +# The following PRR answers are required at alpha release +# List the feature gate name and the components for which it must be enabled +feature-gates: + - name: PersistentVolumeLastPhaseTransitionTime + components: + - kube-apiserver + - kube-controller-manager +disable-supported: true + +# The following PRR answers are required at beta release +metrics: