Skip to content

Commit

Permalink
Merge pull request #750 from ykulazhenkov/pr-software-bridge-enable
Browse files Browse the repository at this point in the history
[software-bridges 5/x] Enable software-bridge implementation
  • Loading branch information
adrianchiris committed Aug 27, 2024
2 parents e1f5b74 + 0b9f45c commit 8358d96
Show file tree
Hide file tree
Showing 14 changed files with 530 additions and 52 deletions.
19 changes: 15 additions & 4 deletions cmd/sriov-network-config-daemon/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ func runServiceCmd(cmd *cobra.Command, args []string) error {
setupLog.V(2).Info("sriov-config-service", "config", sriovConf)
vars.DevMode = sriovConf.UnsupportedNics
vars.ManageSoftwareBridges = sriovConf.ManageSoftwareBridges
vars.OVSDBSocketPath = sriovConf.OVSDBSocketPath

if err := initSupportedNics(); err != nil {
return updateSriovResultErr(setupLog, phaseArg, fmt.Errorf("failed to initialize list of supported NIC ids: %v", err))
Expand Down Expand Up @@ -181,7 +182,7 @@ func callPlugin(setupLog logr.Logger, phase string, conf *systemd.SriovConfig, h
return nil
}

nodeState, err := getNetworkNodeState(setupLog, conf, hostHelpers)
nodeState, err := getNetworkNodeState(setupLog, conf, phase, hostHelpers)
if err != nil {
return err
}
Expand All @@ -207,7 +208,9 @@ func getPlugin(setupLog logr.Logger, phase string,
case consts.Baremetal:
switch phase {
case PhasePre:
configPlugin, err = newGenericPluginFunc(hostHelpers, generic.WithSkipVFConfiguration())
configPlugin, err = newGenericPluginFunc(hostHelpers,
generic.WithSkipVFConfiguration(),
generic.WithSkipBridgeConfiguration())
case PhasePost:
configPlugin, err = newGenericPluginFunc(hostHelpers)
}
Expand All @@ -229,10 +232,11 @@ func getPlugin(setupLog logr.Logger, phase string,
return configPlugin, nil
}

func getNetworkNodeState(setupLog logr.Logger, conf *systemd.SriovConfig,
func getNetworkNodeState(setupLog logr.Logger, conf *systemd.SriovConfig, phase string,
hostHelpers helper.HostHelpersInterface) (*sriovv1.SriovNetworkNodeState, error) {
var (
ifaceStatuses []sriovv1.InterfaceExt
bridges sriovv1.Bridges
err error
)
switch conf.PlatformType {
Expand All @@ -241,6 +245,13 @@ func getNetworkNodeState(setupLog logr.Logger, conf *systemd.SriovConfig,
if err != nil {
return nil, fmt.Errorf("failed to discover sriov devices on the host: %v", err)
}
if phase != PhasePre && vars.ManageSoftwareBridges {
// openvswitch is not available during the pre phase
bridges, err = hostHelpers.DiscoverBridges()
if err != nil {
return nil, fmt.Errorf("failed to discover managed bridges on the host: %v", err)
}
}
case consts.VirtualOpenStack:
platformHelper, err := newPlatformHelperFunc()
if err != nil {
Expand All @@ -257,7 +268,7 @@ func getNetworkNodeState(setupLog logr.Logger, conf *systemd.SriovConfig,
}
return &sriovv1.SriovNetworkNodeState{
Spec: conf.Spec,
Status: sriovv1.SriovNetworkNodeStateStatus{Interfaces: ifaceStatuses},
Status: sriovv1.SriovNetworkNodeStateStatus{Interfaces: ifaceStatuses, Bridges: bridges},
}, nil
}

Expand Down
3 changes: 2 additions & 1 deletion cmd/sriov-network-config-daemon/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ func restoreOrigFuncs() {

func getTestSriovInterfaceConfig(platform int) []byte {
return []byte(fmt.Sprintf(`spec:
dpconfigversion: ""
interfaces:
- pciaddress: 0000:d8:00.0
numvfs: 4
Expand All @@ -57,6 +56,7 @@ func getTestSriovInterfaceConfig(platform int) []byte {
externallymanaged: false
unsupportedNics: false
platformType: %d
manageSoftwareBridges: true
`, platform))
}

Expand Down Expand Up @@ -239,6 +239,7 @@ var _ = Describe("Service", func() {
hostHelpers.EXPECT().DiscoverSriovDevices(hostHelpers).Return([]sriovnetworkv1.InterfaceExt{{
Name: "enp216s0f0np0",
}}, nil)
hostHelpers.EXPECT().DiscoverBridges().Return(sriovnetworkv1.Bridges{}, nil)
genericPlugin.EXPECT().OnNodeStateChange(newNodeStateContainsDeviceMatcher("enp216s0f0np0")).Return(true, false, nil)
genericPlugin.EXPECT().Apply().Return(nil)
Expect(runServiceCmd(&cobra.Command{}, []string{})).NotTo(HaveOccurred())
Expand Down
3 changes: 3 additions & 0 deletions cmd/sriov-network-config-daemon/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ var (
disabledPlugins stringList
parallelNicConfig bool
manageSoftwareBridges bool
ovsSocketPath string
}
)

Expand All @@ -98,6 +99,7 @@ func init() {
startCmd.PersistentFlags().VarP(&startOpts.disabledPlugins, "disable-plugins", "", "comma-separated list of plugins to disable")
startCmd.PersistentFlags().BoolVar(&startOpts.parallelNicConfig, "parallel-nic-config", false, "perform NIC configuration in parallel")
startCmd.PersistentFlags().BoolVar(&startOpts.manageSoftwareBridges, "manage-software-bridges", false, "enable management of software bridges")
startCmd.PersistentFlags().StringVar(&startOpts.ovsSocketPath, "ovs-socket-path", vars.OVSDBSocketPath, "path for OVSDB socket")
}

func runStartCmd(cmd *cobra.Command, args []string) error {
Expand All @@ -113,6 +115,7 @@ func runStartCmd(cmd *cobra.Command, args []string) error {

vars.ParallelNicConfig = startOpts.parallelNicConfig
vars.ManageSoftwareBridges = startOpts.manageSoftwareBridges
vars.OVSDBSocketPath = startOpts.ovsSocketPath

if startOpts.nodeName == "" {
name, ok := os.LookupEnv("NODE_NAME")
Expand Down
19 changes: 16 additions & 3 deletions pkg/daemon/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,29 @@ func (w *NodeStateStatusWriter) Run(stop <-chan struct{}, refresh <-chan Message
func (w *NodeStateStatusWriter) pollNicStatus() error {
log.Log.V(2).Info("pollNicStatus()")
var iface []sriovnetworkv1.InterfaceExt
var bridges sriovnetworkv1.Bridges
var err error

if vars.PlatformType == consts.VirtualOpenStack {
iface, err = w.platformHelper.DiscoverSriovDevicesVirtual()
if err != nil {
return err
}
} else {
iface, err = w.hostHelper.DiscoverSriovDevices(w.hostHelper)
if err != nil {
return err
}
if vars.ManageSoftwareBridges {
bridges, err = w.hostHelper.DiscoverBridges()
if err != nil {
return err
}
}
}
if err != nil {
return err
}

w.status.Interfaces = iface
w.status.Bridges = bridges

return nil
}
Expand Down Expand Up @@ -169,6 +181,7 @@ func (w *NodeStateStatusWriter) updateNodeStateStatusRetry(f func(*sriovnetworkv
func (w *NodeStateStatusWriter) setNodeStateStatus(msg Message) (*sriovnetworkv1.SriovNetworkNodeState, error) {
nodeState, err := w.updateNodeStateStatusRetry(func(nodeState *sriovnetworkv1.SriovNetworkNodeState) {
nodeState.Status.Interfaces = w.status.Interfaces
nodeState.Status.Bridges = w.status.Bridges
if msg.lastSyncError != "" || msg.syncStatus == consts.SyncStatusSucceeded {
// clear lastSyncError when sync Succeeded
nodeState.Status.LastSyncError = msg.lastSyncError
Expand Down
43 changes: 43 additions & 0 deletions pkg/helper/mock/mock_helper.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 38 additions & 1 deletion pkg/host/internal/bridge/ovs/ovs.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ import (
"encoding/json"
"errors"
"fmt"
"os"
"reflect"
"sort"
"strings"
"time"

"github.com/google/uuid"
Expand All @@ -17,6 +19,7 @@ import (

sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1"
ovsStorePkg "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/bridge/ovs/store"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars"
)

Expand Down Expand Up @@ -627,6 +630,35 @@ func updateMap(old, new map[string]string) map[string]string {
return result
}

// returns path for the OVDSB socket
// for unix sockets it is taking into account current FS root and possible symlinks
func getDBSocketPath() (string, error) {
if !strings.HasPrefix(vars.OVSDBSocketPath, "unix://") {
// no need to apply modifications to tcp sockets
return vars.OVSDBSocketPath, nil
}
origPathNoPrefix, _ := strings.CutPrefix(vars.OVSDBSocketPath, "unix://")
resolvedPath := utils.GetHostExtensionPath(origPathNoPrefix)

// in some OSes /var/run is an absolute symlink to /run,
// this can be a problem when we are trying to access OVSDB socket from the daemon POD.
// first we a trying to use original path, if we can't find the OVSDB socket,
// we try to replace /var/run with /run in the socket path
var err error
_, err = os.Stat(resolvedPath)
if err != nil && !os.IsNotExist(err) {
return "", err
}
if os.IsNotExist(err) {
resolvedPath = strings.Replace(resolvedPath, "/var/run/", "/run/", 1)
_, err = os.Stat(resolvedPath)
if err != nil {
return "", err
}
}
return "unix://" + resolvedPath, nil
}

// initialize and return OVSDB client
func getClient(ctx context.Context) (client.Client, error) {
openvSwitchEntry := &OpenvSwitchEntry{}
Expand All @@ -638,8 +670,13 @@ func getClient(ctx context.Context) (client.Client, error) {
return nil, fmt.Errorf("can't create client DB model: %v", err)
}

socketPath, err := getDBSocketPath()
if err != nil {
return nil, fmt.Errorf("can't find OVSDB socket %s: %v", vars.OVSDBSocketPath, err)
}

dbClient, err := client.NewOVSDBClient(clientDBModel,
client.WithEndpoint(vars.OVSDBSocketPath),
client.WithEndpoint(socketPath),
client.WithLogger(&log.Log))
if err != nil {
return nil, fmt.Errorf("can't create DB client: %v", err)
Expand Down
69 changes: 62 additions & 7 deletions pkg/host/internal/bridge/ovs/ovs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (
sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1"
ovsStoreMockPkg "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/internal/bridge/ovs/store/mock"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars"
"github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/fakefilesystem"
"github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/helpers"
)

func getManagedBridges() map[string]*sriovnetworkv1.OVSConfigExt {
Expand Down Expand Up @@ -162,6 +164,14 @@ var _ = Describe("OVS", func() {
)
BeforeEach(func() {
ctx = context.Background()
origInChroot := vars.InChroot
origSocketValue := vars.OVSDBSocketPath
origFSRoot := vars.FilesystemRoot
DeferCleanup(func() {
vars.InChroot = origInChroot
vars.OVSDBSocketPath = origSocketValue
vars.FilesystemRoot = origFSRoot
})
})
Context("setDefaultTimeout", func() {
It("use default", func() {
Expand All @@ -179,7 +189,7 @@ var _ = Describe("OVS", func() {
timeoutCtx, timeoutFunc := context.WithTimeout(ctx, time.Millisecond*100)
defer timeoutFunc()
newCtx, _ := setDefaultTimeout(timeoutCtx)
time.Sleep(time.Millisecond * 200)
time.Sleep(time.Second)
Expect(newCtx.Err()).To(MatchError(context.DeadlineExceeded))
})
It("use explicit timeout - should return noop cancel function", func() {
Expand Down Expand Up @@ -212,6 +222,56 @@ var _ = Describe("OVS", func() {
})
})

Context("client", func() {
It("can't find socket", func() {
helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{})
c, err := getClient(ctx)
Expect(c).To(BeNil())
Expect(err).To(MatchError(ContainSubstring("can't find OVSDB socket")))
})
Context("getDBSocketPath()", func() {
It("tcp socket", func() {
vars.OVSDBSocketPath = "tcp://127.0.0.1:4444"
sock, err := getDBSocketPath()
Expect(err).NotTo(HaveOccurred())
Expect(sock).To(Equal("tcp://127.0.0.1:4444"))
})
It("unix socket - in container", func() {
helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{
Dirs: []string{"/host/ovs"},
Files: map[string][]byte{"/host/ovs/ovsdb.sock": {}},
})
vars.InChroot = false
vars.OVSDBSocketPath = "unix:///ovs/ovsdb.sock"
sock, err := getDBSocketPath()
Expect(err).NotTo(HaveOccurred())
Expect(sock).To(Equal("unix://" + vars.FilesystemRoot + "/host/ovs/ovsdb.sock"))
})
It("unix socket - host", func() {
helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{
Dirs: []string{"/ovs"},
Files: map[string][]byte{"/ovs/ovsdb.sock": {}},
})
vars.InChroot = true
vars.OVSDBSocketPath = "unix:///ovs/ovsdb.sock"
sock, err := getDBSocketPath()
Expect(err).NotTo(HaveOccurred())
Expect(sock).To(Equal("unix://" + vars.FilesystemRoot + "/ovs/ovsdb.sock"))
})
It("unix socket - alternative /var/run path", func() {
helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{
Dirs: []string{"/host/run/ovs"},
Files: map[string][]byte{"/host/run/ovs/ovsdb.sock": {}},
})
vars.InChroot = false
vars.OVSDBSocketPath = "unix:///var/run/ovs/ovsdb.sock"
sock, err := getDBSocketPath()
Expect(err).NotTo(HaveOccurred())
Expect(sock).To(Equal("unix://" + vars.FilesystemRoot + "/host/run/ovs/ovsdb.sock"))
})
})
})

Context("manage bridges", func() {
var (
store *ovsStoreMockPkg.MockStore
Expand All @@ -224,20 +284,15 @@ var _ = Describe("OVS", func() {
ovs Interface
)
BeforeEach(func() {
vars.InChroot = true
tempDir, err = os.MkdirTemp("", "sriov-operator-ovs-test-dir*")
testServerSocket = filepath.Join(tempDir, "ovsdb.sock")
Expect(err).NotTo(HaveOccurred())
testCtrl = gomock.NewController(GinkgoT())
store = ovsStoreMockPkg.NewMockStore(testCtrl)
_ = store
stopServerFunc = startServer("unix", testServerSocket)

origSocketValue := vars.OVSDBSocketPath
vars.OVSDBSocketPath = "unix://" + testServerSocket
DeferCleanup(func() {
vars.OVSDBSocketPath = origSocketValue
})

ovsClient, err = getClient(ctx)
Expect(err).NotTo(HaveOccurred())
ovs = New(store)
Expand Down
Loading

0 comments on commit 8358d96

Please sign in to comment.