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.

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

Add provisioned-by annotations to PVs created by CreateVolumeFromSnapshot kanister function #2705

Merged
merged 7 commits into from
Mar 8, 2024

Conversation

viveksinghggits
Copy link
Contributor

Change Overview

If PVs and PVCs are created by kanister using provider based provisioners and proper annotations are not set, when we delete the PVC, respective PV gets to Failed state and is not cleaned up. Because of that the volume in storage provider is not deleted. This happens because of the migration of provider based provisioner's migration to CSI based provisioners. And if proper annotation is not added provider based PVs are not deleted because CSI would not know what to do.

This PR makes sure that when kanister creates the PV in CreateVolumeFromSnapshot kanister function, we add proper annotation so that when PVCs are deleted, PVs are released properly and deleted.

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • 🐛 Bugfix
  • 🌻 Feature
  • 🗺️ Documentation
  • 🤖 Test

Issues

  • fixes #issue-number

Test Plan

/kanister/pkg/function (provby-pv-annotations*) » go test -check.f "CreateVolumeFromSnapshotTestSuite.TestAddPVProvisionedByAnnotation"
OK: 1 passed
PASS
ok  	github.com/kanisterio/kanister/pkg/function	0.671s
  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

@infraq infraq added this to In Progress in Kanister Feb 28, 2024
@ewhamilton
Copy link
Contributor

LGTM. I'd approve but I don't recall whether that triggers merge on kanister or not. Your option on my feedback. Otherwise I approve and ask Pavan for +1. (I'm AFK all weekend).

Kanister automation moved this from In Progress to Reviewer approved Mar 6, 2024
@pavannd1
Copy link
Contributor

pavannd1 commented Mar 8, 2024

@viveksinghggits are we waiting on any responses for this or is it ready to merge?

@viveksinghggits
Copy link
Contributor Author

@viveksinghggits are we waiting on any responses for this or is it ready to merge?

this is not ready to merge. there were some other bugs that I figured out while working on this and those are being merged as part of
#2732
#2731

once those are merged. We will be able to merge this.

@viveksinghggits
Copy link
Contributor Author

As soon as they are are merged I will add kueue to this.

@mergify mergify bot merged commit 2b86def into master Mar 8, 2024
14 checks passed
@mergify mergify bot deleted the provby-pv-annotations branch March 8, 2024 20:33
Kanister automation moved this from Reviewer approved to Done Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants