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-05 08:17:04 +00:00
Code Activity

nfd-master: fix node update

Browse source 
Update node status before node metadata. This fixes a problem where we
lose track of NFD-managed extended resources in case patching node
status fails. Previously we removed all labels and annotations
(including the one listing our ERs) and only after that updated node
status. If node status update failed we had lost the annotation but
extended resources were still there, leaving them orphaned.
This commit is contained in:
Markus Lehtonen 2023-04-06 21:20:24 +03:00
parent ec014f118b
commit f64c23968a
2 changed files with 18 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -86,6 +86,12 @@ func TestUpdateNodeObject(t *testing.T) {
}
sort.Strings(fakeExtResourceNames)
// Create a list of expected node status patches
statusPatches := []apihelper.JsonPatch{}
for k, v := range fakeExtResources {
statusPatches = append(statusPatches, apihelper.NewJsonPatch("add", "/status/capacity", k, v))
}
mockAPIHelper := new(apihelper.MockAPIHelpers)
mockMaster := newMockMaster(mockAPIHelper)
mockClient := &k8sclient.Clientset{}
@ -108,16 +114,10 @@ func TestUpdateNodeObject(t *testing.T) {
metadataPatches = append(metadataPatches, apihelper.NewJsonPatch("add", "/metadata/annotations", k, v))
}
// Create a list of expected node status patches
statusPatches := []apihelper.JsonPatch{}
for k, v := range fakeExtResources {
statusPatches = append(statusPatches, apihelper.NewJsonPatch("add", "/status/capacity", k, v))
}
mockAPIHelper.On("GetClient").Return(mockClient, nil)
mockAPIHelper.On("GetNode", mockClient, mockNodeName).Return(mockNode, nil).Twice()
mockAPIHelper.On("PatchNode", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(metadataPatches))).Return(nil)
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, fakeAnnotations, fakeExtResources, nil)
Convey("Error is nil", func() {
@ -160,6 +160,7 @@ func TestUpdateNodeObject(t *testing.T) {
expectedError := errors.New("fake error")
mockAPIHelper.On("GetClient").Return(mockClient, nil)
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, fakeAnnotations, fakeExtResources, nil)
@ -328,8 +329,8 @@ func TestSetLabels(t *testing.T) {
mockHelper.On("GetClient").Return(mockClient, nil)
mockHelper.On("GetNode", mockClient, workerName).Return(mockNode, nil).Twice()
mockHelper.On("PatchNode", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(expectedPatches))).Return(nil)
mockHelper.On("PatchNodeStatus", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(expectedStatusPatches))).Return(nil)
mockHelper.On("PatchNode", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(expectedPatches))).Return(nil)
_, err := mockMaster.SetLabels(mockCtx, mockReq)
Convey("No error should be returned", func() {
So(err, ShouldBeNil)
@ -347,8 +348,8 @@ func TestSetLabels(t *testing.T) {
mockMaster.config.LabelWhiteList.Regexp = *regexp.MustCompile("^f.*2$")
mockHelper.On("GetClient").Return(mockClient, nil)
mockHelper.On("GetNode", mockClient, workerName).Return(mockNode, nil)
mockHelper.On("PatchNode", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(expectedPatches))).Return(nil)
mockHelper.On("PatchNodeStatus", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(expectedStatusPatches))).Return(nil)
mockHelper.On("PatchNode", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(expectedPatches))).Return(nil)
_, err := mockMaster.SetLabels(mockCtx, mockReq)
Convey("Error is nil", func() {
So(err, ShouldBeNil)
@ -385,8 +386,8 @@ func TestSetLabels(t *testing.T) {
mockMaster.args.Instance = instance
mockHelper.On("GetClient").Return(mockClient, nil)
mockHelper.On("GetNode", mockClient, workerName).Return(mockNode, nil)
mockHelper.On("PatchNode", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(expectedPatches))).Return(nil)
mockHelper.On("PatchNodeStatus", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(expectedStatusPatches))).Return(nil)
mockHelper.On("PatchNode", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(expectedPatches))).Return(nil)
mockReq := &labeler.SetLabelsRequest{NodeName: workerName, NfdVersion: workerVer, Labels: mockLabels}
_, err := mockMaster.SetLabels(mockCtx, mockReq)
Convey("Error is nil", func() {
@ -410,8 +411,8 @@ func TestSetLabels(t *testing.T) {
mockMaster.config.ResourceLabels = map[string]struct{}{"feature-3": {}, "feature-1": {}}
mockHelper.On("GetClient").Return(mockClient, nil)
mockHelper.On("GetNode", mockClient, workerName).Return(mockNode, nil)
mockHelper.On("PatchNode", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(expectedPatches))).Return(nil)
mockHelper.On("PatchNodeStatus", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(expectedStatusPatches))).Return(nil)
mockHelper.On("PatchNode", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(expectedPatches))).Return(nil)
_, err := mockMaster.SetLabels(mockCtx, mockReq)
Convey("Error is nil", func() {
So(err, ShouldBeNil)

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

@ -878,12 +878,6 @@ func (m *nfdMaster) updateNodeObject(cli *kubernetes.Clientset, nodeName string,
patches := createPatches(oldLabels, node.Labels, labels, "/metadata/labels")
patches = append(patches, createPatches(nil, node.Annotations, annotations, "/metadata/annotations")...)
// Patch the node object in the apiserver
err = m.apihelper.PatchNode(cli, node.Name, patches)
if err != nil {
return fmt.Errorf("error while patching node object: %v", err)
}
// patch node status with extended resource changes
statusPatches := m.createExtendedResourcePatches(node, extendedResources)
err = m.apihelper.PatchNodeStatus(cli, node.Name, statusPatches)
@ -891,6 +885,12 @@ func (m *nfdMaster) updateNodeObject(cli *kubernetes.Clientset, nodeName string,
return fmt.Errorf("error while patching extended resources: %v", err)
}
// Patch the node object in the apiserver
err = m.apihelper.PatchNode(cli, node.Name, patches)
if err != nil {
return fmt.Errorf("error while patching node object: %v", err)
}
if len(patches) > 0 || len(statusPatches) > 0 {
klog.Infof("node %q updated", nodeName)
} else {