Control post-delete hook

[alekseysen]

There are situations where the labels generated by the NFD need to be preserved even after its removal.

One such use case arises when NFD is installed as part of another project. By design, uninstalling the parent project does not remove all its components. However, the remaining components still rely on the labels created by NFD. If NFD is removed, these labels will be deleted, causing dependent components to fail.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: alekseysen (8506bd7)

[k8s-ci-robot]

Welcome @alekseysen!

It looks like this is your first PR to kubernetes-sigs/node-feature-discovery 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/node-feature-discovery has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @alekseysen. 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.

[netlify]

✅ Deploy Preview for kubernetes-sigs-nfd ready!

Name	Link
🔨 Latest commit	8506bd7
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-nfd/deploys/67e57046aba1420008f67424
😎 Deploy Preview	https://deploy-preview-2076--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.

[marquiz]

@alekseysen I didn't fully understand the description. How/why does NFD get uninstalled? Is the problem that uninstalling the parent project also removes NFD or is NFD removed manually in your case?

[Copilot]

Pull Request Overview

This PR implements a new configuration flag ("cleanupLabels") to control whether labels generated by NFD are preserved after removal, which is key to preventing failures in dependent components.

• Added the cleanupLabels flag to the Helm values file.
• Wrapped parts of the post-delete job template in a conditional block based on the new flag.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
deployment/helm/node-feature-discovery/values.yaml	Added a new configuration flag to enable cleanup of labels.
deployment/helm/node-feature-discovery/templates/post-delete-job.yaml	Wrapped resource creation in a conditional block to honor the cleanupLabels setting.

Comments suppressed due to low confidence (2)

deployment/helm/node-feature-discovery/values.yaml:18

• [nitpick] Consider adding an inline comment to clarify the purpose and use case of the 'cleanupLabels' flag for future maintainers.

+cleanupLabels: true

deployment/helm/node-feature-discovery/templates/post-delete-job.yaml:1

• [nitpick] Consider adding a brief comment above the conditional block to clarify why the block is being conditionally rendered based on 'cleanupLabels'.

+{{- if .Values.cleanupLabels }}

[alekseysen]

hi @marquiz,

we've got following use-case: NFD chart is deployed as a sub-chart from our project. Obviously, we've got some workloads (e.g. deployments. daemonsets) which require NFD labels to be scheduled on nodes. These workloads aren't deployed as a part of helm chart.
In a case if our helm chart is uninstalled helm deletes NFD too. This procedure can be done as a part of upgrade/maintenance procedure.

To maintain system integrity and ensure continued functionality, there must be an option that allows any modifications made by NFD to be preserved upon its removal. These changes are essential for system stability, compatibility, and optimized performance, preventing potential disruptions or regressions. Removing these modifications could lead to broken dependencies, security vulnerabilities, or degraded system performance. By retaining these enhancements, organizations can safeguard their infrastructure while ensuring seamless transitions and future adaptability.

[marquiz]

Thank you @alekseysen for the detailed description. I think this change is good, especially as it's behind an option.

We'd also need to document the new value in docs/deployment/helm.md

/ok-to-test

[marquiz]

nitpick about the naming. It cleans up other stuff too (i.e. annotations, extended resources and taints), so I'd suggest something like postDeleteCleanup or smth. WDYT @alekseysen

[marquiz]

Thanks @alekseysen looks good to me.
/assign @ArangoGutierrez

[Copilot]

Pull Request Overview

This PR introduces a new Helm flag, postDeleteCleanup, to control whether the post-delete hook should execute in order to preserve NFD-generated labels after uninstall.

• Adds postDeleteCleanup configuration in the Helm documentation and values file.
• Implements a conditional in the post-delete job template to gate resource creation based on the flag.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
docs/deployment/helm.md	Adds documentation for the new postDeleteCleanup parameter.
deployment/helm/node-feature-discovery/values.yaml	Introduces the postDeleteCleanup flag with a default set to true.
deployment/helm/node-feature-discovery/templates/post-delete-job.yaml	Wraps the post-delete job resource definition with a conditional check for postDeleteCleanup.

Comments suppressed due to low confidence (1)

deployment/helm/node-feature-discovery/templates/post-delete-job.yaml:1

• Ensure that this wrapping if block correctly encapsulates the entire resource definition without interfering with any nested conditionals. Verify that the corresponding '{{- end }}' at the bottom properly closes the newly added block and does not conflict with existing template logic.

{{- if .Values.postDeleteCleanup }}

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alekseysen, marquiz

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:

• OWNERS [marquiz]

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

[marquiz]

@ArangoGutierrez PTAL

[fmuyassarov]

/lgtm
/hold
to give time for @ArangoGutierrez to have a look as well.

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 1fdab651e0d46b7820d9f0e0bb15a19e4e8a5a54

[marquiz]

@ArangoGutierrez any comments?