Skip to content

Commit

Permalink
Refactor renderDevicePluginConfigData
Browse files Browse the repository at this point in the history 
The function mainly creates and updates ResourceConfig
in the list. Split the function into two smaller ones.

Adjust unit test to satisfy `SriovNetworkNodePolicyReconciler`
dependencies through fake client.

Signed-off-by: Andrea Panattoni <[email protected]>
  • Loading branch information
@zeeke
zeeke committed May 8, 2023
1 parent 220a196 commit 17bf75a
Show file tree
Hide file tree
Showing 6 changed files with 1,126 additions and 139 deletions.
266 changes: 147 additions & 119 deletions controllers/sriovnetworknodepolicy_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -588,135 +588,25 @@ func (r *SriovNetworkNodePolicyReconciler) renderDevicePluginConfigData(ctx cont
continue
}

found, i := resourceNameInList(p.Spec.ResourceName, &rcl)
netDeviceSelectors := dptypes.NetDeviceSelectors{}
if found {
if err := json.Unmarshal(*rcl.ResourceList[i].Selectors, &netDeviceSelectors); err != nil {
return rcl, err
}

if p.Spec.NicSelector.Vendor != "" && !sriovnetworkv1.StringInArray(p.Spec.NicSelector.Vendor, netDeviceSelectors.Vendors) {
netDeviceSelectors.Vendors = append(netDeviceSelectors.Vendors, p.Spec.NicSelector.Vendor)
}
if p.Spec.NicSelector.DeviceID != "" {
var deviceID string
if p.Spec.NumVfs == 0 {
deviceID = p.Spec.NicSelector.DeviceID
} else {
deviceID = sriovnetworkv1.GetVfDeviceID(p.Spec.NicSelector.DeviceID)
}
nodeState := &sriovnetworkv1.SriovNetworkNodeState{}
err := r.Get(ctx, types.NamespacedName{Namespace: namespace, Name: node.Name}, nodeState)
if err != nil {
return rcl, err
}

if !sriovnetworkv1.StringInArray(deviceID, netDeviceSelectors.Devices) && deviceID != "" {
netDeviceSelectors.Devices = append(netDeviceSelectors.Devices, deviceID)
}
}
if len(p.Spec.NicSelector.PfNames) > 0 {
netDeviceSelectors.PfNames = sriovnetworkv1.UniqueAppend(netDeviceSelectors.PfNames, p.Spec.NicSelector.PfNames...)
}
// vfio-pci device link type is not detectable
if p.Spec.DeviceType != constants.DeviceTypeVfioPci {
if p.Spec.LinkType != "" {
linkType := constants.LinkTypeEthernet
if strings.EqualFold(p.Spec.LinkType, constants.LinkTypeIB) {
linkType = constants.LinkTypeInfiniband
}
if !sriovnetworkv1.StringInArray(linkType, netDeviceSelectors.LinkTypes) {
netDeviceSelectors.LinkTypes = sriovnetworkv1.UniqueAppend(netDeviceSelectors.LinkTypes, linkType)
}
}
}
if len(p.Spec.NicSelector.RootDevices) > 0 {
netDeviceSelectors.RootDevices = sriovnetworkv1.UniqueAppend(netDeviceSelectors.RootDevices, p.Spec.NicSelector.RootDevices...)
}
// Removed driver constraint for "netdevice" DeviceType
if p.Spec.DeviceType == constants.DeviceTypeVfioPci {
netDeviceSelectors.Drivers = sriovnetworkv1.UniqueAppend(netDeviceSelectors.Drivers, p.Spec.DeviceType)
}
// Enable the selection of devices using NetFilter
if p.Spec.NicSelector.NetFilter != "" {
nodeState := &sriovnetworkv1.SriovNetworkNodeState{}
err := r.Get(ctx, types.NamespacedName{Namespace: namespace, Name: node.Name}, nodeState)
if err == nil {
// Loop through interfaces status to find a match for NetworkID or NetworkTag
for _, intf := range nodeState.Status.Interfaces {
if sriovnetworkv1.NetFilterMatch(p.Spec.NicSelector.NetFilter, intf.NetFilter) {
// Found a match add the Interfaces PciAddress
netDeviceSelectors.PciAddresses = sriovnetworkv1.UniqueAppend(netDeviceSelectors.PciAddresses, intf.PciAddress)
}
}
}
}
found, i := resourceNameInList(p.Spec.ResourceName, &rcl)

netDeviceSelectorsMarshal, err := json.Marshal(netDeviceSelectors)
if found {
err := updateDevicePluginResource(ctx, &rcl.ResourceList[i], &p, nodeState)
if err != nil {
return rcl, err
}
rawNetDeviceSelectors := json.RawMessage(netDeviceSelectorsMarshal)
rcl.ResourceList[i].Selectors = &rawNetDeviceSelectors
logger.Info("Update resource", "Resource", rcl.ResourceList[i])
} else {
rc := &dptypes.ResourceConfig{
ResourceName: p.Spec.ResourceName,
}
netDeviceSelectors.IsRdma = p.Spec.IsRdma
netDeviceSelectors.NeedVhostNet = p.Spec.NeedVhostNet
netDeviceSelectors.VdpaType = dptypes.VdpaType(p.Spec.VdpaType)

if p.Spec.NicSelector.Vendor != "" {
netDeviceSelectors.Vendors = append(netDeviceSelectors.Vendors, p.Spec.NicSelector.Vendor)
}
if p.Spec.NicSelector.DeviceID != "" {
var deviceID string
if p.Spec.NumVfs == 0 {
deviceID = p.Spec.NicSelector.DeviceID
} else {
deviceID = sriovnetworkv1.GetVfDeviceID(p.Spec.NicSelector.DeviceID)
}

if !sriovnetworkv1.StringInArray(deviceID, netDeviceSelectors.Devices) && deviceID != "" {
netDeviceSelectors.Devices = append(netDeviceSelectors.Devices, deviceID)
}
}
if len(p.Spec.NicSelector.PfNames) > 0 {
netDeviceSelectors.PfNames = append(netDeviceSelectors.PfNames, p.Spec.NicSelector.PfNames...)
}
// vfio-pci device link type is not detectable
if p.Spec.DeviceType != constants.DeviceTypeVfioPci {
if p.Spec.LinkType != "" {
linkType := constants.LinkTypeEthernet
if strings.EqualFold(p.Spec.LinkType, constants.LinkTypeIB) {
linkType = constants.LinkTypeInfiniband
}
netDeviceSelectors.LinkTypes = sriovnetworkv1.UniqueAppend(netDeviceSelectors.LinkTypes, linkType)
}
}
if len(p.Spec.NicSelector.RootDevices) > 0 {
netDeviceSelectors.RootDevices = append(netDeviceSelectors.RootDevices, p.Spec.NicSelector.RootDevices...)
}
// Removed driver constraint for "netdevice" DeviceType
if p.Spec.DeviceType == constants.DeviceTypeVfioPci {
netDeviceSelectors.Drivers = append(netDeviceSelectors.Drivers, p.Spec.DeviceType)
}
// Enable the selection of devices using NetFilter
if p.Spec.NicSelector.NetFilter != "" {
nodeState := &sriovnetworkv1.SriovNetworkNodeState{}
err := r.Get(ctx, types.NamespacedName{Namespace: namespace, Name: node.Name}, nodeState)
if err == nil {
// Loop through interfaces status to find a match for NetworkID or NetworkTag
for _, intf := range nodeState.Status.Interfaces {
if sriovnetworkv1.NetFilterMatch(p.Spec.NicSelector.NetFilter, intf.NetFilter) {
// Found a match add the Interfaces PciAddress
netDeviceSelectors.PciAddresses = sriovnetworkv1.UniqueAppend(netDeviceSelectors.PciAddresses, intf.PciAddress)
}
}
}
}
netDeviceSelectorsMarshal, err := json.Marshal(netDeviceSelectors)
rc, err := createDevicePluginResource(ctx, &p, nodeState)
if err != nil {
return rcl, err
}
rawNetDeviceSelectors := json.RawMessage(netDeviceSelectorsMarshal)
rc.Selectors = &rawNetDeviceSelectors
rcl.ResourceList = append(rcl.ResourceList, *rc)
logger.Info("Add resource", "Resource", *rc, "Resource list", rcl.ResourceList)
}
Expand All @@ -732,3 +622,141 @@ func resourceNameInList(name string, rcl *dptypes.ResourceConfList) (bool, int)
}
return false, 0
}

func createDevicePluginResource(
ctx context.Context,
p *sriovnetworkv1.SriovNetworkNodePolicy,
nodeState *sriovnetworkv1.SriovNetworkNodeState) (*dptypes.ResourceConfig, error) {
netDeviceSelectors := dptypes.NetDeviceSelectors{}

rc := &dptypes.ResourceConfig{
ResourceName: p.Spec.ResourceName,
}
netDeviceSelectors.IsRdma = p.Spec.IsRdma
netDeviceSelectors.NeedVhostNet = p.Spec.NeedVhostNet
netDeviceSelectors.VdpaType = dptypes.VdpaType(p.Spec.VdpaType)

if p.Spec.NicSelector.Vendor != "" {
netDeviceSelectors.Vendors = append(netDeviceSelectors.Vendors, p.Spec.NicSelector.Vendor)
}
if p.Spec.NicSelector.DeviceID != "" {
var deviceID string
if p.Spec.NumVfs == 0 {
deviceID = p.Spec.NicSelector.DeviceID
} else {
deviceID = sriovnetworkv1.GetVfDeviceID(p.Spec.NicSelector.DeviceID)
}

if !sriovnetworkv1.StringInArray(deviceID, netDeviceSelectors.Devices) && deviceID != "" {
netDeviceSelectors.Devices = append(netDeviceSelectors.Devices, deviceID)
}
}
if len(p.Spec.NicSelector.PfNames) > 0 {
netDeviceSelectors.PfNames = append(netDeviceSelectors.PfNames, p.Spec.NicSelector.PfNames...)
}
// vfio-pci device link type is not detectable
if p.Spec.DeviceType != constants.DeviceTypeVfioPci {
if p.Spec.LinkType != "" {
linkType := constants.LinkTypeEthernet
if strings.EqualFold(p.Spec.LinkType, constants.LinkTypeIB) {
linkType = constants.LinkTypeInfiniband
}
netDeviceSelectors.LinkTypes = sriovnetworkv1.UniqueAppend(netDeviceSelectors.LinkTypes, linkType)
}
}
if len(p.Spec.NicSelector.RootDevices) > 0 {
netDeviceSelectors.RootDevices = append(netDeviceSelectors.RootDevices, p.Spec.NicSelector.RootDevices...)
}
// Removed driver constraint for "netdevice" DeviceType
if p.Spec.DeviceType == constants.DeviceTypeVfioPci {
netDeviceSelectors.Drivers = append(netDeviceSelectors.Drivers, p.Spec.DeviceType)
}
// Enable the selection of devices using NetFilter
if p.Spec.NicSelector.NetFilter != "" {
// Loop through interfaces status to find a match for NetworkID or NetworkTag
for _, intf := range nodeState.Status.Interfaces {
if sriovnetworkv1.NetFilterMatch(p.Spec.NicSelector.NetFilter, intf.NetFilter) {
// Found a match add the Interfaces PciAddress
netDeviceSelectors.PciAddresses = sriovnetworkv1.UniqueAppend(netDeviceSelectors.PciAddresses, intf.PciAddress)
}
}
}

netDeviceSelectorsMarshal, err := json.Marshal(netDeviceSelectors)
if err != nil {
return nil, err
}
rawNetDeviceSelectors := json.RawMessage(netDeviceSelectorsMarshal)
rc.Selectors = &rawNetDeviceSelectors

return rc, nil
}

func updateDevicePluginResource(
ctx context.Context,
rc *dptypes.ResourceConfig,
p *sriovnetworkv1.SriovNetworkNodePolicy,
nodeState *sriovnetworkv1.SriovNetworkNodeState) error {
netDeviceSelectors := dptypes.NetDeviceSelectors{}

if err := json.Unmarshal(*rc.Selectors, &netDeviceSelectors); err != nil {
return err
}

if p.Spec.NicSelector.Vendor != "" && !sriovnetworkv1.StringInArray(p.Spec.NicSelector.Vendor, netDeviceSelectors.Vendors) {
netDeviceSelectors.Vendors = append(netDeviceSelectors.Vendors, p.Spec.NicSelector.Vendor)
}
if p.Spec.NicSelector.DeviceID != "" {
var deviceID string
if p.Spec.NumVfs == 0 {
deviceID = p.Spec.NicSelector.DeviceID
} else {
deviceID = sriovnetworkv1.GetVfDeviceID(p.Spec.NicSelector.DeviceID)
}

if !sriovnetworkv1.StringInArray(deviceID, netDeviceSelectors.Devices) && deviceID != "" {
netDeviceSelectors.Devices = append(netDeviceSelectors.Devices, deviceID)
}
}
if len(p.Spec.NicSelector.PfNames) > 0 {
netDeviceSelectors.PfNames = sriovnetworkv1.UniqueAppend(netDeviceSelectors.PfNames, p.Spec.NicSelector.PfNames...)
}
// vfio-pci device link type is not detectable
if p.Spec.DeviceType != constants.DeviceTypeVfioPci {
if p.Spec.LinkType != "" {
linkType := constants.LinkTypeEthernet
if strings.EqualFold(p.Spec.LinkType, constants.LinkTypeIB) {
linkType = constants.LinkTypeInfiniband
}
if !sriovnetworkv1.StringInArray(linkType, netDeviceSelectors.LinkTypes) {
netDeviceSelectors.LinkTypes = sriovnetworkv1.UniqueAppend(netDeviceSelectors.LinkTypes, linkType)
}
}
}
if len(p.Spec.NicSelector.RootDevices) > 0 {
netDeviceSelectors.RootDevices = sriovnetworkv1.UniqueAppend(netDeviceSelectors.RootDevices, p.Spec.NicSelector.RootDevices...)
}
// Removed driver constraint for "netdevice" DeviceType
if p.Spec.DeviceType == constants.DeviceTypeVfioPci {
netDeviceSelectors.Drivers = sriovnetworkv1.UniqueAppend(netDeviceSelectors.Drivers, p.Spec.DeviceType)
}
// Enable the selection of devices using NetFilter
if p.Spec.NicSelector.NetFilter != "" {
// Loop through interfaces status to find a match for NetworkID or NetworkTag
for _, intf := range nodeState.Status.Interfaces {
if sriovnetworkv1.NetFilterMatch(p.Spec.NicSelector.NetFilter, intf.NetFilter) {
// Found a match add the Interfaces PciAddress
netDeviceSelectors.PciAddresses = sriovnetworkv1.UniqueAppend(netDeviceSelectors.PciAddresses, intf.PciAddress)
}
}
}

netDeviceSelectorsMarshal, err := json.Marshal(netDeviceSelectors)
if err != nil {
return err
}
rawNetDeviceSelectors := json.RawMessage(netDeviceSelectorsMarshal)
rc.Selectors = &rawNetDeviceSelectors

return nil
}
45 changes: 25 additions & 20 deletions controllers/sriovnetworknodepolicy_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ import (

"github.com/google/go-cmp/cmp"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"

"sigs.k8s.io/controller-runtime/pkg/client/fake"

dptypes "github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/types"

Expand Down Expand Up @@ -129,27 +134,18 @@ func TestNodeSelectorMerge(t *testing.T) {
}
}

func buildSelector(vdpaType dptypes.VdpaType) (json.RawMessage, error) {
netDeviceSelectors := dptypes.NetDeviceSelectors{}
netDeviceSelectors.IsRdma = false
netDeviceSelectors.NeedVhostNet = false
netDeviceSelectors.VdpaType = vdpaType

netDeviceSelectorsMarshal, err := json.Marshal(netDeviceSelectors)
func mustMarshallSelector(t *testing.T, input *dptypes.NetDeviceSelectors) *json.RawMessage {
out, err := json.Marshal(input)
if err != nil {
return nil, err
t.Error(err)
t.FailNow()
return nil
}
rawNetDeviceSelectors := json.RawMessage(netDeviceSelectorsMarshal)
return rawNetDeviceSelectors, nil
ret := json.RawMessage(out)
return &ret
}

func TestRenderDevicePluginConfigData(t *testing.T) {
rawNetDeviceSelectors, err := buildSelector(dptypes.VdpaType(consts.VdpaTypeVirtio))
if err != nil {
t.Error(err)
return
}

table := []struct {
tname string
policy sriovnetworkv1.SriovNetworkNodePolicy
Expand All @@ -168,7 +164,9 @@ func TestRenderDevicePluginConfigData(t *testing.T) {
ResourceList: []dptypes.ResourceConfig{
{
ResourceName: "resourceName",
Selectors: &rawNetDeviceSelectors,
Selectors: mustMarshallSelector(t, &dptypes.NetDeviceSelectors{
VdpaType: dptypes.VdpaType(consts.VdpaTypeVirtio),
}),
},
},
},
Expand All @@ -177,17 +175,24 @@ func TestRenderDevicePluginConfigData(t *testing.T) {

reconciler := SriovNetworkNodePolicyReconciler{}

node := corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1"}}
nodeState := sriovnetworkv1.SriovNetworkNodeState{ObjectMeta: metav1.ObjectMeta{Name: node.Name, Namespace: namespace}}

scheme := runtime.NewScheme()
utilruntime.Must(sriovnetworkv1.AddToScheme(scheme))
reconciler.Client = fake.NewClientBuilder().
WithScheme(scheme).WithObjects(&nodeState).
Build()

for _, tc := range table {
policyList := sriovnetworkv1.SriovNetworkNodePolicyList{Items: []sriovnetworkv1.SriovNetworkNodePolicy{tc.policy}}
node := corev1.Node{}

t.Run(tc.tname, func(t *testing.T) {
resourceList, err := reconciler.renderDevicePluginConfigData(context.TODO(), &policyList, &node)
if err != nil {
t.Error(tc.tname, "renderDevicePluginConfigData has failed")
}

t.Logf("SelectorObj: %v", resourceList.ResourceList[0].SelectorObj)

if !cmp.Equal(resourceList, tc.expResource) {
t.Error(tc.tname, "ResourceConfList not as expected", cmp.Diff(resourceList, tc.expResource))
}
Expand Down
Loading

0 comments on commit 17bf75a

Please sign in to comment.