Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

pkg/utils: move JsonPatch from pkg/apihelper #1568

Merged
merged 1 commit into from
Jan 26, 2024
Merged

pkg/utils: move JsonPatch from pkg/apihelper #1568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions pkg/apihelper/apihelpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
topologyclientset "github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/generated/clientset/versioned"
corev1 "k8s.io/api/core/v1"
k8sclient "k8s.io/client-go/kubernetes"
"sigs.k8s.io/node-feature-discovery/pkg/utils"
)

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

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

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

// GetTopologyClient returns a topologyclientset
GetTopologyClient() (*topologyclientset.Clientset, error)
Expand Down
5 changes: 3 additions & 2 deletions pkg/apihelper/k8shelpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/types"
k8sclient "k8s.io/client-go/kubernetes"
restclient "k8s.io/client-go/rest"
"sigs.k8s.io/node-feature-discovery/pkg/utils"
)

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

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

func (h K8sHelpers) PatchNodeStatus(c *k8sclient.Clientset, nodeName string, patches []JsonPatch) error {
func (h K8sHelpers) PatchNodeStatus(c *k8sclient.Clientset, nodeName string, patches []utils.JsonPatch) error {
if len(patches) > 0 {
data, err := json.Marshal(patches)
if err == nil {
Expand Down
10 changes: 6 additions & 4 deletions pkg/apihelper/mock_APIHelpers.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

120 changes: 60 additions & 60 deletions pkg/nfd-master/nfd-master-internal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,9 @@ func TestUpdateNodeObject(t *testing.T) {
sort.Strings(fakeExtResourceNames)

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

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

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

mockAPIHelper.On("GetClient").Return(mockClient, nil)
Expand Down Expand Up @@ -244,7 +244,7 @@ func TestUpdateMasterNode(t *testing.T) {
mockClient := &k8sclient.Clientset{}
mockNode := newMockNode()
Convey("When update operation succeeds", func() {
expectedPatches := []apihelper.JsonPatch{}
expectedPatches := []utils.JsonPatch{}
mockHelper.On("GetClient").Return(mockClient, nil)
mockHelper.On("GetNode", mockClient, mockNodeName).Return(mockNode, nil)
mockHelper.On("PatchNode", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(expectedPatches))).Return(nil)
Expand Down Expand Up @@ -297,9 +297,9 @@ func TestAddingExtResources(t *testing.T) {
Convey("When there are matching labels", func() {
mockNode := newMockNode()
mockResourceLabels := ExtendedResources{"feature-1": "1", "feature-2": "2"}
expectedPatches := []apihelper.JsonPatch{
apihelper.NewJsonPatch("add", "/status/capacity", "feature-1", "1"),
apihelper.NewJsonPatch("add", "/status/capacity", "feature-2", "2"),
expectedPatches := []utils.JsonPatch{
utils.NewJsonPatch("add", "/status/capacity", "feature-1", "1"),
utils.NewJsonPatch("add", "/status/capacity", "feature-2", "2"),
}
patches := mockMaster.createExtendedResourcePatches(mockNode, mockResourceLabels)
So(sortJsonPatches(patches), ShouldResemble, sortJsonPatches(expectedPatches))
Expand All @@ -317,9 +317,9 @@ func TestAddingExtResources(t *testing.T) {
mockNode := newMockNode()
mockNode.Status.Capacity[corev1.ResourceName("feature-1")] = *resource.NewQuantity(2, resource.BinarySI)
mockResourceLabels := ExtendedResources{"feature-1": "1"}
expectedPatches := []apihelper.JsonPatch{
apihelper.NewJsonPatch("replace", "/status/capacity", "feature-1", "1"),
apihelper.NewJsonPatch("replace", "/status/allocatable", "feature-1", "1"),
expectedPatches := []utils.JsonPatch{
utils.NewJsonPatch("replace", "/status/capacity", "feature-1", "1"),
utils.NewJsonPatch("replace", "/status/allocatable", "feature-1", "1"),
}
patches := mockMaster.createExtendedResourcePatches(mockNode, mockResourceLabels)
So(sortJsonPatches(patches), ShouldResemble, sortJsonPatches(expectedPatches))
Expand Down Expand Up @@ -379,15 +379,15 @@ func TestSetLabels(t *testing.T) {
}
sort.Strings(mockLabelNames)

expectedStatusPatches := []apihelper.JsonPatch{}
expectedStatusPatches := []utils.JsonPatch{}

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

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

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

mockMaster.config.LabelWhiteList.Regexp = *regexp.MustCompile("^f.*2$")
Expand Down Expand Up @@ -434,14 +434,14 @@ func TestSetLabels(t *testing.T) {
vendorProfileLabel: "val-7",
"--invalid-name--": "valid-val",
"valid-name": "--invalid-val--"}
expectedPatches := []apihelper.JsonPatch{
apihelper.NewJsonPatch("add", "/metadata/annotations",
expectedPatches := []utils.JsonPatch{
utils.NewJsonPatch("add", "/metadata/annotations",
instance+"."+nfdv1alpha1.FeatureLabelsAnnotation,
"feature-1,valid.ns/feature-2,"+vendorFeatureLabel+","+vendorProfileLabel),
apihelper.NewJsonPatch("add", "/metadata/labels", "feature.node.kubernetes.io/feature-1", mockLabels["feature.node.kubernetes.io/feature-1"]),
apihelper.NewJsonPatch("add", "/metadata/labels", "valid.ns/feature-2", mockLabels["valid.ns/feature-2"]),
apihelper.NewJsonPatch("add", "/metadata/labels", vendorFeatureLabel, mockLabels[vendorFeatureLabel]),
apihelper.NewJsonPatch("add", "/metadata/labels", vendorProfileLabel, mockLabels[vendorProfileLabel]),
utils.NewJsonPatch("add", "/metadata/labels", "feature.node.kubernetes.io/feature-1", mockLabels["feature.node.kubernetes.io/feature-1"]),
utils.NewJsonPatch("add", "/metadata/labels", "valid.ns/feature-2", mockLabels["valid.ns/feature-2"]),
utils.NewJsonPatch("add", "/metadata/labels", vendorFeatureLabel, mockLabels[vendorFeatureLabel]),
utils.NewJsonPatch("add", "/metadata/labels", vendorProfileLabel, mockLabels[vendorProfileLabel]),
}

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

Convey("When -resource-labels is specified", func() {
mockMaster.config.ExtraLabelNs = map[string]struct{}{"example.io": {}}
expectedPatches := []apihelper.JsonPatch{
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureLabelsAnnotation, "example.io/feature-2"),
apihelper.NewJsonPatch("add", "/metadata/labels", "example.io/feature-2", mockLabels["example.io/feature-2"]),
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.ExtendedResourceAnnotation, "feature-1,feature-3"),
expectedPatches := []utils.JsonPatch{
utils.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureLabelsAnnotation, "example.io/feature-2"),
utils.NewJsonPatch("add", "/metadata/labels", "example.io/feature-2", mockLabels["example.io/feature-2"]),
utils.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.ExtendedResourceAnnotation, "feature-1,feature-3"),
}
expectedStatusPatches := []apihelper.JsonPatch{
apihelper.NewJsonPatch("add", "/status/capacity", "feature.node.kubernetes.io/feature-1", mockLabels["feature.node.kubernetes.io/feature-1"]),
apihelper.NewJsonPatch("add", "/status/capacity", "feature.node.kubernetes.io/feature-3", mockLabels["feature.node.kubernetes.io/feature-3"]),
expectedStatusPatches := []utils.JsonPatch{
utils.NewJsonPatch("add", "/status/capacity", "feature.node.kubernetes.io/feature-1", mockLabels["feature.node.kubernetes.io/feature-1"]),
utils.NewJsonPatch("add", "/status/capacity", "feature.node.kubernetes.io/feature-3", mockLabels["feature.node.kubernetes.io/feature-3"]),
}

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

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

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

Convey("When when there are items to remove add and update", func() {
newItems := map[string]string{"new-key": "new-val", "key-2": "new-2", "key-4": "val-4"}
p := createPatches([]string{"key-1", "key-2", "key-3", "foo"}, existingItems, newItems, jsonPath)
expected := []apihelper.JsonPatch{
apihelper.NewJsonPatch("add", jsonPath, "new-key", newItems["new-key"]),
apihelper.NewJsonPatch("add", jsonPath, "key-4", newItems["key-4"]),
apihelper.NewJsonPatch("replace", jsonPath, "key-2", newItems["key-2"]),
apihelper.NewJsonPatch("remove", jsonPath, "key-1", ""),
apihelper.NewJsonPatch("remove", jsonPath, "key-3", ""),
expected := []utils.JsonPatch{
utils.NewJsonPatch("add", jsonPath, "new-key", newItems["new-key"]),
utils.NewJsonPatch("add", jsonPath, "key-4", newItems["key-4"]),
utils.NewJsonPatch("replace", jsonPath, "key-2", newItems["key-2"]),
utils.NewJsonPatch("remove", jsonPath, "key-1", ""),
utils.NewJsonPatch("remove", jsonPath, "key-3", ""),
}
So(sortJsonPatches(p), ShouldResemble, sortJsonPatches(expected))
})
Expand All @@ -651,14 +651,14 @@ func TestRemoveLabelsWithPrefix(t *testing.T) {

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

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

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

mockClient := &k8sclient.Clientset{}

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

Expand Down Expand Up @@ -909,8 +909,8 @@ func withTimeout(actual interface{}, expected ...interface{}) string {
}
}

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

func sortJsonPatches(p []apihelper.JsonPatch) []apihelper.JsonPatch {
func sortJsonPatches(p []utils.JsonPatch) []utils.JsonPatch {
sort.Slice(p, func(i, j int) bool { return p[i].Path < p[j].Path })
return p
}

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

for k := range n.Labels {
if strings.HasPrefix(k, search) {
p = append(p, apihelper.NewJsonPatch("remove", "/metadata/labels", k, ""))
p = append(p, utils.NewJsonPatch("remove", "/metadata/labels", k, ""))
}
}

Expand Down
Loading