KEP-4958: CSI Sidecars All in one#5153
Conversation
mowangdk
commented
Feb 7, 2025
mowangdk
commented
- One-line PR description: To combine the source code of the CSI sidecars in a monorepo.
- Issue link: CSI Sidecars All in one #4958
- Other comments:
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-ci-robot
added
kind/kep
mowangdk
force-pushed
the
feature/add_csi_aio_proposal
branch
from
d6d990f to
6b692e1
Compare
mauriciopoppe
left a comment
mauriciopoppe
left a comment
There was a problem hiding this comment.
I left some comments, it's looking better every time we review it, good job!
mowangdk
force-pushed
the
feature/add_csi_aio_proposal
branch
2 times, most recently
from
04c2962 to
a57520b
Compare
mowangdk
force-pushed
the
feature/add_csi_aio_proposal
branch
2 times, most recently
from
500c81f to
36ea97f
Compare
|
/retest |
mowangdk
changed the title
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
mowangdk
force-pushed
the
feature/add_csi_aio_proposal
branch
from
46709b6 to
69efda3
Compare
mowangdk
force-pushed
the
feature/add_csi_aio_proposal
branch
from
69efda3 to
3b0596f
Compare
mowangdk
force-pushed
the
feature/add_csi_aio_proposal
branch
from
3b0596f to
9f5da57
Compare
msau42
left a comment
msau42
left a comment
There was a problem hiding this comment.
Overall this looks good to me. Thank you for for detailing a comprehensive migration plan.
| --> | ||
|
|
||
| - [ ] Metrics | ||
| - Metric name: `plugin_execution_duration_seconds{plugin="VolumeBinding",extension_point="Score"}` |
There was a problem hiding this comment.
This looks like a scheduler metric, not the metrics we use in the sidecars
There was a problem hiding this comment.
This section has been updated and was mistakenly copied earlier. Apologies. 🙇♂️
|
@jsafrane Hi Jan~, Could you review this KEP for the upcoming 1.35 release |
mowangdk
force-pushed
the
feature/add_csi_aio_proposal
branch
from
95024c0 to
f7c4f95
Compare
|
/lgtm I think the KEP is overly verbose, but that does not make it unacceptable. |
k8s-ci-robot
added
the
lgtm
|
|
||
| # The milestone at which this feature was, or is targeted to be, at each stage. | ||
| milestone: | ||
| alpha: "v1.34" |
There was a problem hiding this comment.
I believe this still stands.
There was a problem hiding this comment.
Same for the latest-mileston entry above.
soltysh
left a comment
soltysh
left a comment
There was a problem hiding this comment.
/approve
the PRR section
in general the most important bit that I'll be interested in is the migration, and how we can ensure the migration is smooth and as painless as humanly possible.
|
|
||
| # The milestone at which this feature was, or is targeted to be, at each stage. | ||
| milestone: | ||
| alpha: "v1.34" |
There was a problem hiding this comment.
I believe this still stands.
|
|
||
| # The milestone at which this feature was, or is targeted to be, at each stage. | ||
| milestone: | ||
| alpha: "v1.34" |
There was a problem hiding this comment.
Same for the latest-mileston entry above.
|
|
||
| ##### RBAC policy | ||
|
|
||
| We designed the AIO monorepo's RBAC policy to mirror that of individual repos, where each controller maintains its own policy. Driver maintainers should apply proper RBAC when enabling specific controllers in AIO |
There was a problem hiding this comment.
This also matches what kcm has, where each controller has very narrow RBAC rules that explicitly allow it to performed desired actions, and nothing more.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jsafrane, mowangdk, soltysh The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
7 participants
