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

Jump to bottom

deprecate RBD plugin from available in-tree drivers #118552

Merged
merged 2 commits into from Jun 20, 2023
Merged

deprecate RBD plugin from available in-tree drivers #118552

merged 2 commits into from Jun 20, 2023

Conversation

humblec
Copy link
Contributor

@humblec humblec commented Jun 8, 2023

Based on
https://groups.google.com/g/kubernetes-sig-storage/c/h5751_B5LQM, the consensus was to start the deprecation in v1.28.

This commit start the deprecation process of RBD plugin from in-tree drivers.

/kind deprecation

ACTION REQUIRED:
RBD volume plugin ( `kubernetes.io/rbd`) has been deprecated in this release and will be removed in a subsequent release. Alternative is to use RBD CSI driver (https://github.com/ceph/ceph-csi/) in your Kubernetes Cluster.

@k8s-ci-robot k8s-ci-robot added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jun 8, 2023
@humblec
Copy link
Contributor Author

humblec commented Jun 8, 2023

/sig storage
/priority important
/triage accepted

/assign @msau42 @xing-yang @jsafrane

@k8s-ci-robot k8s-ci-robot added sig/storage Categorizes an issue or PR as relevant to SIG Storage. triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Jun 8, 2023
@k8s-ci-robot
Copy link
Contributor

@humblec: The label(s) priority/important cannot be applied, because the repository doesn't have them.

In response to this:

/sig storage
/priority important
/triage accepted

/assign @msau42 @xing-yang @jsafrane

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jun 8, 2023
@humblec
Copy link
Contributor Author

humblec commented Jun 8, 2023

/priority important-longterm

@k8s-ci-robot k8s-ci-robot added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jun 8, 2023
@carlory
Copy link
Member

carlory commented Jun 8, 2023

Hi @humblec, there is an existing pr #118301 about RBD plugin, can you help me review it.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 10, 2023
@humblec
Copy link
Contributor Author

humblec commented Jun 13, 2023

Hi @humblec, there is an existing pr #118301 about RBD plugin, can you help me review it.

@carlory, I filed this PR as mentioned here kubernetes/enhancements#4035 (comment), didn't know in between a PR was filed , appreciate if you can notify upfront to avoid duplicate effort and better tracking.

btw, there are more changes need to be added as in this PR compared to your PR.. please let me know how can we proceed , I am fine with tracking it either here or in the referenced one.. I am rebasing this PR regardless. thanks!

@carlory
Copy link
Member

carlory commented Jun 13, 2023

Sorry, It‘s my mistake. Let's continue with this pr.

@carlory carlory mentioned this pull request Jun 13, 2023
Closed
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 13, 2023
@humblec
Copy link
Contributor Author

humblec commented Jun 13, 2023

Sorry, It‘s my mistake. Let's continue with this pr.

nw at all 👍 , thanks for proactively filing it and its my mistake that, I took it for granted and didn't look into existing PR list.. I have also added you as co-author here 👍 ..

carlory
carlory reviewed Jun 13, 2023
View reviewed changes
Copy link
Member

@carlory carlory left a comment

Choose a reason for hiding this comment

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

the persistentVolumeSource field is invisible to the user.

[root@master ~]# kubectl explain pv.spec.persistentVolumeSource.rbd
KIND:       PersistentVolume
VERSION:    v1

error: field "rbd" does not exist

[root@master ~]# kubectl explain pv.spec.rbd
KIND:       PersistentVolume
VERSION:    v1

FIELD: rbd <RBDPersistentVolumeSource>

so we need remove it.

pkg/api/persistentvolume/util.go Outdated Show resolved Hide resolved
pkg/api/persistentvolume/util_test.go Outdated Show resolved Hide resolved
@carlory
Copy link
Member

carlory commented Jun 13, 2023

ditto

https://github.com/kubernetes/kubernetes/blob/f956675e2a3dbc52b65c0189b4c19d257fb62596/pkg/api/persistentvolume/util.go#L83-L97

@humblec
Copy link
Contributor Author

humblec commented Jun 13, 2023

@carlory I have accommodated the review comments, however at least in tests, it is better to be verbose as its not user facing.. regardless, lets see what others think.

@humblec
deprecate RBD plugin from available in-tree drivers
471fd1a 
Based on https://groups.google.com/g/kubernetes-sig-storage/c/h5751_B5LQM, the
consensus was to start the deprecation in v1.28.

This commit start the deprecation process of RBD plugin from in-tree
drivers.

ACTION REQUIRED:
   RBD volume plugin ( `kubernetes.io/rbd`) has been deprecated in this release
   and will be removed in a subsequent release. Alternative is to use RBD CSI driver
   (https://github.com/ceph/ceph-csi/) in your Kubernetes Cluster.

Signed-off-by: Humble Chirammal <humble.devassy@gmail.com>
@humblec
Change the fieldpath in the deprecated plugin response
f54c15d 
Signed-off-by: Humble Chirammal <humble.devassy@gmail.com>
Signed-off-by: carlory <baofa.fan@daocloud.io>
@humblec
Copy link
Contributor Author

humblec commented Jun 19, 2023

@jsafrane @xing-yang @msau42 can you please help to review this PR ?

@jsafrane
Copy link
Member

/lgtm
/label api-review

@k8s-ci-robot k8s-ci-robot added api-review Categorizes an issue or PR as actively needing an API review. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Jun 19, 2023
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 31a76085e729c7059575a5ff1bfde36cd8280ffe

@humblec
Copy link
Contributor Author

humblec commented Jun 20, 2023

cc @thockin @msau42 for review/approval from API side.

@msau42
Copy link
Member

msau42 commented Jun 20, 2023

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: humblec, msau42

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 20, 2023
@k8s-triage-robot
Copy link

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

  • The PR does have any do-not-merge/* labels
  • The PR does not have the needs-ok-to-test label
  • The PR is mergeable (does not have a needs-rebase label)
  • The PR is approved (has cncf-cla: yes, lgtm, approved labels)
  • The PR is failing tests required for merge

You can:

/retest

@k8s-ci-robot k8s-ci-robot merged commit 9d90b76 into kubernetes:master Jun 20, 2023
12 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.28 milestone Jun 20, 2023
@humblec humblec mentioned this pull request Jul 25, 2023
Merged
@mengjiao-liu mengjiao-liu mentioned this pull request Aug 3, 2023
Merged
@humblec humblec mentioned this pull request Aug 11, 2023
Merged
@carlory carlory mentioned this pull request Apr 26, 2024
Merged
@sveinpj sveinpj mentioned this pull request Apr 26, 2024
Open
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carlory carlory carlory left review comments

@sttts sttts Awaiting requested review from sttts

@thockin thockin Awaiting requested review from thockin

Assignees

@xing-yang xing-yang

@jsafrane jsafrane

@msau42 msau42

Labels
api-review Categorizes an issue or PR as actively needing an API review. approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
v1.28
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@humblec @k8s-ci-robot @carlory @jsafrane @msau42 @k8s-triage-robot @xing-yang