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.

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

Add support for Device Information Spec #287

Merged
merged 7 commits into from
Dec 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
3 changes: 3 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
language: go

go:
"1.13"

matrix:
include:
- name: amd64
Expand Down
18 changes: 18 additions & 0 deletions deployments/k8s-v1.16/sriovdp-daemonset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,19 @@ spec:
mountPath: /var/log
- name: config-volume
mountPath: /etc/pcidp
- name: device-info
mountPath: /var/run/k8s.cni.cncf.io/devinfo/dp
volumes:
- name: devicesock
hostPath:
path: /var/lib/kubelet/
- name: log
hostPath:
path: /var/log
- name: device-info
hostPath:
path: /var/run/k8s.cni.cncf.io/devinfo/dp
type: DirectoryOrCreate
- name: config-volume
configMap:
name: sriovdp-config
Expand Down Expand Up @@ -111,13 +117,19 @@ spec:
mountPath: /var/log
- name: config-volume
mountPath: /etc/pcidp
- name: device-info
mountPath: /var/run/k8s.cni.cncf.io/devinfo/dp
volumes:
- name: devicesock
hostPath:
path: /var/lib/kubelet/
- name: log
hostPath:
path: /var/log
- name: device-info
hostPath:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you need to specify
DirectoryOrCreate type ?

what if the path does not exist on the host ? will it be created ?

https://kubernetes.io/docs/concepts/storage/volumes/#hostpath

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not see any error when I tested it. The nad library would take care of creating the directory so from a DevicePlugin perspective, it should not care. However, it's not a bad idea to add that type flag.
Thanks

path: /var/run/k8s.cni.cncf.io/devinfo/dp
type: DirectoryOrCreate
- name: config-volume
configMap:
name: sriovdp-config
Expand Down Expand Up @@ -172,13 +184,19 @@ spec:
mountPath: /var/log
- name: config-volume
mountPath: /etc/pcidp
- name: device-info
mountPath: /var/run/k8s.cni.cncf.io/devinfo/dp
volumes:
- name: devicesock
hostPath:
path: /var/lib/kubelet/
- name: log
hostPath:
path: /var/log
- name: device-info
hostPath:
path: /var/run/k8s.cni.cncf.io/devinfo/dp
type: DirectoryOrCreate
- name: config-volume
configMap:
name: sriovdp-config
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require (
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
github.com/jaypipes/ghw v0.6.0
github.com/jaypipes/pcidb v0.5.0
github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.1.1-0.20201119153432-9d213757d22d
github.com/onsi/ginkgo v1.12.0
github.com/onsi/gomega v1.9.0
github.com/pkg/errors v0.9.1
Expand Down
105 changes: 105 additions & 0 deletions go.sum

Large diffs are not rendered by default.

8 changes: 7 additions & 1 deletion pkg/factory/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ func (rf *resourceFactory) GetResourcePool(rc *types.ResourceConfig, filteredDev
case types.NetDeviceType:
if len(filteredDevice) > 0 {
if _, ok := filteredDevice[0].(types.PciNetDevice); ok {
rPool = netdevice.NewNetResourcePool(rc, apiDevices, devicePool)
nadUtils := rf.GetNadUtils()
rPool = netdevice.NewNetResourcePool(nadUtils, rc, apiDevices, devicePool)
} else {
err = fmt.Errorf("invalid device list for NetDeviceType")
}
Expand Down Expand Up @@ -179,3 +180,8 @@ func (rf *resourceFactory) GetDeviceFilter(rc *types.ResourceConfig) (interface{
return nil, fmt.Errorf("unable to get deviceFilter, invalid deviceType %s", rc.DeviceType)
}
}

// GetNadUtils returns an instance of NadUtils
func (rf *resourceFactory) GetNadUtils() types.NadUtils {
return netdevice.NewNadUtils()
}
27 changes: 27 additions & 0 deletions pkg/netdevice/nadutils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package netdevice

import (
nettypes "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
nadutils "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/utils"

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

// nadutils implements types.NadUtils interface
// It's purpose is to wrap the utilities provided by github.com/k8snetworkplumbingwg/network-attachment-definition-client
// in order to make mocking easy for Unit Tests
type nadUtils struct {
}

func (nu *nadUtils) SaveDeviceInfoFile(resourceName string, deviceID string, devInfo *nettypes.DeviceInfo) error {
return nadutils.SaveDeviceInfoForDP(resourceName, deviceID, devInfo)
}

func (nu *nadUtils) CleanDeviceInfoFile(resourceName string, deviceID string) error {
return nadutils.CleanDeviceInfoForDP(resourceName, deviceID)
}

// NewNadUtils returns a new NadUtils
func NewNadUtils() types.NadUtils {
return &nadUtils{}
}
46 changes: 45 additions & 1 deletion pkg/netdevice/netResourcePool.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@
package netdevice

import (
"fmt"
"github.com/golang/glog"
"strings"

nettypes "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
pluginapi "k8s.io/kubelet/pkg/apis/deviceplugin/v1beta1"

"github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/resources"
Expand All @@ -26,17 +29,19 @@ import (
type netResourcePool struct {
*resources.ResourcePoolImpl
selectors *types.NetDeviceSelectors
nadutils types.NadUtils
}

var _ types.ResourcePool = &netResourcePool{}

// NewNetResourcePool returns an instance of resourcePool
func NewNetResourcePool(rc *types.ResourceConfig, apiDevices map[string]*pluginapi.Device, devicePool map[string]types.PciDevice) types.ResourcePool {
func NewNetResourcePool(nadutils types.NadUtils, rc *types.ResourceConfig, apiDevices map[string]*pluginapi.Device, devicePool map[string]types.PciDevice) types.ResourcePool {
rp := resources.NewResourcePool(rc, apiDevices, devicePool)
s, _ := rc.SelectorObj.(*types.NetDeviceSelectors)
return &netResourcePool{
ResourcePoolImpl: rp,
selectors: s,
nadutils: nadutils,
}
}

Expand Down Expand Up @@ -80,3 +85,42 @@ func (rp *netResourcePool) GetDeviceSpecs(deviceIDs []string) []*pluginapi.Devic
}
return devSpecs
}

// StoreDeviceInfoFile stores the Device Info files according to the
// k8snetworkplumbingwg/device-info-spec
func (rp *netResourcePool) StoreDeviceInfoFile(resourceNamePrefix string) error {
for id, dev := range rp.GetDevicePool() {
netDev, ok := dev.(types.PciNetDevice)
if !ok {
return fmt.Errorf("StoreDeviceInfoFile: Only pciNetDevices are supported")
}
devInfo := nettypes.DeviceInfo{
Type: nettypes.DeviceInfoTypePCI,
Version: nettypes.DeviceInfoVersion,
Pci: &nettypes.PciDevice{
PciAddress: netDev.GetPciAddr(),
},
}
resource := fmt.Sprintf("%s/%s", resourceNamePrefix, rp.GetConfig().ResourceName)
if err := rp.nadutils.SaveDeviceInfoFile(resource, id, &devInfo); err != nil {
return err
}
}
return nil
}

// CleanDeviceInfoFile cleans the Device Info files
func (rp *netResourcePool) CleanDeviceInfoFile(resourceNamePrefix string) error {
errors := make([]string, 0)
for id := range rp.GetDevicePool() {
resource := fmt.Sprintf("%s/%s", resourceNamePrefix, rp.GetConfig().ResourceName)
if err := rp.nadutils.CleanDeviceInfoFile(resource, id); err != nil {
// Continue trying to clean.
errors = append(errors, err.Error())
}
}
if len(errors) > 0 {
return fmt.Errorf(strings.Join(errors, ","))
}
return nil
}
66 changes: 63 additions & 3 deletions pkg/netdevice/netResourcePool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
package netdevice_test

import (
"fmt"
nettypes "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
"github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/factory"
"github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/netdevice"
"github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/types"
"github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/types/mocks"
Expand All @@ -23,10 +26,13 @@ import (

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
. "github.com/stretchr/testify/mock"
)

var _ = Describe("NetResourcePool", func() {
Context("getting a new instance of the pool", func() {
rf := factory.NewResourceFactory("fake", "fake", true)
nadutils := rf.GetNadUtils()
rc := &types.ResourceConfig{
ResourceName: "fake",
ResourcePrefix: "fake",
Expand All @@ -35,14 +41,16 @@ var _ = Describe("NetResourcePool", func() {
devs := map[string]*v1beta1.Device{}
pcis := map[string]types.PciDevice{}

rp := netdevice.NewNetResourcePool(rc, devs, pcis)
rp := netdevice.NewNetResourcePool(nadutils, rc, devs, pcis)

It("should return a valid instance of the pool", func() {
Expect(rp).ToNot(BeNil())
})
})
Describe("getting DeviceSpecs", func() {
Context("for non-RDMA devices", func() {
rf := factory.NewResourceFactory("fake", "fake", true)
nadutils := rf.GetNadUtils()
rc := &types.ResourceConfig{
ResourceName: "fake",
ResourcePrefix: "fake",
Expand Down Expand Up @@ -78,7 +86,7 @@ var _ = Describe("NetResourcePool", func() {

pcis := map[string]types.PciDevice{"fake1": fake1, "fake2": fake2, "fake3": fake3}

rp := netdevice.NewNetResourcePool(rc, devs, pcis)
rp := netdevice.NewNetResourcePool(nadutils, rc, devs, pcis)

devIDs := []string{"fake1", "fake2"}

Expand All @@ -93,6 +101,8 @@ var _ = Describe("NetResourcePool", func() {
})
})
Context("for RDMA devices", func() {
rf := factory.NewResourceFactory("fake", "fake", true)
nadutils := rf.GetNadUtils()
rc := &types.ResourceConfig{
ResourceName: "fake",
ResourcePrefix: "fake",
Expand Down Expand Up @@ -123,7 +133,7 @@ var _ = Describe("NetResourcePool", func() {

pcis := map[string]types.PciDevice{"fake1": fake1, "fake2": fake2}

rp := netdevice.NewNetResourcePool(rc, devs, pcis)
rp := netdevice.NewNetResourcePool(nadutils, rc, devs, pcis)

devIDs := []string{"fake1", "fake2"}

Expand All @@ -137,4 +147,54 @@ var _ = Describe("NetResourcePool", func() {
})
})
})
Describe("Saving and Cleaning DevInfo files ", func() {
t := GinkgoT()
Context("for valid pci devices", func() {
rc := &types.ResourceConfig{
ResourceName: "fakeResource",
ResourcePrefix: "fakeOrg.io",
SelectorObj: &types.NetDeviceSelectors{
IsRdma: true,
},
}

devs := map[string]*v1beta1.Device{}
fake1 := &mocks.PciNetDevice{}
fake1.On("GetPciAddr").Return("0000:01:00.1")
fake2 := &mocks.PciNetDevice{}
fake2.On("GetPciAddr").Return("0000:01:00.2")
pcis := map[string]types.PciDevice{"fake1": fake1, "fake2": fake2}

It("should call nadutils to create a well formatted DeviceInfo object", func() {
nadutils := &mocks.NadUtils{}
nadutils.On("SaveDeviceInfoFile", "fakeOrg.io/fakeResource", "fake1", Anything).
Return(func(rName, id string, devInfo *nettypes.DeviceInfo) error {
if devInfo.Type != nettypes.DeviceInfoTypePCI || devInfo.Pci == nil || devInfo.Pci.PciAddress != "0000:01:00.1" {
return fmt.Errorf("wrong device info")
}
return nil
})
nadutils.On("SaveDeviceInfoFile", "fakeOrg.io/fakeResource", "fake2", Anything).
Return(func(rName, id string, devInfo *nettypes.DeviceInfo) error {
if devInfo.Type != nettypes.DeviceInfoTypePCI || devInfo.Pci == nil || devInfo.Pci.PciAddress != "0000:01:00.2" {
return fmt.Errorf("wrong device info")
}
return nil
})
rp := netdevice.NewNetResourcePool(nadutils, rc, devs, pcis)
err := rp.StoreDeviceInfoFile("fakeOrg.io")
nadutils.AssertExpectations(t)
Expect(err).ToNot(HaveOccurred())
})
It("should call nadutils to clean the DeviceInfo objects", func() {
nadutils := &mocks.NadUtils{}
nadutils.On("CleanDeviceInfoFile", "fakeOrg.io/fakeResource", "fake1").Return(nil)
nadutils.On("CleanDeviceInfoFile", "fakeOrg.io/fakeResource", "fake2").Return(nil)
rp := netdevice.NewNetResourcePool(nadutils, rc, devs, pcis)
err := rp.CleanDeviceInfoFile("fakeOrg.io")
nadutils.AssertExpectations(t)
Expect(err).ToNot(HaveOccurred())
})
})
})
})
2 changes: 1 addition & 1 deletion pkg/resources/pciDevice.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ package resources
import (
"strconv"

"github.com/jaypipes/ghw"
"github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/types"
"github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/utils"
"github.com/jaypipes/ghw"
pluginapi "k8s.io/kubelet/pkg/apis/deviceplugin/v1beta1"
)

Expand Down
12 changes: 12 additions & 0 deletions pkg/resources/pool_stub.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,15 @@ func (rp *ResourcePoolImpl) DeviceSpecExist(specs []*pluginapi.DeviceSpec, newSp
func (rp *ResourcePoolImpl) GetDevicePool() map[string]types.PciDevice {
return rp.devicePool
}

// StoreDeviceInfoFile does nothing. DeviceType-specific ResourcePools might
// store information according to the k8snetworkplumbingwg/device-info-spec
func (rp *ResourcePoolImpl) StoreDeviceInfoFile(resourceNamePrefix string) error {
return nil
}

// CleanDeviceInfoFile does nothing. DeviceType-specific ResourcePools might
// clean the Device Info file
func (rp *ResourcePoolImpl) CleanDeviceInfoFile(resourceNamePrefix string) error {
return nil
}
14 changes: 13 additions & 1 deletion pkg/resources/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,11 @@ func (rs *resourceServer) Init() error {
func (rs *resourceServer) Start() error {
resourceName := rs.resourcePool.GetResourceName()
_ = rs.cleanUp() // try tp clean up and continue

if err := rs.resourcePool.StoreDeviceInfoFile(rs.resourceNamePrefix); err != nil {
glog.Errorf("%s: error creating DeviceInfo File: %s", rs.resourcePool.GetResourceName(), err.Error())
}

glog.Infof("starting %s device plugin endpoint at: %s\n", resourceName, rs.endPoint)
lis, err := net.Listen("unix", rs.sockPath)
if err != nil {
Expand Down Expand Up @@ -292,8 +297,15 @@ func (rs *resourceServer) Watch() {
}

func (rs *resourceServer) cleanUp() error {
errors := make([]string, 0)
if err := os.Remove(rs.sockPath); err != nil && !os.IsNotExist(err) {
return err
errors = append(errors, err.Error())
}
if err := rs.resourcePool.CleanDeviceInfoFile(rs.resourceNamePrefix); err != nil {
errors = append(errors, err.Error())
}
if len(errors) > 0 {
return fmt.Errorf(strings.Join(errors, ","))
}
return nil
}
Expand Down
Loading