From 2dc42f49413dbf427c94032d30a6d03d38799866 Mon Sep 17 00:00:00 2001 From: renxiangyu Date: Thu, 11 Jan 2024 14:33:51 +0800 Subject: [PATCH] fix: network_manager.go code repeat Signed-off-by: renxiangyu --- pkg/apis/kosmos/v1alpha1/nodeconfig_types.go | 10 + .../network-manager/network_manager.go | 229 +++++++++--------- 2 files changed, 128 insertions(+), 111 deletions(-) diff --git a/pkg/apis/kosmos/v1alpha1/nodeconfig_types.go b/pkg/apis/kosmos/v1alpha1/nodeconfig_types.go index 1f4a77a02..3d73cd2b1 100644 --- a/pkg/apis/kosmos/v1alpha1/nodeconfig_types.go +++ b/pkg/apis/kosmos/v1alpha1/nodeconfig_types.go @@ -10,6 +10,16 @@ import ( // +kubebuilder:resource:scope="Cluster" // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object +const ( + DeviceName = "Devices" + RouteName = "Routes" + IptablesName = "Iptables" + FdbName = "Fdbs" + ArpName = "Arps" + DeleteConfigName = "deleteConfig" + CreateConfigName = "createConfig" +) + type NodeConfig struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` diff --git a/pkg/clusterlink/agent-manager/network-manager/network_manager.go b/pkg/clusterlink/agent-manager/network-manager/network_manager.go index f1898fa55..055cd0d63 100644 --- a/pkg/clusterlink/agent-manager/network-manager/network_manager.go +++ b/pkg/clusterlink/agent-manager/network-manager/network_manager.go @@ -2,6 +2,7 @@ package networkmanager import ( "fmt" + "reflect" "sync" "github.com/pkg/errors" @@ -96,54 +97,60 @@ func (e *NetworkManager) Diff(oldConfig, newConfig *clusterlinkv1alpha1.NodeConf deleteConfig := &clusterlinkv1alpha1.NodeConfigSpec{} createConfig := &clusterlinkv1alpha1.NodeConfigSpec{} isSame := true - // devices: - if flag, deleteRecord, createRecord := compareFunc(oldConfig.Devices, newConfig.Devices, func(a, b clusterlinkv1alpha1.Device) bool { - return a.Compare(b) - }); !flag { - deleteConfig.Devices = deleteRecord - createConfig.Devices = createRecord - isSame = false - } - // routes: - if flag, deleteRecord, createRecord := compareFunc(oldConfig.Routes, newConfig.Routes, func(a, b clusterlinkv1alpha1.Route) bool { - return a.Compare(b) - }); !flag { - deleteConfig.Routes = deleteRecord - createConfig.Routes = createRecord - isSame = false - } - // iptables: - if flag, deleteRecord, createRecord := compareFunc(oldConfig.Iptables, newConfig.Iptables, func(a, b clusterlinkv1alpha1.Iptables) bool { - return a.Compare(b) - }); !flag { - deleteConfig.Iptables = deleteRecord - createConfig.Iptables = createRecord - isSame = false - } - // fdbs: - if flag, deleteRecord, createRecord := compareFunc(oldConfig.Fdbs, newConfig.Fdbs, func(a, b clusterlinkv1alpha1.Fdb) bool { - return a.Compare(b) - }); !flag { - // filter ff:ff:ff:ff:ff:ff - for _, dr := range deleteRecord { - if dr.Mac != "ff:ff:ff:ff:ff:ff" { - deleteConfig.Fdbs = append(deleteConfig.Fdbs, dr) - } + + typeOfOldConfig := reflect.TypeOf(oldConfig) + valueOfDeleteConfig := reflect.ValueOf(&deleteConfig).Elem() + + for i := 0; i < typeOfOldConfig.NumField(); i++ { + fieldName := typeOfOldConfig.Field(i).Name + fieldType := typeOfOldConfig.Field(i).Type + valueByName := valueOfDeleteConfig.FieldByName(fieldName) + + var flag bool + switch fieldName { + case clusterlinkv1alpha1.DeviceName: + flag, deleteConfig.Devices, createConfig.Devices = + compareFunc(oldConfig.Devices, newConfig.Devices, func(a, b clusterlinkv1alpha1.Device) bool { + return a.Compare(b) + }) + case clusterlinkv1alpha1.RouteName: + flag, deleteConfig.Routes, createConfig.Routes = + compareFunc(oldConfig.Routes, newConfig.Routes, func(a, b clusterlinkv1alpha1.Route) bool { + return a.Compare(b) + }) + case clusterlinkv1alpha1.IptablesName: + flag, deleteConfig.Iptables, createConfig.Iptables = + compareFunc(oldConfig.Iptables, newConfig.Iptables, func(a, b clusterlinkv1alpha1.Iptables) bool { + return a.Compare(b) + }) + case clusterlinkv1alpha1.FdbName: + flag, deleteConfig.Fdbs, createConfig.Fdbs = + compareFunc(oldConfig.Fdbs, newConfig.Fdbs, func(a, b clusterlinkv1alpha1.Fdb) bool { + return a.Compare(b) + }) + case clusterlinkv1alpha1.ArpName: + flag, deleteConfig.Arps, createConfig.Arps = + compareFunc(oldConfig.Arps, newConfig.Arps, func(a, b clusterlinkv1alpha1.Arp) bool { + return a.Compare(b) + }) } - createConfig.Fdbs = createRecord - isSame = false - } - // arps: - if flag, deleteRecord, createRecord := compareFunc(oldConfig.Arps, newConfig.Arps, func(a, b clusterlinkv1alpha1.Arp) bool { - return a.Compare(b) - }); !flag { - for _, dr := range deleteRecord { - if dr.Mac != "ff:ff:ff:ff:ff:ff" { - deleteConfig.Arps = append(deleteConfig.Arps, dr) + if !flag { + isSame = flag + if fieldName == clusterlinkv1alpha1.ArpName || fieldName == clusterlinkv1alpha1.FdbName { + len := valueByName.Len() + deleteSlice := reflect.MakeSlice(fieldType, len, len) + for j := 0; j < len; j++ { + fieldByName := valueByName.Index(j).FieldByName("Mac") + if fieldByName.IsValid() { + if fieldByName.Interface().(string) == "ff:ff:ff:ff:ff:ff" { + continue + } + } + deleteSlice.Index(j).Set(valueByName.Index(j)) + } + valueByName.Set(deleteSlice) } } - createConfig.Arps = createRecord - isSame = false } return isSame, deleteConfig, createConfig } @@ -152,79 +159,79 @@ func (e *NetworkManager) LoadSystemConfig() (*clusterlinkv1alpha1.NodeConfigSpec return e.NetworkInterface.LoadSysConfig() } -// nolint:dupl -func (e *NetworkManager) WriteSys(configDiff *ConfigDiff) error { - var errs error +func (e *NetworkManager) delete(value reflect.Value, name string) error { + var err error + switch name { + case clusterlinkv1alpha1.DeviceName: + err = e.NetworkInterface.DeleteDevices(value.Interface().([]clusterlinkv1alpha1.Device)) + case clusterlinkv1alpha1.RouteName: + err = e.NetworkInterface.DeleteRoutes(value.Interface().([]clusterlinkv1alpha1.Route)) + case clusterlinkv1alpha1.IptablesName: + err = e.NetworkInterface.DeleteIptables(value.Interface().([]clusterlinkv1alpha1.Iptables)) + case clusterlinkv1alpha1.FdbName: + err = e.NetworkInterface.DeleteFdbs(value.Interface().([]clusterlinkv1alpha1.Fdb)) + case clusterlinkv1alpha1.ArpName: + err = e.NetworkInterface.DeleteArps(value.Interface().([]clusterlinkv1alpha1.Arp)) + } + return err +} - if configDiff.deleteConfig != nil { - config := configDiff.deleteConfig - if config.Arps != nil { - if err := e.NetworkInterface.DeleteArps(config.Arps); err != nil { - klog.Warning(err) - errs = errors.Wrap(err, fmt.Sprint(errs)) - } - } - if config.Fdbs != nil { - if err := e.NetworkInterface.DeleteFdbs(config.Fdbs); err != nil { - klog.Warning(err) - errs = errors.Wrap(err, fmt.Sprint(errs)) - } - } - if config.Iptables != nil { - if err := e.NetworkInterface.DeleteIptables(config.Iptables); err != nil { - klog.Warning(err) - errs = errors.Wrap(err, fmt.Sprint(errs)) - } - } - if config.Routes != nil { - if err := e.NetworkInterface.DeleteRoutes(config.Routes); err != nil { - klog.Warning(err) - errs = errors.Wrap(err, fmt.Sprint(errs)) - } - } - if config.Devices != nil { - if err := e.NetworkInterface.DeleteDevices(config.Devices); err != nil { - klog.Warning(err) - errs = errors.Wrap(err, fmt.Sprint(errs)) - } - } +func (e *NetworkManager) add(value reflect.Value, name string) error { + var err error + switch name { + case clusterlinkv1alpha1.DeviceName: + err = e.NetworkInterface.AddDevices(value.Interface().([]clusterlinkv1alpha1.Device)) + case clusterlinkv1alpha1.RouteName: + err = e.NetworkInterface.AddRoutes(value.Interface().([]clusterlinkv1alpha1.Route)) + case clusterlinkv1alpha1.IptablesName: + err = e.NetworkInterface.AddIptables(value.Interface().([]clusterlinkv1alpha1.Iptables)) + case clusterlinkv1alpha1.FdbName: + err = e.NetworkInterface.AddFdbs(value.Interface().([]clusterlinkv1alpha1.Fdb)) + case clusterlinkv1alpha1.ArpName: + err = e.NetworkInterface.AddArps(value.Interface().([]clusterlinkv1alpha1.Arp)) } + return err +} - if configDiff.createConfig != nil { - config := configDiff.createConfig - // must create device first - if config.Devices != nil { - if err := e.NetworkInterface.AddDevices(config.Devices); err != nil { - klog.Warning(err) - errs = errors.Wrap(err, fmt.Sprint(errs)) - } - } - if config.Arps != nil { - if err := e.NetworkInterface.AddArps(config.Arps); err != nil { - klog.Warning(err) - errs = errors.Wrap(err, fmt.Sprint(errs)) - } - } - if config.Fdbs != nil { - if err := e.NetworkInterface.AddFdbs(config.Fdbs); err != nil { - klog.Warning(err) - errs = errors.Wrap(err, fmt.Sprint(errs)) - } - } - if config.Iptables != nil { - if err := e.NetworkInterface.AddIptables(config.Iptables); err != nil { - klog.Warning(err) - errs = errors.Wrap(err, fmt.Sprint(errs)) +// nolint:dupl +func (e *NetworkManager) WriteSys(configDiff *ConfigDiff) error { + var errs error + valueOfDiff := reflect.ValueOf(*configDiff) + typeOfDiff := reflect.TypeOf(*configDiff) + for i := 0; i < typeOfDiff.NumField(); i++ { + valueFieldOfDiff := valueOfDiff.Field(i) + typeNameOfDiff := typeOfDiff.Field(i).Name + if !valueFieldOfDiff.IsZero() { + var config clusterlinkv1alpha1.NodeConfigSpec + var isDelete bool + switch typeNameOfDiff { + case clusterlinkv1alpha1.DeleteConfigName: + config = *valueFieldOfDiff.Interface().(*clusterlinkv1alpha1.NodeConfigSpec) + isDelete = true + case clusterlinkv1alpha1.CreateConfigName: + config = *valueFieldOfDiff.Interface().(*clusterlinkv1alpha1.NodeConfigSpec) + isDelete = false } - } - if config.Routes != nil { - if err := e.NetworkInterface.AddRoutes(config.Routes); err != nil { - klog.Warning(err) - errs = errors.Wrap(err, fmt.Sprint(errs)) + valueOf := reflect.ValueOf(config) + typeOf := reflect.TypeOf(config) + for j := 0; j < typeOf.NumField(); j++ { + valueField := valueOf.Field(j) + typeName := typeOf.Field(j).Name + if !valueField.IsZero() { + var err error + if isDelete { + err = e.delete(valueField, typeName) + } else { + err = e.add(valueField, typeName) + } + if err != nil { + klog.Warning(err) + errs = errors.Wrap(err, fmt.Sprint(errs)) + } + } } } } - return errs }