Skip to content

Commit fd2db83

Browse files
committed
Add nad validator test code
Signed-off-by: Jian Wang <[email protected]>
1 parent 9df6c7c commit fd2db83

File tree

3 files changed

+306
-2
lines changed

3 files changed

+306
-2
lines changed

pkg/webhook/nad/validator.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
cniv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
1212
admissionregv1 "k8s.io/api/admissionregistration/v1"
1313
"k8s.io/apimachinery/pkg/runtime"
14+
kubevirtv1 "kubevirt.io/api/core/v1"
1415

1516
"github.com/harvester/harvester-network-controller/pkg/network/iface"
1617
"github.com/harvester/harvester-network-controller/pkg/utils"
@@ -132,14 +133,18 @@ func (v *Validator) checkVmi(nad *cniv1.NetworkAttachmentDefinition) error {
132133
return err
133134
}
134135

136+
return v.generateVmiNoneStopError(nad, vmis)
137+
}
138+
139+
// for convenicen of test code
140+
func (v *Validator) generateVmiNoneStopError(_ *cniv1.NetworkAttachmentDefinition, vmis []*kubevirtv1.VirtualMachineInstance) error {
135141
if len(vmis) > 0 {
136142
vmiNameList := make([]string, 0, len(vmis))
137143
for _, vmi := range vmis {
138144
vmiNameList = append(vmiNameList, vmi.Namespace+"/"+vmi.Name)
139145
}
140146
return fmt.Errorf("it's still used by VM(s) %s which must be stopped at first", strings.Join(vmiNameList, ", "))
141147
}
142-
143148
return nil
144149
}
145150

pkg/webhook/nad/validator_test.go

Lines changed: 299 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,299 @@
1+
package nad
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
kubevirtv1 "kubevirt.io/api/core/v1"
10+
11+
networkv1 "github.com/harvester/harvester-network-controller/pkg/apis/network.harvesterhci.io/v1beta1"
12+
"github.com/harvester/harvester-network-controller/pkg/generated/clientset/versioned/fake"
13+
"github.com/harvester/harvester-network-controller/pkg/utils"
14+
"github.com/harvester/harvester-network-controller/pkg/utils/fakeclients"
15+
harvesterfake "github.com/harvester/harvester/pkg/generated/clientset/versioned/fake"
16+
harvesterfakeclients "github.com/harvester/harvester/pkg/util/fakeclients"
17+
cniv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
18+
)
19+
20+
const (
21+
testCnName = "test-cn"
22+
testNADConfig = "{\"cniVersion\":\"0.3.1\",\"name\":\"net1-vlan\",\"type\":\"bridge\",\"bridge\":\"harvester-br0\",\"promiscMode\":true,\"vlan\":300,\"ipam\":{}}"
23+
testNADName = "testNad"
24+
testVmName = "vm1"
25+
testNamespace = "test"
26+
)
27+
28+
func TestCreateNAD(t *testing.T) {
29+
tests := []struct {
30+
name string
31+
returnErr bool
32+
errKey string
33+
currentCN *networkv1.ClusterNetwork
34+
currentVC *networkv1.VlanConfig
35+
currentNAD *cniv1.NetworkAttachmentDefinition
36+
newNAD *cniv1.NetworkAttachmentDefinition
37+
}{
38+
{
39+
name: "NAD is valid to create",
40+
returnErr: false,
41+
errKey: "",
42+
currentCN: &networkv1.ClusterNetwork{
43+
ObjectMeta: metav1.ObjectMeta{
44+
Name: testCnName,
45+
Annotations: map[string]string{"test": "test"},
46+
},
47+
},
48+
newNAD: &cniv1.NetworkAttachmentDefinition{
49+
ObjectMeta: metav1.ObjectMeta{
50+
Name: testNADName,
51+
Namespace: testNamespace,
52+
Annotations: map[string]string{"test": "test"},
53+
Labels: map[string]string{utils.KeyClusterNetworkLabel: testCnName},
54+
},
55+
Spec: cniv1.NetworkAttachmentDefinitionSpec{
56+
Config: testNADConfig,
57+
},
58+
},
59+
},
60+
{
61+
name: "NAD has invalid config string",
62+
returnErr: true,
63+
errKey: "unmarshal",
64+
currentCN: &networkv1.ClusterNetwork{
65+
ObjectMeta: metav1.ObjectMeta{
66+
Name: testCnName,
67+
Annotations: map[string]string{"test": "test"},
68+
},
69+
},
70+
newNAD: &cniv1.NetworkAttachmentDefinition{
71+
ObjectMeta: metav1.ObjectMeta{
72+
Name: testNADName,
73+
Namespace: testNamespace,
74+
Annotations: map[string]string{"test": "test"},
75+
Labels: map[string]string{utils.KeyClusterNetworkLabel: testCnName},
76+
},
77+
Spec: cniv1.NetworkAttachmentDefinitionSpec{
78+
Config: "{\"cniVersion\",\"name\":\"net1-vlan\",\"type\":\"bridge\",\"bridge\":\"harvester-br0\",\"promiscMode\":true,\"vlan\":300,\"ipam\":{}}",
79+
},
80+
},
81+
},
82+
{
83+
name: "NAD has invalid VLAN id which is -1",
84+
returnErr: true,
85+
errKey: "VLAN ID must",
86+
currentCN: &networkv1.ClusterNetwork{
87+
ObjectMeta: metav1.ObjectMeta{
88+
Name: testCnName,
89+
Annotations: map[string]string{"test": "test"},
90+
},
91+
},
92+
newNAD: &cniv1.NetworkAttachmentDefinition{
93+
ObjectMeta: metav1.ObjectMeta{
94+
Name: testNADName,
95+
Namespace: testNamespace,
96+
Annotations: map[string]string{"test": "test"},
97+
Labels: map[string]string{utils.KeyClusterNetworkLabel: testCnName},
98+
},
99+
Spec: cniv1.NetworkAttachmentDefinitionSpec{
100+
Config: "{\"cniVersion\":\"0.3.1\",\"name\":\"net1-vlan\",\"type\":\"bridge\",\"bridge\":\"harvester-br0\",\"promiscMode\":true,\"vlan\":-1,\"ipam\":{}}",
101+
},
102+
},
103+
},
104+
{
105+
name: "NAD has invalid VLAN id which is 4095",
106+
returnErr: true,
107+
errKey: "VLAN ID must",
108+
currentCN: &networkv1.ClusterNetwork{
109+
ObjectMeta: metav1.ObjectMeta{
110+
Name: testCnName,
111+
Annotations: map[string]string{"test": "test"},
112+
},
113+
},
114+
newNAD: &cniv1.NetworkAttachmentDefinition{
115+
ObjectMeta: metav1.ObjectMeta{
116+
Name: testNADName,
117+
Namespace: testNamespace,
118+
Annotations: map[string]string{"test": "test"},
119+
Labels: map[string]string{utils.KeyClusterNetworkLabel: testCnName},
120+
},
121+
Spec: cniv1.NetworkAttachmentDefinitionSpec{
122+
Config: "{\"cniVersion\":\"0.3.1\",\"name\":\"net1-vlan\",\"type\":\"bridge\",\"bridge\":\"harvester-br0\",\"promiscMode\":true,\"vlan\":4095,\"ipam\":{}}",
123+
},
124+
},
125+
},
126+
{
127+
name: "NAD has too long bridge name",
128+
returnErr: true,
129+
errKey: "the length of the brName",
130+
currentCN: &networkv1.ClusterNetwork{
131+
ObjectMeta: metav1.ObjectMeta{
132+
Name: testCnName,
133+
Annotations: map[string]string{"test": "test"},
134+
},
135+
},
136+
newNAD: &cniv1.NetworkAttachmentDefinition{
137+
ObjectMeta: metav1.ObjectMeta{
138+
Name: testNADName,
139+
Namespace: testNamespace,
140+
Annotations: map[string]string{"test": "test"},
141+
Labels: map[string]string{utils.KeyClusterNetworkLabel: testCnName},
142+
},
143+
Spec: cniv1.NetworkAttachmentDefinitionSpec{
144+
Config: "{\"cniVersion\":\"0.3.1\",\"name\":\"net1-vlan\",\"type\":\"bridge\",\"bridge\":\"harvester-br0-too-long\",\"promiscMode\":true,\"vlan\":300,\"ipam\":{}}",
145+
},
146+
},
147+
},
148+
{
149+
name: "NAD has too short bridge name",
150+
returnErr: true,
151+
errKey: "the suffix of the brName",
152+
currentCN: &networkv1.ClusterNetwork{
153+
ObjectMeta: metav1.ObjectMeta{
154+
Name: testCnName,
155+
Annotations: map[string]string{"test": "test"},
156+
},
157+
},
158+
newNAD: &cniv1.NetworkAttachmentDefinition{
159+
ObjectMeta: metav1.ObjectMeta{
160+
Name: testNADName,
161+
Namespace: testNamespace,
162+
Annotations: map[string]string{"test": "test"},
163+
Labels: map[string]string{utils.KeyClusterNetworkLabel: testCnName},
164+
},
165+
Spec: cniv1.NetworkAttachmentDefinitionSpec{
166+
Config: "{\"cniVersion\":\"0.3.1\",\"name\":\"net1-vlan\",\"type\":\"bridge\",\"bridge\":\"a\",\"promiscMode\":true,\"vlan\":300,\"ipam\":{}}",
167+
},
168+
},
169+
},
170+
{
171+
name: "NAD has invalid bridge suffix",
172+
returnErr: true,
173+
errKey: "the suffix of the brName",
174+
currentCN: &networkv1.ClusterNetwork{
175+
ObjectMeta: metav1.ObjectMeta{
176+
Name: testCnName,
177+
Annotations: map[string]string{"test": "test"},
178+
},
179+
},
180+
newNAD: &cniv1.NetworkAttachmentDefinition{
181+
ObjectMeta: metav1.ObjectMeta{
182+
Name: testNADName,
183+
Namespace: testNamespace,
184+
Annotations: map[string]string{"test": "test"},
185+
Labels: map[string]string{utils.KeyClusterNetworkLabel: testCnName},
186+
},
187+
Spec: cniv1.NetworkAttachmentDefinitionSpec{
188+
Config: "{\"cniVersion\":\"0.3.1\",\"name\":\"net1-vlan\",\"type\":\"bridge\",\"suffix-br-\":\"a\",\"promiscMode\":true,\"vlan\":300,\"ipam\":{}}",
189+
},
190+
},
191+
},
192+
}
193+
194+
for _, tc := range tests {
195+
t.Run(tc.name, func(t *testing.T) {
196+
197+
assert.NotNil(t, tc.newNAD)
198+
if tc.newNAD == nil {
199+
return
200+
}
201+
202+
nchclientset := fake.NewSimpleClientset()
203+
harvesterclientset := harvesterfake.NewSimpleClientset()
204+
205+
vmiCache := harvesterfakeclients.VirtualMachineInstanceCache(harvesterclientset.KubevirtV1().VirtualMachineInstances)
206+
207+
// client to inject test data
208+
vcClient := fakeclients.VlanConfigClient(nchclientset.NetworkV1beta1().VlanConfigs)
209+
cnClient := fakeclients.ClusterNetworkClient(nchclientset.NetworkV1beta1().ClusterNetworks)
210+
211+
if tc.currentVC != nil {
212+
vcClient.Create(tc.currentVC)
213+
}
214+
if tc.currentCN != nil {
215+
cnClient.Create(tc.currentCN)
216+
}
217+
218+
validator := NewNadValidator(vmiCache)
219+
220+
err := validator.Create(nil, tc.newNAD)
221+
if tc.returnErr {
222+
assert.NotNil(t, err)
223+
assert.True(t, strings.Contains(err.Error(), tc.errKey))
224+
}
225+
})
226+
}
227+
}
228+
229+
func TestDeleteNAD(t *testing.T) {
230+
tests := []struct {
231+
name string
232+
returnErr bool
233+
errKey string
234+
currentNAD *cniv1.NetworkAttachmentDefinition
235+
usedVMs []*kubevirtv1.VirtualMachineInstance
236+
}{
237+
{
238+
name: "NAD has used VMs and can not be deleted",
239+
returnErr: true,
240+
errKey: testVmName,
241+
currentNAD: &cniv1.NetworkAttachmentDefinition{
242+
ObjectMeta: metav1.ObjectMeta{
243+
Name: testNADName,
244+
Namespace: testNamespace,
245+
Annotations: map[string]string{"test": "test"},
246+
Labels: map[string]string{utils.KeyClusterNetworkLabel: testCnName},
247+
},
248+
Spec: cniv1.NetworkAttachmentDefinitionSpec{
249+
Config: testNADConfig,
250+
},
251+
},
252+
usedVMs: []*kubevirtv1.VirtualMachineInstance{
253+
{
254+
ObjectMeta: metav1.ObjectMeta{
255+
Name: testVmName,
256+
Namespace: testNamespace,
257+
Annotations: map[string]string{"test": "test"},
258+
},
259+
},
260+
},
261+
},
262+
{
263+
name: "NAD has no used VMs and can be deleted",
264+
returnErr: false,
265+
errKey: "",
266+
currentNAD: &cniv1.NetworkAttachmentDefinition{
267+
ObjectMeta: metav1.ObjectMeta{
268+
Name: testNADName,
269+
Namespace: testNamespace,
270+
Annotations: map[string]string{"test": "test"},
271+
Labels: map[string]string{utils.KeyClusterNetworkLabel: testCnName},
272+
},
273+
Spec: cniv1.NetworkAttachmentDefinitionSpec{
274+
Config: testNADConfig,
275+
},
276+
},
277+
},
278+
}
279+
280+
for _, tc := range tests {
281+
t.Run(tc.name, func(t *testing.T) {
282+
assert.NotNil(t, tc.currentNAD)
283+
if tc.currentNAD == nil {
284+
return
285+
}
286+
287+
harvesterclientset := harvesterfake.NewSimpleClientset()
288+
vmiCache := harvesterfakeclients.VirtualMachineInstanceCache(harvesterclientset.KubevirtV1().VirtualMachineInstances)
289+
validator := NewNadValidator(vmiCache)
290+
291+
// due to fake vmiCache limitation, just test generateVmiNoneStopError() instead of Delete()
292+
err := validator.generateVmiNoneStopError(tc.currentNAD, tc.usedVMs)
293+
if tc.returnErr {
294+
assert.NotNil(t, err)
295+
assert.True(t, strings.Contains(err.Error(), tc.errKey))
296+
}
297+
})
298+
}
299+
}

pkg/webhook/vlanconfig/validator.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func NewVlanConfigValidator(
4646
vcCache ctlnetworkv1.VlanConfigCache,
4747
vsCache ctlnetworkv1.VlanStatusCache,
4848
vmiCache ctlkubevirtv1.VirtualMachineInstanceCache,
49-
cnCache ctlnetworkv1.ClusterNetworkCache) *Validator {
49+
cnCache ctlnetworkv1.ClusterNetworkCache) *Validator {
5050
return &Validator{
5151
nadCache: nadCache,
5252
vcCache: vcCache,

0 commit comments

Comments
 (0)