From 30866610e0e37b5f9c73002abdf0af35b54a4522 Mon Sep 17 00:00:00 2001 From: Fred Rolland Date: Sun, 19 May 2024 14:52:38 +0300 Subject: [PATCH] chore: Remove unused attributes code Signed-off-by: Fred Rolland --- pkg/nodeinfo/attributes.go | 75 ------------------------ pkg/nodeinfo/attributes_test.go | 96 ------------------------------- pkg/nodeinfo/mocks/Provider.go | 92 ----------------------------- pkg/nodeinfo/node_info.go | 14 ----- pkg/nodeinfo/node_info_test.go | 69 ---------------------- pkg/state/dummy_provider.go | 9 --- pkg/state/state_ipoib_cni_test.go | 2 +- pkg/state/state_sriov_dp_test.go | 2 +- 8 files changed, 2 insertions(+), 357 deletions(-) delete mode 100644 pkg/nodeinfo/attributes_test.go delete mode 100644 pkg/nodeinfo/mocks/Provider.go diff --git a/pkg/nodeinfo/attributes.go b/pkg/nodeinfo/attributes.go index 8c937ca9..2bbd8b7d 100644 --- a/pkg/nodeinfo/attributes.go +++ b/pkg/nodeinfo/attributes.go @@ -17,11 +17,7 @@ limitations under the License. package nodeinfo import ( - "github.com/pkg/errors" - corev1 "k8s.io/api/core/v1" logf "sigs.k8s.io/controller-runtime/pkg/log" - - "github.com/Mellanox/network-operator/pkg/consts" ) var log = logf.Log.WithName("nodeinfo") @@ -39,74 +35,3 @@ const ( NodeLabelCudaVersionMajor = "nvidia.com/cuda.driver.major" NodeLabelOSTreeVersion = "feature.node.kubernetes.io/system-os_release.OSTREE_VERSION" ) - -// AttributeType categorizes Attributes of the host. -type AttributeType int - -// Attribute type Enum, add new types before Last and update the mapping below -const ( - // required attrs - AttrTypeHostname = iota - AttrTypeCPUArch - AttrTypeOSName - AttrTypeOSVer - // optional attrs - AttrTypeCudaVersionMajor - AttrTypeOSTreeVersion - - OptionalAttrsStart = AttrTypeCudaVersionMajor -) - -var attrToLabel = []string{ - // AttrTypeHostname - NodeLabelHostname, - // AttrTypeCPUArch - NodeLabelCPUArch, - // AttrTypeOSName - NodeLabelOSName, - // AttrTypeOSVer - NodeLabelOSVer, - // AttrTypeCudaVersionMajor - NodeLabelCudaVersionMajor, - // AttrTypeOSTreeVersion - NodeLabelOSTreeVersion, -} - -// NodeAttributes provides attributes of a specific node -type NodeAttributes struct { - // Node Name - Name string - // Node Attributes - Attributes map[AttributeType]string -} - -// fromLabel adds a new attribute of type attrT to NodeAttributes by extracting value of selectedLabel -func (a *NodeAttributes) fromLabel(attrT AttributeType, nodeLabels map[string]string, selectedLabel string) error { - attrVal, ok := nodeLabels[selectedLabel] - if !ok { - return errors.Errorf("cannot create node attribute, missing label: %s", selectedLabel) - } - - // Note: attrVal may be empty, this could indicate a binary attribute which relies on key existence - a.Attributes[attrT] = attrVal - return nil -} - -// newNodeAttributes creates a new NodeAttributes -func newNodeAttributes(node *corev1.Node) NodeAttributes { - attr := NodeAttributes{ - Name: node.GetName(), - Attributes: make(map[AttributeType]string), - } - var err error - - nLabels := node.GetLabels() - for attrType, label := range attrToLabel { - err = attr.fromLabel(AttributeType(attrType), nLabels, label) - if err != nil && attrType < OptionalAttrsStart { - log.V(consts.LogLevelWarning).Info("Cannot create NodeAttribute", - "attribute", attrType, "error:", err.Error()) - } - } - return attr -} diff --git a/pkg/nodeinfo/attributes_test.go b/pkg/nodeinfo/attributes_test.go deleted file mode 100644 index a6034033..00000000 --- a/pkg/nodeinfo/attributes_test.go +++ /dev/null @@ -1,96 +0,0 @@ -/* -Copyright 2020 NVIDIA - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package nodeinfo - -import ( - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" -) - -var _ = Describe("NodeAttributes tests", func() { - var testNode corev1.Node - - JustBeforeEach(func() { - testNode = corev1.Node{} - testNode.Kind = "Node" - testNode.Name = "test-node" - testNode.Labels = make(map[string]string) - }) - - Context("Create NodeAttributes from node with all relevant labels", func() { - It("Should return NodeAttributes with all attributes", func() { - testNode.Labels[NodeLabelCPUArch] = "amd64" - testNode.Labels[NodeLabelHostname] = "test-host" - testNode.Labels[NodeLabelKernelVerFull] = "5.4.0-generic" - testNode.Labels[NodeLabelOSName] = "ubuntu" - testNode.Labels[NodeLabelOSVer] = "20.04" - attr := newNodeAttributes(&testNode) - - Expect(attr.Name).To(Equal("test-node")) - Expect(attr.Attributes[AttrTypeHostname]).To(Equal(testNode.Labels[NodeLabelHostname])) - Expect(attr.Attributes[AttrTypeOSName]).To(Equal(testNode.Labels[NodeLabelOSName])) - Expect(attr.Attributes[AttrTypeOSVer]).To(Equal(testNode.Labels[NodeLabelOSVer])) - Expect(attr.Attributes[AttrTypeCPUArch]).To(Equal(testNode.Labels[NodeLabelCPUArch])) - }) - }) - - Context("Create NodeAttributes from node with some relevant labels", func() { - It("Should return NodeAttributes with some attributes", func() { - testNode.Labels[NodeLabelHostname] = "test-host" - testNode.Labels[NodeLabelOSName] = "ubuntu" - testNode.Labels[NodeLabelOSVer] = "20.04" - attr := newNodeAttributes(&testNode) - - var exist bool - _, exist = attr.Attributes[AttrTypeHostname] - Expect(exist).To(BeTrue()) - _, exist = attr.Attributes[AttrTypeOSName] - Expect(exist).To(BeTrue()) - _, exist = attr.Attributes[AttrTypeOSVer] - Expect(exist).To(BeTrue()) - _, exist = attr.Attributes[AttrTypeCPUArch] - Expect(exist).To(BeFalse()) - }) - }) - - Context("Create NodeAttributes with no labels", func() { - It("Should return NodeAttributes with no attributes", func() { - attr := newNodeAttributes(&testNode) - Expect(attr.Attributes).To(BeEmpty()) - }) - }) - - Context("Node Attributes from labels", func() { - It("Should return Node Attribute from labels", func() { - testNode.Labels[NodeLabelOSName] = "ubuntu" - attr := NodeAttributes{Attributes: make(map[AttributeType]string)} - - nLabels := testNode.GetLabels() - err := attr.fromLabel(AttrTypeOSName, nLabels, NodeLabelOSName) - Expect(err).ToNot(HaveOccurred()) - Expect(attr.Attributes[AttrTypeOSName]).To(Equal("ubuntu")) - }) - It("Should return no Node Attribute from labels", func() { - attr := NodeAttributes{Attributes: make(map[AttributeType]string)} - - nLabels := testNode.GetLabels() - err := attr.fromLabel(AttrTypeOSName, nLabels, NodeLabelOSName) - Expect(err).To(HaveOccurred()) - }) - }) -}) diff --git a/pkg/nodeinfo/mocks/Provider.go b/pkg/nodeinfo/mocks/Provider.go deleted file mode 100644 index 831428b9..00000000 --- a/pkg/nodeinfo/mocks/Provider.go +++ /dev/null @@ -1,92 +0,0 @@ -// Code generated by mockery v2.32.2. DO NOT EDIT. - -package mocks - -import ( - nodeinfo "github.com/Mellanox/network-operator/pkg/nodeinfo" - mock "github.com/stretchr/testify/mock" -) - -// Provider is an autogenerated mock type for the Provider type -type Provider struct { - mock.Mock -} - -type Provider_Expecter struct { - mock *mock.Mock -} - -func (_m *Provider) EXPECT() *Provider_Expecter { - return &Provider_Expecter{mock: &_m.Mock} -} - -// GetNodesAttributes provides a mock function with given fields: filters -func (_m *Provider) GetNodesAttributes(filters ...nodeinfo.Filter) []nodeinfo.NodeAttributes { - _va := make([]interface{}, len(filters)) - for _i := range filters { - _va[_i] = filters[_i] - } - var _ca []interface{} - _ca = append(_ca, _va...) - ret := _m.Called(_ca...) - - var r0 []nodeinfo.NodeAttributes - if rf, ok := ret.Get(0).(func(...nodeinfo.Filter) []nodeinfo.NodeAttributes); ok { - r0 = rf(filters...) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]nodeinfo.NodeAttributes) - } - } - - return r0 -} - -// Provider_GetNodesAttributes_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetNodesAttributes' -type Provider_GetNodesAttributes_Call struct { - *mock.Call -} - -// GetNodesAttributes is a helper method to define mock.On call -// - filters ...nodeinfo.Filter -func (_e *Provider_Expecter) GetNodesAttributes(filters ...interface{}) *Provider_GetNodesAttributes_Call { - return &Provider_GetNodesAttributes_Call{Call: _e.mock.On("GetNodesAttributes", - append([]interface{}{}, filters...)...)} -} - -func (_c *Provider_GetNodesAttributes_Call) Run(run func(filters ...nodeinfo.Filter)) *Provider_GetNodesAttributes_Call { - _c.Call.Run(func(args mock.Arguments) { - variadicArgs := make([]nodeinfo.Filter, len(args)-0) - for i, a := range args[0:] { - if a != nil { - variadicArgs[i] = a.(nodeinfo.Filter) - } - } - run(variadicArgs...) - }) - return _c -} - -func (_c *Provider_GetNodesAttributes_Call) Return(_a0 []nodeinfo.NodeAttributes) *Provider_GetNodesAttributes_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *Provider_GetNodesAttributes_Call) RunAndReturn(run func(...nodeinfo.Filter) []nodeinfo.NodeAttributes) *Provider_GetNodesAttributes_Call { - _c.Call.Return(run) - return _c -} - -// NewProvider creates a new instance of Provider. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -// The first argument is typically a *testing.T value. -func NewProvider(t interface { - mock.TestingT - Cleanup(func()) -}) *Provider { - mock := &Provider{} - mock.Mock.Test(t) - - t.Cleanup(func() { mock.AssertExpectations(t) }) - - return mock -} diff --git a/pkg/nodeinfo/node_info.go b/pkg/nodeinfo/node_info.go index f1d7473a..4b714c89 100644 --- a/pkg/nodeinfo/node_info.go +++ b/pkg/nodeinfo/node_info.go @@ -35,8 +35,6 @@ var MellanoxNICListOptions = []client.ListOption{ // //go:generate mockery --name Provider type Provider interface { - // GetNodesAttributes retrieves node attributes for nodes matching the filter criteria - GetNodesAttributes(filters ...Filter) []NodeAttributes // GetNodePools partitions nodes into one or more node pools for nodes matching the filter criteria GetNodePools(filters ...Filter) []NodePool } @@ -51,18 +49,6 @@ type provider struct { nodes []*corev1.Node } -// GetNodesAttributes retrieves node attributes for nodes matching the filter criteria -func (p *provider) GetNodesAttributes(filters ...Filter) (attrs []NodeAttributes) { - filtered := p.nodes - for _, filter := range filters { - filtered = filter.Apply(filtered) - } - for _, node := range filtered { - attrs = append(attrs, newNodeAttributes(node)) - } - return attrs -} - // NodePool represent a set of Nodes grouped by common attributes type NodePool struct { Name string diff --git a/pkg/nodeinfo/node_info_test.go b/pkg/nodeinfo/node_info_test.go index 9668b650..e468de0e 100644 --- a/pkg/nodeinfo/node_info_test.go +++ b/pkg/nodeinfo/node_info_test.go @@ -43,75 +43,6 @@ func (df *dummyFilter) Apply(_ []*corev1.Node) []*corev1.Node { } var _ = Describe("nodeinfo Provider tests", func() { - - Context("GetNodesAttributes with provided filters", func() { - It("Should Apply filters repeatedly on the nodes and return node attributes for the filtered nodes", func() { - filter1 := &dummyFilter{filtered: []*corev1.Node{ - { - TypeMeta: metav1.TypeMeta{Kind: "Node"}, - ObjectMeta: metav1.ObjectMeta{Name: "Node-1"}, - }, - { - TypeMeta: metav1.TypeMeta{Kind: "Node"}, - ObjectMeta: metav1.ObjectMeta{Name: "Node-2"}, - }, - }} - filter2 := &dummyFilter{ - filtered: []*corev1.Node{ - { - TypeMeta: metav1.TypeMeta{Kind: "Node"}, - ObjectMeta: metav1.ObjectMeta{Name: "Node-2"}, - }, - }, - } - provider := NewProvider([]*corev1.Node{}) - attrs := provider.GetNodesAttributes(filter1, filter2) - - Expect(filter1.called).To(BeTrue()) - Expect(filter2.called).To(BeTrue()) - Expect(len(attrs)).To(Equal(1)) - Expect(attrs[0].Name).To(Equal("Node-2")) - }) - }) - - Context("GetNodesAttributes with empty list of filters", func() { - It("Should return all nodes attributes", func() { - provider := NewProvider([]*corev1.Node{ - { - TypeMeta: metav1.TypeMeta{Kind: "Node"}, - ObjectMeta: metav1.ObjectMeta{Name: "Node-1"}, - }, - { - TypeMeta: metav1.TypeMeta{Kind: "Node"}, - ObjectMeta: metav1.ObjectMeta{Name: "Node-2"}, - }, - }) - - attrs := provider.GetNodesAttributes() - Expect(len(attrs)).To(Equal(2)) - Expect(attrs[0].Name).To(Equal("Node-1")) - Expect(attrs[1].Name).To(Equal("Node-2")) - }) - }) - - Context("GetNodesAttributes with filter returning no match", func() { - It("Should return an empty list of nodes", func() { - filter := &dummyFilter{filtered: []*corev1.Node{}} - provider := NewProvider([]*corev1.Node{ - { - TypeMeta: metav1.TypeMeta{Kind: "Node"}, - ObjectMeta: metav1.ObjectMeta{Name: "Node-1"}, - }, - { - TypeMeta: metav1.TypeMeta{Kind: "Node"}, - ObjectMeta: metav1.ObjectMeta{Name: "Node-2"}, - }, - }) - attrs := provider.GetNodesAttributes(filter) - Expect(len(attrs)).To(Equal(0)) - }) - }) - Context("GetNodePools with filter", func() { It("Should return an empty list of pools", func() { filter := &dummyFilter{filtered: []*corev1.Node{}} diff --git a/pkg/state/dummy_provider.go b/pkg/state/dummy_provider.go index bf71d670..3cb14dfc 100644 --- a/pkg/state/dummy_provider.go +++ b/pkg/state/dummy_provider.go @@ -42,15 +42,6 @@ func (d *dummyProvider) GetStaticConfig() staticconfig.StaticConfig { return staticconfig.StaticConfig{CniBinDirectory: ""} } -func (d *dummyProvider) GetNodesAttributes(...nodeinfo.Filter) []nodeinfo.NodeAttributes { - nodeAttr := make(map[nodeinfo.AttributeType]string) - nodeAttr[nodeinfo.AttrTypeCPUArch] = "amd64" - nodeAttr[nodeinfo.AttrTypeOSName] = "ubuntu" - nodeAttr[nodeinfo.AttrTypeOSVer] = "20.04" - - return []nodeinfo.NodeAttributes{{Attributes: nodeAttr}} -} - func (d *dummyProvider) GetNodePools(...nodeinfo.Filter) []nodeinfo.NodePool { return []nodeinfo.NodePool{ { diff --git a/pkg/state/state_ipoib_cni_test.go b/pkg/state/state_ipoib_cni_test.go index 2c3cfc87..e1e6ce93 100644 --- a/pkg/state/state_ipoib_cni_test.go +++ b/pkg/state/state_ipoib_cni_test.go @@ -33,7 +33,7 @@ import ( var _ = Describe("IPoIB CNI State tests", func() { - Context("GetNodesAttributes with provide", func() { + Context("Creating NCP with IPoIBCNI", func() { It("Should Apply", func() { client := mocks.ControllerRuntimeClient{} manifestBaseDir := "../../manifests/state-ipoib-cni" diff --git a/pkg/state/state_sriov_dp_test.go b/pkg/state/state_sriov_dp_test.go index 30c906d3..a0dde8e6 100644 --- a/pkg/state/state_sriov_dp_test.go +++ b/pkg/state/state_sriov_dp_test.go @@ -62,7 +62,7 @@ func checkRenderedDpDs(obj *unstructured.Unstructured, imageSpec *mellanoxv1alph var _ = Describe("SR-IOV Device Plugin State tests", func() { - Context("GetNodesAttributes with provide", func() { + Context("When creating NCP with SRIOV-device-plugin", func() { It("Should Apply", func() { client := mocks.ControllerRuntimeClient{} manifestBaseDir := "../../manifests/state-sriov-device-plugin"