node-feature-discovery

node-feature-discovery copied to clipboard

The idea behind this patch is to make it easier for third party apps to import NFD API.

Feb 27 '24 14:02 ArangoGutierrez

✅ Deploy Preview for kubernetes-sigs-nfd ready!

Name	Link
🔨 Latest commit	3434557d7c5d1ea521d02472a46b088906c4fdc4
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-nfd/deploys/66100c4f9ecb0d0008e7a737
😎 Deploy Preview	https://deploy-preview-1600--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.

Feb 27 '24 14:02 netlify[bot]

Thanks @ArangoGutierrez. Maybe we should put it under api/nfd/v1alpha1?

I think is a bit redundant. but I am always open to suggestions, won't let that stop this PR Milestone v016?

Feb 27 '24 14:02 ArangoGutierrez

Thanks @ArangoGutierrez. Maybe we should put it under api/nfd/v1alpha1?

I have edited the path of the module PTAL

Feb 28 '24 10:02 ArangoGutierrez

After https://github.com/kubernetes-sigs/node-feature-discovery/pull/1605, we can restart the conversation here

Mar 12 '24 11:03 ArangoGutierrez

@marquiz PTAL

Mar 12 '24 12:03 ArangoGutierrez

@marquiz rebased PTAL

Apr 05 '24 14:04 ArangoGutierrez

LGTM label has been added.

Apr 09 '24 08:04 k8s-ci-robot

[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

Apr 09 '24 08:04 k8s-ci-robot

@vsoch this helps towards the goal of serving your use case.

Apr 09 '24 08:04 ArangoGutierrez

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 40.89%. Comparing base (86c88f1) to head (3434557). Report is 6 commits behind head on master.

@@             Coverage Diff             @@
##           master    #1600       +/-   ##
===========================================
+ Coverage   30.45%   40.89%   +10.44%     
===========================================
  Files         102       80       -22     
  Lines        9642     7097     -2545     
===========================================
- Hits         2936     2902       -34     
+ Misses       6435     3920     -2515     
- Partials      271      275        +4     

Apr 09 '24 09:04 codecov[bot]

Thanks @ArangoGutierrez ! This is a step in the right direction, but the api still requires some heavy k8s dependencies:

module sigs.k8s.io/node-feature-discovery/api/nfd

go 1.22

require (
	github.com/gogo/protobuf v1.3.2
	github.com/stretchr/testify v1.8.4
	k8s.io/api v0.29.0
	k8s.io/apimachinery v0.29.0
	k8s.io/client-go v0.29.0
)

require (
	github.com/davecgh/go-spew v1.1.1 // indirect
	github.com/emicklei/go-restful/v3 v3.11.0 // indirect
	github.com/evanphx/json-patch v4.12.0+incompatible // indirect
	github.com/go-logr/logr v1.3.0 // indirect
	github.com/go-openapi/jsonpointer v0.19.6 // indirect
	github.com/go-openapi/jsonreference v0.20.2 // indirect
	github.com/go-openapi/swag v0.22.3 // indirect
	github.com/golang/protobuf v1.5.3 // indirect
	github.com/google/gnostic-models v0.6.8 // indirect
	github.com/google/go-cmp v0.6.0 // indirect
	github.com/google/gofuzz v1.2.0 // indirect
	github.com/google/uuid v1.5.0 // indirect
	github.com/josharian/intern v1.0.0 // indirect
	github.com/json-iterator/go v1.1.12 // indirect
	github.com/mailru/easyjson v0.7.7 // indirect
	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
	github.com/modern-go/reflect2 v1.0.2 // indirect
	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
	github.com/pkg/errors v0.9.1 // indirect
	github.com/pmezard/go-difflib v1.0.0 // indirect
	golang.org/x/net v0.20.0 // indirect
	golang.org/x/oauth2 v0.13.0 // indirect
	golang.org/x/sys v0.16.0 // indirect
	golang.org/x/term v0.16.0 // indirect
	golang.org/x/text v0.14.0 // indirect
	golang.org/x/time v0.5.0 // indirect
	golang.org/x/tools v0.17.0 // indirect
	google.golang.org/appengine v1.6.8 // indirect
	google.golang.org/protobuf v1.32.0 // indirect
	gopkg.in/inf.v0 v0.9.1 // indirect
	gopkg.in/yaml.v2 v2.4.0 // indirect
	gopkg.in/yaml.v3 v3.0.1 // indirect
	k8s.io/klog/v2 v2.110.1 // indirect
	k8s.io/kube-openapi v0.0.0-20231010175941-2dd684a91f00 // indirect
	k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect
	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
	sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
	sigs.k8s.io/yaml v1.3.0 // indirect
)

For comparison, my variant got it down to:

https://github.com/converged-computing/nfd-source/blob/main/go.mod

The next stage of work to clean this up would be to go through each k8s dependency in the API and rethink if it's needed - this is what I already did for my variant. For example, here is what is left outside of generated, go.mod and annotations:

v1alpha1/register.go:	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
v1alpha1/register.go:	"k8s.io/apimachinery/pkg/runtime"
v1alpha1/register.go:	"k8s.io/apimachinery/pkg/runtime/schema"
v1alpha1/register.go:	SchemeGroupVersion = schema.GroupVersion{Group: "nfd.k8s-sigs.io", Version: "v1alpha1"}
v1alpha1/types.go:	corev1 "k8s.io/api/core/v1"
v1alpha1/types.go:	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

Both of those files (types.go and register.go) are part of NFD using the api to register for that type I think could be refactored / moved to be outside of the api module. I don't think we can easily use the API until it doesn't pull in Kubernetes.

Thanks for the work and update - it's a step in the right direction!

Apr 09 '24 17:04 vsoch