feat(garbage-collector): garbage-collector to remove stale NFS resouces by mynktl · Pull Request #80 · openebs-archive/dynamic-nfs-provisioner

mynktl · 2021-07-28T13:31:36Z

Signed-off-by: mayank mayank.patel@mayadata.io

Why is this PR required? What issue does it fix?:
This PR fixes #79.

What this PR does?:

This PR adds garbage collector to remove stale nfs resources. Garbage collector runs at an interval of 5 minutes and checks the PVC(backend PVC) created by the nfs-provisioner in the nfs-server namespace. Garbage collector process the PVC from nfs-server namespace having a label "openebs.io/cas-type=nfs-kernel".
To map the backend PVC to NFS PVC(created by a user to provision NFS PV), provisioner adds the following labels to the backend PVC.
- nfs.openebs.io/nfs-pvc-name:
- nfs.openebs.io/nfs-pvc-namespace:
- nfs.openebs.io/nfs-pvc-uid:
If backend PVC is missing any label from the above list, then the garbage collector will not process that backend PVC.

If relevant NFS PVC(nfs-pvc-ns/nfs-pvc-name) exists in cluster and having UID value or, garbage collector couldn't find
the relevant NFS PVC due to network/permission issues, then garbage collector will not perform clean-up for that backend PVC.

If relevant NFS PVC doesn't exist for the backend PVC then the garbage collector will check for the relevant NFS PV resource to process the backend PVC.
If NFS PV resource doesn't exist then the garbage collector will remove the resources created for NFS PV, i.e Service, Deployment, backend PVC.

If NFS PV exists for the backend PVC then the garbage collector will not remove the resources for that PV.
This PR also modifies the NFS-Provisioner provisioning behavior. While provisioning, the provisioner will wait for 1 minute to bound backend PVC(created by provisioner for NFS PV). If backend PVC is not bound within 1 minute period then provisioner will return an error timed out waiting for PVC{ns/name} to bound.

Does this PR require any upgrade changes?: No

If the changes in this PR are manually verified, list down the scenarios covered::

Any additional information for your reviewer? :
Mention if this PR is part of any design or a continuation of previous PRs

Checklist:

Fixes nfs-volume deletion is not idempotent leading to stale nfs server resources #79
PR Title follows the convention of <type>(<scope>): <subject>
Has the change log section been updated?
Commit has unit tests
Commit has integration tests
(Optional) Are upgrade changes included in this PR? If not, mention the issue/PR to track:
(Optional) If documentation changes are required, which issue on https://github.com/openebs/openebs-docs is used to track them:

…ces. Garbage collector runs at an interval of 5 minutes and checks the PVC(backend PVC) created by the nfs-provisioner in the nfs-server namespace. Garbage collector process the PVC from nfs-server namespace having a label "openebs.io/cas-type=nfs-kernel". To map the backend PVC to NFS PVC(created by a user to provision NFS PV), provisioner adds the following labels to the backend PVC. - nfs.openebs.io/nfs-pvc-name: <nfs-pvc-name> - nfs.openebs.io/nfs-pvc-namespace: <nfs-pvc-ns> - nfs.openebs.io/nfs-pvc-uid: <nfs-pvc-uid> If backend PVC is missing any label from the above list, then the garbage collector will not process that backend PVC. If relevant NFS PVC(nfs-pvc-ns/nfs-pvc-name) exists in cluster and having UID value <nfs-pvc-uid> or, garbage collector couldn't find the relevant NFS PVC due to network/permission issues, then garbage collector will not perform clean-up for that backend PVC. If relevant NFS PVC doesn't exist for the backend PVC then the garbage collector will check for the relevant NFS PV resource to process the backend PVC. If NFS PV resource doesn't exist then the garbage collector will remove the resources created for NFS PV, i.e Service, Deployment, backend PVC. If NFS PV exists for the backend PVC then the garbage collector will not remove the resources for that PV. -- This commit also modifies the NFS-Provisioner provisioning behavior. While provisioning, the provisioner will wait for 1 minute to bound backend PVC(created by provisioner for NFS PV). If backend PVC is not bound within 1 minute period then provisioner will return an error "timed out waiting for PVC{ns/name} to bound". Signed-off-by: mayank <mayank.patel@mayadata.io>

Signed-off-by: mayank <mayank.patel@mayadata.io>

codecov-commenter · 2021-07-28T14:55:39Z

Codecov Report

Merging #80 (617b550) into develop (d65ac1f) will increase coverage by 1.79%.
The diff coverage is 77.69%.

@@             Coverage Diff             @@
##           develop      #80      +/-   ##
===========================================
+ Coverage    45.19%   46.99%   +1.79%     
===========================================
  Files           27       29       +2     
  Lines         2239     2349     +110     
===========================================
+ Hits          1012     1104      +92     
- Misses        1157     1167      +10     
- Partials        70       78       +8

Impacted Files	Coverage Δ
provisioner/provisioner.go	`0.00% <0.00%> (ø)`
provisioner/provisioner_kernel_nfs_server.go	`0.00% <0.00%> (ø)`
provisioner/garbage_collector.go	`80.00% <80.00%> (ø)`
provisioner/helper_kernel_nfs_server.go	`80.07% <86.36%> (+3.96%)`	⬆️
provisioner/pvc_tracker.go	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d65ac1f...617b550. Read the comment docs.

- TEST NFS PROVISIONER WITH INVALID BACKEND SC Old behaviour: - Create NFS PVC with NFS SC having invalid Backend SC - Wait for NFS PVC to bound - Check nfs-server pod should be in pending state - Check backend-pvc in pending state New behaviour: - Create NFS PVC with NFS SC having invalid Backend SC - Check NFS PVC is in pending state Signed-off-by: mayank <mayank.patel@mayadata.io>

Signed-off-by: mayank <mayank.patel@mayadata.io>

mittachaitu

Provided few comments... rest of changes looks good to me

provisioner/garbage_collector.go

provisioner/garbage_collector_test.go

provisioner/helper_kernel_nfs_server.go

mittachaitu · 2021-07-30T12:05:10Z

tests/provisioner_with_invalid_backend_sc.go

-	When("verifying nfs-server state", func() {
-		It("should have nfs-server in pending state", func() {


Q: Even with the changes nfs-server deployment will be created right? Why do we need to remove this?

Due to a timing issue. The same behavior is added in TEST GARBAGE COLLECTION OF NFS RESOURCES.

tests/provisioner_with_invalid_backend_sc.go

Signed-off-by: mayank <mayank.patel@mayadata.io>

mittachaitu

changes are good

mayank added 3 commits July 28, 2021 18:57

Adding unit test for garbage collector

e0020ed

Signed-off-by: mayank <mayank.patel@mayadata.io>

updating go module files

b4854eb

Signed-off-by: mayank <mayank.patel@mayadata.io>

mynktl force-pushed the fix-79 branch from a18c339 to b4854eb Compare July 28, 2021 14:51

mayank added 3 commits July 29, 2021 14:05

test(bdd): test to verify garbage collector remove the stale resources

cc121f8

Signed-off-by: mayank <mayank.patel@mayadata.io>

update garbage collector to execute once provisioner starts

5adc616

Signed-off-by: mayank <mayank.patel@mayadata.io>

mynktl marked this pull request as ready for review July 29, 2021 14:21

mynktl requested review from kmova and mittachaitu July 29, 2021 14:21

mittachaitu reviewed Jul 30, 2021

View reviewed changes

mynktl force-pushed the fix-79 branch from 2b2d90c to 5b8b98b Compare August 2, 2021 14:04

Addressing review comments

617b550

Signed-off-by: mayank <mayank.patel@mayadata.io>

mynktl force-pushed the fix-79 branch from 5b8b98b to 617b550 Compare August 3, 2021 02:29

mittachaitu approved these changes Aug 3, 2021

View reviewed changes

kmova approved these changes Aug 9, 2021

View reviewed changes

kmova merged commit 185189b into openebs-archive:develop Aug 9, 2021

celiawa mentioned this pull request Aug 30, 2022

garbage collector not work as expected. #143

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(garbage-collector): garbage-collector to remove stale NFS resouces#80

feat(garbage-collector): garbage-collector to remove stale NFS resouces#80
kmova merged 7 commits intoopenebs-archive:developfrom
mynktl:fix-79

mynktl commented Jul 28, 2021 •

edited

Loading

Uh oh!

codecov-commenter commented Jul 28, 2021 •

edited

Loading

Uh oh!

mittachaitu left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mittachaitu Jul 30, 2021 •

edited

Loading

Uh oh!

mynktl Aug 2, 2021

Uh oh!

Uh oh!

mittachaitu left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		When("verifying nfs-server state", func() {
		It("should have nfs-server in pending state", func() {

Conversation

mynktl commented Jul 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Jul 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mittachaitu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mittachaitu Jul 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mynktl Aug 2, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mittachaitu left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mynktl commented Jul 28, 2021 •

edited

Loading

codecov-commenter commented Jul 28, 2021 •

edited

Loading

mittachaitu Jul 30, 2021 •

edited

Loading