Skip to content

Process PV finalizer for migrated PV when CSIMigration turned off#294

Merged
k8s-ci-robot merged 1 commit intokubernetes-csi:masterfrom
Jiawei0227:master
Mar 18, 2021
Merged

Process PV finalizer for migrated PV when CSIMigration turned off#294
k8s-ci-robot merged 1 commit intokubernetes-csi:masterfrom
Jiawei0227:master

Conversation

@Jiawei0227
Copy link
Copy Markdown
Contributor

@Jiawei0227 Jiawei0227 commented Mar 12, 2021

What type of PR is this?
/kind bug

What this PR does / why we need it:
This PR removes PV external-attacher finalizer when CSIMigration is turned off. Without this change, the in-tree PV controller is not able to delete the PV object when there is no CSI driver in the cluster.

Which issue(s) this PR fixes:

Partially Fixes # kubernetes-csi/external-provisioner#576

Special notes for your reviewer:
This does not fix the issue if the corresponding CSI driver is uninstalled when CSI Migration is turned off. This fix is based on the assumption that CSI driver will still be around for at least a while to remove the finalizer.

Does this PR introduce a user-facing change?:

Fix a bug where external-attacher finalizer not get lifted for migrated PV when CSIMigration is turned off.

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 12, 2021
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 12, 2021
@Jiawei0227
Copy link
Copy Markdown
Contributor Author

/cc @msau42
/assign @jsafrane

@Jiawei0227
Copy link
Copy Markdown
Contributor Author

/cc @msau42

ignore = false
if ann := pv.Annotations; ann != nil {
if migratedToDriver := ann[annMigratedTo]; migratedToDriver == h.attacherName {
ignore = true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would be perhaps better to log a specific message + call pvQueue.Forget and return here. The message logged below is about deletion timestamp.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. I think the deletion timestamp is related. I added a log to identify the scenario that we need to remove the anno

@jsafrane
Copy link
Copy Markdown
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 18, 2021
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

Details 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 Mar 18, 2021
@k8s-ci-robot k8s-ci-robot merged commit 31ad351 into kubernetes-csi:master Mar 18, 2021
andyzhangx added a commit to andyzhangx/external-attacher that referenced this pull request Apr 17, 2026
90925279 Merge pull request kubernetes-csi#298 from torredil/trivy-schedule-trigger
2c9aed3e Merge pull request kubernetes-csi#299 from andyzhangx/patch-12
507ea1e9 chore: fix broken gcb-docker-gcloud image
ab1da0e9 Run Trivy scan on schedule instead of pull requests
de06a09a Merge pull request kubernetes-csi#297 from andyzhangx/patch-11
fc719f31 fix: Update Go version from 1.25.8 to 1.25.9
c24a730e Merge pull request kubernetes-csi#296 from jsafrane/pin-github-actions-sha
59b456b8 fix: pin github action to exact SHA
061f6eed Merge pull request kubernetes-csi#295 from kubernetes-csi/security/update-trivy-action-v0.35.0
6c16f30b security: Update trivy-action to v0.35.0
119a53c3 Merge pull request kubernetes-csi#294 from andyzhangx/patch-10
7c9aa9ba fix: upgrade to go1.25.7 to fix CVE-2026-25679

git-subtree-dir: release-tools
git-subtree-split: 909252797a11793b90f81c1aff9d181b4eebd363
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants