node-feature-discovery

node-feature-discovery copied to clipboard

This PR implements a new CRD-based communication mechanism between nfd-worker and nfd-master, serving as an API for 3rd party extensions and paving way for ditching gRPC in the future, among other things. The PR is marked as a draft to avoid accidental merging.

The PR defined a new NodeFeature CRD. NFD-Worker (or 3rd party extensions) create/update NodeFeature objects whereas nfd-master (or similarly 3rd party users) consumes them and create node labels accordingly. The PR introduces new config options in nfd-worker and nfd-master to enable the new NodeFeature API as well similar options for disabling gRPC communications. Default behavior of NFD is not changes in any way and NodeFeature support must be explicitly enabled.

Having 3rd party extensions in mind, the new NodeFeature type is namespaced (instead of cluster-scoped like NodeFeatureRule), with the goal of making management of NodeFeature objects easier (no clashes of object names, easy cleanup as objects are deleted with namespaces etc...).

This PR is split into a long list of separate commits, with the goal of making the change easier to review by starting to submit subsets of these as separate PRs. The commits can be basically divided into a few subsets:

1. General refactoring/renaming (everything before add CRD for communicating node features and some commits scattered elsewhere)
2. Define the new NodeFeature CRD
3. Change nfd-worker to create NodeFeature objects
4. Change nfd-master to watch/process NodeFeature objects

An example NodeFeature object:

apiVersion: nfd.k8s-sigs.io/v1alpha1
kind: NodeFeature
metadata:
  labels:
    nfd.node.kubernetes.io/node-name: node-1
  name: node-1
  namespace: default
spec:
  # Features for NodeFeatureRule matching
  features:
    flags:
      feature.domain-a:
        elements:
          feature-x: {}
    attributes:
      feature.domain-b:
        elements:
          feature-y: "foo"
    instances:
      feature.domain-c:
        elements:
        - attributes:
            name: "elem-1"
            vendor: "acme"
  # Labels to be created
  labelsRequest:
    vendor-feature.enabled: "true"
    vendor-setting.value: "100"

NOTES: the CRD contains fields for both "raw features" and request to create labels. The reason is that nfd-worker still handles the creation of "built-in" labels. One alternative could be to have only "raw features" in the CRD and turn all built-in labels into a NodeFeatureRule object but that's probably a separate discussion.

TODO:

• [ ] documentation
• [ ] e2e-tests

Implements #828

Skipping CI for Draft Pull Request. If you want CI signal for your change, please convert it to an actual PR. You can still manually trigger a test run with /test all

[APPROVALNOTIFIER] This PR is APPROVED

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

✅ Deploy Preview for kubernetes-sigs-nfd ready!

Name	Link
🔨 Latest commit	3209c14beaa5c84e392695a8929c25a6dd1498aa
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-nfd/deploys/639a331a149f7d000803693b
😎 Deploy Preview	https://deploy-preview-903--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 settings.

Rebased. A big reduction in number of commits but still quite massive

/assign

[eduardo@fedora node-feature-discovery]$ kubectl -n node-feature-discovery get nodefeatures
NAME           AGE
minikube       2m9s
minikube-m02   2m58s

I say with a minimum set of documentation this PR is a go LGTM

This PR is ready to go out of draft into Ready for review /lgtm

I think this starts to be ready for real review. I'll try to split out meaningful smaller pieces out of this. First one is #984

The most important part of this PR i.e. definining the CRD and the basic implementation of the API is now split into a separate PR i.e. #986

#986 merged, thanks @fmuyassarov and @ArangoGutierrez 😊 Next bits are now submitted in #989, #990 and #991 and after those we're almost done

/retitle docs: document NodeFeature API

Now that all the remaining bits have been split into separate PRs I rebased this and re-scoped to be only about the documentation (which was the last remaining piece).

Ready for review but depends on #991 /hold

#991 was merged /unhold

LGTM label has been added.

Git tree hash: 8f34567e89e4e72e9ce9581e4debfcb6112b50b4

[APPROVALNOTIFIER] This PR is APPROVED

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

@fmuyassarov wanna take a look? /hold

@fmuyassarov wanna take a look?

/hold

Yes, please. Will do it in few hours and if looks good will remove the hold.

@fmuyassarov thanks for the review. Comments addressed, PTAL

Looks good now. /lgtm

LGTM label has been added.

Git tree hash: 884cdf69d4be14864bab3d84720245678c3fa699

/hold cancel