From 1ea301d272257141caf6a5ee5a54a5b50d2a0bac Mon Sep 17 00:00:00 2001 From: Markus Lehtonen Date: Thu, 13 Aug 2020 22:19:16 +0300 Subject: [PATCH] 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 | 34 ++++++++++++++++++++ pkg/nfd-master/nfd-master-internal_test.go | 14 ++++---- pkg/nfd-master/nfd-master.go | 37 ++++++++-------------- 3 files changed, 54 insertions(+), 31 deletions(-) create mode 100644 pkg/apihelper/jsonpatch.go diff --git a/pkg/apihelper/jsonpatch.go b/pkg/apihelper/jsonpatch.go new file mode 100644 index 000000000..0939cc8b6 --- /dev/null +++ b/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} +} diff --git a/pkg/nfd-master/nfd-master-internal_test.go b/pkg/nfd-master/nfd-master-internal_test.go index 8476d098e..81f80e273 100644 --- a/pkg/nfd-master/nfd-master-internal_test.go +++ b/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) }) }) diff --git a/pkg/nfd-master/nfd-master.go b/pkg/nfd-master/nfd-master.go index 07fcdd4f8..668c836cf 100644 --- a/pkg/nfd-master/nfd-master.go +++ b/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 { - err = helper.PatchStatus(cli, node.Name, statusOps) + if len(patches) > 0 { + 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 -func getExtendedResourceOps(n *api.Node, extendedResources ExtendedResources) []statusOp { - var statusOps []statusOp +// createExtendedResourcePatches returns a slice of operations to perform on +// the node status +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", "")) - statusOps = append(statusOps, createStatusOp("remove", resource, "allocatable", "")) + patches = append(patches, apihelper.NewJsonPatch("remove", "/status/capacity", resource, "")) + 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)) - statusOps = append(statusOps, createStatusOp("replace", resource, "allocatable", value)) + patches = append(patches, apihelper.NewJsonPatch("replace", "/status/capacity", resource, 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.