Add helm chart for NFD

[adrianchiris]

This commit adds Helm chart for node-feature-discovery

Signed-off-by: Adrian Chiris adrianc@nvidia.com

[k8s-ci-robot]

Hi @adrianchiris. 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/test-infra repository.

[adrianchiris]

Addresses issue #422

[marquiz]

Thanks @adrianchiris for the PR! I'll take a closer look soon

/ok-to-test

[ArangoGutierrez]

Looking good just a couple of suggestions

[adrianchiris]

Hi,
A pair of eyes on this PR is appreciated :)

[marquiz]

Apologies for the delayed review. I had long holidays...

I'm not an expert on helm charts but basically looks good. Just had a few small comments and questions. One thing that I'd like to ask is to update documentation. I.e. a new section in docs/get-started/deployment-and-usage.md describing helm deployment.

In general I think it would be good to keep the raw deployment yaml templates and helm charts as much in sync as possible. I don't know if there's anything we can really do to make to help with this but to just state it somewhere (e.g. comments in all yaml files or smth).

[adrianchiris]

Will update the PR during next week.

Apologies for the delayed review. I had long holidays...

No worries, hope you enjoyed the holidays ! :)

I'm not an expert on helm charts but basically looks good. Just had a few small comments >and questions. One thing that I'd like to ask is to update documentation. I.e. a new section in >docs/get-started/deployment-and-usage.md describing helm deployment.

Will add doc as well. this was actually in my TODO list for this PR. for now ill cover how to deploy with helm from master. as a second step we should look into creating a helm package for each release and setup a helm repository.

In general I think it would be good to keep the raw deployment yaml templates and helm >charts as much in sync as possible. I don't know if there's anything we can really do to make >to help with this but to just state it somewhere (e.g. comments in all yaml files or smth).

This would need to be looked after in PR reviews unfortunately, but ill add a comment in the yaml templates saying that if you change it, helm deployment templates should be changed as well.

[ArangoGutierrez]

Will update the PR during next week.

Apologies for the delayed review. I had long holidays...

No worries, hope you enjoyed the holidays ! :)

I'm not an expert on helm charts but basically looks good. Just had a few small comments >and questions. One thing that I'd like to ask is to update documentation. I.e. a new section in >docs/get-started/deployment-and-usage.md describing helm deployment.

Will add doc as well. this was actually in my TODO list for this PR. for now ill cover how to deploy with helm from master. as a second step we should look into creating a helm package for each release and setup a helm repository.

In general I think it would be good to keep the raw deployment yaml templates and helm >charts as much in sync as possible. I don't know if there's anything we can really do to make >to help with this but to just state it somewhere (e.g. comments in all yaml files or smth).

This would need to be looked after in PR reviews unfortunately, but ill add a comment in the yaml templates saying that if you change it, helm deployment templates should be changed as well.

Awesome, thanks for your contribution, is really appreciated!

[marquiz]

Will add doc as well. this was actually in my TODO list for this PR. for now ill cover how to deploy with helm from master. as a second step we should look into creating a helm package for each release and setup a helm repository.

OK, good

This would need to be looked after in PR reviews unfortunately, but ill add a comment in the yaml templates saying that if you change it, helm deployment templates should be changed as well.

I think this would be enough, thanks

[marquiz]

Thanks for the documentation! Still some minor comments that need to be addressed

[ArangoGutierrez]

/retest

[marquiz]

Thanks @e0ne! Looks like the only open comment is changing (back) the name of the (worker config) configmap to nfd-worker-conf

[marquiz]

Thanks for the updates @e0ne. Still found few minor things to fix. After these are done you could probably squash the PR again and we should be good to go 🤞 😄

[e0ne]

@marquiz could you please help me with CI failure troubleshooting? 'make templates' on my local env doesn't generate any new changes

[ArangoGutierrez]

[eduardo@fedora-ws node-feature-discovery]$ ./scripts/test-infra/verify.sh 
golangci/golangci-lint info checking GitHub for tag 'v1.30.0'
golangci/golangci-lint info found version: 1.30.0 for v1.30.0/linux/amd64
golangci/golangci-lint info installed /home/eduardo/go/bin/golangci-lint
golangci-lint run --timeout 5m0s
[eduardo@fedora-ws node-feature-discovery]$ git branch 
* (HEAD detached at adrianchiris/add-helm-chart)*

/test pull-node-feature-discovery-verify

[marquiz]

@marquiz could you please help me with CI failure troubleshooting? 'make templates' on my local env doesn't generate any new changes

Ach, this must be caused by the latest changes in master. Prow doesn't actually test the revision here, but, this revision merged to master. Rebase on top of master and try make templates again.

[e0ne]

@ArangoGutierrez thank you. I executed it locally and it didn't find any issue. Let's see if retriggered job passes

[ArangoGutierrez]

Yeah @marquiz is right, maybe you need a soft rebase, that should fix it

[e0ne]

@marquiz thanks!

[ArangoGutierrez]

Makes me wonder, why Prow is not setting the needs-rebase label? 🤔

[ArangoGutierrez]

Rebase did the trick

[marquiz]

Makes me wonder, why Prow is not setting the needs-rebase label? 🤔

Because from the git pow, no rebase is needed, merge succeeds just fine. It's just our testing (or verify step) wasn't happy with the merged result. Prow doesn't know the cause behind this failure. Technically, prow could test BOTH versions, of course and suggest a rebase if the merged one fails...

[marquiz]

I don't have anything else to add 😊 Great work @e0ne and @adrianchiris! 🎉 Very good to have helm support for NFD, finally...
/approve

[ArangoGutierrez]

tested on k 1.20+
/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adrianchiris, ArangoGutierrez, 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

[e0ne]

@marquiz @ArangoGutierrez thanks for the guiding me to make this merge happen.

[adrianchiris]

Closes: #422