From b35cbe09b98c23ee66aaac92306d937df014de6f Mon Sep 17 00:00:00 2001 From: Andrea Panattoni Date: Fri, 14 Jul 2023 14:32:24 +0200 Subject: [PATCH 1/5] Set zap log TimeEncoder Set a TimeEncoder for zap logging system to have more speaking timestamp. from `1.6893201844680622e+09` to `2023-07-14T09:52:29.539392293Z` Signed-off-by: Andrea Panattoni --- main.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/main.go b/main.go index f14dee49c..b1e73501f 100644 --- a/main.go +++ b/main.go @@ -25,6 +25,7 @@ import ( netattdefv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" openshiftconfigv1 "github.com/openshift/api/config/v1" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" + "go.uber.org/zap/zapcore" "k8s.io/apimachinery/pkg/api/errors" // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) @@ -76,6 +77,7 @@ func main() { "Enabling this will ensure there is only one active controller manager.") opts := zap.Options{ Development: true, + TimeEncoder: zapcore.RFC3339NanoTimeEncoder, } opts.BindFlags(flag.CommandLine) flag.Parse() From 792575edfc6f6e7e7422cc0eb770953ea3d3240b Mon Sep 17 00:00:00 2001 From: Andrea Panattoni Date: Fri, 14 Jul 2023 14:34:12 +0200 Subject: [PATCH 2/5] Reduce SriovOperator verbosity In `renderDevicePluginConfigData()`, avoid logging the full ResourceList for every resource in the loop. Avoid logging in helper functions, as they can be invoked in loops. Signed-off-by: Andrea Panattoni --- api/v1/helper.go | 1 - controllers/sriovnetworknodepolicy_controller.go | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/api/v1/helper.go b/api/v1/helper.go index 5d2e1e5ad..a942da1e3 100644 --- a/api/v1/helper.go +++ b/api/v1/helper.go @@ -224,7 +224,6 @@ func (p *SriovNetworkNodePolicy) Selected(node *corev1.Node) bool { } return false } - log.Info("Selected():", "node", node.Name) return true } diff --git a/controllers/sriovnetworknodepolicy_controller.go b/controllers/sriovnetworknodepolicy_controller.go index 96a0146ed..eb1177f24 100644 --- a/controllers/sriovnetworknodepolicy_controller.go +++ b/controllers/sriovnetworknodepolicy_controller.go @@ -576,7 +576,7 @@ func renderDsForCR(path string, data *render.RenderData) ([]*uns.Unstructured, e func (r *SriovNetworkNodePolicyReconciler) renderDevicePluginConfigData(pl *sriovnetworkv1.SriovNetworkNodePolicyList, node *corev1.Node) (dptypes.ResourceConfList, error) { logger := log.Log.WithName("renderDevicePluginConfigData") - logger.Info("Start to render device plugin config data") + logger.Info("Start to render device plugin config data", "node", node.Name) rcl := dptypes.ResourceConfList{} for _, p := range pl.Items { if p.Name == constants.DefaultPolicyName { @@ -717,7 +717,7 @@ func (r *SriovNetworkNodePolicyReconciler) renderDevicePluginConfigData(pl *srio rawNetDeviceSelectors := json.RawMessage(netDeviceSelectorsMarshal) rc.Selectors = &rawNetDeviceSelectors rcl.ResourceList = append(rcl.ResourceList, *rc) - logger.Info("Add resource", "Resource", *rc, "Resource list", rcl.ResourceList) + logger.Info("Add resource", "Resource", *rc) } } return rcl, nil From 25b1d3acd67b60a97c4b7a0d3e4606923d4b5207 Mon Sep 17 00:00:00 2001 From: Andrea Panattoni Date: Tue, 4 Jul 2023 14:33:26 +0200 Subject: [PATCH 3/5] Avoid logging `devlink` warning Devlink mode is not supported by every SR-IOV NIC model. Signed-off-by: Andrea Panattoni --- pkg/utils/utils.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 30601847c..b8b04a0ca 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -2,6 +2,7 @@ package utils import ( "bytes" + "errors" "fmt" "io/ioutil" "math/rand" @@ -719,10 +720,16 @@ func generateRandomGUID() net.HardwareAddr { func GetNicSriovMode(pciAddress string) (string, error) { glog.V(2).Infof("GetNicSriovMode(): device %s", pciAddress) + devLink, err := netlink.DevLinkGetDeviceByName("pci", pciAddress) if err != nil { + if errors.Is(err, syscall.ENODEV) { + // the device doesn't support devlink + return "", nil + } return "", err } + return devLink.Attrs.Eswitch.Mode, nil } From e398bdbc1ea52c39b50e7d68ef24d4f9eafafa8a Mon Sep 17 00:00:00 2001 From: Andrea Panattoni Date: Tue, 4 Jul 2023 14:35:00 +0200 Subject: [PATCH 4/5] Reduce vebosity of config-damon Log `getLinkType(): ...` only if LogLevel == 2 Signed-off-by: Andrea Panattoni --- pkg/utils/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index b8b04a0ca..b57bb6d2a 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -666,7 +666,7 @@ func unbindDriverIfNeeded(vfAddr string, isRdma bool) error { } func getLinkType(ifaceStatus sriovnetworkv1.InterfaceExt) string { - glog.Infof("getLinkType(): Device %s", ifaceStatus.PciAddress) + glog.V(2).Infof("getLinkType(): Device %s", ifaceStatus.PciAddress) if ifaceStatus.Name != "" { link, err := netlink.LinkByName(ifaceStatus.Name) if err != nil { From 3692e2b2160819d9efe018a87f723735398bc21c Mon Sep 17 00:00:00 2001 From: Andrea Panattoni Date: Fri, 14 Jul 2023 15:49:47 +0200 Subject: [PATCH 5/5] Avoid reconciling policies multiple times `SriovNetworkNodePolicy` reconcile function lists all node policies and updates all SriovNetworkNodeState. Hence there is no need to resync all of them periodically. Avoid triggering multiple reconciliations when creating multiple policies in a row (that is what happens when using yaml files). Signed-off-by: Andrea Panattoni --- .../sriovnetworknodepolicy_controller.go | 39 ++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/controllers/sriovnetworknodepolicy_controller.go b/controllers/sriovnetworknodepolicy_controller.go index eb1177f24..16504a7b3 100644 --- a/controllers/sriovnetworknodepolicy_controller.go +++ b/controllers/sriovnetworknodepolicy_controller.go @@ -23,6 +23,7 @@ import ( "os" "sort" "strings" + "time" errs "github.com/pkg/errors" appsv1 "k8s.io/api/apps/v1" @@ -35,11 +36,15 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" kscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/util/workqueue" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/source" dptypes "github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/types" @@ -49,6 +54,8 @@ import ( render "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/render" ) +const nodePolicySyncEventName = "node-policy-sync-event" + // SriovNetworkNodePolicyReconciler reconciles a SriovNetworkNodePolicy object type SriovNetworkNodePolicyReconciler struct { client.Client @@ -69,8 +76,12 @@ type SriovNetworkNodePolicyReconciler struct { // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.8.3/pkg/reconcile func (r *SriovNetworkNodePolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - reqLogger := log.FromContext(ctx).WithValues("sriovnetworknodepolicy", req.NamespacedName) + // Only handle node-policy-sync-event + if req.Name != nodePolicySyncEventName || req.Namespace != "" { + return reconcile.Result{}, nil + } + reqLogger := log.FromContext(ctx) reqLogger.Info("Reconciling") defaultPolicy := &sriovnetworkv1.SriovNetworkNodePolicy{} @@ -152,8 +163,34 @@ func (r *SriovNetworkNodePolicyReconciler) Reconcile(ctx context.Context, req ct // SetupWithManager sets up the controller with the Manager. func (r *SriovNetworkNodePolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { + qHandler := func(q workqueue.RateLimitingInterface) { + q.AddAfter(reconcile.Request{NamespacedName: types.NamespacedName{ + Namespace: "", + Name: nodePolicySyncEventName, + }}, time.Second) + } + + delayedEventHandler := handler.Funcs{ + CreateFunc: func(e event.CreateEvent, q workqueue.RateLimitingInterface) { + log.Log.WithName("SriovNetworkNodePolicy"). + Info("Enqueuing sync for create event", "resource", e.Object.GetName()) + qHandler(q) + }, + UpdateFunc: func(e event.UpdateEvent, q workqueue.RateLimitingInterface) { + log.Log.WithName("SriovNetworkNodePolicy"). + Info("Enqueuing sync for update event", "resource", e.ObjectNew.GetName()) + qHandler(q) + }, + DeleteFunc: func(e event.DeleteEvent, q workqueue.RateLimitingInterface) { + log.Log.WithName("SriovNetworkNodePolicy"). + Info("Enqueuing sync for delete event", "resource", e.Object.GetName()) + qHandler(q) + }, + } + return ctrl.NewControllerManagedBy(mgr). For(&sriovnetworkv1.SriovNetworkNodePolicy{}). + Watches(&source.Kind{Type: &sriovnetworkv1.SriovNetworkNodePolicy{}}, delayedEventHandler). Complete(r) }