nfd-master: change statusOp to a more generalized JSON patch

Generalize and rename 'statusOp' type to a more flexible 'JsonPatch'.
Move it to the apihelper package.

pkg/apihelper/jsonpatch.go

@@ -0,0 +1,34 @@
+/*
+Copyright 2020 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
+
+import (
+	"path/filepath"
+	"strings"
+)
+
+// JsonPatch is a json marshaling helper used for patching API objects
+type JsonPatch struct {
+	Op    string `json:"op"`
+	Path  string `json:"path"`
+	Value string `json:"value,omitempty"`
+}
+
+// NewJsonPatch returns a new JsonPatch object
+func NewJsonPatch(verb string, path string, key string, value string) JsonPatch {
+	return JsonPatch{verb, filepath.Join(path, strings.ReplaceAll(key, "/", "~1")), value}
+}

pkg/nfd-master/nfd-master-internal_test.go

@@ -189,14 +189,14 @@ func TestAddingExtResources(t *testing.T) {
 		Convey("When there are no matching labels", func() {
 			mockNode := newMockNode()
 			mockResourceLabels := ExtendedResources{}
-			resourceOps := getExtendedResourceOps(mockNode, mockResourceLabels)
+			resourceOps := createExtendedResourcePatches(mockNode, mockResourceLabels)
 			So(len(resourceOps), ShouldEqual, 0)
 		})
 
 		Convey("When there are matching labels", func() {
 			mockNode := newMockNode()
 			mockResourceLabels := ExtendedResources{LabelNs + "/feature-1": "1", LabelNs + "feature-2": "2"}
-			resourceOps := getExtendedResourceOps(mockNode, mockResourceLabels)
+			resourceOps := createExtendedResourcePatches(mockNode, mockResourceLabels)
 			So(len(resourceOps), ShouldBeGreaterThan, 0)
 		})
 
@@ -204,7 +204,7 @@ func TestAddingExtResources(t *testing.T) {
 			mockNode := newMockNode()
 			mockNode.Status.Capacity[api.ResourceName(LabelNs+"/feature-1")] = *resource.NewQuantity(1, resource.BinarySI)
 			mockResourceLabels := ExtendedResources{LabelNs + "/feature-1": "1"}
-			resourceOps := getExtendedResourceOps(mockNode, mockResourceLabels)
+			resourceOps := createExtendedResourcePatches(mockNode, mockResourceLabels)
 			So(len(resourceOps), ShouldEqual, 0)
 		})
 
@@ -212,7 +212,7 @@ func TestAddingExtResources(t *testing.T) {
 			mockNode := newMockNode()
 			mockNode.Status.Capacity[api.ResourceName(LabelNs+"/feature-1")] = *resource.NewQuantity(2, resource.BinarySI)
 			mockResourceLabels := ExtendedResources{LabelNs + "/feature-1": "1"}
-			resourceOps := getExtendedResourceOps(mockNode, mockResourceLabels)
+			resourceOps := createExtendedResourcePatches(mockNode, mockResourceLabels)
 			So(len(resourceOps), ShouldBeGreaterThan, 0)
 		})
 	})
@@ -226,7 +226,7 @@ func TestRemovingExtResources(t *testing.T) {
 			mockNode.Annotations[AnnotationNs+"/extended-resources"] = "feature-1,feature-2"
 			mockNode.Status.Capacity[api.ResourceName(LabelNs+"/feature-1")] = *resource.NewQuantity(1, resource.BinarySI)
 			mockNode.Status.Capacity[api.ResourceName(LabelNs+"/feature-2")] = *resource.NewQuantity(2, resource.BinarySI)
-			resourceOps := getExtendedResourceOps(mockNode, mockResourceLabels)
+			resourceOps := createExtendedResourcePatches(mockNode, mockResourceLabels)
 			So(len(resourceOps), ShouldEqual, 0)
 		})
 		Convey("When the related label is gone", func() {
@@ -235,7 +235,7 @@ func TestRemovingExtResources(t *testing.T) {
 			mockNode.Annotations[AnnotationNs+"/extended-resources"] = "feature-4,feature-2"
 			mockNode.Status.Capacity[api.ResourceName(LabelNs+"/feature-4")] = *resource.NewQuantity(4, resource.BinarySI)
 			mockNode.Status.Capacity[api.ResourceName(LabelNs+"/feature-2")] = *resource.NewQuantity(2, resource.BinarySI)
-			resourceOps := getExtendedResourceOps(mockNode, mockResourceLabels)
+			resourceOps := createExtendedResourcePatches(mockNode, mockResourceLabels)
 			So(len(resourceOps), ShouldBeGreaterThan, 0)
 		})
 		Convey("When the extended resource is no longer wanted", func() {
@@ -244,7 +244,7 @@ func TestRemovingExtResources(t *testing.T) {
 			mockNode.Status.Capacity[api.ResourceName(LabelNs+"/feature-2")] = *resource.NewQuantity(2, resource.BinarySI)
 			mockResourceLabels := ExtendedResources{LabelNs + "/feature-2": "2"}
 			mockNode.Annotations[AnnotationNs+"/extended-resources"] = "feature-1,feature-2"
-			resourceOps := getExtendedResourceOps(mockNode, mockResourceLabels)
+			resourceOps := createExtendedResourcePatches(mockNode, mockResourceLabels)
 			So(len(resourceOps), ShouldBeGreaterThan, 0)
 		})
 	})

pkg/nfd-master/nfd-master.go

@@ -93,18 +93,6 @@ type nfdMaster struct {
 	apihelper apihelper.APIHelpers
 }
 
-// statusOp is a json marshaling helper used for patching node status
-type statusOp struct {
-	Op    string `json:"op"`
-	Path  string `json:"path"`
-	Value string `json:"value,omitempty"`
-}
-
-func createStatusOp(verb string, resource string, path string, value string) statusOp {
-	res := strings.ReplaceAll(resource, "/", "~1")
-	return statusOp{verb, "/status/" + path + "/" + res, value}
-}
-
 // Create new NfdMaster server instance.
 func NewNfdMaster(args Args) (NfdMaster, error) {
 	nfd := &nfdMaster{args: args, ready: make(chan bool, 1)}
@@ -397,7 +385,7 @@ func updateNodeFeatures(helper apihelper.APIHelpers, nodeName string, labels Lab
 	}
 
 	// Resolve publishable extended resources before node is modified
-	statusOps := getExtendedResourceOps(node, extendedResources)
+	patches := createExtendedResourcePatches(node, extendedResources)
 
 	// Remove old labels
 	if l, ok := node.Annotations[AnnotationNs+"/feature-labels"]; ok {
@@ -427,8 +415,8 @@ func updateNodeFeatures(helper apihelper.APIHelpers, nodeName string, labels Lab
 	}
 
 	// patch node status with extended resource changes
-	if len(statusOps) > 0 {
+	if len(patches) > 0 {
-		err = helper.PatchStatus(cli, node.Name, statusOps)
+		err = helper.PatchStatus(cli, node.Name, patches)
 		if err != nil {
 			stderrLogger.Printf("error while patching extended resources: %s", err.Error())
 			return err
@@ -454,9 +442,10 @@ func removeLabels(n *api.Node, labelNames []string) {
 	}
 }
 
-// getExtendedResourceOps returns a slice of operations to perform on the node status
+// createExtendedResourcePatches returns a slice of operations to perform on
-func getExtendedResourceOps(n *api.Node, extendedResources ExtendedResources) []statusOp {
+// the node status
-	var statusOps []statusOp
+func createExtendedResourcePatches(n *api.Node, extendedResources ExtendedResources) []apihelper.JsonPatch {
+	var patches []apihelper.JsonPatch
 
 	// Form a list of namespaced resource names managed by us
 	if old, ok := n.Annotations[AnnotationNs+"/extended-resources"]; ok {
@@ -471,8 +460,8 @@ func getExtendedResourceOps(n *api.Node, extendedResources ExtendedResources) []
 			if _, ok := n.Status.Capacity[api.ResourceName(resource)]; ok {
 				// check if the ext resource is still needed
 				if _, extResNeeded := extendedResources[resource]; !extResNeeded {
-					statusOps = append(statusOps, createStatusOp("remove", resource, "capacity", ""))
+					patches = append(patches, apihelper.NewJsonPatch("remove", "/status/capacity", resource, ""))
-					statusOps = append(statusOps, createStatusOp("remove", resource, "allocatable", ""))
+					patches = append(patches, apihelper.NewJsonPatch("remove", "/status/allocatable", resource, ""))
 				}
 			}
 		}
@@ -484,16 +473,16 @@ func getExtendedResourceOps(n *api.Node, extendedResources ExtendedResources) []
 		if quantity, ok := n.Status.Capacity[api.ResourceName(resource)]; ok {
 			val, _ := quantity.AsInt64()
 			if strconv.FormatInt(val, 10) != value {
-				statusOps = append(statusOps, createStatusOp("replace", resource, "capacity", value))
+				patches = append(patches, apihelper.NewJsonPatch("replace", "/status/capacity", resource, value))
-				statusOps = append(statusOps, createStatusOp("replace", resource, "allocatable", value))
+				patches = append(patches, apihelper.NewJsonPatch("replace", "/status/allocatable", resource, value))
 			}
 		} else {
-			statusOps = append(statusOps, createStatusOp("add", resource, "capacity", value))
+			patches = append(patches, apihelper.NewJsonPatch("add", "/status/capacity", resource, value))
 			// "allocatable" gets added implicitly after adding to capacity
 		}
 	}
 
-	return statusOps
+	return patches
 }
 
 // Add NFD labels to a Node object.