From 3edf314c610fa420c31655f8b156c33166a16679 Mon Sep 17 00:00:00 2001 From: Radim Hrazdil <32546791+rhrazdil@users.noreply.github.com> Date: Mon, 21 Feb 2022 16:11:23 +0100 Subject: [PATCH] [release 0.52] Fix numeric iface name in route (#993) * Filter CNI veth interfaces by type and DeviceState (un/managed) (#809) * Filter interfaces by type and DeviceState Instead of filtering interfaces in NNS by name regex, use type attribute and deviceState obtained from NetworkManager. Drop veth interfaces that are not being managed by NetworkManager. Signed-off-by: Radim Hrazdil * e2e: Add test verifying interface_filter isn't failing Since NetworkManager client could fail to connect to dbus (potentially), we should make sure that we catch such events. When the NetworkManager client fails to connect, the NNS is not filtered and error message is logged by handler. This commit adds test verifying that handler log doesn't contain such message. Signed-off-by: Radim Hrazdil * Remove unused INTERFACES_FILTER variable Signed-off-by: Radim Hrazdil * Update section about vlan filtering from nns It is now not possible to adjust vlan filtering, as we stricly filter only unmanaged veth interfaces Signed-off-by: Radim Hrazdil * Fix issue with Route next-hop-interface being interpreted as float64 (#987) * unit test filtering of ifaces with numeric names Signed-off-by: Radim Hrazdil * Fix next-hop-interface with numeric names being interpreted as float Signed-off-by: Radim Hrazdil * Use centos 8 Stream as base image for handler Signed-off-by: Radim Hrazdil --- Makefile | 2 +- build/Dockerfile | 2 +- controllers/node_controller.go | 4 +- controllers/node_controller_test.go | 18 +- deploy/handler/operator.yaml | 2 - docs/user-guide/101-reporting.md | 23 +- pkg/state/filter.go | 95 ++++++-- pkg/state/filter_test.go | 337 ++++++++++++++++------------ pkg/state/type.go | 40 +++- test/e2e/handler/nns_filter_test.go | 25 +++ 10 files changed, 347 insertions(+), 201 deletions(-) create mode 100644 test/e2e/handler/nns_filter_test.go diff --git a/Makefile b/Makefile index fe6c399651..93dc39bf13 100644 --- a/Makefile +++ b/Makefile @@ -162,7 +162,7 @@ push-operator: operator push: push-handler push-operator test/unit: $(GO) - INTERFACES_FILTER="" NODE_NAME=node01 $(GINKGO) $(unit_test_args) $(WHAT) + NODE_NAME=node01 $(GINKGO) $(unit_test_args) $(WHAT) test-e2e-handler: $(GO) KUBECONFIG=$(KUBECONFIG) OPERATOR_NAMESPACE=$(OPERATOR_NAMESPACE) $(GINKGO) $(e2e_test_args) ./test/e2e/handler ... -- $(E2E_TEST_SUITE_ARGS) diff --git a/build/Dockerfile b/build/Dockerfile index 66e7ed2020..419a63755c 100644 --- a/build/Dockerfile +++ b/build/Dockerfile @@ -1,4 +1,4 @@ -FROM centos:8 +FROM quay.io/centos/centos:stream8 ARG NMSTATE_COPR_REPO ARG NM_COPR_REPO RUN dnf install -b -y dnf-plugins-core && \ diff --git a/controllers/node_controller.go b/controllers/node_controller.go index b7b4da46e7..b6cf10c43e 100644 --- a/controllers/node_controller.go +++ b/controllers/node_controller.go @@ -54,6 +54,7 @@ type NodeReconciler struct { lastState shared.State nmstateUpdater NmstateUpdater nmstatectlShow NmstatectlShow + deviceInfo state.DeviceInfoer } // Reconcile reads that state of the cluster for a Node object and makes changes based on the state read @@ -68,7 +69,7 @@ func (r *NodeReconciler) Reconcile(ctx context.Context, request ctrl.Request) (c return ctrl.Result{}, err } - currentState, err := state.FilterOut(shared.NewState(currentStateRaw)) + currentState, err := state.FilterOut(shared.NewState(currentStateRaw), r.deviceInfo) if err != nil { return ctrl.Result{}, err } @@ -118,6 +119,7 @@ func (r *NodeReconciler) SetupWithManager(mgr ctrl.Manager) error { r.nmstateUpdater = nmstate.CreateOrUpdateNodeNetworkState r.nmstatectlShow = nmstatectl.Show + r.deviceInfo = state.DeviceInfo{} // By default all this functors return true so controller watch all events, // but we only want to watch create/delete for current node. diff --git a/controllers/node_controller_test.go b/controllers/node_controller_test.go index 02bcf75b44..cf9113e08c 100644 --- a/controllers/node_controller_test.go +++ b/controllers/node_controller_test.go @@ -19,6 +19,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" + networkmanager "github.com/phoracek/networkmanager-go/src" + "github.com/nmstate/kubernetes-nmstate/api/shared" nmstatev1beta1 "github.com/nmstate/kubernetes-nmstate/api/v1beta1" "github.com/nmstate/kubernetes-nmstate/pkg/nmstatectl" @@ -26,6 +28,15 @@ import ( "github.com/nmstate/kubernetes-nmstate/pkg/state" ) +type fakeDeviceInfo struct{} + +func (fakeDeviceInfo) DeviceStates() (map[string]networkmanager.DeviceState, error) { + return map[string]networkmanager.DeviceState{ + "eth1": networkmanager.DeviceStateActivated, + "eth2": networkmanager.DeviceStateActivated, + }, nil +} + var _ = Describe("Node controller reconcile", func() { var ( cl client.Client @@ -65,6 +76,7 @@ var _ = Describe("Node controller reconcile", func() { reconciler.Scheme = s reconciler.nmstateUpdater = nmstate.CreateOrUpdateNodeNetworkState reconciler.nmstatectlShow = nmstatectl.Show + reconciler.deviceInfo = fakeDeviceInfo{} reconciler.lastState = shared.NewState("lastState") observedState = ` --- @@ -76,8 +88,9 @@ routes: running: [] config: [] ` + var err error - filteredOutObservedState, err = state.FilterOut(shared.NewState(observedState)) + filteredOutObservedState, err = state.FilterOut(shared.NewState(observedState), reconciler.deviceInfo) Expect(err).ToNot(HaveOccurred()) reconciler.nmstatectlShow = func() (string, error) { @@ -154,6 +167,7 @@ routes: config: [] ` ) + BeforeEach(func() { By("Set last state") reconciler.lastState = filteredOutObservedState @@ -171,7 +185,7 @@ routes: obtainedNNS := nmstatev1beta1.NodeNetworkState{} err = cl.Get(context.TODO(), types.NamespacedName{Name: existingNodeName}, &obtainedNNS) Expect(err).ToNot(HaveOccurred()) - filteredOutExpectedState, err := state.FilterOut(shared.NewState(expectedStateRaw)) + filteredOutExpectedState, err := state.FilterOut(shared.NewState(expectedStateRaw), reconciler.deviceInfo) Expect(err).ToNot(HaveOccurred()) Expect(obtainedNNS.Status.CurrentState.String()).To(Equal(filteredOutExpectedState.String())) }) diff --git a/deploy/handler/operator.yaml b/deploy/handler/operator.yaml index 5814c67463..4af69badd5 100644 --- a/deploy/handler/operator.yaml +++ b/deploy/handler/operator.yaml @@ -249,8 +249,6 @@ spec: valueFrom: fieldRef: fieldPath: spec.nodeName - - name: INTERFACES_FILTER - value: "{veth*,cali*}" - name: ENABLE_PROFILER value: "False" - name: PROFILER_PORT diff --git a/docs/user-guide/101-reporting.md b/docs/user-guide/101-reporting.md index b085ac6ba2..69d88f45e0 100644 --- a/docs/user-guide/101-reporting.md +++ b/docs/user-guide/101-reporting.md @@ -164,27 +164,10 @@ observed state is fresh enough. The reported state is updated every 5 seconds. -## Filter reported interfaces +## Node Network State interfaces filtering -By default, all `veth*` interfaces are omitted from the report in order not to -clutter the output with all Pod connections. This filtering can be adjusted via -environment variable `INTERFACES_FILTER`at nmstate-handler daemonset. - -| Filter | Effect | -| --- | --- | -| `"veth*"` | Omit all interfaces starting with `veth` from the report (default) | -| `""` | Disable the filter, show all interfaces found on the node | -| `"{veth*,vnet*}"` | Omit all interfaces starting with either `veth` or `vnet` | - - -To override the default configuration, use `set env` command. Additionally, in -order for the config to take effect, all handlers will be restarted: - -```json -kubectl set env -n nmstate daemonset/nmstate-handler INTERFACES_FILTER="" -``` - -The `NodeNetworkState` will now list all interfaces seen on the host. +All unmanaged `veth` interfaces are omitted from the report in order to not +clutter the output with all Pod connections. ## Continue reading diff --git a/pkg/state/filter.go b/pkg/state/filter.go index 2b5a0a2180..79b968558e 100644 --- a/pkg/state/filter.go +++ b/pkg/state/filter.go @@ -1,47 +1,86 @@ package state import ( - "os" + networkmanager "github.com/phoracek/networkmanager-go/src" - "github.com/gobwas/glob" "github.com/nmstate/kubernetes-nmstate/api/shared" "github.com/nmstate/kubernetes-nmstate/pkg/environment" goyaml "gopkg.in/yaml.v2" + logf "sigs.k8s.io/controller-runtime/pkg/log" yaml "sigs.k8s.io/yaml" ) +const ( + INTERFACE_FILTER = "interface_filter" +) + var ( - interfacesFilterGlobFromEnv glob.Glob + filterLog = logf.Log.WithName(INTERFACE_FILTER) ) func init() { if !environment.IsHandler() { return } - interfacesFilter, isSet := os.LookupEnv("INTERFACES_FILTER") - if !isSet { - panic("INTERFACES_FILTER is mandatory") +} + +type DeviceInfoer interface { + DeviceStates() (map[string]networkmanager.DeviceState, error) +} + +type DeviceInfo struct{} + +func (d DeviceInfo) DeviceStates() (map[string]networkmanager.DeviceState, error) { + nmClient, err := networkmanager.NewClientPrivate() + if err != nil { + filterLog.Error(err, "failed to initialize NetworkManager client") + return nil, err } - interfacesFilterGlobFromEnv = glob.MustCompile(interfacesFilter) + defer nmClient.Close() + + devices, err := nmClient.GetDevices() + if err != nil { + filterLog.Error(err, "failed to list NetworkManager devices") + return nil, err + } + + ifaceStates := map[string]networkmanager.DeviceState{} + for _, device := range devices { + ifaceStates[device.Interface] = device.State + } + return ifaceStates, nil } -func FilterOut(currentState shared.State) (shared.State, error) { - return filterOut(currentState, interfacesFilterGlobFromEnv) +func FilterOut(currentState shared.State, deviceInfo DeviceInfoer) (shared.State, error) { + devStates, err := deviceInfo.DeviceStates() + if err != nil { + filterLog.Error(err, "failed getting interface states, cannot filter managed interfaces") + return currentState, nil + } + return filterOut(currentState, devStates) } -func filterOutRoutes(routes []interface{}, interfacesFilterGlob glob.Glob) []interface{} { - filteredRoutes := []interface{}{} +func filterOutRoutes(routes []routeState, filteredInterfaces []interfaceState) []routeState { + filteredRoutes := []routeState{} for _, route := range routes { - name := route.(map[string]interface{})["next-hop-interface"] - if !interfacesFilterGlob.Match(name.(string)) { + name := route.NextHopInterface + if isInInterfaces(name, filteredInterfaces) { filteredRoutes = append(filteredRoutes, route) } } - return filteredRoutes } +func isInInterfaces(interfaceName string, interfaces []interfaceState) bool { + for _, iface := range interfaces { + if iface.Name == interfaceName { + return true + } + } + return false +} + func filterOutDynamicAttributes(iface map[string]interface{}) { // The gc-timer and hello-time are deep into linux-bridge like this // - bridge: @@ -74,10 +113,14 @@ func filterOutDynamicAttributes(iface map[string]interface{}) { delete(options, "hello-timer") } -func filterOutInterfaces(ifacesState []interfaceState, interfacesFilterGlob glob.Glob) []interfaceState { +func filterOutInterfaces(ifacesState []interfaceState, deviceStates map[string]networkmanager.DeviceState) []interfaceState { + if deviceStates == nil { + return ifacesState + } + filteredInterfaces := []interfaceState{} for _, iface := range ifacesState { - if !interfacesFilterGlob.Match(iface.Name) { + if iface.Data["type"] != "veth" || deviceStates[iface.Name] != networkmanager.DeviceStateUnmanaged { filterOutDynamicAttributes(iface.Data) filteredInterfaces = append(filteredInterfaces, iface) } @@ -85,21 +128,21 @@ func filterOutInterfaces(ifacesState []interfaceState, interfacesFilterGlob glob return filteredInterfaces } -func filterOut(currentState shared.State, interfacesFilterGlob glob.Glob) (shared.State, error) { +func filterOut(currentState shared.State, deviceStates map[string]networkmanager.DeviceState) (shared.State, error) { var state rootState if err := yaml.Unmarshal(currentState.Raw, &state); err != nil { return currentState, err } - if err := normalizeInterfacesNames(currentState.Raw, &state); err != nil { return currentState, err } - state.Interfaces = filterOutInterfaces(state.Interfaces, interfacesFilterGlob) + state.Interfaces = filterOutInterfaces(state.Interfaces, deviceStates) if state.Routes != nil { - state.Routes.Running = filterOutRoutes(state.Routes.Running, interfacesFilterGlob) - state.Routes.Config = filterOutRoutes(state.Routes.Config, interfacesFilterGlob) + state.Routes.Running = filterOutRoutes(state.Routes.Running, state.Interfaces) + state.Routes.Config = filterOutRoutes(state.Routes.Config, state.Interfaces) } + filteredState, err := yaml.Marshal(state) if err != nil { return currentState, err @@ -115,8 +158,18 @@ func normalizeInterfacesNames(rawState []byte, state *rootState) error { if err := goyaml.Unmarshal(rawState, &stateForNormalization); err != nil { return err } + for i, iface := range stateForNormalization.Interfaces { state.Interfaces[i].Name = iface.Name } + + if stateForNormalization.Routes != nil { + for i, route := range stateForNormalization.Routes.Config { + state.Routes.Config[i].NextHopInterface = route.NextHopInterface + } + for i, route := range stateForNormalization.Routes.Running { + state.Routes.Running[i].NextHopInterface = route.NextHopInterface + } + } return nil } diff --git a/pkg/state/filter_test.go b/pkg/state/filter_test.go index 8e771001aa..f9df1d386e 100644 --- a/pkg/state/filter_test.go +++ b/pkg/state/filter_test.go @@ -1,17 +1,18 @@ package state import ( - "github.com/gobwas/glob" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + networkmanager "github.com/phoracek/networkmanager-go/src" + nmstate "github.com/nmstate/kubernetes-nmstate/api/shared" ) var _ = Describe("FilterOut", func() { var ( state, filteredState nmstate.State - interfacesFilterGlob glob.Glob + ifaceStates map[string]networkmanager.DeviceState ) Context("when there is a linux bridge with gc-timer and hello-timer", func() { @@ -64,7 +65,10 @@ routes: next-hop-interface: eth1 table-id: 254 `) - + ifaceStates = map[string]networkmanager.DeviceState{ + "eth1": networkmanager.DeviceStateActivated, + "br1": networkmanager.DeviceStateActivated, + } filteredState = nmstate.NewState(` interfaces: - name: eth1 @@ -111,24 +115,20 @@ routes: next-hop-interface: eth1 table-id: 254 `) - interfacesFilterGlob = glob.MustCompile("") }) - It("should remove them from linux-bridge", func() { - returnedState, err := filterOut(state, interfacesFilterGlob) + It("should remove dynamic attributes from linux-bridge interface", func() { + returnedState, err := filterOut(state, ifaceStates) Expect(err).ToNot(HaveOccurred()) Expect(returnedState).To(MatchYAML(filteredState)) }) }) - Context("when the filter is set to empty and there is a list of interfaces", func() { + + Context("when there is managed veth interface", func() { BeforeEach(func() { - state = nmstate.NewState(` -interfaces: -- name: eth1 - state: up - type: ethernet + state = nmstate.NewState(`interfaces: - name: vethab6030bd state: down - type: ethernet + type: veth routes: config: [] running: @@ -137,31 +137,38 @@ routes: next-hop-address: "" next-hop-interface: vethab6030bd table-id: 254 - - destination: 0.0.0.0/0 - metric: 102 - next-hop-address: 192.168.66.2 - next-hop-interface: eth1 +`) + ifaceStates = map[string]networkmanager.DeviceState{ + "vethab6030bd": networkmanager.DeviceStateActivated, + } + filteredState = nmstate.NewState(`interfaces: +- name: vethab6030bd + state: down + type: veth +routes: + config: [] + running: + - destination: fd10:244::8c40/128 + metric: 1024 + next-hop-address: "" + next-hop-interface: vethab6030bd table-id: 254 `) - interfacesFilterGlob = glob.MustCompile("") }) - It("should keep all interfaces intact", func() { - returnedState, err := filterOut(state, interfacesFilterGlob) - Expect(err).ToNot(HaveOccurred()) - Expect(returnedState).To(MatchYAML(state)) + It("should keep managed veth interface", func() { + returnedState, err := filterOut(state, ifaceStates) + Expect(err).NotTo(HaveOccurred()) + Expect(returnedState).To(MatchYAML(filteredState)) }) }) - Context("when the filter is matching one of the interfaces in the list", func() { + Context("when there is unmanaged veth interface", func() { BeforeEach(func() { state = nmstate.NewState(`interfaces: -- name: eth1 - state: up - type: ethernet - name: vethab6030bd state: down - type: ethernet + type: veth routes: config: [] running: @@ -170,215 +177,249 @@ routes: next-hop-address: "" next-hop-interface: vethab6030bd table-id: 254 - - destination: 0.0.0.0/0 - metric: 102 - next-hop-address: 192.168.66.2 - next-hop-interface: eth1 - table-id: 254 - `) - filteredState = nmstate.NewState(`interfaces: -- name: eth1 - state: up - type: ethernet + ifaceStates = map[string]networkmanager.DeviceState{ + "vethab6030bd": networkmanager.DeviceStateUnmanaged, + } + filteredState = nmstate.NewState(`interfaces: [] routes: config: [] - running: - - destination: 0.0.0.0/0 - metric: 102 - next-hop-address: 192.168.66.2 - next-hop-interface: eth1 - table-id: 254 + running: [] `) - interfacesFilterGlob = glob.MustCompile("veth*") }) - It("should filter out matching interface and keep the others", func() { - returnedState, err := filterOut(state, interfacesFilterGlob) + It("should filter unmanaged veth interface", func() { + returnedState, err := filterOut(state, ifaceStates) Expect(err).NotTo(HaveOccurred()) Expect(returnedState).To(MatchYAML(filteredState)) }) }) - Context("when the filter is matching multiple interfaces in the list", func() { + Context("when there are multiple managed and unmanaged veth interfaces", func() { BeforeEach(func() { state = nmstate.NewState(`interfaces: - name: eth1 state: up type: ethernet -- name: vethab6030bd +- name: veth101 state: down - type: ethernet + type: veth +- name: veth102 + state: down + type: veth - name: vethjyuftrgv state: down - type: ethernet + type: veth +- name: vethvasziovs + state: down + type: veth routes: config: [] running: - destination: fd10:244::8c40/128 metric: 1024 next-hop-address: "" - next-hop-interface: vethab6030bd + next-hop-interface: veth101 + table-id: 254 + - destination: fd10:244::8c40/128 + metric: 1024 + next-hop-address: "" + next-hop-interface: veth102 table-id: 254 - destination: fd10:244::8c40/128 metric: 1024 next-hop-address: "" next-hop-interface: vethjyuftrgv table-id: 254 + - destination: fd10:244::8c40/128 + metric: 1024 + next-hop-address: "" + next-hop-interface: vethvasziovs + table-id: 254 - destination: 0.0.0.0/0 metric: 102 next-hop-address: 192.168.66.2 next-hop-interface: eth1 table-id: 254 `) + ifaceStates = map[string]networkmanager.DeviceState{ + "veth101": networkmanager.DeviceStateActivated, + "veth102": networkmanager.DeviceStateUnmanaged, + "vethjyuftrgv": networkmanager.DeviceStateActivated, + "vethvasziovs": networkmanager.DeviceStateUnmanaged, + } filteredState = nmstate.NewState(`interfaces: - name: eth1 state: up type: ethernet +- name: veth101 + state: down + type: veth +- name: vethjyuftrgv + state: down + type: veth routes: config: [] running: + - destination: fd10:244::8c40/128 + metric: 1024 + next-hop-address: "" + next-hop-interface: veth101 + table-id: 254 + - destination: fd10:244::8c40/128 + metric: 1024 + next-hop-address: "" + next-hop-interface: vethjyuftrgv + table-id: 254 - destination: 0.0.0.0/0 metric: 102 next-hop-address: 192.168.66.2 next-hop-interface: eth1 table-id: 254 `) - interfacesFilterGlob = glob.MustCompile("veth*") }) - - It("should filter out all matching interfaces and keep the others", func() { - returnedState, err := filterOut(state, interfacesFilterGlob) + It("should filter out all unmanaged veth interfaces", func() { + returnedState, err := filterOut(state, ifaceStates) Expect(err).ToNot(HaveOccurred()) Expect(returnedState).To(MatchYAML(filteredState)) }) - }) - Context("when the filter is matching multiple prefixes", func() { - BeforeEach(func() { - state = nmstate.NewState(`interfaces: + Context("when we fail to get deviceStates", func() { + BeforeEach(func() { + ifaceStates = nil + filteredState = nmstate.NewState(`interfaces: - name: eth1 state: up type: ethernet -- name: vethab6030bd +- name: veth101 state: down - type: ethernet -- name: vnet2b730a2b@if3 + type: veth +- name: veth102 state: down - type: ethernet + type: veth +- name: vethjyuftrgv + state: down + type: veth +- name: vethvasziovs + state: down + type: veth routes: config: [] running: - destination: fd10:244::8c40/128 metric: 1024 next-hop-address: "" - next-hop-interface: vethab6030bd + next-hop-interface: veth101 table-id: 254 - destination: fd10:244::8c40/128 metric: 1024 next-hop-address: "" - next-hop-interface: vnet2b730a2b + next-hop-interface: veth102 table-id: 254 - - destination: 0.0.0.0/0 - metric: 102 - next-hop-address: 192.168.66.2 - next-hop-interface: eth1 + - destination: fd10:244::8c40/128 + metric: 1024 + next-hop-address: "" + next-hop-interface: vethjyuftrgv + table-id: 254 + - destination: fd10:244::8c40/128 + metric: 1024 + next-hop-address: "" + next-hop-interface: vethvasziovs table-id: 254 -`) - filteredState = nmstate.NewState(`interfaces: -- name: eth1 - state: up - type: ethernet -routes: - config: [] - running: - destination: 0.0.0.0/0 metric: 102 next-hop-address: 192.168.66.2 next-hop-interface: eth1 table-id: 254 `) - interfacesFilterGlob = glob.MustCompile("{veth*,vnet*}") - }) - - It("it should filter out all interfaces matching any of these prefixes and keep the others", func() { - returnedState, err := filterOut(state, interfacesFilterGlob) - Expect(err).ToNot(HaveOccurred()) - Expect(returnedState).To(MatchYAML(filteredState)) - }) - }) - Context("when there is a linux bridge without 'bridge' options because is down", func() { - BeforeEach(func() { - state = nmstate.NewState(` -interfaces: -- name: br1 - type: linux-bridge - state: down -`) - - filteredState = nmstate.NewState(` -interfaces: -- name: br1 - type: linux-bridge - state: down -`) - interfacesFilterGlob = glob.MustCompile("") - }) - It("should keep the bridge as it is", func() { - returnedState, err := filterOut(state, interfacesFilterGlob) - Expect(err).ToNot(HaveOccurred()) - Expect(returnedState).To(MatchYAML(filteredState)) + }) + It("should keep the state intact", func() { + returnedState, err := filterOut(state, ifaceStates) + Expect(err).ToNot(HaveOccurred()) + Expect(returnedState).To(MatchYAML(filteredState)) + }) }) }) - Context("when the interfaces has numeric characters quoted", func() { + Context("when the interfaces have numeric characters", func() { BeforeEach(func() { - state = nmstate.NewState(` -interfaces: -- name: eth0 -- name: '0' -- name: '1101010' -- name: '0.0' -- name: '1.0' -- name: '0xfe' -- name: '60.e+02' + ifaceStates = map[string]networkmanager.DeviceState{ + "0": networkmanager.DeviceStateUnmanaged, + "1101010": networkmanager.DeviceStateUnmanaged, + "0.0": networkmanager.DeviceStateUnmanaged, + "1.0": networkmanager.DeviceStateUnmanaged, + "0xfe": networkmanager.DeviceStateUnmanaged, + "60.e+02": networkmanager.DeviceStateUnmanaged, + "10e+02": networkmanager.DeviceStateUnmanaged, + "70e+02": networkmanager.DeviceStateUnmanaged, + "94475496822e234": networkmanager.DeviceStateUnmanaged, + } + state = nmstate.NewState(`interfaces: + - name: eth0 + type: ethernet + - name: '0' + type: veth + - name: '1101010' + type: veth + - name: '0.0' + type: veth + - name: '1.0' + type: veth + - name: '0xfe' + type: veth + - name: '60.e+02' + type: veth + - name: 10e+02 + type: veth + - name: 70e+02 + type: veth + - name: 94475496822e234 + type: veth +routes: + config: [] + running: + - destination: fd10:244::8c40/128 + metric: 1024 + next-hop-address: 10.21.21.10 + next-hop-interface: eth0 + table-id: 254 + - destination: fd10:244::8c40/128 + metric: 1024 + next-hop-address: 10.21.21.10 + next-hop-interface: 94475496822e234 + table-id: 254 + - destination: fd10:244::8c40/128 + metric: 1024 + next-hop-address: 10.21.21.10 + next-hop-interface: '94475496822e234' + table-id: 254 + - destination: fd10:244::8c40/128 + metric: 1024 + next-hop-address: 10.21.21.10 + next-hop-interface: 70e+02 + table-id: 254 + - destination: fd10:244::8c40/128 + metric: 1024 + next-hop-address: 10.21.21.10 + next-hop-interface: 60.e+02 + table-id: 254 `) - filteredState = nmstate.NewState(` -interfaces: + filteredState = nmstate.NewState(`interfaces: - name: eth0 -- name: '1101010' -- name: '1.0' -- name: '60.e+02' + type: ethernet +routes: + config: [] + running: + - destination: fd10:244::8c40/128 + metric: 1024 + next-hop-address: 10.21.21.10 + next-hop-interface: eth0 + table-id: 254 `) - interfacesFilterGlob = glob.MustCompile("0*") }) It("should filter out interfaces correctly", func() { - returnedState, err := filterOut(state, interfacesFilterGlob) - Expect(err).NotTo(HaveOccurred()) - Expect(returnedState).To(MatchYAML(filteredState)) - }) - }) - - // See https://github.com/yaml/pyyaml/issues/173 for why this scenario is checked. - Context("when the interfaces names have numbers in scientific notation without dot", func() { - BeforeEach(func() { - state = nmstate.NewState(` -interfaces: -- name: eth0 -- name: 10e+02 -- name: 60e+02 -`) - filteredState = nmstate.NewState(` -interfaces: -- name: eth0 -- name: "60e+02" -`) - interfacesFilterGlob = glob.MustCompile("10e*") - }) - - It("does not filter out interfaces correctly and does not represent them correctly", func() { - returnedState, err := filterOut(state, interfacesFilterGlob) + returnedState, err := filterOut(state, ifaceStates) Expect(err).NotTo(HaveOccurred()) Expect(returnedState).To(MatchYAML(filteredState)) }) diff --git a/pkg/state/type.go b/pkg/state/type.go index 7d18053e4f..e77814bfa8 100644 --- a/pkg/state/type.go +++ b/pkg/state/type.go @@ -3,17 +3,23 @@ package state import ( "encoding/json" "fmt" + "sigs.k8s.io/yaml" ) type rootState struct { - Interfaces []interfaceState `json:"interfaces" yaml:"interfaces"` - Routes *routesState `json:"routes,omitempty" yaml:"routes,omitempty"` + Interfaces []interfaceState `json:"interfaces" yaml:"interfaces"` + Routes *routes `json:"routes,omitempty" yaml:"routes,omitempty"` +} + +type routes struct { + Config []routeState `json:"config" yaml:"config"` + Running []routeState `json:"running" yaml:"running"` } -type routesState struct { - Config []interface{} `json:"config" yaml:"config"` - Running []interface{} `json:"running" yaml:"running"` +type routeState struct { + routeFields `yaml:",inline"` + Data map[string]interface{} } type interfaceState struct { @@ -26,6 +32,11 @@ type interfaceFields struct { Name string `json:"name" yaml:"name"` } +// routeFields allows unmarshaling directly into the defined fields +type routeFields struct { + NextHopInterface string `json:"next-hop-interface" yaml:"next-hop-interface"` +} + func (i interfaceState) MarshalJSON() (output []byte, err error) { i.Data["name"] = i.Name return json.Marshal(i.Data) @@ -44,3 +55,22 @@ func (i *interfaceState) UnmarshalJSON(b []byte) error { i.interfaceFields = ifaceFields return nil } + +func (r routeState) MarshalJSON() (output []byte, err error) { + r.Data["next-hop-interface"] = r.NextHopInterface + return json.Marshal(r.Data) +} + +func (r *routeState) UnmarshalJSON(b []byte) error { + if err := yaml.Unmarshal(b, &r.Data); err != nil { + return fmt.Errorf("failed Unmarshaling b: %w", err) + } + + var fields routeFields + if err := yaml.Unmarshal(b, &fields); err != nil { + return fmt.Errorf("failed Unmarchaling raw: %w", err) + } + r.Data["next-hop-interface"] = fields.NextHopInterface + r.routeFields = fields + return nil +} diff --git a/test/e2e/handler/nns_filter_test.go b/test/e2e/handler/nns_filter_test.go new file mode 100644 index 0000000000..b397e9a734 --- /dev/null +++ b/test/e2e/handler/nns_filter_test.go @@ -0,0 +1,25 @@ +package handler + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "github.com/nmstate/kubernetes-nmstate/pkg/state" + "github.com/nmstate/kubernetes-nmstate/test/cmd" + "k8s.io/apimachinery/pkg/types" +) + +var _ = Describe("[nns] NNS Interface filter", func() { + BeforeEach(func() { + // Make sure NNSes are present + for _, node := range nodes { + key := types.NamespacedName{Name: node} + _ = nodeNetworkState(key) + } + }) + It("should not log errors related to NNS interface filtering", func() { + combinedHandlerLogs, err := cmd.Kubectl("logs", "-lname=nmstate-handler", "-n", "nmstate") + Expect(err).ToNot(HaveOccurred()) + Expect(combinedHandlerLogs).ToNot(ContainSubstring(state.INTERFACE_FILTER)) + }) +})