From 8783bfb78eba2a40818e51c12023bc84c7a6f9d3 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 | 237 +++++++++--------- .../network-manager/network_manager_test.go | 91 +++++++ 3 files changed, 222 insertions(+), 116 deletions(-) create mode 100644 pkg/clusterlink/agent-manager/network-manager/network_manager_test.go 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..5a82736c0 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" @@ -29,9 +30,9 @@ type NetworkManager struct { } type ConfigDiff struct { - deleteConfig *clusterlinkv1alpha1.NodeConfigSpec + DeleteConfig *clusterlinkv1alpha1.NodeConfigSpec // updateConfig *clusterlinkv1alpha1.NodeConfigSpec - createConfig *clusterlinkv1alpha1.NodeConfigSpec + CreateConfig *clusterlinkv1alpha1.NodeConfigSpec } func NewNetworkManager(network network.NetWork) *NetworkManager { @@ -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,77 @@ 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, typeName string) error { + var err error + switch typeName { + 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)) + default: + err = fmt.Errorf("not found this value, name: %s", typeName) + } + 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, typeName string) error { + var err error + switch typeName { + 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)) + default: + err = fmt.Errorf("not found this value, name: %s", typeName) } + 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)) - } - } - if config.Routes != nil { - if err := e.NetworkInterface.AddRoutes(config.Routes); err != nil { - klog.Warning(err) - errs = errors.Wrap(err, fmt.Sprint(errs)) +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).Elem() + typeNameOfDiff := typeOfDiff.Field(i).Name + if !valueFieldOfDiff.IsZero() { + config := valueFieldOfDiff.Interface().(clusterlinkv1alpha1.NodeConfigSpec) + 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 + switch typeNameOfDiff { + case clusterlinkv1alpha1.DeleteConfigName: + err = e.delete(valueField, typeName) + case clusterlinkv1alpha1.CreateConfigName: + err = e.add(valueField, typeName) + default: + errs = fmt.Errorf("not found this value, name: %s", typeNameOfDiff) + return errs + } + if err != nil { + klog.Warning(err) + errs = errors.Wrap(err, fmt.Sprint(errs)) + } + } } } } - return errs } @@ -282,8 +287,8 @@ func (e *NetworkManager) UpdateSync() NodeConfigSyncStatus { } err = e.WriteSys(&ConfigDiff{ - deleteConfig: deleteConfig, - createConfig: createConfig, + DeleteConfig: deleteConfig, + CreateConfig: createConfig, }) if err != nil { e.Status = NodeConfigSyncException diff --git a/pkg/clusterlink/agent-manager/network-manager/network_manager_test.go b/pkg/clusterlink/agent-manager/network-manager/network_manager_test.go new file mode 100644 index 000000000..a68606271 --- /dev/null +++ b/pkg/clusterlink/agent-manager/network-manager/network_manager_test.go @@ -0,0 +1,91 @@ +package networkmanager + +import ( + "fmt" + "testing" + + clusterlinkv1alpha1 "github.com/kosmos.io/kosmos/pkg/apis/kosmos/v1alpha1" + "github.com/kosmos.io/kosmos/pkg/clusterlink/network" +) + +func TestNetworkManager_Diff(t *testing.T) { + type args struct { + oldConfig *clusterlinkv1alpha1.NodeConfigSpec + newConfig *clusterlinkv1alpha1.NodeConfigSpec + deleteConfig *clusterlinkv1alpha1.NodeConfigSpec + createConfig *clusterlinkv1alpha1.NodeConfigSpec + } + + tests := []struct { + name string + args args + }{ + {name: "Test1", args: args{ + oldConfig: &clusterlinkv1alpha1.NodeConfigSpec{ + Devices: []clusterlinkv1alpha1.Device{{"vxlan1", "vx-local", "210.168.11.13/8", "7a:0e:b7:89:90:45", "*", 55, 4877}}, + Routes: []clusterlinkv1alpha1.Route{{"10.234.0.0/16", "220.168.11.4", "vx-bridge"}}, + Iptables: []clusterlinkv1alpha1.Iptables{{"nat", "POSTROUTING", "-s 220.0.0.0/8 -j MASQUERADE"}}, + Fdbs: []clusterlinkv1alpha1.Fdb{{"192.168.11.7", "b2:52:aa:8d:37:b1", "vx-local"}, {"192.168.11.7", "ff:ff:ff:ff:ff:ff", "vx-local"}}, + Arps: []clusterlinkv1alpha1.Arp{{"220.168.11.4", "8a:5c:50:6b:30:99", "vx-bridge"}}, + }, + newConfig: &clusterlinkv1alpha1.NodeConfigSpec{ + Devices: []clusterlinkv1alpha1.Device{{"vxlan", "vx-local", "210.168.11.13/8", "7a:0e:b7:89:90:45", "*", 55, 4877}}, + Routes: []clusterlinkv1alpha1.Route{{"10.234.0.0/16", "220.168.11.4", "vx-bridge"}}, + Iptables: []clusterlinkv1alpha1.Iptables{{"nat", "POSTROUTING", "-s 220.0.0.0/8 -j MASQUERADE"}}, + Fdbs: []clusterlinkv1alpha1.Fdb{{"192.168.11.7", "ff:ff:ff:ff:ff:f1", "vx-local"}}, + Arps: []clusterlinkv1alpha1.Arp{{"220.168.11.4", "8a:5c:50:6b:30:99", "vx-bridge"}}, + }, + deleteConfig: &clusterlinkv1alpha1.NodeConfigSpec{}, + createConfig: &clusterlinkv1alpha1.NodeConfigSpec{}, + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + net := network.NewNetWork(false) + nManager := NewNetworkManager(net) + flag, deleteConfig, createConfig := nManager.Diff(tt.args.oldConfig, tt.args.newConfig) + fmt.Println("flag: ", flag) + fmt.Println("deleteConfig: ", deleteConfig) + fmt.Println("createConfig: ", createConfig) + }) + } +} + +func TestNetworkManager_WriteSys(t *testing.T) { + type args struct { + configDiff *ConfigDiff + } + + tests := []struct { + name string + args args + }{ + {"Test1", args{ + configDiff: &ConfigDiff{ + DeleteConfig: &clusterlinkv1alpha1.NodeConfigSpec{ + Devices: []clusterlinkv1alpha1.Device{{"vxlan1", "vx-local", "210.168.11.13/8", "7a:0e:b7:89:90:45", "*", 55, 4877}}, + Routes: []clusterlinkv1alpha1.Route{}, + Iptables: []clusterlinkv1alpha1.Iptables{}, + Fdbs: []clusterlinkv1alpha1.Fdb{{"192.168.11.7", "b2:52:aa:8d:37:b1", "vx-local"}}, + Arps: []clusterlinkv1alpha1.Arp{}, + }, + CreateConfig: &clusterlinkv1alpha1.NodeConfigSpec{ + Devices: []clusterlinkv1alpha1.Device{{"vxlan1", "vx-local", "210.168.11.13/8", "7a:0e:b7:89:90:45", "*", 55, 4877}}, + Routes: []clusterlinkv1alpha1.Route{}, + Iptables: []clusterlinkv1alpha1.Iptables{}, + Fdbs: []clusterlinkv1alpha1.Fdb{{"192.168.11.7", "ff:ff:ff:ff:ff:ff", "vx-local"}}, + Arps: []clusterlinkv1alpha1.Arp{}, + }, + }, + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + net := network.NewNetWork(false) + nManager := NewNetworkManager(net) + err := nManager.WriteSys(tt.args.configDiff) + fmt.Println(err) + }) + } + +}