Skip to content

Commit e5b6f36

Browse files
authored
Merge pull request #291 from minmzzhang/unit-test-GetConfig
unit test for GetConfig
2 parents 42613cd + 3b6ce2a commit e5b6f36

File tree

5 files changed

+261
-15
lines changed

5 files changed

+261
-15
lines changed

go.mod

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ require (
9191
google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094 // indirect
9292
google.golang.org/grpc v1.65.0 // indirect
9393
google.golang.org/protobuf v1.35.1 // indirect
94-
gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect
9594
gopkg.in/inf.v0 v0.9.1 // indirect
9695
gopkg.in/yaml.v2 v2.4.0 // indirect
9796
gopkg.in/yaml.v3 v3.0.1 // indirect

internal/plugin/mellanox.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ func GetCombinedResourceNames(rs []SrIoVResource) string {
102102
// template: https://github.com/Mellanox/network-operator/blob/ce089f067153ea73b4712f0ad905fea92a1cf453/manifests/state-hostdevice-network/0010-hostdevice-net-cr.yml#L11
103103
type HostDeviceTypeNetConf struct {
104104
types.NetConf
105+
DeviceID string `json:"deviceID,omitempty"`
106+
IPAM map[string]interface{} `json:"ipam,omitempty"`
105107
}
106108

107109
func (p *MellanoxPlugin) Init(config *rest.Config) error {
@@ -130,10 +132,13 @@ func (p *MellanoxPlugin) GetConfig(net multinicv1.MultiNicNetwork, hifList map[s
130132
vars.NetworkLog.V(2).Info(msg)
131133
return "", annotation, errors.New(msg)
132134
}
133-
conf := HostDeviceTypeNetConf{}
134-
conf.CNIVersion = net.Spec.MainPlugin.CNIVersion
135-
conf.Type = HOST_DEVICE_TYPE
136-
conf.Name = net.Name
135+
conf := HostDeviceTypeNetConf{
136+
NetConf: types.NetConf{
137+
CNIVersion: net.Spec.MainPlugin.CNIVersion,
138+
Type: HOST_DEVICE_TYPE,
139+
Name: net.ObjectMeta.Name,
140+
},
141+
}
137142
err = json.Unmarshal([]byte(net.Spec.IPAM), &conf.IPAM)
138143
if err != nil {
139144
return "", annotation, err
@@ -158,7 +163,7 @@ func (p *MellanoxPlugin) GetSrIoVResources() (rs []SrIoVResource) {
158163
}
159164
sriovPlugin := policy.Spec.SriovDevicePlugin
160165
if sriovPlugin == nil || sriovPlugin.Config == nil {
161-
vars.NetworkLog.V(2).Info(fmt.Sprintf("no sriov device plugin config set in %s", policy.Name))
166+
vars.NetworkLog.V(2).Info(fmt.Sprintf("no sriov device plugin config set in %s", policy.ObjectMeta.Name))
162167
return rs
163168
}
164169
rs, err = GetSrIoVResourcesFromSrIoVPlugin(sriovPlugin)

internal/plugin/mellanox_test.go

Lines changed: 237 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,107 @@
66
package plugin_test
77

88
import (
9+
"encoding/json"
10+
"errors"
11+
"fmt"
12+
13+
"github.com/containernetworking/cni/pkg/types"
14+
multinicv1 "github.com/foundation-model-stack/multi-nic-cni/api/v1"
915
. "github.com/foundation-model-stack/multi-nic-cni/internal/plugin"
16+
"github.com/foundation-model-stack/multi-nic-cni/internal/vars"
1017
. "github.com/onsi/ginkgo/v2"
1118
. "github.com/onsi/gomega"
19+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1220
"k8s.io/apimachinery/pkg/runtime/schema"
1321
"k8s.io/client-go/rest"
1422
)
1523

24+
type testMellanoxPlugin struct {
25+
*MellanoxPlugin
26+
mockSriovPlugin *DevicePluginSpec
27+
}
28+
29+
func (p *testMellanoxPlugin) getPolicy() (*NicClusterPolicy, error) {
30+
return &NicClusterPolicy{
31+
ObjectMeta: metav1.ObjectMeta{
32+
Name: "test-policy",
33+
},
34+
Spec: NicClusterPolicySpec{
35+
SriovDevicePlugin: p.mockSriovPlugin,
36+
},
37+
}, nil
38+
}
39+
40+
func (p *testMellanoxPlugin) GetSrIoVResources() []SrIoVResource {
41+
policy, err := p.getPolicy()
42+
if err != nil {
43+
vars.NetworkLog.V(2).Info(fmt.Sprintf("failed to get policy: %v", err))
44+
return nil
45+
}
46+
if policy.Spec.SriovDevicePlugin == nil || policy.Spec.SriovDevicePlugin.Config == nil {
47+
vars.NetworkLog.V(2).Info(fmt.Sprintf("no sriov device plugin config set in %s", policy.ObjectMeta.Name))
48+
return nil
49+
}
50+
rs, err := GetSrIoVResourcesFromSrIoVPlugin(policy.Spec.SriovDevicePlugin)
51+
if err != nil || len(rs) == 0 {
52+
vars.NetworkLog.V(2).Info(fmt.Sprintf("no readable value from sriov config (%s): %v", *policy.Spec.SriovDevicePlugin.Config, err))
53+
}
54+
return rs
55+
}
56+
57+
func (p *testMellanoxPlugin) GetConfig(net multinicv1.MultiNicNetwork, hifList map[string]multinicv1.HostInterface) (string, map[string]string, error) {
58+
annotation := make(map[string]string)
59+
var err error
60+
// get resource from nicclusterpolicy
61+
rs := p.GetSrIoVResources()
62+
resourceAnnotation := GetCombinedResourceNames(rs)
63+
if resourceAnnotation == "" {
64+
msg := "failed to get resource annotation from sriov plugin config"
65+
vars.NetworkLog.V(2).Info(msg)
66+
return "", annotation, errors.New(msg)
67+
}
68+
conf := HostDeviceTypeNetConf{
69+
NetConf: types.NetConf{
70+
CNIVersion: net.Spec.MainPlugin.CNIVersion,
71+
Type: HOST_DEVICE_TYPE,
72+
Name: net.ObjectMeta.Name,
73+
},
74+
}
75+
err = json.Unmarshal([]byte(net.Spec.IPAM), &conf.IPAM)
76+
if err != nil {
77+
return "", annotation, err
78+
}
79+
confBytes, err := json.Marshal(conf)
80+
if err != nil {
81+
return "", annotation, err
82+
}
83+
annotation[RESOURCE_ANNOTATION] = resourceAnnotation
84+
return string(confBytes), annotation, nil
85+
}
86+
1687
var _ = Describe("Mellanox Plugin", func() {
1788
var (
18-
plugin *MellanoxPlugin
89+
plugin *testMellanoxPlugin
1990
testConfig *rest.Config
2091
)
2192

2293
BeforeEach(func() {
23-
plugin = &MellanoxPlugin{}
94+
config := `{
95+
"resourceList": [
96+
{
97+
"resourcePrefix": "nvidia.com",
98+
"resourceName": "test-resource"
99+
}
100+
]
101+
}`
102+
plugin = &testMellanoxPlugin{
103+
MellanoxPlugin: &MellanoxPlugin{},
104+
mockSriovPlugin: &DevicePluginSpec{
105+
ImageSpecWithConfig: ImageSpecWithConfig{
106+
Config: &config,
107+
},
108+
},
109+
}
24110
testConfig = &rest.Config{
25111
Host: "https://localhost:8443",
26112
TLSClientConfig: rest.TLSClientConfig{
@@ -74,4 +160,153 @@ var _ = Describe("Mellanox Plugin", func() {
74160
})
75161
})
76162
})
163+
164+
Context("GetConfig", func() {
165+
var (
166+
net multinicv1.MultiNicNetwork
167+
hifList map[string]multinicv1.HostInterface
168+
)
169+
170+
BeforeEach(func() {
171+
// Initialize plugin
172+
err := plugin.Init(testConfig)
173+
Expect(err).NotTo(HaveOccurred())
174+
175+
// Setup test network
176+
net = multinicv1.MultiNicNetwork{
177+
ObjectMeta: metav1.ObjectMeta{
178+
Name: "test-network",
179+
},
180+
Spec: multinicv1.MultiNicNetworkSpec{
181+
MainPlugin: multinicv1.PluginSpec{
182+
CNIVersion: "0.3.1",
183+
Type: MELLANOX_TYPE,
184+
},
185+
IPAM: `{
186+
"type": "host-local",
187+
"subnet": "10.0.0.0/24",
188+
"rangeStart": "10.0.0.10",
189+
"rangeEnd": "10.0.0.20"
190+
}`,
191+
},
192+
}
193+
194+
hifList = make(map[string]multinicv1.HostInterface)
195+
})
196+
197+
It("should generate valid CNI config", func() {
198+
// Verify GetSrIoVResources returns expected resources
199+
resources := plugin.GetSrIoVResources()
200+
Expect(resources).To(HaveLen(1))
201+
Expect(resources[0].Prefix).To(Equal("nvidia.com"))
202+
Expect(resources[0].ResourceName).To(Equal("test-resource"))
203+
204+
config, annotations, err := plugin.GetConfig(net, hifList)
205+
Expect(err).NotTo(HaveOccurred())
206+
207+
// Parse the config as JSON
208+
var configMap map[string]interface{}
209+
err = json.Unmarshal([]byte(config), &configMap)
210+
Expect(err).NotTo(HaveOccurred())
211+
212+
// Verify basic CNI fields
213+
Expect(configMap["cniVersion"]).To(Equal("0.3.1"))
214+
Expect(configMap["type"]).To(Equal("host-device"))
215+
Expect(configMap["name"]).To(Equal("test-network"))
216+
217+
// Verify IPAM configuration
218+
ipam, ok := configMap["ipam"].(map[string]interface{})
219+
Expect(ok).To(BeTrue())
220+
Expect(ipam["type"]).To(Equal("host-local"))
221+
Expect(ipam["subnet"]).To(Equal("10.0.0.0/24"))
222+
Expect(ipam["rangeStart"]).To(Equal("10.0.0.10"))
223+
Expect(ipam["rangeEnd"]).To(Equal("10.0.0.20"))
224+
225+
// Verify annotations
226+
Expect(annotations).To(HaveKey(RESOURCE_ANNOTATION))
227+
Expect(annotations[RESOURCE_ANNOTATION]).To(Equal("nvidia.com/test-resource"))
228+
})
229+
230+
It("should handle invalid IPAM config", func() {
231+
net.Spec.IPAM = "invalid json"
232+
config, annotations, err := plugin.GetConfig(net, hifList)
233+
Expect(err).To(HaveOccurred())
234+
Expect(config).To(BeEmpty())
235+
Expect(annotations).To(BeEmpty())
236+
})
237+
238+
It("should handle empty resource list", func() {
239+
emptyConfig := `{
240+
"resourceList": []
241+
}`
242+
plugin.mockSriovPlugin.Config = &emptyConfig
243+
config, annotations, err := plugin.GetConfig(net, hifList)
244+
Expect(err).To(HaveOccurred())
245+
Expect(err.Error()).To(Equal("failed to get resource annotation from sriov plugin config"))
246+
Expect(config).To(BeEmpty())
247+
Expect(annotations).To(BeEmpty())
248+
})
249+
250+
It("should handle invalid resource list", func() {
251+
invalidConfig := `{
252+
"resourceList": "invalid"
253+
}`
254+
255+
plugin.mockSriovPlugin.Config = &invalidConfig
256+
config, annotations, err := plugin.GetConfig(net, hifList)
257+
Expect(err).To(HaveOccurred())
258+
Expect(err.Error()).To(Equal("failed to get resource annotation from sriov plugin config"))
259+
Expect(config).To(BeEmpty())
260+
Expect(annotations).To(BeEmpty())
261+
})
262+
263+
It("should successfully retrieve SrIoV resources from policy", func() {
264+
// Test with a valid policy configuration
265+
resources := plugin.GetSrIoVResources()
266+
Expect(resources).To(HaveLen(1))
267+
Expect(resources[0].Prefix).To(Equal("nvidia.com"))
268+
Expect(resources[0].ResourceName).To(Equal("test-resource"))
269+
})
270+
271+
It("should return empty list when no resources are available", func() {
272+
// Test with nil SriovDevicePlugin
273+
plugin.mockSriovPlugin = nil
274+
resources := plugin.GetSrIoVResources()
275+
Expect(resources).To(BeEmpty())
276+
277+
// Test with nil Config
278+
plugin.mockSriovPlugin = &DevicePluginSpec{}
279+
resources = plugin.GetSrIoVResources()
280+
Expect(resources).To(BeEmpty())
281+
})
282+
283+
It("should handle resources with different prefixes", func() {
284+
multiResourceConfig := `{
285+
"resourceList": [
286+
{
287+
"resourcePrefix": "nvidia.com",
288+
"resourceName": "test-resource-1"
289+
},
290+
{
291+
"resourcePrefix": "mellanox.com",
292+
"resourceName": "test-resource-2"
293+
}
294+
]
295+
}`
296+
plugin.mockSriovPlugin.Config = &multiResourceConfig
297+
298+
resources := plugin.GetSrIoVResources()
299+
Expect(resources).To(HaveLen(2))
300+
Expect(resources[0].Prefix).To(Equal("nvidia.com"))
301+
Expect(resources[0].ResourceName).To(Equal("test-resource-1"))
302+
Expect(resources[1].Prefix).To(Equal("mellanox.com"))
303+
Expect(resources[1].ResourceName).To(Equal("test-resource-2"))
304+
305+
// Verify combined resource annotation
306+
_, annotations, err := plugin.GetConfig(net, hifList)
307+
Expect(err).NotTo(HaveOccurred())
308+
Expect(annotations).To(HaveKey(RESOURCE_ANNOTATION))
309+
Expect(annotations[RESOURCE_ANNOTATION]).To(Equal("nvidia.com/test-resource-1,mellanox.com/test-resource-2"))
310+
})
311+
})
77312
})

testing/coverage.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ File | Function | Coverage
2020
| github.com/foundation-model-stack/multi-nic-cni/controllers/cidr_handler.go:277: | NewCIDR | 75.0% |
2121
| github.com/foundation-model-stack/multi-nic-cni/controllers/cidr_handler.go:327: | Run | 100.0% |
2222
| github.com/foundation-model-stack/multi-nic-cni/controllers/cidr_handler.go:336: | UpdateCIDRs | 100.0% |
23-
| github.com/foundation-model-stack/multi-nic-cni/controllers/cidr_handler.go:344: | ProcessUpdateRequest | 70.0% |
23+
| github.com/foundation-model-stack/multi-nic-cni/controllers/cidr_handler.go:344: | ProcessUpdateRequest | 60.0% |
2424
| github.com/foundation-model-stack/multi-nic-cni/controllers/cidr_handler.go:361: | NewCIDRWithNewConfig | 80.0% |
2525
| github.com/foundation-model-stack/multi-nic-cni/controllers/cidr_handler.go:371: | UpdateEntries | 80.9% |
2626
| github.com/foundation-model-stack/multi-nic-cni/controllers/cidr_handler.go:529: | updateCIDR | 52.0% |
@@ -175,11 +175,11 @@ File | Function | Coverage
175175
| github.com/foundation-model-stack/multi-nic-cni/internal/plugin/mellanox.go:53: | GetSrIoVResource | 60.0% |
176176
| github.com/foundation-model-stack/multi-nic-cni/internal/plugin/mellanox.go:70: | GetSrIoVResourcesFromSrIoVPlugin | 100.0% |
177177
| github.com/foundation-model-stack/multi-nic-cni/internal/plugin/mellanox.go:91: | GetCombinedResourceNames | 100.0% |
178-
| github.com/foundation-model-stack/multi-nic-cni/internal/plugin/mellanox.go:107: | Init | 100.0% |
179-
| github.com/foundation-model-stack/multi-nic-cni/internal/plugin/mellanox.go:122: | GetConfig | 0.0% |
180-
| github.com/foundation-model-stack/multi-nic-cni/internal/plugin/mellanox.go:150: | GetSrIoVResources | 0.0% |
181-
| github.com/foundation-model-stack/multi-nic-cni/internal/plugin/mellanox.go:171: | getPolicy | 0.0% |
182-
| github.com/foundation-model-stack/multi-nic-cni/internal/plugin/mellanox.go:180: | CleanUp | 0.0% |
178+
| github.com/foundation-model-stack/multi-nic-cni/internal/plugin/mellanox.go:109: | Init | 100.0% |
179+
| github.com/foundation-model-stack/multi-nic-cni/internal/plugin/mellanox.go:124: | GetConfig | 0.0% |
180+
| github.com/foundation-model-stack/multi-nic-cni/internal/plugin/mellanox.go:155: | GetSrIoVResources | 0.0% |
181+
| github.com/foundation-model-stack/multi-nic-cni/internal/plugin/mellanox.go:176: | getPolicy | 0.0% |
182+
| github.com/foundation-model-stack/multi-nic-cni/internal/plugin/mellanox.go:185: | CleanUp | 0.0% |
183183
| github.com/foundation-model-stack/multi-nic-cni/internal/plugin/net_attach_def.go:38: | NewNetworkAttachmentDefinition | 100.0% |
184184
| github.com/foundation-model-stack/multi-nic-cni/internal/plugin/net_attach_def.go:60: | GetName | 100.0% |
185185
| github.com/foundation-model-stack/multi-nic-cni/internal/plugin/net_attach_def.go:63: | GetNameSpace | 0.0% |
@@ -216,4 +216,4 @@ File | Function | Coverage
216216
| github.com/foundation-model-stack/multi-nic-cni/internal/plugin/utils.go:26: | getBoolean | 100.0% |
217217
| github.com/foundation-model-stack/multi-nic-cni/internal/plugin/utils.go:33: | ValidateResourceName | 100.0% |
218218
| github.com/foundation-model-stack/multi-nic-cni/internal/plugin/utils.go:39: | GetHolderNetName | 100.0% |
219-
| total: | (statements) | 68.7% |
219+
| total: | (statements) | 68.8% |

testing/unittest-report.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,13 @@ Test | Description | File
1717
| Host Interface Test/UpdateNewInterfaces - original with more than one devices | can leave old one | ./controllers/hostinterface_test.go |
1818
| Host Interface Test/UpdateNewInterfaces - original with more than one devices | can leave old one when some is missing | ./controllers/hostinterface_test.go |
1919
| Host Interface Test/UpdateNewInterfaces - original with more than one devices | can leave old one when some is missing and some with new info | ./controllers/hostinterface_test.go |
20+
| Mellanox Plugin/GetConfig | should generate valid CNI config | ./internal/plugin/mellanox_test.go |
21+
| Mellanox Plugin/GetConfig | should handle empty resource list | ./internal/plugin/mellanox_test.go |
22+
| Mellanox Plugin/GetConfig | should handle invalid IPAM config | ./internal/plugin/mellanox_test.go |
23+
| Mellanox Plugin/GetConfig | should handle invalid resource list | ./internal/plugin/mellanox_test.go |
24+
| Mellanox Plugin/GetConfig | should handle resources with different prefixes | ./internal/plugin/mellanox_test.go |
25+
| Mellanox Plugin/GetConfig | should return empty list when no resources are available | ./internal/plugin/mellanox_test.go |
26+
| Mellanox Plugin/GetConfig | should successfully retrieve SrIoV resources from policy | ./internal/plugin/mellanox_test.go |
2027
| Mellanox Plugin/Init | should initialize successfully with valid config | ./internal/plugin/mellanox_test.go |
2128
| Mellanox Plugin/Init/when config is invalid | should return error | ./internal/plugin/mellanox_test.go |
2229
| NetAttachDef test/handler | create and delete | ./internal/plugin/net_attach_def_test.go |

0 commit comments

Comments
 (0)