Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Release Signoff Checklist for KEP-1495 Volume Populators #5023

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+44 −25
Open

Update Release Signoff Checklist for KEP-1495 Volume Populators #5023

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions keps/prod-readiness/sig-storage/1495.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@ alpha:
approver: "@deads2k"
beta:
approver: "@deads2k"
stable:
approver: "@deads2k"
59 changes: 38 additions & 21 deletions keps/sig-storage/1495-volume-populators/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
- [Risks and Mitigations](#risks-and-mitigations)
- [Design Details](#design-details)
- [Test Plan](#test-plan)
- [Unit Tests](#unit-tests)
- [e2e Tests](#e2e-tests)
- [Graduation Criteria](#graduation-criteria)
- [Alpha -> Beta Graduation](#alpha---beta-graduation)
- [Beta -> GA Graduation](#beta---ga-graduation)
Expand All @@ -28,6 +30,7 @@
- [Dependencies](#dependencies)
- [Scalability](#scalability)
- [Troubleshooting](#troubleshooting)
- [Drawbacks](#drawbacks)
- [Implementation History](#implementation-history)
- [Alternatives](#alternatives)
- [Validating webhook](#validating-webhook)
Expand All @@ -38,27 +41,25 @@
**ACTION REQUIRED:** In order to merge code into a release, there must be an issue in [kubernetes/enhancements] referencing this KEP and targeting a release milestone **before [Enhancement Freeze](https://github.com/kubernetes/sig-release/tree/master/releases)
of the targeted release**.

For enhancements that make changes to code or processes/procedures in core Kubernetes i.e., [kubernetes/kubernetes], we require the following Release Signoff checklist to be completed.

Check these off as they are completed for the Release Team to track. These checklist items _must_ be updated for the enhancement to be released.

- [ ] kubernetes/enhancements issue in release milestone, which links to KEP (this should be a link to the KEP location in kubernetes/enhancements, not the initial KEP PR)
- [ ] KEP approvers have set the KEP status to `implementable`
- [ ] Design details are appropriately documented
- [ ] Test plan is in place, giving consideration to SIG Architecture and SIG Testing input
- [ ] Graduation criteria is in place
- [X] (R) Enhancement issue in release milestone, which links to KEP dir in [kubernetes/enhancements] (not the initial KEP PR)
- [X] (R) KEP approvers have approved the KEP status as `implementable`
- [X] (R) Design details are appropriately documented
- [X] (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 ([pull-kubernetes-csi-lib-volume-populator dashboard](https://testgrid.k8s.io/sig-storage-csi-other#pull-kubernetes-csi-lib-volume-populator), [pull-kubernetes-csi-volume-data-source-validator dashboard](https://testgrid.k8s.io/sig-storage-csi-other#pull-kubernetes-csi-volume-data-source-validator))
- [ ] (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
- [X] (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)
- [X] (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

**Note:** Any PRs to move a KEP to `implementable` or significant changes once it is marked `implementable` should be approved by each of the KEP approvers. If any of those approvers is no longer appropriate than changes to that list should be approved by the remaining approvers and/or the owning SIG (or SIG-arch for cross cutting KEPs).

**Note:** This checklist is iterative and should be reviewed and updated every time this enhancement is being considered for a milestone.
- [X] User-facing documentation has been created in [kubernetes/website], for publication to [kubernetes.io]
- [X] 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://github.com/kubernetes/enhancements/issues
[kubernetes/kubernetes]: https://github.com/kubernetes/kubernetes
[kubernetes/website]: https://github.com/kubernetes/website
[kubernetes/enhancements]: https://git.k8s.io/enhancements
[kubernetes/kubernetes]: https://git.k8s.io/kubernetes
[kubernetes/website]: https://git.k8s.io/website

## Summary

Expand Down Expand Up @@ -337,6 +338,18 @@ To test the `volume-data-source-validator`, we need to check the following cases
- Creation of a PVC with a CRD `DataSourceRef` that's registered by a
volume populator causes no events.

#### Unit Tests

- [controller tests](https://github.com/kubernetes-csi/lib-volume-populator/blob/master/populator-machinery/controller_test.go)
- [util tests](https://github.com/kubernetes-csi/lib-volume-populator/blob/master/populator-machinery/util_test.go)
- [metrics tests](https://github.com/kubernetes-csi/lib-volume-populator/blob/master/populator-machinery/metrics_test.go)

#### e2e Tests

- [provisioning.go Creating VolumePopulator CR datasource and Provision](https://github.com/kubernetes/kubernetes/blob/master/test/e2e/storage/testsuites/provisioning.go#L367)
- [pull-kubernetes-csi-lib-volume-populator dashboard](https://testgrid.k8s.io/sig-storage-csi-other#pull-kubernetes-csi-lib-volume-populator)
- [pull-kubernetes-csi-volume-data-source-validator dashboard](https://testgrid.k8s.io/sig-storage-csi-other#pull-kubernetes-csi-volume-data-source-validator)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also include a link of the test code?
Looks like both tests failed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reply here: https://github.com/kubernetes/kubernetes/blob/master/test/e2e/storage/testsuites/provisioning.go#L367

And I will update the pointer in the KEP.

Copy link
Contributor Author

@sunnylovestiramisu sunnylovestiramisu Jan 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the test failure hmmm, need to look into it, I tried to do 'make all' also 'make test' locally on the lib-volume-populator it succeeded, also there is no new changes after Jan 2nd, and there was a sucess run on Jan 3rd.

The latest test runs are from dependabot PRs hmm? https://prow.k8s.io/job-history/gs/kubernetes-ci-logs/pr-logs/directory/pull-kubernetes-csi-lib-volume-populator

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I triggered one test to run successfully on the dashboard: https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-csi_lib-volume-populator/185/pull-kubernetes-csi-lib-volume-populator/1879962450263019520

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! I couldn't find the test though. If I search for "VolumeSnapshotDataSource" or "should provision storage with snapshot data source", I can see a number of tests. If I search for "should provision storage with any volume data source", I couldn't find it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I screenshot and highlighted both:

vp-tests


sunnylovestiramisu marked this conversation as resolved.
Show resolved Hide resolved
### Graduation Criteria

#### Alpha -> Beta Graduation
Expand All @@ -357,7 +370,7 @@ To test the `volume-data-source-validator`, we need to check the following cases

- Distributions including data populators as part of their distros (possibly
a backup/restore implementation layered on top)
- Allowing time for feedback
- Feedback issues are addressed, please [GA Blocker list](https://github.com/kubernetes-csi/lib-volume-populator/issues?q=is%3Aissue+%22GA+Blocker%22)

### Upgrade / Downgrade Strategy

Expand Down Expand Up @@ -434,8 +447,6 @@ fields of API types, flags, etc.?**

### Monitoring Requirements

_This section must be completed when targeting beta graduation to a release._

* **How can an operator determine if the feature is in use by workloads?**
Simply look at the data source field of the PVCs. Non-empty data sources with
a Kind other than snapshot and PVC indicate this feature is in use. Also the
Expand Down Expand Up @@ -573,6 +584,12 @@ resource usage (CPU, RAM, disk, IO, ...) in any components?**
is determined, a populator-specific investigation will be needed, starting
from looking at the logs for that populator.

## Drawbacks

`DataSource` and `DataSourceRef` cannot be pointed to different resources.
While this is by design, it does not solve certain use case like restore volume
from VolumeSnapshot, and then do volume population.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand this use case. Can you elaborate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the VP KEP design: If DataSource is set valid AND DataSourceRef is set differently; reject

Which means DataSource and DataSourceRef cannot be different. The use case I described: customer sets snapshot in the DataSource field, creating a volume from the snapshot. And then do volume population via volume populator.

This case is currently not supported.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, the volume population will overwrite the volume created from snapshot?


## Implementation History

- The idea of data populators has been discussed abstractly in SIG-storage
Expand Down
8 changes: 4 additions & 4 deletions keps/sig-storage/1495-volume-populators/kep.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ participating-sigs:
- sig-api-machinery
status: implementable
creation-date: 2019-12-03
last-updated: 2022-01-31
last-updated: 2024-01-16
reviewers:
- "@thockin"
- "@saad-ali"
Expand All @@ -18,12 +18,12 @@ approvers:
- "@saad-ali"
replaces:
- "/keps/sig-storage/20200120-generic-data-populators.md"
stage: beta
latest-milestone: "v1.23"
stage: stable
latest-milestone: "v1.33"
milestone:
alpha: "v1.18"
beta: "v1.24"
stable: "v1.25"
stable: "v1.33"
feature-gates:
- name: AnyVolumeDataSource
components:
Expand Down