Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
OpenShift: enable OVS HW offload for multiple MCPs
Browse files Browse the repository at this point in the history
One node can only be added in one custom MCP pool by
design of MCO. Since current ovs-hw-offload MCs are
applied to hardcoded MCP ovs-hw-offload-worker, other
existing custome MCPs cannot be enabled with ovs hw
offload configuration.

This commits allows ovs-hw-offload MCs be applied
to multiple MCPs as specified in default SriovNetworkNodeConfig.
zshi-redhat committed May 7, 2021
1 parent 12b734a commit f5c463c
Showing 14 changed files with 330 additions and 409 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -63,7 +63,7 @@ _build-%:
_plugin-%: vet
@hack/build-plugins.sh $*

plugins: _plugin-intel _plugin-mellanox _plugin-generic _plugin-virtual _plugin-mco _plugin-k8s
plugins: _plugin-intel _plugin-mellanox _plugin-generic _plugin-virtual _plugin-k8s

clean:
@rm -rf $(TARGET_DIR)
30 changes: 15 additions & 15 deletions controllers/helper.go
Original file line number Diff line number Diff line change
@@ -22,21 +22,21 @@ import (
)

const (
ResyncPeriod = 5 * time.Minute
DEFAULT_CONFIG_NAME = "default"
CONFIG_DAEMON_PATH = "./bindata/manifests/daemon"
INJECTOR_WEBHOOK_PATH = "./bindata/manifests/webhook"
OPERATOR_WEBHOOK_PATH = "./bindata/manifests/operator-webhook"
SERVICE_CA_CONFIGMAP_ANNOTATION = "service.beta.openshift.io/inject-cabundle"
INJECTOR_WEBHOOK_NAME = "network-resources-injector-config"
OPERATOR_WEBHOOK_NAME = "sriov-operator-webhook-config"
DEPRECATED_OPERATOR_WEBHOOK_NAME = "operator-webhook-config"
PLUGIN_PATH = "./bindata/manifests/plugins"
DAEMON_PATH = "./bindata/manifests/daemon"
DEFAULT_POLICY_NAME = "default"
CONFIGMAP_NAME = "device-plugin-config"
DP_CONFIG_FILENAME = "config.json"
HwOffloadNodeLabel = "ovs-hw-offload-worker"
ResyncPeriod = 5 * time.Minute
DEFAULT_CONFIG_NAME = "default"
CONFIG_DAEMON_PATH = "./bindata/manifests/daemon"
INJECTOR_WEBHOOK_PATH = "./bindata/manifests/webhook"
OPERATOR_WEBHOOK_PATH = "./bindata/manifests/operator-webhook"
SERVICE_CA_CONFIGMAP_ANNOTATION = "service.beta.openshift.io/inject-cabundle"
INJECTOR_WEBHOOK_NAME = "network-resources-injector-config"
OPERATOR_WEBHOOK_NAME = "sriov-operator-webhook-config"
DEPRECATED_OPERATOR_WEBHOOK_NAME = "operator-webhook-config"
PLUGIN_PATH = "./bindata/manifests/plugins"
DAEMON_PATH = "./bindata/manifests/daemon"
DEFAULT_POLICY_NAME = "default"
CONFIGMAP_NAME = "device-plugin-config"
DP_CONFIG_FILENAME = "config.json"
OVS_HWOL_MACHINE_CONFIG_NAME_SUFFIX = "ovs-hw-offload"

linkTypeEthernet = "ether"
linkTypeInfiniband = "infiniband"
157 changes: 154 additions & 3 deletions controllers/sriovnetworknodeconfig_controller.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,25 @@
package controllers

import (
"bytes"
"context"
"fmt"

"github.com/go-logr/logr"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1"
render "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/render"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils"
mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
)

// SriovNetworkNodeConfigReconciler reconciles a SriovNetworkNodeConfig object
@@ -32,11 +43,35 @@ type SriovNetworkNodeConfigReconciler struct {
// - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
func (r *SriovNetworkNodeConfigReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
_ = context.Background()
_ = r.Log.WithValues("sriovnetworknodeconfig", req.NamespacedName)
reqLogger := r.Log.WithValues("sriovnetworknodeconfig", req.NamespacedName)

// your logic here
defaultConfig := &sriovnetworkv1.SriovNetworkNodeConfig{}
err := r.Get(context.TODO(), types.NamespacedName{
Name: DEFAULT_CONFIG_NAME, Namespace: namespace}, defaultConfig)
if err != nil {
if errors.IsNotFound(err) {
// Default Config object not found, create it.
defaultConfig.SetNamespace(namespace)
defaultConfig.SetName(DEFAULT_CONFIG_NAME)
err = r.Create(context.TODO(), defaultConfig)
if err != nil {
reqLogger.Error(err, "Failed to create default Operator Config", "Namespace",
namespace, "Name", DEFAULT_CONFIG_NAME)
return reconcile.Result{}, err
}
return reconcile.Result{}, nil
}
// Error reading the object - requeue the request.
return reconcile.Result{}, err
}

return ctrl.Result{}, nil
if utils.ClusterType == utils.ClusterTypeOpenshift {
if err = r.syncOvsHardwareOffloadMachineConfig(defaultConfig); err != nil {
return reconcile.Result{}, err
}
}

return reconcile.Result{RequeueAfter: ResyncPeriod}, nil
}

// SetupWithManager sets up the controller with the Manager.
@@ -45,3 +80,119 @@ func (r *SriovNetworkNodeConfigReconciler) SetupWithManager(mgr ctrl.Manager) er
For(&sriovnetworkv1.SriovNetworkNodeConfig{}).
Complete(r)
}

func (r *SriovNetworkNodeConfigReconciler) syncOvsHardwareOffloadMachineConfig(dc *sriovnetworkv1.SriovNetworkNodeConfig) error {
logger := r.Log.WithName("syncOvsHardwareOffloadMachineConfig")

data := render.MakeRenderData()
mcpMap := make(map[string]bool)
nodeOffloadStatus := []sriovnetworkv1.OvsHardwareOffloadConfigStatus{}

mcpList := &mcfgv1.MachineConfigPoolList{}
err := r.List(context.TODO(), mcpList, &client.ListOptions{})
if err != nil {
return fmt.Errorf("Failed to get MachineConfigPoolList: %v", err)
}

for _, mcp := range mcpList.Items {
selector, err := metav1.LabelSelectorAsSelector(mcp.Spec.NodeSelector)
if err != nil {
return fmt.Errorf("Invalid label selector in MachineConfigPool: %s, %v", mcp.GetName(), err)
}
// Node is selected when its label(s) are included in NodeSelector
for _, ovsHWOLConfig := range dc.Spec.OvsHardwareOffload {
if selector.Matches(labels.Set(ovsHWOLConfig.NodeSelector)) {
// OVS Hardware Offload is not supported on master nodes
if mcp.GetName() == "master" {
logger.Info("OVS Hardware Offload is configured on master nodes which is not supported, ignoring.")
continue
}
mcpMap[mcp.GetName()] = true
break
}
}
}

for mcpName, enable := range mcpMap {
mcName := "00-" + mcpName + "-" + OVS_HWOL_MACHINE_CONFIG_NAME_SUFFIX
mc, err := render.GenerateMachineConfig("bindata/manifests/switchdev-config", mcName, mcpName, true, &data)
if err != nil {
return err
}

foundMC := &mcfgv1.MachineConfig{}
err = r.Get(context.TODO(), types.NamespacedName{Name: mcName}, foundMC)
if err != nil {
if errors.IsNotFound(err) {
if enable {
err = r.Create(context.TODO(), mc)
if err != nil {
return fmt.Errorf("Couldn't create MachineConfig: %v", err)
}
nodeOffloadStatus = append(nodeOffloadStatus,
sriovnetworkv1.OvsHardwareOffloadConfigStatus{Nodes: []string{mcpName}})
logger.Info("Created MachineConfig CR in MachineConfigPool", mcName, mcpName)
}
} else {
return fmt.Errorf("Failed to get MachineConfig: %v", err)
}
} else {
if enable {
if bytes.Compare(foundMC.Spec.Config.Raw, mc.Spec.Config.Raw) == 0 {
logger.Info("MachineConfig already exists, updating")
err = r.Update(context.TODO(), foundMC)
if err != nil {
return fmt.Errorf("Couldn't update MachineConfig: %v", err)
}
nodeOffloadStatus = append(nodeOffloadStatus,
sriovnetworkv1.OvsHardwareOffloadConfigStatus{Nodes: []string{mcpName}})
} else {
logger.Info("No content change, skip updating MC")
}
} else {
logger.Info("offload disabled, delete MachineConfig")
err = r.Delete(context.TODO(), foundMC)
if err != nil {
return fmt.Errorf("Couldn't delete MachineConfig: %v", err)
}
}
}
}

// Remove legacy MCs
for _, mcp := range mcpList.Items {
found := false
for mcpName := range mcpMap {
if mcp.Name == mcpName {
found = true
break
}
}
if !found {
mcName := "00-" + mcp.Name + "-" + OVS_HWOL_MACHINE_CONFIG_NAME_SUFFIX
foundMC := &mcfgv1.MachineConfig{}
err = r.Get(context.TODO(), types.NamespacedName{Name: mcName}, foundMC)
if err != nil {
if !errors.IsNotFound(err) {
err = r.Delete(context.TODO(), foundMC)
if err != nil {
return fmt.Errorf("Couldn't delete MachineConfig: %v", err)
}
}
}
}
}

// Update SriovOperatorConfig.Status
if equality.Semantic.DeepDerivative(dc.Status.OvsHardwareOffload, nodeOffloadStatus) {
logger.Info("Default SriovOperatorConfig status changed, updating")
dc.Status.OvsHardwareOffload = nodeOffloadStatus
err := r.Status().Update(context.TODO(), dc)
if err != nil {
logger.Error(err, "Fail to update OvsHardwareConfigStatus in default SriovOperatorConfig status")
return err
}
}

return nil
}
67 changes: 67 additions & 0 deletions controllers/sriovnetworknodeconfig_controller_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package controllers

import (
goctx "context"

"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1"
util "github.com/k8snetworkplumbingwg/sriov-network-operator/test/util"
)

var _ = Describe("Operator", func() {
Context("When is up", func() {
It("should be able to create machine config for ovs hardware offload", func() {
config := &sriovnetworkv1.SriovNetworkNodeConfig{}
err := util.WaitForNamespacedObject(config, k8sClient, testNamespace, "default", interval, timeout)
Expect(err).NotTo(HaveOccurred())

mcpName := "worker"
mc := &mcfgv1.MachineConfig{}
mcName := "00-" + mcpName + "-" + OVS_HWOL_MACHINE_CONFIG_NAME_SUFFIX
err = k8sClient.Get(goctx.TODO(), types.NamespacedName{Name: mcName, Namespace: testNamespace}, mc)
Expect(errors.IsNotFound(err)).Should(BeTrue())

mcp := &mcfgv1.MachineConfigPool{}
err = k8sClient.Get(goctx.TODO(), types.NamespacedName{Name: mcpName, Namespace: testNamespace}, mcp)
Expect(errors.IsNotFound(err)).Should(BeTrue())

mcp = &mcfgv1.MachineConfigPool{
ObjectMeta: metav1.ObjectMeta{
Name: mcpName,
},
Spec: mcfgv1.MachineConfigPoolSpec{
NodeSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"node-role.kubernetes.io/worker": "",
},
},
},
}
err = k8sClient.Create(goctx.TODO(), mcp)
Expect(err).NotTo(HaveOccurred())

config.Spec.OvsHardwareOffload = []sriovnetworkv1.OvsHardwareOffloadConfig{
{
NodeSelector: map[string]string{"node-role.kubernetes.io/worker": ""},
},
}
err = k8sClient.Update(goctx.TODO(), config)
Expect(err).NotTo(HaveOccurred())
Eventually(func() error {
mc := &mcfgv1.MachineConfig{}
err := k8sClient.Get(goctx.TODO(), types.NamespacedName{Name: mcName, Namespace: testNamespace}, mc)
if err != nil {
return err
}
return nil
}, timeout*3, interval).Should(Succeed())
})
})
})
Loading

0 comments on commit f5c463c

Please sign in to comment.