Skip setting vlan when parameter is not present

k8snetworkplumbingwg / sriov-cni

DPDK & SR-IOV CNI plugin

Apache License 2.0

307 stars 146 forks source link

Skip setting vlan when parameter is not present #296

Closed mlguerrero12 closed 5 months ago

mlguerrero12 commented 6 months ago

Fixes https://github.com/k8snetworkplumbingwg/sriov-cni/issues/291

coveralls commented 6 months ago

Pull Request Test Coverage Report for Build 8924595799

Details

37 of 41 (90.24%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+2.1%) to 49.569%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/sriov/sriov.go	4	8	50.0%
<!--	Total:	37	41	90.24%	-->

Totals
Change from base Build 8835333415:	2.1%
Covered Lines:	632
Relevant Lines:	1275

💛 - Coveralls

mlguerrero12 commented 5 months ago

great work!

please just extend a bit more our unit test to cover all the cases related to vlans
*conf.Vlan,
*conf.VlanQoS,
sriovtypes.VlanProtoInt[*conf.VlanProto]
to be sure we call the function when needed (vlan is not empty)

Thanks @SchSeba .

There are many cases related to vlan in config_test.go. Regarding the calling of the function, yes, there is one missing case, so I added unit tests for ApplyVFConfig which covers all optional parameters.

mlguerrero12 commented 5 months ago

@Eoghan1232, @e0ne, please have a look

mlguerrero12 commented 5 months ago

Hi @adrianchiris, could you please have a look at this? Thanks!

Eoghan1232 commented 5 months ago

working my way through my emails now, so getting around to the PR's