Skip to content

Commit 43a9239

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #1568 from marquiz/devel/apihelper-refactor-4
pkg/utils: move JsonPatch from pkg/apihelper
2 parents 5d30be1 + 53003cb commit 43a9239

File tree

8 files changed

+91
-86
lines changed

8 files changed

+91
-86
lines changed

‎pkg/apihelper/apihelpers.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
topologyclientset "github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/generated/clientset/versioned"
2323
corev1 "k8s.io/api/core/v1"
2424
k8sclient "k8s.io/client-go/kubernetes"
25+
"sigs.k8s.io/node-feature-discovery/pkg/utils"
2526
)
2627

2728
// APIHelpers represents a set of API helpers for Kubernetes
@@ -39,10 +40,10 @@ type APIHelpers interface {
3940
UpdateNode(*k8sclient.Clientset, *corev1.Node) error
4041

4142
// PatchNode updates the node object via the API server using a client.
42-
PatchNode(*k8sclient.Clientset, string, []JsonPatch) error
43+
PatchNode(*k8sclient.Clientset, string, []utils.JsonPatch) error
4344

4445
// PatchNodeStatus updates the node status via the API server using a client.
45-
PatchNodeStatus(*k8sclient.Clientset, string, []JsonPatch) error
46+
PatchNodeStatus(*k8sclient.Clientset, string, []utils.JsonPatch) error
4647

4748
// GetTopologyClient returns a topologyclientset
4849
GetTopologyClient() (*topologyclientset.Clientset, error)

‎pkg/apihelper/k8shelpers.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"k8s.io/apimachinery/pkg/types"
2727
k8sclient "k8s.io/client-go/kubernetes"
2828
restclient "k8s.io/client-go/rest"
29+
"sigs.k8s.io/node-feature-discovery/pkg/utils"
2930
)
3031

3132
// K8sHelpers implements APIHelpers
@@ -77,7 +78,7 @@ func (h K8sHelpers) UpdateNode(c *k8sclient.Clientset, n *corev1.Node) error {
7778
return nil
7879
}
7980

80-
func (h K8sHelpers) PatchNode(c *k8sclient.Clientset, nodeName string, patches []JsonPatch) error {
81+
func (h K8sHelpers) PatchNode(c *k8sclient.Clientset, nodeName string, patches []utils.JsonPatch) error {
8182
if len(patches) > 0 {
8283
data, err := json.Marshal(patches)
8384
if err == nil {
@@ -88,7 +89,7 @@ func (h K8sHelpers) PatchNode(c *k8sclient.Clientset, nodeName string, patches [
8889
return nil
8990
}
9091

91-
func (h K8sHelpers) PatchNodeStatus(c *k8sclient.Clientset, nodeName string, patches []JsonPatch) error {
92+
func (h K8sHelpers) PatchNodeStatus(c *k8sclient.Clientset, nodeName string, patches []utils.JsonPatch) error {
9293
if len(patches) > 0 {
9394
data, err := json.Marshal(patches)
9495
if err == nil {

‎pkg/apihelper/mock_APIHelpers.go

Lines changed: 6 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 60 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -151,9 +151,9 @@ func TestUpdateNodeObject(t *testing.T) {
151151
sort.Strings(fakeExtResourceNames)
152152

153153
// Create a list of expected node status patches
154-
statusPatches := []apihelper.JsonPatch{}
154+
statusPatches := []utils.JsonPatch{}
155155
for k, v := range fakeExtResources {
156-
statusPatches = append(statusPatches, apihelper.NewJsonPatch("add", "/status/capacity", k, v))
156+
statusPatches = append(statusPatches, utils.NewJsonPatch("add", "/status/capacity", k, v))
157157
}
158158

159159
mockAPIHelper := new(apihelper.MockAPIHelpers)
@@ -166,17 +166,17 @@ func TestUpdateNodeObject(t *testing.T) {
166166

167167
Convey("When I successfully update the node with feature labels", func() {
168168
// Create a list of expected node metadata patches
169-
metadataPatches := []apihelper.JsonPatch{
170-
apihelper.NewJsonPatch("replace", "/metadata/annotations", nfdv1alpha1.AnnotationNs+"/feature-labels", strings.Join(fakeFeatureLabelNames, ",")),
171-
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureAnnotationsTrackingAnnotation, strings.Join(fakeFeatureAnnotationsNames, ",")),
172-
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.AnnotationNs+"/extended-resources", strings.Join(fakeExtResourceNames, ",")),
173-
apihelper.NewJsonPatch("remove", "/metadata/labels", nfdv1alpha1.FeatureLabelNs+"/old-feature", ""),
169+
metadataPatches := []utils.JsonPatch{
170+
utils.NewJsonPatch("replace", "/metadata/annotations", nfdv1alpha1.AnnotationNs+"/feature-labels", strings.Join(fakeFeatureLabelNames, ",")),
171+
utils.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureAnnotationsTrackingAnnotation, strings.Join(fakeFeatureAnnotationsNames, ",")),
172+
utils.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.AnnotationNs+"/extended-resources", strings.Join(fakeExtResourceNames, ",")),
173+
utils.NewJsonPatch("remove", "/metadata/labels", nfdv1alpha1.FeatureLabelNs+"/old-feature", ""),
174174
}
175175
for k, v := range fakeFeatureLabels {
176-
metadataPatches = append(metadataPatches, apihelper.NewJsonPatch("add", "/metadata/labels", k, v))
176+
metadataPatches = append(metadataPatches, utils.NewJsonPatch("add", "/metadata/labels", k, v))
177177
}
178178
for k, v := range fakeAnnotations {
179-
metadataPatches = append(metadataPatches, apihelper.NewJsonPatch("add", "/metadata/annotations", k, v))
179+
metadataPatches = append(metadataPatches, utils.NewJsonPatch("add", "/metadata/annotations", k, v))
180180
}
181181

182182
mockAPIHelper.On("GetClient").Return(mockClient, nil)
@@ -244,7 +244,7 @@ func TestUpdateMasterNode(t *testing.T) {
244244
mockClient := &k8sclient.Clientset{}
245245
mockNode := newMockNode()
246246
Convey("When update operation succeeds", func() {
247-
expectedPatches := []apihelper.JsonPatch{}
247+
expectedPatches := []utils.JsonPatch{}
248248
mockHelper.On("GetClient").Return(mockClient, nil)
249249
mockHelper.On("GetNode", mockClient, mockNodeName).Return(mockNode, nil)
250250
mockHelper.On("PatchNode", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(expectedPatches))).Return(nil)
@@ -297,9 +297,9 @@ func TestAddingExtResources(t *testing.T) {
297297
Convey("When there are matching labels", func() {
298298
mockNode := newMockNode()
299299
mockResourceLabels := ExtendedResources{"feature-1": "1", "feature-2": "2"}
300-
expectedPatches := []apihelper.JsonPatch{
301-
apihelper.NewJsonPatch("add", "/status/capacity", "feature-1", "1"),
302-
apihelper.NewJsonPatch("add", "/status/capacity", "feature-2", "2"),
300+
expectedPatches := []utils.JsonPatch{
301+
utils.NewJsonPatch("add", "/status/capacity", "feature-1", "1"),
302+
utils.NewJsonPatch("add", "/status/capacity", "feature-2", "2"),
303303
}
304304
patches := mockMaster.createExtendedResourcePatches(mockNode, mockResourceLabels)
305305
So(sortJsonPatches(patches), ShouldResemble, sortJsonPatches(expectedPatches))
@@ -317,9 +317,9 @@ func TestAddingExtResources(t *testing.T) {
317317
mockNode := newMockNode()
318318
mockNode.Status.Capacity[corev1.ResourceName("feature-1")] = *resource.NewQuantity(2, resource.BinarySI)
319319
mockResourceLabels := ExtendedResources{"feature-1": "1"}
320-
expectedPatches := []apihelper.JsonPatch{
321-
apihelper.NewJsonPatch("replace", "/status/capacity", "feature-1", "1"),
322-
apihelper.NewJsonPatch("replace", "/status/allocatable", "feature-1", "1"),
320+
expectedPatches := []utils.JsonPatch{
321+
utils.NewJsonPatch("replace", "/status/capacity", "feature-1", "1"),
322+
utils.NewJsonPatch("replace", "/status/allocatable", "feature-1", "1"),
323323
}
324324
patches := mockMaster.createExtendedResourcePatches(mockNode, mockResourceLabels)
325325
So(sortJsonPatches(patches), ShouldResemble, sortJsonPatches(expectedPatches))
@@ -379,15 +379,15 @@ func TestSetLabels(t *testing.T) {
379379
}
380380
sort.Strings(mockLabelNames)
381381

382-
expectedStatusPatches := []apihelper.JsonPatch{}
382+
expectedStatusPatches := []utils.JsonPatch{}
383383

384384
Convey("When node update succeeds", func() {
385385
mockMaster.config.ExtraLabelNs = map[string]struct{}{"example.io": {}}
386-
expectedPatches := []apihelper.JsonPatch{
387-
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureLabelsAnnotation, strings.Join(mockLabelNames, ",")),
386+
expectedPatches := []utils.JsonPatch{
387+
utils.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureLabelsAnnotation, strings.Join(mockLabelNames, ",")),
388388
}
389389
for k, v := range mockLabels {
390-
expectedPatches = append(expectedPatches, apihelper.NewJsonPatch("add", "/metadata/labels", k, v))
390+
expectedPatches = append(expectedPatches, utils.NewJsonPatch("add", "/metadata/labels", k, v))
391391
}
392392

393393
mockHelper.On("GetClient").Return(mockClient, nil)
@@ -402,9 +402,9 @@ func TestSetLabels(t *testing.T) {
402402

403403
Convey("When -label-whitelist is specified", func() {
404404
mockMaster.config.ExtraLabelNs = map[string]struct{}{"example.io": {}}
405-
expectedPatches := []apihelper.JsonPatch{
406-
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureLabelsAnnotation, "example.io/feature-2"),
407-
apihelper.NewJsonPatch("add", "/metadata/labels", "example.io/feature-2", mockLabels["example.io/feature-2"]),
405+
expectedPatches := []utils.JsonPatch{
406+
utils.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureLabelsAnnotation, "example.io/feature-2"),
407+
utils.NewJsonPatch("add", "/metadata/labels", "example.io/feature-2", mockLabels["example.io/feature-2"]),
408408
}
409409

410410
mockMaster.config.LabelWhiteList.Regexp = *regexp.MustCompile("^f.*2$")
@@ -434,14 +434,14 @@ func TestSetLabels(t *testing.T) {
434434
vendorProfileLabel: "val-7",
435435
"--invalid-name--": "valid-val",
436436
"valid-name": "--invalid-val--"}
437-
expectedPatches := []apihelper.JsonPatch{
438-
apihelper.NewJsonPatch("add", "/metadata/annotations",
437+
expectedPatches := []utils.JsonPatch{
438+
utils.NewJsonPatch("add", "/metadata/annotations",
439439
instance+"."+nfdv1alpha1.FeatureLabelsAnnotation,
440440
"feature-1,valid.ns/feature-2,"+vendorFeatureLabel+","+vendorProfileLabel),
441-
apihelper.NewJsonPatch("add", "/metadata/labels", "feature.node.kubernetes.io/feature-1", mockLabels["feature.node.kubernetes.io/feature-1"]),
442-
apihelper.NewJsonPatch("add", "/metadata/labels", "valid.ns/feature-2", mockLabels["valid.ns/feature-2"]),
443-
apihelper.NewJsonPatch("add", "/metadata/labels", vendorFeatureLabel, mockLabels[vendorFeatureLabel]),
444-
apihelper.NewJsonPatch("add", "/metadata/labels", vendorProfileLabel, mockLabels[vendorProfileLabel]),
441+
utils.NewJsonPatch("add", "/metadata/labels", "feature.node.kubernetes.io/feature-1", mockLabels["feature.node.kubernetes.io/feature-1"]),
442+
utils.NewJsonPatch("add", "/metadata/labels", "valid.ns/feature-2", mockLabels["valid.ns/feature-2"]),
443+
utils.NewJsonPatch("add", "/metadata/labels", vendorFeatureLabel, mockLabels[vendorFeatureLabel]),
444+
utils.NewJsonPatch("add", "/metadata/labels", vendorProfileLabel, mockLabels[vendorProfileLabel]),
445445
}
446446

447447
mockMaster.deniedNs.normal = map[string]struct{}{"random.denied.ns": {}}
@@ -461,14 +461,14 @@ func TestSetLabels(t *testing.T) {
461461

462462
Convey("When -resource-labels is specified", func() {
463463
mockMaster.config.ExtraLabelNs = map[string]struct{}{"example.io": {}}
464-
expectedPatches := []apihelper.JsonPatch{
465-
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureLabelsAnnotation, "example.io/feature-2"),
466-
apihelper.NewJsonPatch("add", "/metadata/labels", "example.io/feature-2", mockLabels["example.io/feature-2"]),
467-
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.ExtendedResourceAnnotation, "feature-1,feature-3"),
464+
expectedPatches := []utils.JsonPatch{
465+
utils.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureLabelsAnnotation, "example.io/feature-2"),
466+
utils.NewJsonPatch("add", "/metadata/labels", "example.io/feature-2", mockLabels["example.io/feature-2"]),
467+
utils.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.ExtendedResourceAnnotation, "feature-1,feature-3"),
468468
}
469-
expectedStatusPatches := []apihelper.JsonPatch{
470-
apihelper.NewJsonPatch("add", "/status/capacity", "feature.node.kubernetes.io/feature-1", mockLabels["feature.node.kubernetes.io/feature-1"]),
471-
apihelper.NewJsonPatch("add", "/status/capacity", "feature.node.kubernetes.io/feature-3", mockLabels["feature.node.kubernetes.io/feature-3"]),
469+
expectedStatusPatches := []utils.JsonPatch{
470+
utils.NewJsonPatch("add", "/status/capacity", "feature.node.kubernetes.io/feature-1", mockLabels["feature.node.kubernetes.io/feature-1"]),
471+
utils.NewJsonPatch("add", "/status/capacity", "feature.node.kubernetes.io/feature-3", mockLabels["feature.node.kubernetes.io/feature-3"]),
472472
}
473473

474474
mockMaster.config.ResourceLabels = map[string]struct{}{"feature.node.kubernetes.io/feature-3": {}, "feature-1": {}}
@@ -605,32 +605,32 @@ func TestCreatePatches(t *testing.T) {
605605

606606
Convey("When when there are itmes to remoe but none to add or update", func() {
607607
p := createPatches([]string{"key-2", "key-3", "foo"}, existingItems, map[string]string{}, jsonPath)
608-
expected := []apihelper.JsonPatch{
609-
apihelper.NewJsonPatch("remove", jsonPath, "key-2", ""),
610-
apihelper.NewJsonPatch("remove", jsonPath, "key-3", ""),
608+
expected := []utils.JsonPatch{
609+
utils.NewJsonPatch("remove", jsonPath, "key-2", ""),
610+
utils.NewJsonPatch("remove", jsonPath, "key-3", ""),
611611
}
612612
So(sortJsonPatches(p), ShouldResemble, sortJsonPatches(expected))
613613
})
614614

615615
Convey("When when there are no itmes to remove but new items to add", func() {
616616
newItems := map[string]string{"new-key": "new-val", "key-1": "new-1"}
617617
p := createPatches([]string{"key-1"}, existingItems, newItems, jsonPath)
618-
expected := []apihelper.JsonPatch{
619-
apihelper.NewJsonPatch("add", jsonPath, "new-key", newItems["new-key"]),
620-
apihelper.NewJsonPatch("replace", jsonPath, "key-1", newItems["key-1"]),
618+
expected := []utils.JsonPatch{
619+
utils.NewJsonPatch("add", jsonPath, "new-key", newItems["new-key"]),
620+
utils.NewJsonPatch("replace", jsonPath, "key-1", newItems["key-1"]),
621621
}
622622
So(sortJsonPatches(p), ShouldResemble, sortJsonPatches(expected))
623623
})
624624

625625
Convey("When when there are items to remove add and update", func() {
626626
newItems := map[string]string{"new-key": "new-val", "key-2": "new-2", "key-4": "val-4"}
627627
p := createPatches([]string{"key-1", "key-2", "key-3", "foo"}, existingItems, newItems, jsonPath)
628-
expected := []apihelper.JsonPatch{
629-
apihelper.NewJsonPatch("add", jsonPath, "new-key", newItems["new-key"]),
630-
apihelper.NewJsonPatch("add", jsonPath, "key-4", newItems["key-4"]),
631-
apihelper.NewJsonPatch("replace", jsonPath, "key-2", newItems["key-2"]),
632-
apihelper.NewJsonPatch("remove", jsonPath, "key-1", ""),
633-
apihelper.NewJsonPatch("remove", jsonPath, "key-3", ""),
628+
expected := []utils.JsonPatch{
629+
utils.NewJsonPatch("add", jsonPath, "new-key", newItems["new-key"]),
630+
utils.NewJsonPatch("add", jsonPath, "key-4", newItems["key-4"]),
631+
utils.NewJsonPatch("replace", jsonPath, "key-2", newItems["key-2"]),
632+
utils.NewJsonPatch("remove", jsonPath, "key-1", ""),
633+
utils.NewJsonPatch("remove", jsonPath, "key-3", ""),
634634
}
635635
So(sortJsonPatches(p), ShouldResemble, sortJsonPatches(expected))
636636
})
@@ -651,14 +651,14 @@ func TestRemoveLabelsWithPrefix(t *testing.T) {
651651

652652
Convey("a unique label should be removed", func() {
653653
p := removeLabelsWithPrefix(n, "single")
654-
So(p, ShouldResemble, []apihelper.JsonPatch{apihelper.NewJsonPatch("remove", "/metadata/labels", "single-label", "")})
654+
So(p, ShouldResemble, []utils.JsonPatch{utils.NewJsonPatch("remove", "/metadata/labels", "single-label", "")})
655655
})
656656

657657
Convey("a non-unique search string should remove all matching keys", func() {
658658
p := removeLabelsWithPrefix(n, "multiple")
659-
So(sortJsonPatches(p), ShouldResemble, sortJsonPatches([]apihelper.JsonPatch{
660-
apihelper.NewJsonPatch("remove", "/metadata/labels", "multiple_A", ""),
661-
apihelper.NewJsonPatch("remove", "/metadata/labels", "multiple_B", ""),
659+
So(sortJsonPatches(p), ShouldResemble, sortJsonPatches([]utils.JsonPatch{
660+
utils.NewJsonPatch("remove", "/metadata/labels", "multiple_A", ""),
661+
utils.NewJsonPatch("remove", "/metadata/labels", "multiple_B", ""),
662662
}))
663663
})
664664

@@ -851,8 +851,8 @@ func BenchmarkNfdAPIUpdateAllNodes(b *testing.B) {
851851

852852
mockClient := &k8sclient.Clientset{}
853853

854-
statusPatches := []apihelper.JsonPatch{}
855-
metadataPatches := []apihelper.JsonPatch{
854+
statusPatches := []utils.JsonPatch{}
855+
metadataPatches := []utils.JsonPatch{
856856
{Op: "add", Path: "/metadata/annotations/nfd.node.kubernetes.io~1feature-labels", Value: ""}, {Op: "add", Path: "/metadata/annotations/nfd.node.kubernetes.io~1extended-resources", Value: ""},
857857
}
858858

@@ -909,8 +909,8 @@ func withTimeout(actual interface{}, expected ...interface{}) string {
909909
}
910910
}
911911

912-
func jsonPatchMatcher(expected []apihelper.JsonPatch) func([]apihelper.JsonPatch) bool {
913-
return func(actual []apihelper.JsonPatch) bool {
912+
func jsonPatchMatcher(expected []utils.JsonPatch) func([]utils.JsonPatch) bool {
913+
return func(actual []utils.JsonPatch) bool {
914914
// We don't care about modifying the original slices
915915
ok, msg := assertions.So(sortJsonPatches(actual), ShouldResemble, sortJsonPatches(expected))
916916
if !ok {
@@ -926,18 +926,18 @@ func jsonPatchMatcher(expected []apihelper.JsonPatch) func([]apihelper.JsonPatch
926926
}
927927
}
928928

929-
func sortJsonPatches(p []apihelper.JsonPatch) []apihelper.JsonPatch {
929+
func sortJsonPatches(p []utils.JsonPatch) []utils.JsonPatch {
930930
sort.Slice(p, func(i, j int) bool { return p[i].Path < p[j].Path })
931931
return p
932932
}
933933

934934
// Remove any labels having the given prefix
935-
func removeLabelsWithPrefix(n *corev1.Node, search string) []apihelper.JsonPatch {
936-
var p []apihelper.JsonPatch
935+
func removeLabelsWithPrefix(n *corev1.Node, search string) []utils.JsonPatch {
936+
var p []utils.JsonPatch
937937

938938
for k := range n.Labels {
939939
if strings.HasPrefix(k, search) {
940-
p = append(p, apihelper.NewJsonPatch("remove", "/metadata/labels", k, ""))
940+
p = append(p, utils.NewJsonPatch("remove", "/metadata/labels", k, ""))
941941
}
942942
}
943943

0 commit comments

Comments
 (0)