mirrors/node-feature-discovery
mirrors/node-feature-discovery
1
0
Fork 0
mirror of https://github.com/kubernetes-sigs/node-feature-discovery.git synced 2025-03-15 04:57:56 +00:00
Code Activity

Refactor APIHelpers

Browse source 
Remove functionality that was not interacting with Kubernetes API.
Makes the architecture a bit simpler and simplifies testing.
This commit is contained in:
Markus Lehtonen 2019-02-12 15:31:59 +02:00
parent 35d26001e4
commit 75a8f0c146
6 changed files with 123 additions and 185 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
pkg/apihelper/apihelpers.go
View file

@ -29,22 +29,6 @@ type APIHelpers interface {
// GetNode returns the Kubernetes node on which this container is running.
GetNode(*k8sclient.Clientset, string) (*api.Node, error)
// RemoveLabelsWithPrefix removes labels from the supplied node that contain the
// search string provided. In order to publish the changes, the node must
// subsequently be updated via the API server using the client library.
RemoveLabelsWithPrefix(*api.Node, string)
// RemoveLabels removes NFD labels from a node object
RemoveLabels(*api.Node, []string)
// AddLabels adds new NFD labels to the node object.
// In order to publish the labels, the node must be subsequently updated via the
// API server using the client library.
AddLabels(*api.Node, map[string]string)
// Add annotations
AddAnnotations(*api.Node, map[string]string)
// UpdateNode updates the node via the API server using a client.
UpdateNode(*k8sclient.Clientset, *api.Node) error
}

32
pkg/apihelper/k8shelpers.go
View file

@ -17,8 +17,6 @@ limitations under the License.
package apihelper
import (
"strings"
api "k8s.io/api/core/v1"
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8sclient "k8s.io/client-go/kubernetes"
@ -54,36 +52,6 @@ func (h K8sHelpers) GetNode(cli *k8sclient.Clientset, nodeName string) (*api.Nod
return node, nil
}
// RemoveLabelsWithPrefix searches through all labels on Node n and removes
// any where the key contain the search string.
func (h K8sHelpers) RemoveLabelsWithPrefix(n *api.Node, search string) {
for k := range n.Labels {
if strings.Contains(k, search) {
delete(n.Labels, k)
}
}
}
// RemoveLabels removes given NFD labels
func (h K8sHelpers) RemoveLabels(n *api.Node, labelNames []string) {
for _, l := range labelNames {
delete(n.Labels, h.LabelNs+l)
}
}
func (h K8sHelpers) AddLabels(n *api.Node, labels map[string]string) {
for k, v := range labels {
n.Labels[h.LabelNs+k] = v
}
}
// Add Annotations to the Node object
func (h K8sHelpers) AddAnnotations(n *api.Node, annotations map[string]string) {
for k, v := range annotations {
n.Annotations[h.AnnotationNs+k] = v
}
}
func (h K8sHelpers) UpdateNode(c *k8sclient.Clientset, n *api.Node) error {
// Send the updated node to the apiserver.
_, err := c.Core().Nodes().Update(n)

90
pkg/apihelper/k8shelpers_test.go
View file

@ -1,90 +0,0 @@
/*
Copyright 2019 The Kubernetes Authors.
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 apihelper_test
import (
"testing"
. "github.com/smartystreets/goconvey/convey"
api "k8s.io/api/core/v1"
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/node-feature-discovery/pkg/apihelper"
)
func TestAddLabels(t *testing.T) {
Convey("When adding labels", t, func() {
labelNs := "test.nfd/"
helper := apihelper.K8sHelpers{LabelNs: labelNs}
labels := map[string]string{}
n := &api.Node{
ObjectMeta: meta_v1.ObjectMeta{
Labels: map[string]string{},
},
}
Convey("If no labels are passed", func() {
helper.AddLabels(n, labels)
Convey("None should be added", func() {
So(len(n.Labels), ShouldEqual, 0)
})
})
Convey("They should be added to the node.Labels", func() {
test1 := "test1"
labels[test1] = "true"
helper.AddLabels(n, labels)
So(n.Labels, ShouldContainKey, labelNs+test1)
})
})
}
func TestRemoveLabelsWithPrefix(t *testing.T) {
Convey("When removing labels", t, func() {
helper := apihelper.K8sHelpers{}
n := &api.Node{
ObjectMeta: meta_v1.ObjectMeta{
Labels: map[string]string{
"single": "123",
"multiple_A": "a",
"multiple_B": "b",
},
},
}
Convey("a unique label should be removed", func() {
helper.RemoveLabelsWithPrefix(n, "single")
So(len(n.Labels), ShouldEqual, 2)
So(n.Labels, ShouldNotContainKey, "single")
})
Convey("a non-unique search string should remove all matching keys", func() {
helper.RemoveLabelsWithPrefix(n, "multiple")
So(len(n.Labels), ShouldEqual, 1)
So(n.Labels, ShouldNotContainKey, "multiple_A")
So(n.Labels, ShouldNotContainKey, "multiple_B")
})
Convey("a search string with no matches should not alter labels", func() {
helper.RemoveLabelsWithPrefix(n, "unique")
So(n.Labels, ShouldContainKey, "single")
So(n.Labels, ShouldContainKey, "multiple_A")
So(n.Labels, ShouldContainKey, "multiple_B")
So(len(n.Labels), ShouldEqual, 3)
})
})
}

20
pkg/apihelper/mock_APIHelpers.go
View file

@ -13,16 +13,6 @@ type MockAPIHelpers struct {
mock.Mock
}
// AddAnnotations provides a mock function with given fields: _a0, _a1
func (_m *MockAPIHelpers) AddAnnotations(_a0 *v1.Node, _a1 map[string]string) {
_m.Called(_a0, _a1)
}
// AddLabels provides a mock function with given fields: _a0, _a1
func (_m *MockAPIHelpers) AddLabels(_a0 *v1.Node, _a1 map[string]string) {
_m.Called(_a0, _a1)
}
// GetClient provides a mock function with given fields:
func (_m *MockAPIHelpers) GetClient() (*kubernetes.Clientset, error) {
ret := _m.Called()
@ -69,16 +59,6 @@ func (_m *MockAPIHelpers) GetNode(_a0 *kubernetes.Clientset, _a1 string) (*v1.No
return r0, r1
}
// RemoveLabels provides a mock function with given fields: _a0, _a1
func (_m *MockAPIHelpers) RemoveLabels(_a0 *v1.Node, _a1 []string) {
_m.Called(_a0, _a1)
}
// RemoveLabelsWithPrefix provides a mock function with given fields: _a0, _a1
func (_m *MockAPIHelpers) RemoveLabelsWithPrefix(_a0 *v1.Node, _a1 string) {
_m.Called(_a0, _a1)
}
// UpdateNode provides a mock function with given fields: _a0, _a1
func (_m *MockAPIHelpers) UpdateNode(_a0 *kubernetes.Clientset, _a1 *v1.Node) error {
ret := _m.Called(_a0, _a1)

104
pkg/nfd-master/nfd-master-internal_test.go
View file

@ -21,10 +21,10 @@ import (
"testing"
. "github.com/smartystreets/goconvey/convey"
"github.com/stretchr/testify/mock"
"github.com/vektra/errors"
"golang.org/x/net/context"
api "k8s.io/api/core/v1"
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8sclient "k8s.io/client-go/kubernetes"
"sigs.k8s.io/node-feature-discovery/pkg/apihelper"
"sigs.k8s.io/node-feature-discovery/pkg/labeler"
@ -39,6 +39,13 @@ func init() {
nodeName = mockNodeName
}
func newMockNode() *api.Node {
n := api.Node{}
n.Labels = map[string]string{}
n.Annotations = map[string]string{}
return &n
}
func TestUpdateNodeFeatures(t *testing.T) {
Convey("When I update the node using fake client", t, func() {
fakeFeatureLabels := map[string]string{"source-feature.1": "val1", "source-feature.2": "val2", "source-feature.3": "val3"}
@ -50,23 +57,31 @@ func TestUpdateNodeFeatures(t *testing.T) {
fakeAnnotations["feature-labels"] = strings.Join(fakeFeatureLabelNames, ",")
mockAPIHelper := new(apihelper.MockAPIHelpers)
mockNode := &api.Node{}
mockClient := &k8sclient.Clientset{}
// Mock node with old features
mockNode := newMockNode()
mockNode.Labels[labelNs+"old-feature"] = "old-value"
mockNode.Annotations[annotationNs+"feature-labels"] = "old-feature"
Convey("When I successfully update the node with feature labels", func() {
mockAPIHelper.On("GetClient").Return(mockClient, nil)
mockAPIHelper.On("GetNode", mockClient, mockNodeName).Return(mockNode, nil).Once()
mockAPIHelper.On("AddLabels", mockNode, fakeFeatureLabels).Return().Once()
mockAPIHelper.On("RemoveLabelsWithPrefix", mockNode, labelNs).Return().Once()
mockAPIHelper.On("RemoveLabelsWithPrefix", mockNode, "node.alpha.kubernetes-incubator.io/nfd").Return().Once()
mockAPIHelper.On("RemoveLabelsWithPrefix", mockNode, "node.alpha.kubernetes-incubator.io/node-feature-discovery").Return().Once()
mockAPIHelper.On("AddAnnotations", mockNode, fakeAnnotations).Return().Once()
mockAPIHelper.On("UpdateNode", mockClient, mockNode).Return(nil).Once()
err := updateNodeFeatures(mockAPIHelper, mockNodeName, fakeFeatureLabels, fakeAnnotations)
Convey("Error is nil", func() {
So(err, ShouldBeNil)
})
Convey("Node object should have updated with labels and annotations", func() {
So(len(mockNode.Labels), ShouldEqual, len(fakeFeatureLabels))
for k, v := range fakeFeatureLabels {
So(mockNode.Labels[labelNs+k], ShouldEqual, v)
}
So(len(mockNode.Annotations), ShouldEqual, len(fakeAnnotations))
for k, v := range fakeFeatureLabels {
So(mockNode.Labels[labelNs+k], ShouldEqual, v)
}
})
})
Convey("When I fail to update the node with feature labels", func() {
@ -104,11 +119,6 @@ func TestUpdateNodeFeatures(t *testing.T) {
expectedError := errors.New("fake error")
mockAPIHelper.On("GetClient").Return(mockClient, nil)
mockAPIHelper.On("GetNode", mockClient, mockNodeName).Return(mockNode, nil).Once()
mockAPIHelper.On("RemoveLabelsWithPrefix", mockNode, labelNs).Return().Once()
mockAPIHelper.On("RemoveLabelsWithPrefix", mockNode, "node.alpha.kubernetes-incubator.io/nfd").Return().Once()
mockAPIHelper.On("RemoveLabelsWithPrefix", mockNode, "node.alpha.kubernetes-incubator.io/node-feature-discovery").Return().Once()
mockAPIHelper.On("AddLabels", mockNode, fakeFeatureLabels).Return().Once()
mockAPIHelper.On("AddAnnotations", mockNode, fakeAnnotations).Return().Once()
mockAPIHelper.On("UpdateNode", mockClient, mockNode).Return(expectedError).Once()
err := updateNodeFeatures(mockAPIHelper, mockNodeName, fakeFeatureLabels, fakeAnnotations)
@ -124,11 +134,10 @@ func TestUpdateMasterNode(t *testing.T) {
Convey("When updating the nfd-master node", t, func() {
mockHelper := &apihelper.MockAPIHelpers{}
mockClient := &k8sclient.Clientset{}
mockNode := &api.Node{}
mockNode := newMockNode()
Convey("When update operation succeeds", func() {
mockHelper.On("GetClient").Return(mockClient, nil)
mockHelper.On("GetNode", mockClient, mockNodeName).Return(mockNode, nil)
mockHelper.On("AddAnnotations", mockNode, map[string]string{"master.version": version.Get()})
mockHelper.On("UpdateNode", mockClient, mockNode).Return(nil)
err := updateMasterNode(mockHelper)
Convey("No error should be returned", func() {
@ -157,7 +166,6 @@ func TestUpdateMasterNode(t *testing.T) {
Convey("When updating node object fails", func() {
mockHelper.On("GetClient").Return(mockClient, nil)
mockHelper.On("GetNode", mockClient, mockNodeName).Return(mockNode, nil)
mockHelper.On("AddAnnotations", mock.Anything, mock.Anything)
mockHelper.On("UpdateNode", mockClient, mockNode).Return(mockErr)
err := updateMasterNode(mockHelper)
Convey("An error should be returned", func() {
@ -173,7 +181,7 @@ func TestSetLabels(t *testing.T) {
const workerVer = "0.1-test"
mockHelper := &apihelper.MockAPIHelpers{}
mockClient := &k8sclient.Clientset{}
mockNode := &api.Node{}
mockNode := newMockNode()
mockServer := labelerServer{args: Args{}, apiHelper: mockHelper}
mockCtx := context.Background()
mockReq := &labeler.SetLabelsRequest{NodeName: workerName, NfdVersion: workerVer, Labels: map[string]string{"feature-1": "val-1"}}
@ -181,9 +189,6 @@ func TestSetLabels(t *testing.T) {
Convey("When node update succeeds", func() {
mockHelper.On("GetClient").Return(mockClient, nil)
mockHelper.On("GetNode", mockClient, workerName).Return(mockNode, nil)
mockHelper.On("RemoveLabelsWithPrefix", mockNode, mock.Anything).Return()
mockHelper.On("AddLabels", mockNode, mock.Anything).Return()
mockHelper.On("AddAnnotations", mockNode, map[string]string{"worker.version": workerVer, "feature-labels": "feature-1"})
mockHelper.On("UpdateNode", mockClient, mockNode).Return(nil)
_, err := mockServer.SetLabels(mockCtx, mockReq)
Convey("No error should be returned", func() {
@ -209,3 +214,64 @@ func TestSetLabels(t *testing.T) {
})
})
}
func TestAddLabels(t *testing.T) {
Convey("When adding labels", t, func() {
labels := map[string]string{}
n := &api.Node{
ObjectMeta: meta_v1.ObjectMeta{
Labels: map[string]string{},
},
}
Convey("If no labels are passed", func() {
addLabels(n, labels)
Convey("None should be added", func() {
So(len(n.Labels), ShouldEqual, 0)
})
})
Convey("They should be added to the node.Labels", func() {
test1 := "test1"
labels[test1] = "true"
addLabels(n, labels)
So(n.Labels, ShouldContainKey, labelNs+test1)
})
})
}
func TestRemoveLabelsWithPrefix(t *testing.T) {
Convey("When removing labels", t, func() {
n := &api.Node{
ObjectMeta: meta_v1.ObjectMeta{
Labels: map[string]string{
"single-label": "123",
"multiple_A": "a",
"multiple_B": "b",
},
},
}
Convey("a unique label should be removed", func() {
removeLabelsWithPrefix(n, "single")
So(len(n.Labels), ShouldEqual, 2)
So(n.Labels, ShouldNotContainKey, "single")
})
Convey("a non-unique search string should remove all matching keys", func() {
removeLabelsWithPrefix(n, "multiple")
So(len(n.Labels), ShouldEqual, 1)
So(n.Labels, ShouldNotContainKey, "multiple_A")
So(n.Labels, ShouldNotContainKey, "multiple_B")
})
Convey("a search string with no matches should not alter labels", func() {
removeLabelsWithPrefix(n, "unique")
So(n.Labels, ShouldContainKey, "single-label")
So(n.Labels, ShouldContainKey, "multiple_A")
So(n.Labels, ShouldContainKey, "multiple_B")
So(len(n.Labels), ShouldEqual, 3)
})
})
}

46
pkg/nfd-master/nfd-master.go
View file

@ -33,6 +33,7 @@ import (
"google.golang.org/grpc"
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/peer"
api "k8s.io/api/core/v1"
"sigs.k8s.io/node-feature-discovery/pkg/apihelper"
pb "sigs.k8s.io/node-feature-discovery/pkg/labeler"
"sigs.k8s.io/node-feature-discovery/pkg/version"
@ -109,8 +110,7 @@ func (m *nfdMaster) Run() error {
stdoutLogger.Printf("NodeName: '%s'", nodeName)
// Initialize Kubernetes API helpers
helper := apihelper.APIHelpers(apihelper.K8sHelpers{AnnotationNs: annotationNs,
LabelNs: labelNs})
helper := apihelper.APIHelpers(apihelper.K8sHelpers{})
if !m.args.NoPublish {
err := updateMasterNode(helper)
@ -192,7 +192,7 @@ func updateMasterNode(helper apihelper.APIHelpers) error {
}
// Advertise NFD version as an annotation
helper.AddAnnotations(node, Annotations{"master.version": version.Get()})
addAnnotations(node, Annotations{"master.version": version.Get()})
err = helper.UpdateNode(cli, node)
if err != nil {
stderrLogger.Printf("can't update node: %s", err.Error())
@ -282,18 +282,18 @@ func updateNodeFeatures(helper apihelper.APIHelpers, nodeName string, labels Lab
// Remove old labels
if l, ok := node.Annotations[annotationNs+"feature-labels"]; ok {
oldLabels := strings.Split(l, ",")
helper.RemoveLabels(node, oldLabels)
removeLabels(node, oldLabels)
}
// Also, remove all labels with the old prefix, and the old version label
helper.RemoveLabelsWithPrefix(node, "node.alpha.kubernetes-incubator.io/nfd")
helper.RemoveLabelsWithPrefix(node, "node.alpha.kubernetes-incubator.io/node-feature-discovery")
removeLabelsWithPrefix(node, "node.alpha.kubernetes-incubator.io/nfd")
removeLabelsWithPrefix(node, "node.alpha.kubernetes-incubator.io/node-feature-discovery")
// Add labels to the node object.
helper.AddLabels(node, labels)
addLabels(node, labels)
// Add annotations
helper.AddAnnotations(node, annotations)
addAnnotations(node, annotations)
// Send the updated node to the apiserver.
err = helper.UpdateNode(cli, node)
@ -304,3 +304,33 @@ func updateNodeFeatures(helper apihelper.APIHelpers, nodeName string, labels Lab
return nil
}
// Remove any labels having the given prefix
func removeLabelsWithPrefix(n *api.Node, search string) {
for k := range n.Labels {
if strings.HasPrefix(k, search) {
delete(n.Labels, k)
}
}
}
// Removes NFD labels from a Node object
func removeLabels(n *api.Node, labelNames []string) {
for _, l := range labelNames {
delete(n.Labels, labelNs+l)
}
}
// Add NFD labels to a Node object.
func addLabels(n *api.Node, labels map[string]string) {
for k, v := range labels {
n.Labels[labelNs+k] = v
}
}
// Add Annotations to a Node object
func addAnnotations(n *api.Node, annotations map[string]string) {
for k, v := range annotations {
n.Annotations[annotationNs+k] = v
}
}