node-feature-discovery
node-feature-discovery copied to clipboard
kubernetes-sigs
nfd-gc: Remove stale NRT objects
Remove stale NRT objects whose creator pod does not exist anymore.
Closes #1586
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: ozhuraki Once this PR has been reviewed and has the lgtm label, please assign marquiz for approval. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Hi @ozhuraki. Thanks for your PR.
I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.
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.
Deploy Preview for kubernetes-sigs-nfd ready!
| Name | Link |
|---|---|
| Latest commit | b5fee2235afbad9dcc7e0473c1780703f5e6fc7f |
| Latest deploy log | https://app.netlify.com/sites/kubernetes-sigs-nfd/deploys/670eb02cb2c76d00083c3cb5 |
| Deploy Preview | https://deploy-preview-1700--kubernetes-sigs-nfd.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
@ozhuraki @ArangoGutierrez one question, if we upgrade NFD-gc on existing cluster with this feature, it would remove all NRTs in the cluster cause none of them, has the owner-pod labels? Is it acceptable? Even if NRTs would be recreated after some time automatically it can lead to some downtime for components leveraging NRTs like topo-aware-scheduler.
/cc @ffromani
@PiotrProkop Thanks for the input. Yes, this would happen.
I updated it so that NRT is garbage collected only when it has label, please take a look.
Codecov Report
Attention: Patch coverage is 60.00000% with 6 lines in your changes are missing coverage. Please review.
Project coverage is 39.90%. Comparing base (
b9770b1) to head (4822109). Report is 14 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #1700 +/- ##
==========================================
+ Coverage 39.84% 39.90% +0.05%
==========================================
Files 80 80
Lines 6841 6854 +13
==========================================
+ Hits 2726 2735 +9
- Misses 3861 3864 +3
- Partials 254 255 +1
| Files | Coverage Δ | |
|---|---|---|
| pkg/nfd-topology-updater/nfd-topology-updater.go | 2.49% <0.00%> (-0.02%) |
:arrow_down: |
| pkg/nfd-gc/nfd-gc.go | 35.94% <69.23%> (+3.09%) |
:arrow_up: |
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
Codecov Report
Attention: Patch coverage is 29.41176% with 12 lines in your changes missing coverage. Please review.
Project coverage is 39.52%. Comparing base (
b9770b1) to head (665abb9). Report is 142 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #1700 +/- ##
==========================================
- Coverage 39.84% 39.52% -0.33%
==========================================
Files 80 80
Lines 6841 7170 +329
==========================================
+ Hits 2726 2834 +108
- Misses 3861 4075 +214
- Partials 254 261 +7
| Files | Coverage Δ | |
|---|---|---|
| pkg/nfd-topology-updater/nfd-topology-updater.go | 2.50% <0.00%> (-0.01%) |
:arrow_down: |
| pkg/nfd-gc/nfd-gc.go | 32.69% <31.25%> (-0.17%) |
:arrow_down: |
@marquiz @PiotrProkop
Thanks for the help here and apologies, missed your comments.
Updated per your suggestion to solve the race and rebased, please take a look
PR needs rebase.
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.