Skip to content

Commit

Permalink
Merge pull request #1470 from marquiz/devel/nfd-master-refactor
Browse files Browse the repository at this point in the history 
nfd-master: drop stale variables
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Nov 24, 2023
2 parents 38ed148 + 678d7e8 commit 1497cc6
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 26 deletions.
10 changes: 5 additions & 5 deletions pkg/nfd-master/nfd-master-internal_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func TestUpdateNodeObject(t *testing.T) {
mockAPIHelper.On("GetNode", mockClient, mockNodeName).Return(mockNode, nil).Twice()
mockAPIHelper.On("PatchNodeStatus", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(statusPatches))).Return(nil)
mockAPIHelper.On("PatchNode", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(metadataPatches))).Return(nil)
err := mockMaster.updateNodeObject(mockClient, mockNodeName, fakeFeatureLabels, Annotations{}, fakeAnnotations, fakeExtResources, nil)
err := mockMaster.updateNodeObject(mockClient, mockNodeName, fakeFeatureLabels, fakeAnnotations, fakeExtResources, nil)

Convey("Error is nil", func() {
So(err, ShouldBeNil)
Expand All @@ -193,7 +193,7 @@ func TestUpdateNodeObject(t *testing.T) {
Convey("When I fail to update the node with feature labels", func() {
expectedError := fmt.Errorf("no client is passed, client: <nil>")
mockAPIHelper.On("GetClient").Return(nil, expectedError)
err := mockMaster.updateNodeObject(nil, mockNodeName, fakeFeatureLabels, Annotations{}, fakeAnnotations, fakeExtResources, nil)
err := mockMaster.updateNodeObject(nil, mockNodeName, fakeFeatureLabels, fakeAnnotations, fakeExtResources, nil)

Convey("Error is produced", func() {
So(err, ShouldResemble, expectedError)
Expand All @@ -203,7 +203,7 @@ func TestUpdateNodeObject(t *testing.T) {
Convey("When I fail to get a mock client while updating feature labels", func() {
expectedError := fmt.Errorf("no client is passed, client: <nil>")
mockAPIHelper.On("GetClient").Return(nil, expectedError)
err := mockMaster.updateNodeObject(nil, mockNodeName, fakeFeatureLabels, Annotations{}, fakeAnnotations, fakeExtResources, nil)
err := mockMaster.updateNodeObject(nil, mockNodeName, fakeFeatureLabels, fakeAnnotations, fakeExtResources, nil)

Convey("Error is produced", func() {
So(err, ShouldResemble, expectedError)
Expand All @@ -214,7 +214,7 @@ func TestUpdateNodeObject(t *testing.T) {
expectedError := errors.New("fake error")
mockAPIHelper.On("GetClient").Return(mockClient, nil)
mockAPIHelper.On("GetNode", mockClient, mockNodeName).Return(nil, expectedError).Twice()
err := mockMaster.updateNodeObject(mockClient, mockNodeName, fakeFeatureLabels, Annotations{}, fakeAnnotations, fakeExtResources, nil)
err := mockMaster.updateNodeObject(mockClient, mockNodeName, fakeFeatureLabels, fakeAnnotations, fakeExtResources, nil)

Convey("Error is produced", func() {
So(err, ShouldEqual, expectedError)
Expand All @@ -227,7 +227,7 @@ func TestUpdateNodeObject(t *testing.T) {
mockAPIHelper.On("GetNode", mockClient, mockNodeName).Return(mockNode, nil).Twice()
mockAPIHelper.On("PatchNodeStatus", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(statusPatches))).Return(nil)
mockAPIHelper.On("PatchNode", mockClient, mockNodeName, mock.Anything).Return(expectedError).Twice()
err := mockMaster.updateNodeObject(mockClient, mockNodeName, fakeFeatureLabels, Annotations{}, fakeAnnotations, fakeExtResources, nil)
err := mockMaster.updateNodeObject(mockClient, mockNodeName, fakeFeatureLabels, fakeAnnotations, fakeExtResources, nil)

Convey("Error is produced", func() {
So(err.Error(), ShouldEndWith, expectedError.Error())
Expand Down
33 changes: 12 additions & 21 deletions pkg/nfd-master/nfd-master.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ func (m *nfdMaster) prune() error {
klog.InfoS("pruning node...", "nodeName", node.Name)

// Prune labels and extended resources
err := m.updateNodeObject(cli, node.Name, Labels{}, Annotations{}, Annotations{}, ExtendedResources{}, []corev1.Taint{})
err := m.updateNodeObject(cli, node.Name, Labels{}, Annotations{}, ExtendedResources{}, []corev1.Taint{})
if err != nil {
nodeUpdateFailures.Inc()
return fmt.Errorf("failed to prune node %q: %v", node.Name, err)
Expand Down Expand Up @@ -693,10 +693,8 @@ func (m *nfdMaster) SetLabels(c context.Context, r *pb.SetLabelsRequest) (*pb.Se
return &pb.SetLabelsReply{}, err
}

annotations := Annotations{}

// Create labels et al
if err := m.refreshNodeFeatures(cli, r.NodeName, annotations, r.GetLabels(), r.GetFeatures()); err != nil {
if err := m.refreshNodeFeatures(cli, r.NodeName, r.GetLabels(), r.GetFeatures()); err != nil {
nodeUpdateFailures.Inc()
return &pb.SetLabelsReply{}, err
}
Expand Down Expand Up @@ -760,8 +758,6 @@ func (m *nfdMaster) nfdAPIUpdateOneNode(nodeName string) error {

features := nfdv1alpha1.NewNodeFeatureSpec()

annotations := Annotations{}

if len(objs) > 0 {
// Merge in features
//
Expand All @@ -782,7 +778,7 @@ func (m *nfdMaster) nfdAPIUpdateOneNode(nodeName string) error {
if err != nil {
return err
}
if err := m.refreshNodeFeatures(cli, nodeName, annotations, features.Labels, &features.Features); err != nil {
if err := m.refreshNodeFeatures(cli, nodeName, features.Labels, &features.Features); err != nil {
return err
}

Expand Down Expand Up @@ -837,7 +833,7 @@ func filterExtendedResource(name, value string, features *nfdv1alpha1.Features)
return q.String(), nil
}

func (m *nfdMaster) refreshNodeFeatures(cli *kubernetes.Clientset, nodeName string, nfdAnnotations Annotations, labels map[string]string, features *nfdv1alpha1.Features) error {
func (m *nfdMaster) refreshNodeFeatures(cli *kubernetes.Clientset, nodeName string, labels map[string]string, features *nfdv1alpha1.Features) error {
if labels == nil {
labels = make(map[string]string)
}
Expand All @@ -860,15 +856,15 @@ func (m *nfdMaster) refreshNodeFeatures(cli *kubernetes.Clientset, nodeName stri
extendedResources = filterExtendedResources(features, extendedResources)

// Annotations
featureAnnotations := m.filterFeatureAnnotations(crAnnotations)
annotations := m.filterFeatureAnnotations(crAnnotations)

// Taints
var taints []corev1.Taint
if m.config.EnableTaints {
taints = filterTaints(crTaints)
}

err := m.updateNodeObject(cli, nodeName, labels, nfdAnnotations, featureAnnotations, extendedResources, taints)
err := m.updateNodeObject(cli, nodeName, labels, annotations, extendedResources, taints)
if err != nil {
klog.ErrorS(err, "failed to update node", "nodeName", nodeName)
return err
Expand Down Expand Up @@ -1040,7 +1036,7 @@ func (m *nfdMaster) processNodeFeatureRule(nodeName string, features *nfdv1alpha
// updateNodeObject ensures the Kubernetes node object is up to date,
// creating new labels and extended resources where necessary and removing
// outdated ones. Also updates the corresponding annotations.
func (m *nfdMaster) updateNodeObject(cli *kubernetes.Clientset, nodeName string, labels Labels, nfdAnnotations, featureAnnotations Annotations, extendedResources ExtendedResources, taints []corev1.Taint) error {
func (m *nfdMaster) updateNodeObject(cli *kubernetes.Clientset, nodeName string, labels Labels, featureAnnotations Annotations, extendedResources ExtendedResources, taints []corev1.Taint) error {
if cli == nil {
return fmt.Errorf("no client is passed, client: %v", cli)
}
Expand All @@ -1051,6 +1047,8 @@ func (m *nfdMaster) updateNodeObject(cli *kubernetes.Clientset, nodeName string,
return err
}

annotations := make(Annotations)

// Store names of labels in an annotation
if len(labels) > 0 {
labelKeys := make([]string, 0, len(labels))
Expand All @@ -1059,7 +1057,7 @@ func (m *nfdMaster) updateNodeObject(cli *kubernetes.Clientset, nodeName string,
labelKeys = append(labelKeys, strings.TrimPrefix(key, nfdv1alpha1.FeatureLabelNs+"/"))
}
sort.Strings(labelKeys)
nfdAnnotations[m.instanceAnnotation(nfdv1alpha1.FeatureLabelsAnnotation)] = strings.Join(labelKeys, ",")
annotations[m.instanceAnnotation(nfdv1alpha1.FeatureLabelsAnnotation)] = strings.Join(labelKeys, ",")
}

// Store names of extended resources in an annotation
Expand All @@ -1070,11 +1068,10 @@ func (m *nfdMaster) updateNodeObject(cli *kubernetes.Clientset, nodeName string,
extendedResourceKeys = append(extendedResourceKeys, strings.TrimPrefix(key, nfdv1alpha1.FeatureLabelNs+"/"))
}
sort.Strings(extendedResourceKeys)
nfdAnnotations[m.instanceAnnotation(nfdv1alpha1.ExtendedResourceAnnotation)] = strings.Join(extendedResourceKeys, ",")
annotations[m.instanceAnnotation(nfdv1alpha1.ExtendedResourceAnnotation)] = strings.Join(extendedResourceKeys, ",")
}

// Store feature annotations
annotations := make(Annotations)
if len(featureAnnotations) > 0 {
// Store names of feature annotations in an annotation
annotationKeys := make([]string, 0, len(featureAnnotations))
Expand All @@ -1083,18 +1080,12 @@ func (m *nfdMaster) updateNodeObject(cli *kubernetes.Clientset, nodeName string,
annotationKeys = append(annotationKeys, strings.TrimPrefix(key, nfdv1alpha1.FeatureAnnotationNs+"/"))
}
sort.Strings(annotationKeys)
nfdAnnotations[m.instanceAnnotation(nfdv1alpha1.FeatureAnnotationsTrackingAnnotation)] = strings.Join(annotationKeys, ",")
annotations[m.instanceAnnotation(nfdv1alpha1.FeatureAnnotationsTrackingAnnotation)] = strings.Join(annotationKeys, ",")
for k, v := range featureAnnotations {
annotations[k] = v
}
}

if len(nfdAnnotations) > 0 {
for k, v := range nfdAnnotations {
annotations[k] = v
}
}

// Create JSON patches for changes in labels and annotations
oldLabels := stringToNsNames(node.Annotations[m.instanceAnnotation(nfdv1alpha1.FeatureLabelsAnnotation)], nfdv1alpha1.FeatureLabelNs)
oldAnnotations := stringToNsNames(node.Annotations[m.instanceAnnotation(nfdv1alpha1.FeatureAnnotationsTrackingAnnotation)], nfdv1alpha1.FeatureAnnotationNs)
Expand Down

0 comments on commit 1497cc6

Please sign in to comment.