Remove PersistentVolumeLabel admission plugin #124505

jsafrane · 2024-04-24T10:44:21Z

What type of PR is this?

/kind bug
/kind regression

What this PR does / why we need it:

Remove useless admission plugin.

It has been deprecated for years.
All in-tree cloud providers were removed, so the admission plugin does not have
any way to get PV labels.
- Since Azure and vSphere cloud providers are removed and this admission still tries to use them, this fixes PersistentVolumeLabel admission blocks PVs on Azure + vSphere #124504
There is a replacement in https://github.com/kubernetes-sigs/cloud-pv-admission-labeler

Action required: Removed admission plugin PersistentVolumeLabel. Please use https://github.com/kubernetes-sigs/cloud-pv-admission-labeler instead if you need a similar functionality.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

jsafrane · 2024-04-24T10:44:42Z

/sig storage

jsafrane · 2024-04-24T11:14:08Z

/priority important-soon
/triage accepted

k8s-triage-robot · 2024-04-24T17:56:47Z

Unknown CLA label state. Rechecking for CLA labels.

Send feedback to sig-contributor-experience at kubernetes/community.

/check-cla
/easycla

dims · 2024-05-03T14:07:53Z

/approve
/lgtm

k8s-ci-robot · 2024-05-03T14:07:59Z

LGTM label has been added.

Git tree hash: 4fbe1bfff8434fcd86b21777462aa3d27d9c83fa

andrewsykim

/hold

Left a comment about deleting the admission plugin instead

andrewsykim · 2024-05-03T14:43:34Z

plugin/pkg/admission/storage/persistentvolume/label/admission.go

@@ -205,18 +203,6 @@ func (l *persistentVolumeLabel) findVolumeLabels(volume *api.PersistentVolume) (
 			return nil, fmt.Errorf("error querying GCE PD volume %s: %v", volume.Spec.GCEPersistentDisk.PDName, err)
 		}
 		return labels, nil
-	case volume.Spec.AzureDisk != nil:


With this PR and #124519, I suggest just removing the entire admission plugin. It has been deprecated for many releases and we published an external alternative https://github.com/kubernetes-sigs/cloud-pv-admission-labeler

I did not know about https://github.com/kubernetes-sigs/cloud-pv-admission-labeler. I agree that removing the admission is the best way forward now.

dims · 2024-05-07T14:34:31Z

need to remove:

kubernetes/hack/make-rules/test-cmd.sh

Line 59 in e798b9c

    
           DISABLE_ADMISSION_PLUGINS="ServiceAccount,PersistentVolumeLabel,DefaultStorageClass,DefaultTolerationSeconds,MutatingAdmissionWebhook,ValidatingAdmissionWebhook,StorageObjectInUseProtection"

dims · 2024-05-08T12:55:48Z

xref: #52617

Remove useless admission plugin. * It has been deprecated for years. * All in-tree cloud providers were removed, so the admission plugin does not have any way to get PV labels. * There is a replacement in https://github.com/kubernetes-sigs/cloud-pv-admission-labeler

k8s-ci-robot · 2024-05-09T10:17:37Z

@jsafrane: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-kubernetes-e2e-kind-kms	`ff402f3`	link	false	`/test pull-kubernetes-e2e-kind-kms`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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-sigs/prow repository. I understand the commands that are listed here.

jsafrane · 2024-05-09T10:42:51Z

hack/make-rules/test-cmd.sh updated.

~~@dims how do I re-generate docs/admin/kube-apiserver.md and similar docs?~~ the admission plugin is already removed from there.

dims · 2024-05-09T11:42:13Z

/retest

dims · 2024-05-09T11:42:33Z

/approve
/lgtm

k8s-ci-robot · 2024-05-09T11:42:38Z

LGTM label has been added.

Git tree hash: 5ff7acbdf0542434bf85a774f7890f96b03b81d1

k8s-ci-robot · 2024-05-09T11:42:51Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, jsafrane

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:

~~hack/OWNERS~~ [dims]
~~pkg/kubeapiserver/OWNERS~~ [dims]
~~plugin/pkg/admission/storage/persistentvolume/label/OWNERS~~ [dims]

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

dims · 2024-05-09T14:48:35Z

/hold cancel

( removing hold as we are now cleaning up per @andrewsykim 's comment Left a comment about deleting the admission plugin instead )

thanks @jsafrane

k8s-ci-robot added sig/storage Categorizes an issue or PR as relevant to SIG Storage. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Apr 24, 2024

k8s-ci-robot requested review from andrewsykim and dims April 24, 2024 10:44

jsafrane mentioned this pull request Apr 24, 2024

PersistentVolumeLabel admission blocks PVs on Azure + vSphere #124504

Closed

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 24, 2024

jsafrane mentioned this pull request Apr 25, 2024

PersistentVolumeLabel admission plugin provides wrong Azure region #124525

Open

jsafrane mentioned this pull request May 3, 2024

Remove gcp in-tree cloud provider and credential providers #124519

Merged

k8s-ci-robot assigned dims May 3, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 3, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 3, 2024

andrewsykim reviewed May 3, 2024

View reviewed changes

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 3, 2024

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels May 7, 2024

jsafrane changed the title ~~Remove support for Azure and vSphere PVs in PersistentVolumeLabel admission~~ Remove PersistentVolumeLabel admission plugin May 7, 2024

k8s-ci-robot added area/apiserver sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels May 7, 2024

jsafrane force-pushed the clean-pvlabeler branch from 5703e1f to ff402f3 Compare May 7, 2024 13:27

jsafrane force-pushed the clean-pvlabeler branch 2 times, most recently from 60c241a to 1152526 Compare May 7, 2024 13:51

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 9, 2024

jsafrane force-pushed the clean-pvlabeler branch from 1152526 to e7a6ed2 Compare May 9, 2024 09:23

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 9, 2024

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 9, 2024

k8s-ci-robot merged commit 3d24b96 into kubernetes:master May 9, 2024
14 checks passed

k8s-ci-robot added this to the v1.31 milestone May 9, 2024

carlory mentioned this pull request May 10, 2024

remove PersistentVolumeLabel admission plugin kubernetes/website#46302

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove PersistentVolumeLabel admission plugin #124505

Remove PersistentVolumeLabel admission plugin #124505

jsafrane commented Apr 24, 2024 •

edited

jsafrane commented Apr 24, 2024

jsafrane commented Apr 24, 2024

k8s-triage-robot commented Apr 24, 2024

dims commented May 3, 2024

k8s-ci-robot commented May 3, 2024

andrewsykim left a comment

andrewsykim May 3, 2024 •

edited

jsafrane May 7, 2024

dims commented May 7, 2024

dims commented May 8, 2024

k8s-ci-robot commented May 9, 2024 •

edited

jsafrane commented May 9, 2024 •

edited

dims commented May 9, 2024

dims commented May 9, 2024

k8s-ci-robot commented May 9, 2024

k8s-ci-robot commented May 9, 2024

dims commented May 9, 2024

Remove PersistentVolumeLabel admission plugin #124505

Remove PersistentVolumeLabel admission plugin #124505

Conversation

jsafrane commented Apr 24, 2024 • edited

What type of PR is this?

What this PR does / why we need it:

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

jsafrane commented Apr 24, 2024

jsafrane commented Apr 24, 2024

k8s-triage-robot commented Apr 24, 2024

dims commented May 3, 2024

k8s-ci-robot commented May 3, 2024

andrewsykim left a comment

Choose a reason for hiding this comment

andrewsykim May 3, 2024 • edited

Choose a reason for hiding this comment

jsafrane May 7, 2024

Choose a reason for hiding this comment

dims commented May 7, 2024

dims commented May 8, 2024

k8s-ci-robot commented May 9, 2024 • edited

jsafrane commented May 9, 2024 • edited

dims commented May 9, 2024

dims commented May 9, 2024

k8s-ci-robot commented May 9, 2024

k8s-ci-robot commented May 9, 2024

dims commented May 9, 2024

jsafrane commented Apr 24, 2024 •

edited

andrewsykim May 3, 2024 •

edited

k8s-ci-robot commented May 9, 2024 •

edited

jsafrane commented May 9, 2024 •

edited