From 33a4f8c530684163bebebeefe5dd490f415134f9 Mon Sep 17 00:00:00 2001 From: Nadia Pinaeva Date: Mon, 25 Aug 2025 13:50:28 +0200 Subject: [PATCH 01/15] [docs] Move local testing guide to its own page from CI Signed-off-by: Nadia Pinaeva --- docs/ci/ci.md | 286 -------------------- docs/developer-guide/local_testing_guide.md | 263 ++++++++++++++++++ 2 files changed, 263 insertions(+), 286 deletions(-) diff --git a/docs/ci/ci.md b/docs/ci/ci.md index 1d17671925..c708aa2e07 100644 --- a/docs/ci/ci.md +++ b/docs/ci/ci.md @@ -86,292 +86,6 @@ To reduce the explosion of tests being run in CI, the test cases run are limited using an `exclude:` statement in [ovn-kubernetes/.github/workflows/test.yml](https://github.com/ovn-org/ovn-kubernetes/blob/master/.github/workflows/test.yml). -## Running CI Locally - -This section describes how to run CI tests on a local deployment. This may be -useful for expanding the CI test coverage or testing a private fix before -creating a pull request. - -### Download and Build Kubernetes Components - -#### Go Version - -Older versions of Kubernetes do not build with newer versions of Go, -specifically Kubernetes v1.16.4 doesn't build with Go version 1.13.x. If this is -a version of Kubernetes that needs to be tested with, as a workaround, Go -version 1.12.1 can be downloaded to a local directory and the $PATH variable -updated only where kubernetes is being built. - -``` -$ go version -go version go1.13.8 linux/amd64 - -$ mkdir -p /home/$USER/src/golang/go1-12-1/; cd /home/$USER/src/golang/go1-12-1/ -$ wget https://dl.google.com/go/go1.12.1.linux-amd64.tar.gz -$ tar -xzf go1.12.1.linux-amd64.tar.gz -$ PATH=/home/$USER/src/golang/go1-12-1/go/bin:$GOPATH/src/k8s.io/kubernetes/_output/local/bin/linux/amd64:$PATH -``` - -#### Download and Build Kubernetes Components (E2E Tests, ginkgo, kubectl): - -Determine which version of Kubernetes is currently used in CI (See -[ovn-kubernetes/.github/workflows/test.yml](https://github.com/ovn-org/ovn-kubernetes/blob/master/.github/workflows/test.yml)) -and set the environmental variable `K8S_VERSION` to the same value. Also make sure to export a GOPATH which points to -your go directory with `export GOPATH=(...)`. - -``` -K8S_VERSION=v1.33.1 -git clone --single-branch --branch $K8S_VERSION https://github.com/kubernetes/kubernetes.git $GOPATH/src/k8s.io/kubernetes/ -pushd $GOPATH/src/k8s.io/kubernetes/ -make WHAT="test/e2e/e2e.test vendor/github.com/onsi/ginkgo/ginkgo cmd/kubectl" -rm -rf .git - -sudo cp _output/local/go/bin/e2e.test /usr/local/bin/. -sudo cp _output/local/go/bin/kubectl /usr/local/bin/kubectl-$K8S_VERSION -sudo ln -s /usr/local/bin/kubectl-$K8S_VERSION /usr/local/bin/kubectl -cp _output/local/go/bin/ginkgo /usr/local/bin/. -popd -``` - -If you have any failures during the build, verify $PATH has been updated to -point to correct GO version. Also may need to change settings on some of the -generated binaries. For example: - -``` -chmod +x $GOPATH/src/k8s.io/kubernetes/_output/bin/deepcopy-gen -``` - -### Export environment variables - -Before setting up KIND and before running the actual tests, export essential environment variables. - -The environment variables and their values depend on the actual test scenario that you want to run. - -Look at the `e2e` action (search for `name: e2e`) in [ovn-kubernetes/.github/workflows/test.yml](https://github.com/ovn-org/ovn-kubernetes/blob/master/.github/workflows/test.yml). Prior to installing kind, set the following environment variables according to your needs: -``` -export KIND_CLUSTER_NAME=ovn -export KIND_INSTALL_INGRESS=[true|false] -export KIND_ALLOW_SYSTEM_WRITES=[true|false] -export PARALLEL=[true|false] -export JOB_NAME=(... job name ...) -export OVN_HYBRID_OVERLAY_ENABLE=[true|false] -export OVN_MULTICAST_ENABLE=[true|false] -export OVN_EMPTY_LB_EVENTS=[true|false] -export OVN_HA=[true|false] -export OVN_DISABLE_SNAT_MULTIPLE_GWS=[true|false] -export OVN_GATEWAY_MODE=["local"|"shared"] -export PLATFORM_IPV4_SUPPORT=[true|false] -export PLATFORM_IPV6_SUPPORT=[true|false] -# not required for the OVN Kind installation script, but export this already for later -OVN_SECOND_BRIDGE=[true|false] -``` - -You can refer to a recent CI run from any pull request in [https://github.com/ovn-org/ovn-kubernetes/actions](https://github.com/ovn-org/ovn-kubernetes/actions) to get a valid set of settings. - -As an example for the `control-plane-noHA-local-ipv4-snatGW-1br` job, the settings are at time of this writing: -``` -export KIND_CLUSTER_NAME=ovn -export KIND_INSTALL_INGRESS=true -export KIND_ALLOW_SYSTEM_WRITES=true -export PARALLEL=true -export JOB_NAME=control-plane-noHA-local-ipv4-snatGW-1br -export OVN_HYBRID_OVERLAY_ENABLE=true -export OVN_MULTICAST_ENABLE=true -export OVN_EMPTY_LB_EVENTS=true -export OVN_HA=false -export OVN_DISABLE_SNAT_MULTIPLE_GWS=false -export OVN_GATEWAY_MODE="local" -export PLATFORM_IPV4_SUPPORT=true -export PLATFORM_IPV6_SUPPORT=false -# not required for the OVN Kind installation script, but export this already for later -export OVN_SECOND_BRIDGE=false -``` - -### KIND - -Kubernetes in Docker (KIND) is used to deploy Kubernetes locally where a docker -container is created per Kubernetes node. The CI tests run on this Kubernetes -deployment. Therefore, KIND will need to be installed locally. - -Generic instructions for installing and running OVNKubernetes with KIND can be found at: -[https://github.com/ovn-org/ovn-kubernetes/blob/master/docs/kind.md](https://github.com/ovn-org/ovn-kubernetes/blob/master/docs/kind.md) - -Make sure to set the required environment variables first (see section above). Then, deploy kind: -``` -$ pushd -$ ./kind.sh -$ popd -``` - -### Run Tests - -To run the tests locally, run a KIND deployment as described above. The E2E -tests look for the kube config file in a special location, so make a copy: - -``` -cp ~/ovn.conf ~/.kube/kind-config-kind -``` - -To run the desired shard, first make sure that the necessary environment variables are exported (see section above). -Then, go to the location of your local copy of the `ovn-kubernetes` repository: -``` -$ REPO=$GOPATH/src/github.com/ovn-org/ovn-kubernetes -$ cd $REPO -``` - -#### Running a suite of shards or control-plane tests - -Finally, run the the shard that you want to test against (each shard can take 30+ minutes to complete) -``` -$ pushd test -# run either -$ make shard-network -# or -$ make shard-conformance -# or -$ GITHUB_WORKSPACE="$REPO" make control-plane -# or -$ make conformance -$ popd -``` - -#### Running a single E2E test - -To run a single E2E test instead, target the shard-test action, as follows: - -``` -$ cd $REPO -$ pushd test -$ make shard-test WHAT="should enforce egress policy allowing traffic to a server in a different namespace based on PodSelector and NamespaceSelector" -$ popd -``` - -As a reminder, shards use the [E2E framework](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-testing/e2e-tests.md). The value of `WHAT=` will be used to modify the `--focus` parameter. Individual tests can be retrieved from [https://github.com/kubernetes/kubernetes/tree/master/test/e2e](https://github.com/kubernetes/kubernetes/tree/master/test/e2e). For network tests, one could run: -~~~ -grep ginkgo.It $GOPATH/src/k8s.io/kubernetes/test/e2e/network/ -Ri -~~~ - -For example: -~~~ -# grep ginkgo.It $GOPATH/src/k8s.io/kubernetes/test/e2e/network/ -Ri | head -1 -/root/go/src/k8s.io/kubernetes/test/e2e/network/conntrack.go: ginkgo.It("should be able to preserve UDP traffic when server pod cycles for a NodePort service", func() { -# make shard-test WHAT="should enforce policy to allow traffic from pods within server namespace based on PodSelector" -(...) -+ case "$SHARD" in -++ echo should be able to preserve UDP traffic when server pod cycles for a NodePort service -++ sed 's/ /\\s/g' -+ FOCUS='should\sbe\sable\sto\spreserve\sUDP\straffic\swhen\sserver\spod\scycles\sfor\sa\sNodePort\sservice' -+ export KUBERNETES_CONFORMANCE_TEST=y -+ KUBERNETES_CONFORMANCE_TEST=y -+ export KUBE_CONTAINER_RUNTIME=remote -+ KUBE_CONTAINER_RUNTIME=remote -+ export KUBE_CONTAINER_RUNTIME_ENDPOINT=unix:///run/containerd/containerd.sock -+ KUBE_CONTAINER_RUNTIME_ENDPOINT=unix:///run/containerd/containerd.sock -+ export KUBE_CONTAINER_RUNTIME_NAME=containerd -+ KUBE_CONTAINER_RUNTIME_NAME=containerd -+ export FLAKE_ATTEMPTS=5 -+ FLAKE_ATTEMPTS=5 -+ export NUM_NODES=20 -+ NUM_NODES=20 -+ export NUM_WORKER_NODES=3 -+ NUM_WORKER_NODES=3 -+ ginkgo --nodes=20 '--focus=should\sbe\sable\sto\spreserve\sUDP\straffic\swhen\sserver\spod\scycles\sfor\sa\sNodePort\sservice' '--skip=Networking\sIPerf\sIPv[46]|\[Feature:PerformanceDNS\]|Disruptive|DisruptionController|\[sig-apps\]\sCronJob|\[sig-storage\]|\[Feature:Federation\]|should\shave\sipv4\sand\sipv6\sinternal\snode\sip|should\shave\sipv4\sand\sipv6\snode\spodCIDRs|kube-proxy|should\sset\sTCP\sCLOSE_WAIT\stimeout|should\shave\ssession\saffinity\stimeout\swork|named\sport.+\[Feature:NetworkPolicy\]|\[Feature:SCTP\]|service.kubernetes.io/headless|should\sresolve\sconnection\sreset\sissue\s#74839|sig-api-machinery|\[Feature:NoSNAT\]|Services.+(ESIPP|cleanup\sfinalizer)|configMap\snameserver|ClusterDns\s\[Feature:Example\]|should\sset\sdefault\svalue\son\snew\sIngressClass|should\sprevent\sIngress\screation\sif\smore\sthan\s1\sIngressClass\smarked\sas\sdefault|\[Feature:Networking-IPv6\]|\[Feature:.*DualStack.*\]' --flake-attempts=5 /usr/local/bin/e2e.test -- --kubeconfig=/root/ovn.conf --provider=local --dump-logs-on-failure=false --report-dir=/root/ovn-kubernetes/test/_artifacts --disable-log-dump=true --num-nodes=3 -Running Suite: Kubernetes e2e suite -(...) -• [SLOW TEST:28.091 seconds] -[sig-network] Conntrack -/root/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e/network/framework.go:23 - should be able to preserve UDP traffic when server pod cycles for a NodePort service - /root/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e/network/conntrack.go:128 ------------------------------- -{"msg":"PASSED [sig-network] Conntrack should be able to preserve UDP traffic when server pod cycles for a NodePort service","total":-1,"completed":1,"skipped":229,"failed":0} -Aug 17 14:46:42.842: INFO: Running AfterSuite actions on all nodes - - -Aug 17 14:46:15.264: INFO: Running AfterSuite actions on all nodes -Aug 17 14:46:42.885: INFO: Running AfterSuite actions on node 1 -Aug 17 14:46:42.885: INFO: Skipping dumping logs from cluster - - -Ran 1 of 5667 Specs in 30.921 seconds -SUCCESS! -- 1 Passed | 0 Failed | 0 Flaked | 0 Pending | 5666 Skipped - - -Ginkgo ran 1 suite in 38.489055861s -Test Suite Passed -~~~ - -#### Running a control-plane test - -All local tests are defined as `control-plane` tests. To run a single `control-plane` test, target the `control-plane` -action and append the `WHAT=` parameter, as follows: - -``` -$ cd $REPO -$ pushd test -$ make control-plane WHAT="should be able to send multicast UDP traffic between nodes" -$ popd -``` - -The value of `WHAT=` will be used to modify the `-ginkgo.focus` parameter. Individual tests can be retrieved from this -repository under [test/e2e](https://github.com/ovn-org/ovn-kubernetes/tree/master/test/e2e). To see a list of individual -tests, one could run: -~~~ -grep -R ginkgo.It test/ -~~~ - -For example: -~~~ -# grep -R ginkgo.It . | head -1 -./e2e/multicast.go: ginkgo.It("should be able to send multicast UDP traffic between nodes", func() { -# make control-plane WHAT="should be able to send multicast UDP traffic between nodes" -(...) -+ go test -timeout=0 -v . -ginkgo.v -ginkgo.focus 'should\sbe\sable\sto\ssend\smulticast\sUDP\straffic\sbetween\snodes' -ginkgo.flakeAttempts 2 '-ginkgo.skip=recovering from deleting db files while maintain connectivity|Should validate connectivity before and after deleting all the db-pods at once in HA mode|Should be allowed to node local cluster-networked endpoints by nodeport services with externalTrafficPolicy=local|e2e ingress to host-networked pods traffic validation|host to host-networked pods traffic validation' -provider skeleton -kubeconfig /root/ovn.conf --num-nodes=2 --report-dir=/root/ovn-kubernetes/test/_artifacts --report-prefix=control-plane_ -I0817 15:26:21.762483 1197731 test_context.go:457] Tolerating taints "node-role.kubernetes.io/control-plane" when considering if nodes are ready -=== RUN TestE2e -I0817 15:26:21.762635 1197731 e2e_suite_test.go:67] Saving reports to /root/ovn-kubernetes/test/_artifacts -Running Suite: E2e Suite -(...) -• [SLOW TEST:12.332 seconds] -Multicast -/root/ovn-kubernetes/test/e2e/multicast.go:25 - should be able to send multicast UDP traffic between nodes - /root/ovn-kubernetes/test/e2e/multicast.go:75 ------------------------------- -SSSSSSSSSSSS -JUnit report was created: /root/ovn-kubernetes/test/_artifacts/junit_control-plane_01.xml - -Ran 1 of 60 Specs in 12.333 seconds -SUCCESS! -- 1 Passed | 0 Failed | 0 Flaked | 0 Pending | 59 Skipped ---- PASS: TestE2e (12.34s) -PASS -ok github.com/ovn-org/ovn-kubernetes/test/e2e 12.371s -+ popd -~/ovn-kubernetes/test -~~~ - -### IPv6 tests - -To skip the IPv4 only tests (in a IPv6 only deployment), pass the -`PLATFORM_IPV6_SUPPORT=true` environmental variable to `make`: - -``` -$ cd $GOPATH/src/github.com/ovn-org/ovn-kubernetes - -$ pushd test -$ PLATFORM_IPV6_SUPPORT=true make shard-conformance -$ popd -``` - -Github CI doesn´t offer IPv6 connectivity, so IPv6 only tests are always -skipped. To run those tests locally, comment out the following line from -[ovn-kubernetes/test/scripts/e2e-kind.sh](https://github.com/ovn-org/ovn-kubernetes/blob/master/test/scripts/e2e-kind.sh) - -``` -# Github CI doesn´t offer IPv6 connectivity, so always skip IPv6 only tests. -SKIPPED_TESTS=$SKIPPED_TESTS$IPV6_ONLY_TESTS -``` - # Conformance Tests We have a conformance test suit that can be invoked using the `make conformance` command. diff --git a/docs/developer-guide/local_testing_guide.md b/docs/developer-guide/local_testing_guide.md index e69de29bb2..5158fb2df9 100644 --- a/docs/developer-guide/local_testing_guide.md +++ b/docs/developer-guide/local_testing_guide.md @@ -0,0 +1,263 @@ +# Running CI Locally + +This section describes how to run CI tests on a local machine. This may be +useful for expanding the CI test coverage or testing a private fix before +creating a pull request. + +## Download and Build Kubernetes Components + +### Go Version + +Older versions of Kubernetes do not build with newer versions of Go, +specifically Kubernetes v1.16.4 doesn't build with Go version 1.13.x. If this is +a version of Kubernetes that needs to be tested with, as a workaround, Go +version 1.12.1 can be downloaded to a local directory and the $PATH variable +updated only where kubernetes is being built. + +``` +$ go version +go version go1.13.8 linux/amd64 + +$ mkdir -p /home/$USER/src/golang/go1-12-1/; cd /home/$USER/src/golang/go1-12-1/ +$ wget https://dl.google.com/go/go1.12.1.linux-amd64.tar.gz +$ tar -xzf go1.12.1.linux-amd64.tar.gz +$ PATH=/home/$USER/src/golang/go1-12-1/go/bin:$GOPATH/src/k8s.io/kubernetes/_output/local/bin/linux/amd64:$PATH +``` + +### Download and Build Kubernetes Components (E2E Tests, ginkgo, kubectl): + +Determine which version of Kubernetes is currently used in CI (See +[ovn-kubernetes/.github/workflows/test.yml](https://github.com/ovn-org/ovn-kubernetes/blob/master/.github/workflows/test.yml)) +and set the environmental variable `K8S_VERSION` to the same value. Also make sure to export a GOPATH which points to +your go directory with `export GOPATH=(...)`. + +``` +K8S_VERSION=v1.33.1 +git clone --single-branch --branch $K8S_VERSION https://github.com/kubernetes/kubernetes.git $GOPATH/src/k8s.io/kubernetes/ +pushd $GOPATH/src/k8s.io/kubernetes/ +make WHAT="test/e2e/e2e.test vendor/github.com/onsi/ginkgo/ginkgo cmd/kubectl" +rm -rf .git + +sudo cp _output/local/go/bin/e2e.test /usr/local/bin/. +sudo cp _output/local/go/bin/kubectl /usr/local/bin/kubectl-$K8S_VERSION +sudo ln -s /usr/local/bin/kubectl-$K8S_VERSION /usr/local/bin/kubectl +cp _output/local/go/bin/ginkgo /usr/local/bin/. +popd +``` + +If you have any failures during the build, verify $PATH has been updated to +point to correct GO version. Also may need to change settings on some of the +generated binaries. For example: + +``` +chmod +x $GOPATH/src/k8s.io/kubernetes/_output/bin/deepcopy-gen +``` + +## Export environment variables + +Before setting up KIND and before running the actual tests, export essential environment variables. + +The environment variables and their values depend on the actual test scenario that you want to run. + +Look at the `e2e` action (search for `name: e2e`) in [ovn-kubernetes/.github/workflows/test.yml](https://github.com/ovn-org/ovn-kubernetes/blob/master/.github/workflows/test.yml). Prior to installing kind, set the following environment variables according to your needs: +``` +export KIND_CLUSTER_NAME=ovn +export KIND_INSTALL_INGRESS=[true|false] +export KIND_ALLOW_SYSTEM_WRITES=[true|false] +export PARALLEL=[true|false] +export JOB_NAME=(... job name ...) +export OVN_HYBRID_OVERLAY_ENABLE=[true|false] +export OVN_MULTICAST_ENABLE=[true|false] +export OVN_EMPTY_LB_EVENTS=[true|false] +export OVN_HA=[true|false] +export OVN_DISABLE_SNAT_MULTIPLE_GWS=[true|false] +export OVN_GATEWAY_MODE=["local"|"shared"] +export PLATFORM_IPV4_SUPPORT=[true|false] +export PLATFORM_IPV6_SUPPORT=[true|false] +# not required for the OVN Kind installation script, but export this already for later +export OVN_SECOND_BRIDGE=[true|false] +``` + +You can refer to a recent CI run from any pull request in [https://github.com/ovn-org/ovn-kubernetes/actions](https://github.com/ovn-org/ovn-kubernetes/actions) to get a valid set of settings. + +As an example for the `control-plane-noHA-local-ipv4-snatGW-1br` job, the settings are at time of this writing: +``` +export KIND_CLUSTER_NAME=ovn +export KIND_INSTALL_INGRESS=true +export KIND_ALLOW_SYSTEM_WRITES=true +export PARALLEL=true +export JOB_NAME=control-plane-noHA-local-ipv4-snatGW-1br +export OVN_HYBRID_OVERLAY_ENABLE=true +export OVN_MULTICAST_ENABLE=true +export OVN_EMPTY_LB_EVENTS=true +export OVN_HA=false +export OVN_DISABLE_SNAT_MULTIPLE_GWS=false +export OVN_GATEWAY_MODE="local" +export PLATFORM_IPV4_SUPPORT=true +export PLATFORM_IPV6_SUPPORT=false +# not required for the OVN Kind installation script, but export this already for later +export OVN_SECOND_BRIDGE=false +``` + +## KIND + +Kubernetes in Docker (KIND) is used to deploy Kubernetes locally where a docker (or podman) +container is created per Kubernetes node. The CI tests run on this Kubernetes +deployment. Therefore, KIND will need to be installed locally. + +Generic instructions for installing and running OVN Kubernetes with KIND can be found at: +[OVN-Kubernetes KIND Setup](https://ovn-kubernetes.io/installation/launching-ovn-kubernetes-on-kind/) + +Make sure to set the required environment variables first (see section above). Then, deploy kind: +``` +$ pushd contrib +$ ./kind.sh +$ popd +``` + +## Run Tests + +To run the tests locally, run a KIND deployment as described above. The E2E +tests look for the kube config file in a special location, so make a copy: + +``` +cp ~/ovn.conf ~/.kube/kind-config-kind +``` + +To run the desired shard, first make sure that the necessary environment variables are exported (see section above). +Then, go to the location of your local copy of the `ovn-kubernetes` repository: +``` +$ REPO=$GOPATH/src/github.com/ovn-org/ovn-kubernetes +$ cd $REPO +``` + +### Running a suite of shards or control-plane tests + +Finally, run the the shard that you want to test against (each shard can take 30+ minutes to complete) +``` +$ pushd test +# run either +$ make shard-network +# or +$ make shard-conformance +# or +$ GITHUB_WORKSPACE="$REPO" make control-plane +# or +$ make conformance +$ popd +``` + +### Running a single E2E test + +To run a single E2E test instead, target the shard-test action, as follows: + +``` +$ cd $REPO +$ pushd test +$ make shard-test WHAT="should enforce egress policy allowing traffic to a server in a different namespace based on PodSelector and NamespaceSelector" +$ popd +``` + +As a reminder, shards use the [E2E framework](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-testing/e2e-tests.md). The value of `WHAT=` will be used to modify the `--focus` parameter. Individual tests can be retrieved from [https://github.com/kubernetes/kubernetes/tree/master/test/e2e](https://github.com/kubernetes/kubernetes/tree/master/test/e2e). For network tests, one could run: +~~~ +grep ginkgo.It $GOPATH/src/k8s.io/kubernetes/test/e2e/network/ -Ri +~~~ + +For example: +~~~ +# grep ginkgo.It $GOPATH/src/k8s.io/kubernetes/test/e2e/network/ -Ri | head -1 +/root/go/src/k8s.io/kubernetes/test/e2e/network/conntrack.go: ginkgo.It("should be able to preserve UDP traffic when server pod cycles for a NodePort service", func() { +# make shard-test WHAT="should enforce policy to allow traffic from pods within server namespace based on PodSelector" +(...) ++ case "$SHARD" in +++ echo should be able to preserve UDP traffic when server pod cycles for a NodePort service +++ sed 's/ /\\s/g' ++ FOCUS='should\sbe\sable\sto\spreserve\sUDP\straffic\swhen\sserver\spod\scycles\sfor\sa\sNodePort\sservice' ++ export KUBERNETES_CONFORMANCE_TEST=y ++ KUBERNETES_CONFORMANCE_TEST=y ++ export KUBE_CONTAINER_RUNTIME=remote ++ KUBE_CONTAINER_RUNTIME=remote ++ export KUBE_CONTAINER_RUNTIME_ENDPOINT=unix:///run/containerd/containerd.sock ++ KUBE_CONTAINER_RUNTIME_ENDPOINT=unix:///run/containerd/containerd.sock ++ export KUBE_CONTAINER_RUNTIME_NAME=containerd ++ KUBE_CONTAINER_RUNTIME_NAME=containerd ++ export FLAKE_ATTEMPTS=5 ++ FLAKE_ATTEMPTS=5 ++ export NUM_NODES=20 ++ NUM_NODES=20 ++ export NUM_WORKER_NODES=3 ++ NUM_WORKER_NODES=3 ++ ginkgo --nodes=20 '--focus=should\sbe\sable\sto\spreserve\sUDP\straffic\swhen\sserver\spod\scycles\sfor\sa\sNodePort\sservice' '--skip=Networking\sIPerf\sIPv[46]|\[Feature:PerformanceDNS\]|Disruptive|DisruptionController|\[sig-apps\]\sCronJob|\[sig-storage\]|\[Feature:Federation\]|should\shave\sipv4\sand\sipv6\sinternal\snode\sip|should\shave\sipv4\sand\sipv6\snode\spodCIDRs|kube-proxy|should\sset\sTCP\sCLOSE_WAIT\stimeout|should\shave\ssession\saffinity\stimeout\swork|named\sport.+\[Feature:NetworkPolicy\]|\[Feature:SCTP\]|service.kubernetes.io/headless|should\sresolve\sconnection\sreset\sissue\s#74839|sig-api-machinery|\[Feature:NoSNAT\]|Services.+(ESIPP|cleanup\sfinalizer)|configMap\snameserver|ClusterDns\s\[Feature:Example\]|should\sset\sdefault\svalue\son\snew\sIngressClass|should\sprevent\sIngress\screation\sif\smore\sthan\s1\sIngressClass\smarked\sas\sdefault|\[Feature:Networking-IPv6\]|\[Feature:.*DualStack.*\]' --flake-attempts=5 /usr/local/bin/e2e.test -- --kubeconfig=/root/ovn.conf --provider=local --dump-logs-on-failure=false --report-dir=/root/ovn-kubernetes/test/_artifacts --disable-log-dump=true --num-nodes=3 +Running Suite: Kubernetes e2e suite +(...) +• [SLOW TEST:28.091 seconds] +[sig-network] Conntrack +/root/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e/network/framework.go:23 + should be able to preserve UDP traffic when server pod cycles for a NodePort service + /root/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e/network/conntrack.go:128 +------------------------------ +{"msg":"PASSED [sig-network] Conntrack should be able to preserve UDP traffic when server pod cycles for a NodePort service","total":-1,"completed":1,"skipped":229,"failed":0} +Aug 17 14:46:42.842: INFO: Running AfterSuite actions on all nodes + + +Aug 17 14:46:15.264: INFO: Running AfterSuite actions on all nodes +Aug 17 14:46:42.885: INFO: Running AfterSuite actions on node 1 +Aug 17 14:46:42.885: INFO: Skipping dumping logs from cluster + + +Ran 1 of 5667 Specs in 30.921 seconds +SUCCESS! -- 1 Passed | 0 Failed | 0 Flaked | 0 Pending | 5666 Skipped + + +Ginkgo ran 1 suite in 38.489055861s +Test Suite Passed +~~~ + +### Running a control-plane test + +All local tests are defined as `control-plane` tests. To run a single `control-plane` test, target the `control-plane` +action and append the `WHAT=` parameter, as follows: + +``` +$ cd $REPO +$ pushd test +$ make control-plane WHAT="should be able to send multicast UDP traffic between nodes" +$ popd +``` + +The value of `WHAT=` will be used to modify the `-ginkgo.focus` parameter. Individual tests can be retrieved from this +repository under [test/e2e](https://github.com/ovn-org/ovn-kubernetes/tree/master/test/e2e). To see a list of individual +tests, one could run: +~~~ +grep -R ginkgo.It test/ +~~~ + +For example: +~~~ +# grep -R ginkgo.It . | head -1 +./e2e/multicast.go: ginkgo.It("should be able to send multicast UDP traffic between nodes", func() { +# make control-plane WHAT="should be able to send multicast UDP traffic between nodes" +(...) ++ go test -timeout=0 -v . -ginkgo.v -ginkgo.focus 'should\sbe\sable\sto\ssend\smulticast\sUDP\straffic\sbetween\snodes' -ginkgo.flakeAttempts 2 '-ginkgo.skip=recovering from deleting db files while maintain connectivity|Should validate connectivity before and after deleting all the db-pods at once in HA mode|Should be allowed to node local cluster-networked endpoints by nodeport services with externalTrafficPolicy=local|e2e ingress to host-networked pods traffic validation|host to host-networked pods traffic validation' -provider skeleton -kubeconfig /root/ovn.conf --num-nodes=2 --report-dir=/root/ovn-kubernetes/test/_artifacts --report-prefix=control-plane_ +I0817 15:26:21.762483 1197731 test_context.go:457] Tolerating taints "node-role.kubernetes.io/control-plane" when considering if nodes are ready +=== RUN TestE2e +I0817 15:26:21.762635 1197731 e2e_suite_test.go:67] Saving reports to /root/ovn-kubernetes/test/_artifacts +Running Suite: E2e Suite +(...) +• [SLOW TEST:12.332 seconds] +Multicast +/root/ovn-kubernetes/test/e2e/multicast.go:25 + should be able to send multicast UDP traffic between nodes + /root/ovn-kubernetes/test/e2e/multicast.go:75 +------------------------------ +SSSSSSSSSSSS +JUnit report was created: /root/ovn-kubernetes/test/_artifacts/junit_control-plane_01.xml + +Ran 1 of 60 Specs in 12.333 seconds +SUCCESS! -- 1 Passed | 0 Failed | 0 Flaked | 0 Pending | 59 Skipped +--- PASS: TestE2e (12.34s) +PASS +ok github.com/ovn-org/ovn-kubernetes/test/e2e 12.371s ++ popd +~/ovn-kubernetes/test +~~~ From aa188fb54d57568419c50b5bcd4cba1fe3c426b3 Mon Sep 17 00:00:00 2001 From: Nadia Pinaeva Date: Mon, 25 Aug 2025 14:00:59 +0200 Subject: [PATCH 02/15] [docs] Add instructions for CI failures. Signed-off-by: Nadia Pinaeva --- docs/ci/ci.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/docs/ci/ci.md b/docs/ci/ci.md index c708aa2e07..93fc60b09c 100644 --- a/docs/ci/ci.md +++ b/docs/ci/ci.md @@ -22,6 +22,30 @@ are also run periodically (twice daily) using an OVN-Kubernetes build based on t The following sections should help you understand (and if needed modify) the set of tests that run and how to run these tests locally. +## CI fails: what do I do? + +Some tests are known to be flaky, see [`kind/ci-flake` issues.](https://github.com/ovn-kubernetes/ovn-kubernetes/issues?q=is%3Aissue%20state%3Aopen%20label%3Akind%2Fci-flake) +At the end of your failed test run, you will see something like: + +``` +Summarizing 1 Failure: + [FAIL] e2e egress firewall policy validation with external containers [It] Should validate the egress firewall policy functionality for allowed IP + /home/runner/work/ovn-kubernetes/ovn-kubernetes/test/e2e/egress_firewall.go:130 +``` +then search for "e2e egress firewall policy validation" in the open issues. + +If you find an issue that matches your failure, update the issue with your job link. +If the issue doesn't exist, it either means the failure is introduced in your PR or it is a new flake. +Try to run the same test locally multiple times, and if doesn't fail, report a new flake. +Reporting a new flake is fairly straightforward, but you can use already open issues as an example. +It may also be useful sometimes to search through the closed issues to see if the flake was reported +previously and (not really) fixed, then reopening it with the new job failure. + +Only after following these steps ^, you can comment `/retest-failed` on your PR to trigger a retest of the failed tests. +A rocket emoji reaction on your comment should apper when the retest is triggered. + +Before running this command, please reference existing or newly opened issues to justify the retest request. + ## Understanding the CI Test Suite The tests are broken into 2 categories, `shard` tests which execute tests from the Kubernetes E2E test suite and the From 8db8ca1de453c00817037bee3bb85a21c3263d47 Mon Sep 17 00:00:00 2001 From: Nadia Pinaeva Date: Fri, 29 Aug 2025 09:58:57 +0200 Subject: [PATCH 03/15] Use "OVN-Kubernetes" consistently as project name for CNFC correctness Signed-off-by: Nadia Pinaeva --- README.md | 6 +++--- SECURITY.md | 2 +- contrib/kind.sh | 2 +- ...ovn.org_clusteruserdefinednetworks.yaml.j2 | 4 ++-- .../k8s.ovn.org_userdefinednetworks.yaml.j2 | 2 +- .../userdefinednetwork-api-spec.md | 2 +- docs/developer-guide/local_testing_guide.md | 2 +- docs/features/hardware-offload/dpu-support.md | 10 +++++----- .../multiple-networks/multi-homing.md | 4 ++-- docs/features/multiple-networks/multi-vtep.md | 6 +++--- docs/installation/INSTALL.KUBEADM.md | 20 +++++++++---------- .../launching-ovn-kubernetes-on-kind.md | 2 +- docs/okeps/okep-4368-template.md | 2 +- docs/okeps/okep-4380-network-qos.md | 2 +- docs/okeps/okep-5085-localnet-api.md | 4 ++-- docs/okeps/okep-5193-user-defined-networks.md | 4 ++-- .../okep-5233-preconfigured-udn-addresses.md | 2 +- docs/okeps/okep-5296-bgp.md | 4 ++-- go-controller/README.md | 2 +- .../cmd/ovn-kube-util/app/readiness-probe.go | 2 +- .../cmd/ovnkube-trace/ovnkube-trace.go | 2 +- .../pkg/crd/userdefinednetwork/v1/shared.go | 2 +- .../layer2_user_defined_network_controller.go | 2 +- .../layer3_user_defined_network_controller.go | 2 +- go-controller/pkg/ovn/ovn.go | 2 +- helm/basic-deploy.sh | 4 ++-- test/scripts/upgrade-ovn.sh | 2 +- 27 files changed, 50 insertions(+), 50 deletions(-) diff --git a/README.md b/README.md index 588e6ea420..1c6cda9281 100644 --- a/README.md +++ b/README.md @@ -35,9 +35,9 @@ Here are some links to help in your ovn-kubernetes journey: - [Welcome to ovn-kubernetes](https://ovn-kubernetes.io/) for overview of ovn-kubernetes. - [Architecture of ovn-kubernetes](https://ovn-kubernetes.io/design/architecture/) -- [Deploying OVN Kubernetes cluster using KIND](https://ovn-kubernetes.io/installation/launching-ovn-kubernetes-on-kind/) -- [Deploying OVN Kubernetes CNI using Helm](https://ovn-kubernetes.io/installation/launching-ovn-kubernetes-with-helm/) -- [Contributing to OVN Kubernetes](https://ovn-kubernetes.io/governance/CONTRIBUTING/) for how to get involved +- [Deploying OVN-Kubernetes cluster using KIND](https://ovn-kubernetes.io/installation/launching-ovn-kubernetes-on-kind/) +- [Deploying OVN-Kubernetes CNI using Helm](https://ovn-kubernetes.io/installation/launching-ovn-kubernetes-with-helm/) +- [Contributing to OVN-Kubernetes](https://ovn-kubernetes.io/governance/CONTRIBUTING/) for how to get involved in our project - [Meet the Community](https://ovn-kubernetes.io/governance/MEETINGS/) for details on community meeting details. diff --git a/SECURITY.md b/SECURITY.md index e4f0f2870b..8c28e365b6 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -1,6 +1,6 @@ # Security Policy -OVNKubernetes repo uses the [dependabot](https://docs.github.com/en/code-security/dependabot/dependabot-security-updates/configuring-dependabot-security-updates) which does automatic security updates by scanning the repo and opening PRs to update the effected libraries. +OVN-Kubernetes repo uses the [dependabot](https://docs.github.com/en/code-security/dependabot/dependabot-security-updates/configuring-dependabot-security-updates) which does automatic security updates by scanning the repo and opening PRs to update the effected libraries. ## Reporting a Vulnerability diff --git a/contrib/kind.sh b/contrib/kind.sh index cff76b68ef..9ff8031312 100755 --- a/contrib/kind.sh +++ b/contrib/kind.sh @@ -121,7 +121,7 @@ echo "-npz | --nodes-per-zone If interconnect is enabled, echo "-mtu Define the overlay mtu" echo "--isolated Deploy with an isolated environment (no default gateway)" echo "--delete Delete current cluster" -echo "--deploy Deploy ovn kubernetes without restarting kind" +echo "--deploy Deploy ovn-kubernetes without restarting kind" echo "--add-nodes Adds nodes to an existing cluster. The number of nodes to be added is specified by --num-workers. Also use -ic if the cluster is using interconnect." echo "-dns | --enable-dnsnameresolver Enable DNSNameResolver for resolving the DNS names used in the DNS rules of EgressFirewall." echo "-obs | --observability Enable OVN Observability feature." diff --git a/dist/templates/k8s.ovn.org_clusteruserdefinednetworks.yaml.j2 b/dist/templates/k8s.ovn.org_clusteruserdefinednetworks.yaml.j2 index 88901528d4..4cf86fd906 100644 --- a/dist/templates/k8s.ovn.org_clusteruserdefinednetworks.yaml.j2 +++ b/dist/templates/k8s.ovn.org_clusteruserdefinednetworks.yaml.j2 @@ -145,7 +145,7 @@ spec: description: |- Lifecycle controls IP addresses management lifecycle. - The only allowed value is Persistent. When set, the IP addresses assigned by OVN Kubernetes will be persisted in an + The only allowed value is Persistent. When set, the IP addresses assigned by OVN-Kubernetes will be persisted in an `ipamclaims.k8s.cni.cncf.io` object. These IP addresses will be reused by other pods if requested. Only supported when mode is `Enabled`. enum: @@ -460,7 +460,7 @@ spec: description: |- Lifecycle controls IP addresses management lifecycle. - The only allowed value is Persistent. When set, the IP addresses assigned by OVN Kubernetes will be persisted in an + The only allowed value is Persistent. When set, the IP addresses assigned by OVN-Kubernetes will be persisted in an `ipamclaims.k8s.cni.cncf.io` object. These IP addresses will be reused by other pods if requested. Only supported when mode is `Enabled`. enum: diff --git a/dist/templates/k8s.ovn.org_userdefinednetworks.yaml.j2 b/dist/templates/k8s.ovn.org_userdefinednetworks.yaml.j2 index 9b855382a9..b922cb5f94 100644 --- a/dist/templates/k8s.ovn.org_userdefinednetworks.yaml.j2 +++ b/dist/templates/k8s.ovn.org_userdefinednetworks.yaml.j2 @@ -93,7 +93,7 @@ spec: description: |- Lifecycle controls IP addresses management lifecycle. - The only allowed value is Persistent. When set, the IP addresses assigned by OVN Kubernetes will be persisted in an + The only allowed value is Persistent. When set, the IP addresses assigned by OVN-Kubernetes will be persisted in an `ipamclaims.k8s.cni.cncf.io` object. These IP addresses will be reused by other pods if requested. Only supported when mode is `Enabled`. enum: diff --git a/docs/api-reference/userdefinednetwork-api-spec.md b/docs/api-reference/userdefinednetwork-api-spec.md index e053dcb1f7..4c679ffc14 100644 --- a/docs/api-reference/userdefinednetwork-api-spec.md +++ b/docs/api-reference/userdefinednetwork-api-spec.md @@ -153,7 +153,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `mode` _[IPAMMode](#ipammode)_ | Mode controls how much of the IP configuration will be managed by OVN.
`Enabled` means OVN-Kubernetes will apply IP configuration to the SDN infrastructure and it will also assign IPs
from the selected subnet to the individual pods.
`Disabled` means OVN-Kubernetes will only assign MAC addresses and provide layer 2 communication, letting users
configure IP addresses for the pods.
`Disabled` is only available for Secondary networks.
By disabling IPAM, any Kubernetes features that rely on selecting pods by IP will no longer function
(such as network policy, services, etc). Additionally, IP port security will also be disabled for interfaces attached to this network.
Defaults to `Enabled`. | | Enum: [Enabled Disabled]
| -| `lifecycle` _[NetworkIPAMLifecycle](#networkipamlifecycle)_ | Lifecycle controls IP addresses management lifecycle.

The only allowed value is Persistent. When set, the IP addresses assigned by OVN Kubernetes will be persisted in an
`ipamclaims.k8s.cni.cncf.io` object. These IP addresses will be reused by other pods if requested.
Only supported when mode is `Enabled`. | | Enum: [Persistent]
| +| `lifecycle` _[NetworkIPAMLifecycle](#networkipamlifecycle)_ | Lifecycle controls IP addresses management lifecycle.

The only allowed value is Persistent. When set, the IP addresses assigned by OVN-Kubernetes will be persisted in an
`ipamclaims.k8s.cni.cncf.io` object. These IP addresses will be reused by other pods if requested.
Only supported when mode is `Enabled`. | | Enum: [Persistent]
| #### IPAMMode diff --git a/docs/developer-guide/local_testing_guide.md b/docs/developer-guide/local_testing_guide.md index 5158fb2df9..b15a3d314c 100644 --- a/docs/developer-guide/local_testing_guide.md +++ b/docs/developer-guide/local_testing_guide.md @@ -105,7 +105,7 @@ Kubernetes in Docker (KIND) is used to deploy Kubernetes locally where a docker container is created per Kubernetes node. The CI tests run on this Kubernetes deployment. Therefore, KIND will need to be installed locally. -Generic instructions for installing and running OVN Kubernetes with KIND can be found at: +Generic instructions for installing and running OVN-Kubernetes with KIND can be found at: [OVN-Kubernetes KIND Setup](https://ovn-kubernetes.io/installation/launching-ovn-kubernetes-on-kind/) Make sure to set the required environment variables first (see section above). Then, deploy kind: diff --git a/docs/features/hardware-offload/dpu-support.md b/docs/features/hardware-offload/dpu-support.md index 6ac6a5ca7d..2c5c23e028 100644 --- a/docs/features/hardware-offload/dpu-support.md +++ b/docs/features/hardware-offload/dpu-support.md @@ -18,7 +18,7 @@ Any vendor that manufactures a DPU which supports the above model should work wi Design document can be found [here](https://docs.google.com/document/d/11IoMKiohK7hIyIE36FJmwJv46DEBx52a4fqvrpCBBcg/edit?usp=sharing). -## OVN Kubernetes in a DPU-Accelerated Environment +## OVN-Kubernetes in a DPU-Accelerated Environment The **ovn-kubernetes** deployment will have two parts one on the host and another on the DPU side. @@ -29,10 +29,10 @@ These aforementioned parts are expected to be deployed also on two different Kub ### Host Cluster --- -#### OVN Kubernetes control plane related component +#### OVN-Kubernetes control plane related component - ovn-cluster-manager -#### OVN Kubernetes components on a Standard Host (Non-DPU) +#### OVN-Kubernetes components on a Standard Host (Non-DPU) - local-nb-ovsdb - local-sb-ovsdb - run-ovn-northd @@ -40,7 +40,7 @@ These aforementioned parts are expected to be deployed also on two different Kub - ovn-controller - ovs-metrics -#### OVN Kubernetes component on a DPU-Enabled Host +#### OVN-Kubernetes component on a DPU-Enabled Host - ovn-node For detailed configuration of gateway interfaces in DPU host mode, see [DPU Gateway Interface Configuration](dpu-gateway-interface.md). @@ -48,7 +48,7 @@ For detailed configuration of gateway interfaces in DPU host mode, see [DPU Gate ### DPU Cluster --- -#### OVN Kubernetes components +#### OVN-Kubernetes components - local-nb-ovsdb - local-sb-ovsdb - run-ovn-northd diff --git a/docs/features/multiple-networks/multi-homing.md b/docs/features/multiple-networks/multi-homing.md index c134509d0b..eee72f47df 100644 --- a/docs/features/multiple-networks/multi-homing.md +++ b/docs/features/multiple-networks/multi-homing.md @@ -163,7 +163,7 @@ spec: - `excludeSubnets` (string, optional): a comma separated list of CIDRs / IPs. These IPs will be removed from the assignable IP pool, and never handed over to the pods. -- `allowPersistentIPs` (boolean, optional): persist the OVN Kubernetes assigned +- `allowPersistentIPs` (boolean, optional): persist the OVN-Kubernetes assigned IP addresses in a `ipamclaims.k8s.cni.cncf.io` object. This IP addresses will be reused by other pods if requested. Useful for KubeVirt VMs. Only makes sense if the `subnets` attribute is also defined. @@ -220,7 +220,7 @@ localnet network. These IPs will be removed from the assignable IP pool, and never handed over to the pods. - `vlanID` (integer, optional): assign VLAN tag. Defaults to none. -- `allowPersistentIPs` (boolean, optional): persist the OVN Kubernetes assigned +- `allowPersistentIPs` (boolean, optional): persist the OVN-Kubernetes assigned IP addresses in a `ipamclaims.k8s.cni.cncf.io` object. This IP addresses will be reused by other pods if requested. Useful for KubeVirt VMs. Only makes sense if the `subnets` attribute is also defined. diff --git a/docs/features/multiple-networks/multi-vtep.md b/docs/features/multiple-networks/multi-vtep.md index 8bc49ab31c..7de470926d 100644 --- a/docs/features/multiple-networks/multi-vtep.md +++ b/docs/features/multiple-networks/multi-vtep.md @@ -35,9 +35,9 @@ and traffic segregation. * vfR is VF representor. -## How to enable this feature on an OVN Kubernetes cluster? +## How to enable this feature on an OVN-Kubernetes cluster? -On OVN Kubernetes side, no additional configuration required to enable this feature. +On OVN-Kubernetes side, no additional configuration required to enable this feature. This feature depends on a specific underlay network setup; it cannot be turned on without an adequate underlay network configuration. @@ -83,7 +83,7 @@ OVS Database on chassis: added to define the mapping between PF interface name and its VTEP IP. -### OVN Kubernetes Implementation Details +### OVN-Kubernetes Implementation Details To support this feature, the VTEP interfaces must be configured in advance. This can be accomplished using a system network management tool, such as diff --git a/docs/installation/INSTALL.KUBEADM.md b/docs/installation/INSTALL.KUBEADM.md index 5eea767053..6160e78f8c 100644 --- a/docs/installation/INSTALL.KUBEADM.md +++ b/docs/installation/INSTALL.KUBEADM.md @@ -1,4 +1,4 @@ -The following is a walkthrough for an installation in an environment with 4 virtual machines, and a cluster deployed with `kubeadm`. This shall serve as a guide for people who are curious enough to deploy OVN Kubernetes on a manually created cluster and to play around with the components. +The following is a walkthrough for an installation in an environment with 4 virtual machines, and a cluster deployed with `kubeadm`. This shall serve as a guide for people who are curious enough to deploy OVN-Kubernetes on a manually created cluster and to play around with the components. Note that the resulting environment might be highly unstable. @@ -8,7 +8,7 @@ If your goal is to set up an environment quickly or to set up a development envi ### Overview -The environment consists of 4 libvirt/qemu virtual machines, all deployed with Rocky Linux 8 or CentOS 8. `node1` will serve as the sole master node and nodes `node2` and `node3` as the worker nodes. `gw1` will be the default gateway for the cluster via the `Isolated Network`. It will also host an HTTP registry to store the OVN Kubernetes images. +The environment consists of 4 libvirt/qemu virtual machines, all deployed with Rocky Linux 8 or CentOS 8. `node1` will serve as the sole master node and nodes `node2` and `node3` as the worker nodes. `gw1` will be the default gateway for the cluster via the `Isolated Network`. It will also host an HTTP registry to store the OVN-Kubernetes images. ~~~ to hypervisor to hypervisor to hypervisor @@ -135,7 +135,7 @@ reboot ### node1 through node3 base setup -You must install Open vSwitch on `node1` through `node3`. You will then connect `enp7s0` to an OVS bridge called `br-ex`. This bridge will be used later by OVN Kubernetes. +You must install Open vSwitch on `node1` through `node3`. You will then connect `enp7s0` to an OVS bridge called `br-ex`. This bridge will be used later by OVN-Kubernetes. Furthermore, you must assign IP addresses to `br-ex` and point the nodes' default route via `br-ex` to `gw1`. #### Set hostnames @@ -155,7 +155,7 @@ reboot #### Remove firewalld -Make sure to uninstall firewalld. Otherwise, it will block the kubernetes management ports (that can easily be fixed by configuration) and it will also preempt and block the OVN Kubernetes installed NAT and FORWARD rules (this is more difficult to remediate). The easiest fix is hence not to use firewalld at all: +Make sure to uninstall firewalld. Otherwise, it will block the kubernetes management ports (that can easily be fixed by configuration) and it will also preempt and block the OVN-Kubernetes installed NAT and FORWARD rules (this is more difficult to remediate). The easiest fix is hence not to use firewalld at all: ~~~ systemctl disable --now firewalld yum remove -y firewalld @@ -377,7 +377,7 @@ sudo yum install -y kubelet kubeadm kubectl --disableexcludes=kubernetes sudo systemctl enable --now kubelet ~~~ -## Deploying a cluster with OVN Kubernetes +## Deploying a cluster with OVN-Kubernetes Execute the following instructions **only** on the master node, `node1`. @@ -406,9 +406,9 @@ kube-system kube-proxy-vm44k 1/1 Running 0 kube-system kube-scheduler-node1 1/1 Running 3 28s 192.168.122.205 node1 ~~~ -Now, deploy OVN Kubernetes - see below. +Now, deploy OVN-Kubernetes - see below. -### Deploying OVN Kubernetes on node1 +### Deploying OVN-Kubernetes on node1 Install build dependencies and create a softlink for `pip` to `pip3`: ~~~ @@ -425,7 +425,7 @@ source ~/.bashrc go version ~~~ -Now, clone the OVN Kubernetes repository: +Now, clone the OVN-Kubernetes repository: ~~~ mkdir -p $HOME/work/src/github.com/ovn-org cd $HOME/work/src/github.com/ovn-org @@ -599,7 +599,7 @@ Installed: Complete! ~~~ -### Uninstalling OVN Kubernetes +### Uninstalling OVN-Kubernetes In order to uninstall OVN kubernetes: ~~~ @@ -616,7 +616,7 @@ br-int might be added by OVN, but the files for it are not created in /var/run/o 2021-08-24T12:42:43.810Z|00025|rconn|WARN|unix:/var/run/openvswitch/br-int.mgmt: connection failed (No such file or directory) ~~~ -The best workaroud is to pre-create br-int before the OVN Kubernetes installation: +The best workaroud is to pre-create br-int before the OVN-Kubernetes installation: ~~~ ovs-vsctl add-br br-int ~~~ diff --git a/docs/installation/launching-ovn-kubernetes-on-kind.md b/docs/installation/launching-ovn-kubernetes-on-kind.md index 183f738884..659ba5e460 100644 --- a/docs/installation/launching-ovn-kubernetes-on-kind.md +++ b/docs/installation/launching-ovn-kubernetes-on-kind.md @@ -177,7 +177,7 @@ usage: kind.sh [[[-cf |--config-file ] [-kt|keep-taint] [-ha|--ha-enabled] -nqe | --network-qos-enable Enable network QoS. DEFAULT: Disabled. -lr |--local-kind-registry Will start and connect a kind local registry to push/retrieve images --delete Delete current cluster ---deploy Deploy ovn kubernetes without restarting kind +--deploy Deploy ovn-kubernetes without restarting kind ``` As seen above, if you do not specify any options the script will assume the default values. diff --git a/docs/okeps/okep-4368-template.md b/docs/okeps/okep-4368-template.md index 03f2a62ef3..0f14591dd1 100644 --- a/docs/okeps/okep-4368-template.md +++ b/docs/okeps/okep-4368-template.md @@ -94,7 +94,7 @@ to include the path to your new OKEP (i.e Feature Title: okeps/) ## Risks, Known Limitations and Mitigations -## OVN Kubernetes Version Skew +## OVN-Kubernetes Version Skew which version is this feature planned to be introduced in? check repo milestones/releases to get this information for diff --git a/docs/okeps/okep-4380-network-qos.md b/docs/okeps/okep-4380-network-qos.md index d792bbd4ce..eb64470270 100644 --- a/docs/okeps/okep-4380-network-qos.md +++ b/docs/okeps/okep-4380-network-qos.md @@ -551,7 +551,7 @@ To be discussed. ## Risks, Known Limitations and Mitigations -## OVN Kubernetes Version Skew +## OVN-Kubernetes Version Skew To be discussed. diff --git a/docs/okeps/okep-5085-localnet-api.md b/docs/okeps/okep-5085-localnet-api.md index 07be81a48f..fe5d862817 100644 --- a/docs/okeps/okep-5085-localnet-api.md +++ b/docs/okeps/okep-5085-localnet-api.md @@ -90,7 +90,7 @@ spec: 5. `vlanID` - VLAN tag assigned to traffic. 6. `mtu` - maximum transmission unit for a network 7. `allowPersistentIPs` - persist the OVN Kubernetes assigned IP addresses in a `ipamclaims.k8s.cni.cncf.io` object. These IP addresses will be + persist the OVN-Kubernetes assigned IP addresses in a `ipamclaims.k8s.cni.cncf.io` object. These IP addresses will be reused by other pods if requested. Useful for [KubeVirt](https://kubevirt.io/) VMs. #### Extend ClusterUserDefinedNetwork CRD @@ -588,7 +588,7 @@ To mitigate this, the mentioned validation should be done by the CUDN CRD contro In a scenario where a CUDN CR has at least one exclude-subnet that is not within the range of the topology subnet, the controller will not create the corresponding NAD and will report an error in the status. -## OVN Kubernetes Version Skew +## OVN-Kubernetes Version Skew ## Alternatives diff --git a/docs/okeps/okep-5193-user-defined-networks.md b/docs/okeps/okep-5193-user-defined-networks.md index 93da13c51c..010fc84452 100644 --- a/docs/okeps/okep-5193-user-defined-networks.md +++ b/docs/okeps/okep-5193-user-defined-networks.md @@ -581,7 +581,7 @@ type IPAMSpec struct { // Lifecycle controls IP addresses management lifecycle. // - // The only allowed value is Persistent. When set, OVN Kubernetes assigned IP addresses will be persisted in an + // The only allowed value is Persistent. When set, OVN-Kubernetes assigned IP addresses will be persisted in an // `ipamclaims.k8s.cni.cncf.io` object. These IP addresses will be reused by other pods if requested. // Only supported when "mode" is `Enabled`. // @@ -1473,7 +1473,7 @@ addresses are used. With this new design, users will need to reconfigure their s desired number of networks. Note, API changes will need to be made in order to support changing the masquerade subnet post-installation. -## OVN Kubernetes Version Skew +## OVN-Kubernetes Version Skew UDN will be delivered in version 1.1.0. diff --git a/docs/okeps/okep-5233-preconfigured-udn-addresses.md b/docs/okeps/okep-5233-preconfigured-udn-addresses.md index 332fec24fb..2dc7db6178 100644 --- a/docs/okeps/okep-5233-preconfigured-udn-addresses.md +++ b/docs/okeps/okep-5233-preconfigured-udn-addresses.md @@ -490,7 +490,7 @@ OVN-Kubernetes for a dynamically allocated IP. To mitigate these conflicts, user MAC address and recreate the workload. For importing workloads that already use this prefix, a future enhancement could add a field to the Layer2 spec allowing users to specify a custom MAC prefix for the UDN. -## OVN Kubernetes Version Skew +## OVN-Kubernetes Version Skew ## Alternatives diff --git a/docs/okeps/okep-5296-bgp.md b/docs/okeps/okep-5296-bgp.md index 295a36c55d..fb605937ba 100644 --- a/docs/okeps/okep-5296-bgp.md +++ b/docs/okeps/okep-5296-bgp.md @@ -623,7 +623,7 @@ leak routes to the pod subnets from each user-defined VRF into the default VRF r ### Testing Details * E2E upstream with a framework (potentially [containerlab.dev](containerlab.dev) to simulate a routed spine and leaf - topology with integration using OVN Kubernetes. + topology with integration using OVN-Kubernetes. * Testing using transport none for some networks, and Geneve for non-BGP enabled networks. * Testing to cover BGP functionality including MEG, Egress IP, Egress QoS, etc. * Scale testing to determine impact of FRR-K8S footprint on large scale deployments. @@ -656,7 +656,7 @@ default environment without enabling these extra options/features. Reliance on FRR is another minor risk, with no presence from the OVN-Kubernetes team involved in that project. -## OVN Kubernetes Version Skew +## OVN-Kubernetes Version Skew BGP will be delivered in version 1.1.0. diff --git a/go-controller/README.md b/go-controller/README.md index fb73a74e38..d0c075bfcc 100644 --- a/go-controller/README.md +++ b/go-controller/README.md @@ -1,4 +1,4 @@ -# ovn kubernetes go-controller +# ovn-kubernetes go-controller The golang based ovn controller is a reliable way to deploy the OVN SDN using kubernetes clients and watchers based on golang. diff --git a/go-controller/cmd/ovn-kube-util/app/readiness-probe.go b/go-controller/cmd/ovn-kube-util/app/readiness-probe.go index a3b55fcfc8..29baca87c1 100644 --- a/go-controller/cmd/ovn-kube-util/app/readiness-probe.go +++ b/go-controller/cmd/ovn-kube-util/app/readiness-probe.go @@ -156,7 +156,7 @@ func ovnNodeReadiness(_ string) error { confFile := "/etc/cni/net.d/10-ovn-kubernetes.conf" _, err := os.Stat(confFile) if os.IsNotExist(err) { - return fmt.Errorf("OVN Kubernetes config file %q doesn't exist", confFile) + return fmt.Errorf("OVN-Kubernetes config file %q doesn't exist", confFile) } return nil } diff --git a/go-controller/cmd/ovnkube-trace/ovnkube-trace.go b/go-controller/cmd/ovnkube-trace/ovnkube-trace.go index 96cea038d2..3e0bb29167 100644 --- a/go-controller/cmd/ovnkube-trace/ovnkube-trace.go +++ b/go-controller/cmd/ovnkube-trace/ovnkube-trace.go @@ -1241,7 +1241,7 @@ func main() { klog.Exitf(" Unexpected error: %v", err) } - klog.V(5).Infof("OVN Kubernetes namespace is %s", ovnNamespace) + klog.V(5).Infof("OVN-Kubernetes namespace is %s", ovnNamespace) if *dumpVRFTableIDs { nodesVRFTableIDs, err := findUserDefinedNetworkVRFTableIDs(coreclient, restconfig, ovnNamespace) if err != nil { diff --git a/go-controller/pkg/crd/userdefinednetwork/v1/shared.go b/go-controller/pkg/crd/userdefinednetwork/v1/shared.go index 9787415ddc..987a54c1cd 100644 --- a/go-controller/pkg/crd/userdefinednetwork/v1/shared.go +++ b/go-controller/pkg/crd/userdefinednetwork/v1/shared.go @@ -199,7 +199,7 @@ type IPAMConfig struct { // Lifecycle controls IP addresses management lifecycle. // - // The only allowed value is Persistent. When set, the IP addresses assigned by OVN Kubernetes will be persisted in an + // The only allowed value is Persistent. When set, the IP addresses assigned by OVN-Kubernetes will be persisted in an // `ipamclaims.k8s.cni.cncf.io` object. These IP addresses will be reused by other pods if requested. // Only supported when mode is `Enabled`. // diff --git a/go-controller/pkg/ovn/layer2_user_defined_network_controller.go b/go-controller/pkg/ovn/layer2_user_defined_network_controller.go index eb7bb05abd..5194faed93 100644 --- a/go-controller/pkg/ovn/layer2_user_defined_network_controller.go +++ b/go-controller/pkg/ovn/layer2_user_defined_network_controller.go @@ -888,7 +888,7 @@ func (oc *Layer2UserDefinedNetworkController) StartServiceController(wg *sync.Wa // do not use LB templates for UDNs - OVN bug https://issues.redhat.com/browse/FDP-988 err := oc.svcController.Run(5, oc.stopChan, wg, runRepair, useLBGroups, false) if err != nil { - return fmt.Errorf("error running OVN Kubernetes Services controller for network %s: %v", oc.GetNetworkName(), err) + return fmt.Errorf("error running OVN-Kubernetes Services controller for network %s: %v", oc.GetNetworkName(), err) } return nil } diff --git a/go-controller/pkg/ovn/layer3_user_defined_network_controller.go b/go-controller/pkg/ovn/layer3_user_defined_network_controller.go index 815a8b4c9c..81b4aecc60 100644 --- a/go-controller/pkg/ovn/layer3_user_defined_network_controller.go +++ b/go-controller/pkg/ovn/layer3_user_defined_network_controller.go @@ -1141,7 +1141,7 @@ func (oc *Layer3UserDefinedNetworkController) StartServiceController(wg *sync.Wa // do not use LB templates for UDNs - OVN bug https://issues.redhat.com/browse/FDP-988 err := oc.svcController.Run(5, oc.stopChan, wg, runRepair, useLBGroups, false) if err != nil { - return fmt.Errorf("error running OVN Kubernetes Services controller for network %s: %v", oc.GetNetworkName(), err) + return fmt.Errorf("error running OVN-Kubernetes Services controller for network %s: %v", oc.GetNetworkName(), err) } return nil } diff --git a/go-controller/pkg/ovn/ovn.go b/go-controller/pkg/ovn/ovn.go index d935bca85f..50ab9e6448 100644 --- a/go-controller/pkg/ovn/ovn.go +++ b/go-controller/pkg/ovn/ovn.go @@ -459,7 +459,7 @@ func (oc *DefaultNetworkController) StartServiceController(wg *sync.WaitGroup, r err := oc.svcController.Run(5, oc.stopChan, wg, runRepair, useLBGroups, oc.svcTemplateSupport) if err != nil { - return fmt.Errorf("error running OVN Kubernetes Services controller: %v", err) + return fmt.Errorf("error running OVN-Kubernetes Services controller: %v", err) } return nil } diff --git a/helm/basic-deploy.sh b/helm/basic-deploy.sh index e6603bd8fb..f9b3599dc7 100755 --- a/helm/basic-deploy.sh +++ b/helm/basic-deploy.sh @@ -4,7 +4,7 @@ function usage() { echo "Usage: $0 [OPTIONS]" echo "" - echo "This script deploys a kind cluster and configures it to use OVN Kubernetes CNI." + echo "This script deploys a kind cluster and configures it to use OVN-Kubernetes CNI." echo "" echo "Options:" echo " BUILD_IMAGE=${BUILD_IMAGE:-false} Set to true to build the Docker image instead of pulling it." @@ -88,7 +88,7 @@ if [[ "$OVN_INTERCONNECT" == "true" ]]; then done fi -# Deploy OVN Kubernetes using Helm +# Deploy OVN-Kubernetes using Helm cd ${DIR}/ovn-kubernetes helm install ovn-kubernetes . -f ${VALUES_FILE} \ --set k8sAPIServer="https://$(kubectl get pods -n kube-system -l component=kube-apiserver -o jsonpath='{.items[0].status.hostIP}'):6443" \ diff --git a/test/scripts/upgrade-ovn.sh b/test/scripts/upgrade-ovn.sh index da40800d36..a2fc50dfe3 100755 --- a/test/scripts/upgrade-ovn.sh +++ b/test/scripts/upgrade-ovn.sh @@ -21,7 +21,7 @@ kubectl_wait_pods() { OVN_TIMEOUT=1400s fi if ! kubectl wait -n ovn-kubernetes --for=condition=ready pods --all --timeout=${OVN_TIMEOUT} ; then - echo "some pods in OVN Kubernetes are not running" + echo "some pods in OVN-Kubernetes are not running" kubectl get pods -A -o wide || true kubectl describe po -n ovn-kubernetes exit 1 From c8b18bd71a0726dbed9230153f89511c2dec74b6 Mon Sep 17 00:00:00 2001 From: Ram Lavi Date: Mon, 7 Jul 2025 11:09:18 +0300 Subject: [PATCH 04/15] pod-allocator: Introduce MAC addresses manager Enable tracking used MAC addresses with owner identification. Enabling MAC addresses conflict detection when multiple entities try to use the same address within the same network. The MAC manager will be integrated with cluster-manager's pod-allocator code in follow-up commits. Since pod-allocator run by multiple Goroutines, use Mutex to prevent race conditions on reserve and release MACs. Signed-off-by: Ram Lavi Co-authored-by: Or Mergi --- .../pkg/allocator/mac/mac_suite_test.go | 13 ++++ .../pkg/allocator/mac/reservation.go | 69 ++++++++++++++++++ .../pkg/allocator/mac/reservation_test.go | 73 +++++++++++++++++++ 3 files changed, 155 insertions(+) create mode 100644 go-controller/pkg/allocator/mac/mac_suite_test.go create mode 100644 go-controller/pkg/allocator/mac/reservation.go create mode 100644 go-controller/pkg/allocator/mac/reservation_test.go diff --git a/go-controller/pkg/allocator/mac/mac_suite_test.go b/go-controller/pkg/allocator/mac/mac_suite_test.go new file mode 100644 index 0000000000..da52f4ae17 --- /dev/null +++ b/go-controller/pkg/allocator/mac/mac_suite_test.go @@ -0,0 +1,13 @@ +package mac_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestMAC(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "MAC Suite") +} diff --git a/go-controller/pkg/allocator/mac/reservation.go b/go-controller/pkg/allocator/mac/reservation.go new file mode 100644 index 0000000000..7cf5291396 --- /dev/null +++ b/go-controller/pkg/allocator/mac/reservation.go @@ -0,0 +1,69 @@ +package mac + +import ( + "errors" + "net" + "sync" +) + +// ReservationManager tracks reserved MAC addresses requests of pods and detect MAC conflicts, +// where one pod request static MAC address that is used by another pod. +type ReservationManager struct { + // lock for storing a MAC reservation. + lock sync.Mutex + // store for reserved MAC address request by owner. Key is MAC address, value is owner identifier. + store map[string]string +} + +// NewManager creates a new ReservationManager. +func NewManager() *ReservationManager { + return &ReservationManager{ + store: make(map[string]string), + } +} + +var ErrReserveMACConflict = errors.New("MAC address already in use") +var ErrMACReserved = errors.New("MAC address already reserved for the given owner") + +// Reserve stores the address reservation and its owner. +// Returns an error ErrReserveMACConflict when "mac" is already reserved by different owner. +// Returns an error ErrMACReserved when "mac" is already reserved by the given owner. +func (n *ReservationManager) Reserve(owner string, mac net.HardwareAddr) error { + n.lock.Lock() + defer n.lock.Unlock() + + macKey := mac.String() + currentOwner, macReserved := n.store[macKey] + if macReserved && currentOwner != owner { + return ErrReserveMACConflict + } + if macReserved { + return ErrMACReserved + } + + n.store[macKey] = owner + + return nil +} + +var ErrReleaseMismatchOwner = errors.New("MAC reserved for different owner") + +// Release MAC address from store of the given owner. +// Return an error ErrReleaseMismatchOwner when "mac" reserved for different owner than the given one. +func (n *ReservationManager) Release(owner string, mac net.HardwareAddr) error { + n.lock.Lock() + defer n.lock.Unlock() + + macKey := mac.String() + currentOwner, macReserved := n.store[macKey] + if !macReserved { + return nil + } + if currentOwner != owner { + return ErrReleaseMismatchOwner + } + + delete(n.store, macKey) + + return nil +} diff --git a/go-controller/pkg/allocator/mac/reservation_test.go b/go-controller/pkg/allocator/mac/reservation_test.go new file mode 100644 index 0000000000..a463948c42 --- /dev/null +++ b/go-controller/pkg/allocator/mac/reservation_test.go @@ -0,0 +1,73 @@ +package mac_test + +import ( + "fmt" + "net" + + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/mac" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("ReservationManager", func() { + const owner1 = "namespace1/pod1" + const owner2 = "namespace2/pod2" + + var testMgr *mac.ReservationManager + var mac1 net.HardwareAddr + + BeforeEach(func() { + var err error + mac1, err = net.ParseMAC("aa:bb:cc:dd:ee:f1") + Expect(err).NotTo(HaveOccurred()) + + testMgr = mac.NewManager() + }) + + Context("reserve", func() { + It("should fail on repeated reservation for the same owner", func() { + Expect(testMgr.Reserve(owner1, mac1)).To(Succeed()) + Expect(testMgr.Reserve(owner1, mac1)).To(MatchError(mac.ErrMACReserved)) + }) + + It("should fail reserve existing MAC for different owner", func() { + Expect(testMgr.Reserve(owner1, mac1)).To(Succeed()) + Expect(testMgr.Reserve(owner2, mac1)).To(MatchError(mac.ErrReserveMACConflict), + "different owner should raise a conflict") + }) + + It("should succeed", func() { + for i := 0; i < 5; i++ { + owner := fmt.Sprintf("ns%d/test", i) + mac := net.HardwareAddr(fmt.Sprintf("02:02:02:02:02:0%d", i)) + Expect(testMgr.Reserve(owner, mac)).To(Succeed()) + } + }) + }) + + Context("release a reserved mac", func() { + BeforeEach(func() { + By("reserve mac1 for owner1") + Expect(testMgr.Reserve(owner1, mac1)).To(Succeed()) + }) + + It("should not release MAC given wrong owner", func() { + Expect(testMgr.Release(owner2, mac1)).To(MatchError(mac.ErrReleaseMismatchOwner)) + + Expect(testMgr.Reserve(owner2, mac1)).To(MatchError(mac.ErrReserveMACConflict), + "mac1 reserved for owner1, it should raise a conflict") + }) + + It("should succeed", func() { + Expect(testMgr.Release(owner1, mac1)).To(Succeed()) + + Expect(testMgr.Reserve(owner2, mac1)).To(Succeed(), + "reserving mac1 for different owner should not raise a conflict") + }) + }) + + It("release non reserved mac should succeed (no-op)", func() { + Expect(testMgr.Release(owner1, mac1)).To(Succeed()) + }) +}) From 1d5045a8364c1e0224478161651ea69cd3423b3d Mon Sep 17 00:00:00 2001 From: Ram Lavi Date: Thu, 10 Jul 2025 15:13:54 +0300 Subject: [PATCH 05/15] clustermanager,allocator: Detect MAC conflict on pod allocation Integrate the MAC manager to podAllocator, instantiate the MAC manager on primary L2 networks UDNs with persistentIPs enabled, when EnablePreconfiguredUDNAddresses is enabled. The pod-allocator is instantiated for each network, thus network isolation is maintained. MACs can reused in different UDNs. On pod allocation, record the used MAC address and its owner-id, if already used raise MAC conflict error. Compose the owner-id in the following format: / E.g: Given pod namespace=blue, name=mypod, owner-id is blue/mypod To allow VM migration scenario, where two pods should use the same MAC, relax MAC conflicts by composing the owner-id from the associated VM name: / E.g: Given pod namespace=blue, name=virt-launcher-myvm-abc123 VM name=myvm, owner id is "blue/mypod". The VM name is reflected by the "vm.kubevirt.io/name" label In addition, in a scenario of repeated request (same mac & owner) that was already handled, being rollback due to failure (e.g.: pod update failure), do not release the reserved MAC as part of the pod-allocation rollback. MAC addresses release on pod deletion, and initializing the MAC manager on start up will be done in follow-up commits. Signed-off-by: Ram Lavi Co-authored-by: Or Mergi --- .../pkg/allocator/mac/reservation.go | 5 + go-controller/pkg/allocator/pod/macs.go | 21 +++ .../pkg/allocator/pod/pod_annotation.go | 53 ++++++- .../pkg/allocator/pod/pod_annotation_test.go | 55 +++++++ .../network_cluster_controller.go | 13 +- .../pkg/clustermanager/pod/allocator_test.go | 143 +++++++++++++++--- 6 files changed, 271 insertions(+), 19 deletions(-) create mode 100644 go-controller/pkg/allocator/pod/macs.go diff --git a/go-controller/pkg/allocator/mac/reservation.go b/go-controller/pkg/allocator/mac/reservation.go index 7cf5291396..b3cbc3777a 100644 --- a/go-controller/pkg/allocator/mac/reservation.go +++ b/go-controller/pkg/allocator/mac/reservation.go @@ -6,6 +6,11 @@ import ( "sync" ) +type Register interface { + Reserve(owner string, mac net.HardwareAddr) error + Release(owner string, mac net.HardwareAddr) error +} + // ReservationManager tracks reserved MAC addresses requests of pods and detect MAC conflicts, // where one pod request static MAC address that is used by another pod. type ReservationManager struct { diff --git a/go-controller/pkg/allocator/pod/macs.go b/go-controller/pkg/allocator/pod/macs.go new file mode 100644 index 0000000000..68df0b9dff --- /dev/null +++ b/go-controller/pkg/allocator/pod/macs.go @@ -0,0 +1,21 @@ +package pod + +import ( + "fmt" + + kubevirtv1 "kubevirt.io/api/core/v1" + + corev1 "k8s.io/api/core/v1" +) + +// macOwner compose the owner identifier reserved for MAC addresses management. +// Returns "/" for regular pods and "/" for VMs. +func macOwner(pod *corev1.Pod) string { + // Check if this is a VM pod and persistent IPs are enabled + if vmName, ok := pod.Labels[kubevirtv1.VirtualMachineNameLabel]; ok { + return fmt.Sprintf("%s/%s", pod.Namespace, vmName) + } + + // Default to pod-based identifier + return fmt.Sprintf("%s/%s", pod.Namespace, pod.Name) +} diff --git a/go-controller/pkg/allocator/pod/pod_annotation.go b/go-controller/pkg/allocator/pod/pod_annotation.go index eed6bab488..0929111cb5 100644 --- a/go-controller/pkg/allocator/pod/pod_annotation.go +++ b/go-controller/pkg/allocator/pod/pod_annotation.go @@ -1,6 +1,7 @@ package pod import ( + "errors" "fmt" "net" @@ -15,6 +16,7 @@ import ( "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/id" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/ip" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/ip/subnet" + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/mac" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/generator/udn" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/kube" @@ -30,20 +32,34 @@ type PodAnnotationAllocator struct { netInfo util.NetInfo ipamClaimsReconciler persistentips.PersistentAllocations + macRegistry mac.Register } +type AllocatorOption func(*PodAnnotationAllocator) + func NewPodAnnotationAllocator( netInfo util.NetInfo, podLister listers.PodLister, kube kube.InterfaceOVN, claimsReconciler persistentips.PersistentAllocations, + opts ...AllocatorOption, ) *PodAnnotationAllocator { - return &PodAnnotationAllocator{ + p := &PodAnnotationAllocator{ podLister: podLister, kube: kube, netInfo: netInfo, ipamClaimsReconciler: claimsReconciler, } + for _, opt := range opts { + opt(p) + } + return p +} + +func WithMACRegistry(m mac.Register) AllocatorOption { + return func(p *PodAnnotationAllocator) { + p.macRegistry = m + } } // AllocatePodAnnotation allocates the PodAnnotation which includes IPs, a mac @@ -75,6 +91,7 @@ func (allocator *PodAnnotationAllocator) AllocatePodAnnotation( pod, network, allocator.ipamClaimsReconciler, + allocator.macRegistry, reallocateIP, networkRole, ) @@ -89,6 +106,7 @@ func allocatePodAnnotation( pod *corev1.Pod, network *nadapi.NetworkSelectionElement, claimsReconciler persistentips.PersistentAllocations, + macRegistry mac.Register, reallocateIP bool, networkRole string) ( updatedPod *corev1.Pod, @@ -108,6 +126,7 @@ func allocatePodAnnotation( pod, network, claimsReconciler, + macRegistry, reallocateIP, networkRole, ) @@ -159,6 +178,7 @@ func (allocator *PodAnnotationAllocator) AllocatePodAnnotationWithTunnelID( pod, network, allocator.ipamClaimsReconciler, + allocator.macRegistry, reallocateIP, networkRole, ) @@ -174,6 +194,7 @@ func allocatePodAnnotationWithTunnelID( pod *corev1.Pod, network *nadapi.NetworkSelectionElement, claimsReconciler persistentips.PersistentAllocations, + macRegistry mac.Register, reallocateIP bool, networkRole string) ( updatedPod *corev1.Pod, @@ -190,6 +211,7 @@ func allocatePodAnnotationWithTunnelID( pod, network, claimsReconciler, + macRegistry, reallocateIP, networkRole, ) @@ -264,6 +286,7 @@ func allocatePodAnnotationWithRollback( pod *corev1.Pod, network *nadapi.NetworkSelectionElement, claimsReconciler persistentips.PersistentAllocations, + macRegistry mac.Register, reallocateIP bool, networkRole string) ( updatedPod *corev1.Pod, @@ -276,6 +299,8 @@ func allocatePodAnnotationWithRollback( nadName = util.GetNADName(network.Namespace, network.Name) } podDesc := fmt.Sprintf("%s/%s/%s", nadName, pod.Namespace, pod.Name) + macOwnerID := macOwner(pod) + networkName := netInfo.GetNetworkName() // the IPs we allocate in this function need to be released back to the IPAM // pool if there is some error in any step past the point the IPs were @@ -283,12 +308,23 @@ func allocatePodAnnotationWithRollback( // for defer to work correctly. var releaseIPs []*net.IPNet var releaseID int + var releaseMAC net.HardwareAddr rollback = func() { if releaseID != 0 { idAllocator.ReleaseID() klog.V(5).Infof("Released ID %d", releaseID) releaseID = 0 } + + if len(releaseMAC) > 0 && macRegistry != nil { + if rerr := macRegistry.Release(macOwnerID, releaseMAC); rerr != nil { + klog.Errorf("Failed to release MAC %q on rollback, owner: %q, network: %q: %v", releaseMAC.String(), macOwnerID, networkName, rerr) + } else { + klog.V(5).Infof("Released MAC %q on rollback, owner: %q, network: %q", releaseMAC.String(), macOwnerID, networkName) + } + releaseMAC = nil + } + if len(releaseIPs) == 0 { return } @@ -436,6 +472,21 @@ func allocatePodAnnotationWithRollback( if err != nil { return } + if macRegistry != nil { + if rerr := macRegistry.Reserve(macOwnerID, tentative.MAC); rerr != nil { + // repeated requests are no-op because mac already reserved + if !errors.Is(rerr, mac.ErrMACReserved) { + // avoid leaking the network name because this error may reflect of a pod event, which is visible to non-admins. + err = fmt.Errorf("failed to reserve MAC address %q for owner %q on network attachment %q: %w", + tentative.MAC, macOwnerID, nadName, rerr) + klog.Errorf("%v, network-name: %q", err, networkName) + return + } + } else { + klog.V(5).Infof("Reserved MAC %q for owner %q on network %q nad %q", tentative.MAC, macOwnerID, networkName, nadName) + releaseMAC = tentative.MAC + } + } // handle routes & gateways err = AddRoutesGatewayIP(netInfo, node, pod, tentative, network) diff --git a/go-controller/pkg/allocator/pod/pod_annotation_test.go b/go-controller/pkg/allocator/pod/pod_annotation_test.go index f8044b62fa..d2719bd8c1 100644 --- a/go-controller/pkg/allocator/pod/pod_annotation_test.go +++ b/go-controller/pkg/allocator/pod/pod_annotation_test.go @@ -18,6 +18,7 @@ import ( "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/id" ipam "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/ip" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/ip/subnet" + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/mac" ovncnitypes "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/cni/types" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/persistentips" @@ -89,6 +90,22 @@ func ipamClaimKey(namespace string, claimName string) string { return fmt.Sprintf("%s/%s", namespace, claimName) } +type macRegistryStub struct { + reserveErr error + releaseMAC net.HardwareAddr + reservedMAC net.HardwareAddr +} + +func (m *macRegistryStub) Reserve(_ string, mac net.HardwareAddr) error { + m.reservedMAC = mac + return m.reserveErr +} + +func (m *macRegistryStub) Release(_ string, mac net.HardwareAddr) error { + m.releaseMAC = mac + return nil +} + func Test_allocatePodAnnotationWithRollback(t *testing.T) { randomMac, err := util.GenerateRandMAC() if err != nil { @@ -104,6 +121,7 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) { type args struct { ipAllocator subnet.NamedAllocator idAllocator id.NamedAllocator + macRegistry *macRegistryStub network *nadapi.NetworkSelectionElement ipamClaim *ipamclaimsapi.IPAMClaim reallocate bool @@ -125,6 +143,8 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) { wantPodAnnotation *util.PodAnnotation wantReleasedIPs []*net.IPNet wantReleasedIPsOnRollback []*net.IPNet + wantReservedMAC net.HardwareAddr + wantReleaseMACOnRollback net.HardwareAddr wantReleaseID bool wantRelasedIDOnRollback bool wantErr bool @@ -1015,6 +1035,21 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) { }, wantErr: true, // Should fail because ErrAllocated is not skipped }, + { + // In a scenario of VM migration multiple pods using the same network configuration including the MAC address. + // When the migration destination pod is created, the pod-allocator should relax ErrMACReserved error + // to allow the migration destination pod use the same MAC as the migration source pod, for the migration to succeed. + name: "macRegistry should not release already reserved MAC on rollback", + args: args{ + network: &nadapi.NetworkSelectionElement{MacRequest: requestedMAC}, + macRegistry: &macRegistryStub{reserveErr: mac.ErrMACReserved}, + }, + wantPodAnnotation: &util.PodAnnotation{ + MAC: requestedMACParsed, + }, + wantReservedMAC: requestedMACParsed, + wantReleaseMACOnRollback: nil, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1033,6 +1068,13 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) { config.OVNKubernetesFeature.EnableMultiNetwork = !tt.multiNetworkDisabled config.OVNKubernetesFeature.EnableNetworkSegmentation = true config.OVNKubernetesFeature.EnablePreconfiguredUDNAddresses = tt.enablePreconfiguredUDNAddresses + + var macRegistry mac.Register + macRegistry = mac.NewManager() + if tt.args.macRegistry != nil { + macRegistry = tt.args.macRegistry + } + config.IPv4Mode = true if tt.isSingleStackIPv6 { config.IPv4Mode = false @@ -1117,6 +1159,7 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) { pod, network, claimsReconciler, + macRegistry, tt.args.reallocate, tt.role, ) @@ -1133,6 +1176,12 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) { tt.args.idAllocator.(*idAllocatorStub).releasedID = false } + if tt.args.macRegistry != nil { + reservedMAC := tt.args.macRegistry.reservedMAC + g.Expect(reservedMAC).To(gomega.Equal(tt.wantReservedMAC), "Reserve MAC on error behaved unexpectedly") + tt.args.macRegistry.reservedMAC = nil + } + rollback() if tt.args.ipAllocator != nil { @@ -1145,6 +1194,12 @@ func Test_allocatePodAnnotationWithRollback(t *testing.T) { g.Expect(releasedID).To(gomega.Equal(tt.wantRelasedIDOnRollback), "Release ID on rollback behaved unexpectedly") } + if tt.args.macRegistry != nil { + releaseMAC := tt.args.macRegistry.releaseMAC + g.Expect(releaseMAC).To(gomega.Equal(tt.wantReleaseMACOnRollback), "Release MAC on rollback behaved unexpectedly") + tt.args.macRegistry.releaseMAC = nil + } + if tt.wantErr { // check the expected error after we have checked above that the // rollback has behaved as expected diff --git a/go-controller/pkg/clustermanager/network_cluster_controller.go b/go-controller/pkg/clustermanager/network_cluster_controller.go index 219fb6d9d9..4391fc7e32 100644 --- a/go-controller/pkg/clustermanager/network_cluster_controller.go +++ b/go-controller/pkg/clustermanager/network_cluster_controller.go @@ -21,6 +21,7 @@ import ( "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/id" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/ip/subnet" + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/mac" annotationalloc "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/pod" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/clustermanager/node" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/clustermanager/pod" @@ -229,7 +230,8 @@ func (ncc *networkClusterController) init() error { ipamClaimsReconciler persistentips.PersistentAllocations ) - if ncc.allowPersistentIPs() { + persistentIPsEnabled := ncc.allowPersistentIPs() + if persistentIPsEnabled { ncc.retryIPAMClaims = ncc.newRetryFramework(factory.IPAMClaimsType, true) ncc.ipamClaimReconciler = persistentips.NewIPAMClaimReconciler( ncc.kube, @@ -239,11 +241,20 @@ func (ncc *networkClusterController) init() error { ipamClaimsReconciler = ncc.ipamClaimReconciler } + var podAllocOpts []annotationalloc.AllocatorOption + if util.IsPreconfiguredUDNAddressesEnabled() && + ncc.IsPrimaryNetwork() && + persistentIPsEnabled && + ncc.TopologyType() == types.Layer2Topology { + podAllocOpts = append(podAllocOpts, annotationalloc.WithMACRegistry(mac.NewManager())) + } + podAllocationAnnotator = annotationalloc.NewPodAnnotationAllocator( ncc.GetNetInfo(), ncc.watchFactory.PodCoreInformer().Lister(), ncc.kube, ipamClaimsReconciler, + podAllocOpts..., ) ncc.podAllocator = pod.NewPodAllocator( diff --git a/go-controller/pkg/clustermanager/pod/allocator_test.go b/go-controller/pkg/clustermanager/pod/allocator_test.go index 51bab90fc5..c6b3bc250c 100644 --- a/go-controller/pkg/clustermanager/pod/allocator_test.go +++ b/go-controller/pkg/clustermanager/pod/allocator_test.go @@ -3,6 +3,7 @@ package pod import ( "context" "encoding/json" + "errors" "fmt" "net" "sync" @@ -26,6 +27,7 @@ import ( "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/id" ipallocator "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/ip" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/ip/subnet" + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/mac" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/pod" ovncnitypes "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/cni/types" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config" @@ -43,6 +45,7 @@ type testPod struct { hostNetwork bool completed bool network *nadapi.NetworkSelectionElement + labels map[string]string } func (p testPod) getPod(t *testing.T) *corev1.Pod { @@ -53,6 +56,7 @@ func (p testPod) getPod(t *testing.T) *corev1.Pod { UID: apitypes.UID("pod"), Namespace: "namespace", Annotations: map[string]string{}, + Labels: p.labels, }, Spec: corev1.PodSpec{ HostNetwork: p.hostNetwork, @@ -169,6 +173,22 @@ func (nas *namedAllocatorStub) ReleaseIPs([]*net.IPNet) error { return nil } +type macRegistryStub struct { + reservedMAC net.HardwareAddr + ownerID string + reserveErr, releaseErr error +} + +func (m *macRegistryStub) Reserve(owner string, mac net.HardwareAddr) error { + m.ownerID = owner + m.reservedMAC = mac + return m.reserveErr +} +func (m *macRegistryStub) Release(owner string, _ net.HardwareAddr) error { + m.ownerID = owner + return m.releaseErr +} + func TestPodAllocator_reconcileForNAD(t *testing.T) { type args struct { old *testPod @@ -178,20 +198,23 @@ func TestPodAllocator_reconcileForNAD(t *testing.T) { release bool } tests := []struct { - name string - args args - ipam bool - idAllocation bool - tracked bool - role string - expectAllocate bool - expectIPRelease bool - expectIDRelease bool - expectTracked bool - fullIPPool bool - expectEvents []string - expectError string - podAnnotation *util.PodAnnotation + name string + args args + ipam bool + idAllocation bool + macRegistry *macRegistryStub + tracked bool + role string + expectAllocate bool + expectIPRelease bool + expectIDRelease bool + expectMACReserve *net.HardwareAddr + expectMACOwnerID string + expectTracked bool + fullIPPool bool + expectEvents []string + expectError string + podAnnotation *util.PodAnnotation }{ { name: "Pod not scheduled", @@ -541,6 +564,73 @@ func TestPodAllocator_reconcileForNAD(t *testing.T) { expectEvents: []string{"Warning ErrorAllocatingPod failed to update pod namespace/pod: failed to ensure requested or annotated IPs [10.1.130.0/24] for namespace/nad/namespace/pod: subnet address pool exhausted"}, expectError: "failed to update pod namespace/pod: failed to ensure requested or annotated IPs [10.1.130.0/24] for namespace/nad/namespace/pod: subnet address pool exhausted", }, + + // podAllocator's macRegistry record mac on pod creation + { + name: "macRegistry should record pod's MAC", + macRegistry: &macRegistryStub{}, + args: args{ + new: &testPod{ + scheduled: true, + // use predictable MAC address for testing. + network: &nadapi.NetworkSelectionElement{Namespace: "namespace", Name: "nad", MacRequest: "0a:0a:0a:0a:0a:0a"}, + }, + }, + expectMACReserve: &net.HardwareAddr{0x0a, 0x0a, 0x0a, 0x0a, 0x0a, 0x0a}, + expectAllocate: true, + }, + { + name: "should fail when macRegistry fail to reserve pod's MAC", + macRegistry: &macRegistryStub{reserveErr: errors.New("test reserve failure")}, + args: args{ + new: &testPod{ + scheduled: true, + // use predictable MAC address for testing. + network: &nadapi.NetworkSelectionElement{Namespace: "namespace", Name: "nad", MacRequest: "0a:0a:0a:0a:0a:0a"}, + }, + }, + expectError: `failed to update pod namespace/pod: failed to reserve MAC address "0a:0a:0a:0a:0a:0a" for owner "namespace/pod" on network attachment "namespace/nad": test reserve failure`, + }, + { + name: "should NOT fail when macRegistry gets repeated reserve requests (same mac and owner)", + macRegistry: &macRegistryStub{reserveErr: mac.ErrMACReserved}, + args: args{ + new: &testPod{ + scheduled: true, + network: &nadapi.NetworkSelectionElement{Namespace: "namespace", Name: "nad"}, + }, + }, + expectAllocate: true, + }, + // podAllocator compose MAC owner IDs as expected + { + name: "should compose MAC owner ID from pod.namespace and pod.name", + macRegistry: &macRegistryStub{}, + args: args{ + new: &testPod{ + network: &nadapi.NetworkSelectionElement{Namespace: "namespace", Name: "nad"}, + scheduled: true, + }, + }, + expectMACOwnerID: "namespace/pod", + expectAllocate: true, + }, + { + // In a scenario of VM migration, migration destination and source pods use the same network configuration, + // including MAC address. Given VM pods, composing the owner ID from the VM name relaxes MAC conflict errors, + // when VM is migrated (where migration source and destination pods share the same MAC). + name: "Given pod with VM label, should compose MAC owner ID from pod.namespace and VM label", + expectMACOwnerID: "namespace/myvm", + macRegistry: &macRegistryStub{}, + args: args{ + new: &testPod{ + network: &nadapi.NetworkSelectionElement{Namespace: "namespace", Name: "nad"}, + scheduled: true, + labels: map[string]string{"vm.kubevirt.io/name": "myvm"}, + }, + }, + expectAllocate: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -606,11 +696,16 @@ func TestPodAllocator_reconcileForNAD(t *testing.T) { }) } + var opts []pod.AllocatorOption + if tt.macRegistry != nil { + opts = append(opts, pod.WithMACRegistry(tt.macRegistry)) + } podAnnotationAllocator := pod.NewPodAnnotationAllocator( netInfo, podListerMock, kubeMock, ipamClaimsReconciler, + opts..., ) testNs := "namespace" @@ -666,9 +761,17 @@ func TestPodAllocator_reconcileForNAD(t *testing.T) { } if tt.podAnnotation != nil { - new.Annotations, err = util.MarshalPodAnnotation(new.Annotations, tt.podAnnotation, "namespace/nad") - if err != nil { - t.Fatalf("failed to set pod annotations: %v", err) + if new != nil { + new.Annotations, err = util.MarshalPodAnnotation(new.Annotations, tt.podAnnotation, "namespace/nad") + if err != nil { + t.Fatalf("failed to set pod annotations: %v", err) + } + } + if old != nil { + old.Annotations, err = util.MarshalPodAnnotation(old.Annotations, tt.podAnnotation, "namespace/nad") + if err != nil { + t.Fatalf("failed to set pod annotations: %v", err) + } } } @@ -694,6 +797,12 @@ func TestPodAllocator_reconcileForNAD(t *testing.T) { if tt.expectTracked != a.releasedPods["namespace/nad"].Has("pod") { t.Errorf("expected pod tracked to be %v but it was %v", tt.expectTracked, a.releasedPods["namespace/nad"].Has("pod")) } + if tt.expectMACReserve != nil && tt.macRegistry.reservedMAC.String() != tt.expectMACReserve.String() { + t.Errorf("expected pod MAC reserved to be %v but it was %v", tt.expectMACReserve, tt.macRegistry.reservedMAC) + } + if tt.expectMACOwnerID != "" && tt.expectMACOwnerID != tt.macRegistry.ownerID { + t.Errorf("expected pod MAC owner ID to be %v but it was %v", tt.expectMACOwnerID, tt.macRegistry.ownerID) + } var obtainedEvents []string for { From 0b38dd8a638f1f38eeba1716482cd34ddac564e4 Mon Sep 17 00:00:00 2001 From: Or Mergi Date: Mon, 18 Aug 2025 16:35:15 +0300 Subject: [PATCH 06/15] clustermanager,allocator: Emit pod event reflecting MAC conflicts Emit pod event when MAC conflict is detected during pod allocation process. Avoid user-defined network name leak to pod events, as they are visible by non cluster-admin users. Signed-off-by: Or Mergi --- go-controller/pkg/clustermanager/pod/allocator.go | 4 +++- .../pkg/clustermanager/pod/allocator_test.go | 13 +++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/go-controller/pkg/clustermanager/pod/allocator.go b/go-controller/pkg/clustermanager/pod/allocator.go index bd9c28c956..44e37b9bd7 100644 --- a/go-controller/pkg/clustermanager/pod/allocator.go +++ b/go-controller/pkg/clustermanager/pod/allocator.go @@ -19,6 +19,7 @@ import ( "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/id" ipallocator "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/ip" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/ip/subnet" + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/mac" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/pod" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/networkmanager" @@ -365,7 +366,8 @@ func (a *PodAllocator) allocatePodOnNAD(pod *corev1.Pod, nad string, network *ne if err != nil { if errors.Is(err, ipallocator.ErrFull) || - errors.Is(err, ipallocator.ErrAllocated) { + errors.Is(err, ipallocator.ErrAllocated) || + errors.Is(err, mac.ErrReserveMACConflict) { a.recordPodErrorEvent(pod, err) } return err diff --git a/go-controller/pkg/clustermanager/pod/allocator_test.go b/go-controller/pkg/clustermanager/pod/allocator_test.go index c6b3bc250c..a5bb603838 100644 --- a/go-controller/pkg/clustermanager/pod/allocator_test.go +++ b/go-controller/pkg/clustermanager/pod/allocator_test.go @@ -591,6 +591,19 @@ func TestPodAllocator_reconcileForNAD(t *testing.T) { }, expectError: `failed to update pod namespace/pod: failed to reserve MAC address "0a:0a:0a:0a:0a:0a" for owner "namespace/pod" on network attachment "namespace/nad": test reserve failure`, }, + { + name: "should emit pod event when macRegistry fail to reserve pod's MAC due to MAC conflict", + macRegistry: &macRegistryStub{reserveErr: mac.ErrReserveMACConflict}, + args: args{ + new: &testPod{ + scheduled: true, + // use predictable MAC address for testing. + network: &nadapi.NetworkSelectionElement{Namespace: "namespace", Name: "nad", MacRequest: "0a:0a:0a:0a:0a:0a"}, + }, + }, + expectError: `failed to update pod namespace/pod: failed to reserve MAC address "0a:0a:0a:0a:0a:0a" for owner "namespace/pod" on network attachment "namespace/nad": MAC address already in use`, + expectEvents: []string{`Warning ErrorAllocatingPod failed to update pod namespace/pod: failed to reserve MAC address "0a:0a:0a:0a:0a:0a" for owner "namespace/pod" on network attachment "namespace/nad": MAC address already in use`}, + }, { name: "should NOT fail when macRegistry gets repeated reserve requests (same mac and owner)", macRegistry: &macRegistryStub{reserveErr: mac.ErrMACReserved}, From b6965d943007c04d66d5727a15ee166e04009c7e Mon Sep 17 00:00:00 2001 From: Ram Lavi Date: Wed, 16 Jul 2025 14:53:49 +0300 Subject: [PATCH 07/15] clustermanager,allocator: Release reserved MAC on pod deletion On pod deletion, remove the MAC address used by the pod from the MAC manager store. To allow VM migration scenario, do not release the MAC when there is at least one VM pod that is not in complete state. Resolve the VM pod owner-id by composing the owner-id from the associated VM name. Initializing the MAC manager on start up will be done in follow-up commits. Signed-off-by: Ram Lavi Co-authored-by: Or Mergi --- go-controller/pkg/allocator/pod/macs.go | 42 ++++ .../pkg/clustermanager/pod/allocator.go | 8 + .../pkg/clustermanager/pod/allocator_test.go | 207 ++++++++++++++++-- go-controller/pkg/kubevirt/pod.go | 28 ++- 4 files changed, 256 insertions(+), 29 deletions(-) diff --git a/go-controller/pkg/allocator/pod/macs.go b/go-controller/pkg/allocator/pod/macs.go index 68df0b9dff..1aa23dc486 100644 --- a/go-controller/pkg/allocator/pod/macs.go +++ b/go-controller/pkg/allocator/pod/macs.go @@ -1,11 +1,17 @@ package pod import ( + "errors" "fmt" + "net" kubevirtv1 "kubevirt.io/api/core/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/klog/v2" + + allocmac "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/mac" + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/kubevirt" ) // macOwner compose the owner identifier reserved for MAC addresses management. @@ -19,3 +25,39 @@ func macOwner(pod *corev1.Pod) string { // Default to pod-based identifier return fmt.Sprintf("%s/%s", pod.Namespace, pod.Name) } + +// ReleasePodReservedMacAddress releases pod's reserved MAC address, if exists. +// It removes the used MAC address, from pod network annotation, and remove it from the MAC manager store. +func (allocator *PodAnnotationAllocator) ReleasePodReservedMacAddress(pod *corev1.Pod, mac net.HardwareAddr) error { + networkName := allocator.netInfo.GetNetworkName() + if allocator.macRegistry == nil { + klog.V(5).Infof("No MAC registry defined for network %q, skipping MAC address release", networkName) + return nil + } + + macOwnerID := macOwner(pod) + if vmKey := kubevirt.ExtractVMNameFromPod(pod); vmKey != nil { + allVMPodsCompleted, err := kubevirt.AllVMPodsAreCompleted(allocator.podLister, pod) + if err != nil { + return fmt.Errorf("failed checking all VM %q pods are completed: %v", vmKey, err) + } + if !allVMPodsCompleted { + klog.V(5).Infof(`Retaining MAC address %q for owner %q on network %q because its in use by another VM pod`, + mac, macOwnerID, networkName) + return nil + } + } + + if err := allocator.macRegistry.Release(macOwnerID, mac); err != nil { + if errors.Is(err, allocmac.ErrReleaseMismatchOwner) { + // the given pod is not the original MAC owner thus there is no point to retry. avoid retries by not returning an error. + klog.Errorf(`Failed to release MAC %q for owner %q on network %q, because its originally reserved for different owner`, mac, macOwnerID, networkName) + } else { + return fmt.Errorf("failed to release MAC address %q for owner %q on network %q: %v", mac, macOwnerID, networkName, err) + } + } else { + klog.V(5).Infof("Released MAC %q owned by %q on network %q", mac, macOwnerID, networkName) + } + + return nil +} diff --git a/go-controller/pkg/clustermanager/pod/allocator.go b/go-controller/pkg/clustermanager/pod/allocator.go index 44e37b9bd7..e061532ad6 100644 --- a/go-controller/pkg/clustermanager/pod/allocator.go +++ b/go-controller/pkg/clustermanager/pod/allocator.go @@ -275,6 +275,7 @@ func (a *PodAllocator) releasePodOnNAD(pod *corev1.Pod, nad string, network *net hasIPAMClaim = false } if hasIPAMClaim { + var err error ipamClaim, err := a.ipamClaimsReconciler.FindIPAMClaim(network.IPAMClaimReference, network.Namespace) hasIPAMClaim = ipamClaim != nil && len(ipamClaim.Status.IPs) > 0 if apierrors.IsNotFound(err) { @@ -316,6 +317,13 @@ func (a *PodAllocator) releasePodOnNAD(pod *corev1.Pod, nad string, network *net klog.V(5).Infof("Released IPs %v", util.StringSlice(podAnnotation.IPs)) } + if doRelease { + if err := a.podAnnotationAllocator.ReleasePodReservedMacAddress(pod, podAnnotation.MAC); err != nil { + return fmt.Errorf(`failed to release pod "%s/%s" mac %q: %v`, + pod.Namespace, pod.Name, podAnnotation.MAC, err) + } + } + if podDeleted { a.deleteReleasedPod(nad, string(pod.UID)) } else { diff --git a/go-controller/pkg/clustermanager/pod/allocator_test.go b/go-controller/pkg/clustermanager/pod/allocator_test.go index a5bb603838..72eeb16061 100644 --- a/go-controller/pkg/clustermanager/pod/allocator_test.go +++ b/go-controller/pkg/clustermanager/pod/allocator_test.go @@ -174,9 +174,9 @@ func (nas *namedAllocatorStub) ReleaseIPs([]*net.IPNet) error { } type macRegistryStub struct { - reservedMAC net.HardwareAddr - ownerID string - reserveErr, releaseErr error + reservedMAC, releasedMAC net.HardwareAddr + ownerID string + reserveErr, releaseErr error } func (m *macRegistryStub) Reserve(owner string, mac net.HardwareAddr) error { @@ -184,8 +184,9 @@ func (m *macRegistryStub) Reserve(owner string, mac net.HardwareAddr) error { m.reservedMAC = mac return m.reserveErr } -func (m *macRegistryStub) Release(owner string, _ net.HardwareAddr) error { +func (m *macRegistryStub) Release(owner string, mac net.HardwareAddr) error { m.ownerID = owner + m.releasedMAC = mac return m.releaseErr } @@ -198,23 +199,26 @@ func TestPodAllocator_reconcileForNAD(t *testing.T) { release bool } tests := []struct { - name string - args args - ipam bool - idAllocation bool - macRegistry *macRegistryStub - tracked bool - role string - expectAllocate bool - expectIPRelease bool - expectIDRelease bool - expectMACReserve *net.HardwareAddr - expectMACOwnerID string - expectTracked bool - fullIPPool bool - expectEvents []string - expectError string - podAnnotation *util.PodAnnotation + name string + args args + ipam bool + idAllocation bool + macRegistry *macRegistryStub + tracked bool + role string + expectAllocate bool + expectIPRelease bool + expectIDRelease bool + expectMACReserve *net.HardwareAddr + expectMACRelease *net.HardwareAddr + expectMACOwnerID string + expectTracked bool + fullIPPool bool + expectEvents []string + expectError string + podAnnotation *util.PodAnnotation + newPodCopyRunning bool + podListerErr error }{ { name: "Pod not scheduled", @@ -615,6 +619,113 @@ func TestPodAllocator_reconcileForNAD(t *testing.T) { }, expectAllocate: true, }, + // podAllocator's macRegistry remove mac record on pod complete/deleted + { + name: "Pod completed, macRegistry should release pod's MAC", + ipam: true, + macRegistry: &macRegistryStub{}, + podAnnotation: &util.PodAnnotation{MAC: net.HardwareAddr{0x0a, 0x0a, 0x0a, 0x0a, 0x0a, 0x0a}}, + args: args{ + release: true, + new: &testPod{ + scheduled: true, + completed: true, + network: &nadapi.NetworkSelectionElement{Namespace: "namespace", Name: "nad"}, + }, + }, + expectMACRelease: &net.HardwareAddr{0x0a, 0x0a, 0x0a, 0x0a, 0x0a, 0x0a}, + expectIPRelease: true, + expectTracked: true, + }, + { + name: "Pod completed, has VM label, macRegistry should release pod's MAC", + ipam: true, + macRegistry: &macRegistryStub{}, + podAnnotation: &util.PodAnnotation{MAC: net.HardwareAddr{0x0a, 0x0a, 0x0a, 0x0a, 0x0a, 0x0a}}, + args: args{ + release: true, + new: &testPod{ + scheduled: true, + completed: true, + network: &nadapi.NetworkSelectionElement{Namespace: "namespace", Name: "nad"}, + labels: map[string]string{"vm.kubevirt.io/name": "myvm"}, + }, + }, + expectMACRelease: &net.HardwareAddr{0x0a, 0x0a, 0x0a, 0x0a, 0x0a, 0x0a}, + expectIPRelease: true, + expectTracked: true, + }, + { + name: "Pod completed, should fail when macRegistry fail to release pod MAC", + ipam: true, + macRegistry: &macRegistryStub{releaseErr: errors.New("test release failure")}, + podAnnotation: &util.PodAnnotation{MAC: net.HardwareAddr{0x0a, 0x0a, 0x0a, 0x0a, 0x0a, 0x0a}}, + args: args{ + release: true, + new: &testPod{ + scheduled: true, + completed: true, + // use predictable MAC address for testing. + network: &nadapi.NetworkSelectionElement{Namespace: "namespace", Name: "nad", MacRequest: "0a:0a:0a:0a:0a:0a"}, + }, + }, + expectError: `failed to release pod "namespace/pod" mac "0a:0a:0a:0a:0a:0a": failed to release MAC address "0a:0a:0a:0a:0a:0a" for owner "namespace/pod" on network "": test release failure`, + expectIPRelease: true, + }, + { + // In a scenario of VM migration, migration destination and source pods use the same network configuration, + // including MAC address. The MAC address should not be released as long there is at least one VM pod running. + name: "Pod completed, has VM label, macRegistry should NOT release MAC when not all associated VM pods are in completed state", + ipam: true, + macRegistry: &macRegistryStub{}, + podAnnotation: &util.PodAnnotation{MAC: net.HardwareAddr{0x0a, 0x0a, 0x0a, 0x0a, 0x0a, 0x0a}}, + args: args{ + release: true, + new: &testPod{ + scheduled: true, + completed: true, + network: &nadapi.NetworkSelectionElement{Namespace: "namespace", Name: "nad"}, + labels: map[string]string{"vm.kubevirt.io/name": ""}, + }, + }, + newPodCopyRunning: true, + expectTracked: true, + expectIPRelease: true, + }, + { + name: "Pod completed, has VM label, macRegistry should fail when checking associated VM pods are in complete state", + ipam: true, + macRegistry: &macRegistryStub{}, + podListerErr: errors.New("test error"), + podAnnotation: &util.PodAnnotation{MAC: net.HardwareAddr{0x0a, 0x0a, 0x0a, 0x0a, 0x0a, 0x0a}}, + args: args{ + release: true, + new: &testPod{ + scheduled: true, + completed: true, + network: &nadapi.NetworkSelectionElement{Namespace: "namespace", Name: "nad"}, + labels: map[string]string{"vm.kubevirt.io/name": "myvm"}, + }, + }, + expectError: `failed to release pod "namespace/pod" mac "0a:0a:0a:0a:0a:0a": failed checking all VM "namespace/myvm" pods are completed: failed finding related pods for pod namespace/pod when checking if they are completed: test error`, + expectIPRelease: true, + }, + { + name: "Pod completed, should NOT fail when macRegistry fail to release pod's MAC due to miss-match owner error", + ipam: true, + macRegistry: &macRegistryStub{releaseErr: mac.ErrReleaseMismatchOwner}, + podAnnotation: &util.PodAnnotation{MAC: net.HardwareAddr{0x0a, 0x0a, 0x0a, 0x0a, 0x0a, 0x0a}}, + args: args{ + release: true, + new: &testPod{ + scheduled: true, + completed: true, + network: &nadapi.NetworkSelectionElement{Namespace: "namespace", Name: "nad"}, + }, + }, + expectIPRelease: true, + expectTracked: true, + }, // podAllocator compose MAC owner IDs as expected { name: "should compose MAC owner ID from pod.namespace and pod.name", @@ -628,6 +739,21 @@ func TestPodAllocator_reconcileForNAD(t *testing.T) { expectMACOwnerID: "namespace/pod", expectAllocate: true, }, + { + name: "Pod completed, should compose MAC owner ID from pod.namespace and pod.name", + ipam: true, + macRegistry: &macRegistryStub{}, + args: args{ + release: true, + new: &testPod{ + scheduled: true, completed: true, + network: &nadapi.NetworkSelectionElement{Namespace: "namespace", Name: "nad"}, + }, + }, + expectMACOwnerID: "namespace/pod", + expectTracked: true, + expectIPRelease: true, + }, { // In a scenario of VM migration, migration destination and source pods use the same network configuration, // including MAC address. Given VM pods, composing the owner ID from the VM name relaxes MAC conflict errors, @@ -644,6 +770,25 @@ func TestPodAllocator_reconcileForNAD(t *testing.T) { }, expectAllocate: true, }, + { + // In a scenario of VM migration, migration destination and source pods use the same network configuration, + // including MAC address. Given VM pods, composing the owner ID from the VM name relaxes MAC conflict errors, + // when VM is migrated (where migration source and destination pods share the same MAC). + name: "Pod completed, has VM label, should compose MAC owner ID from pod.namespace and VM label", + ipam: true, + macRegistry: &macRegistryStub{}, + args: args{ + release: true, + new: &testPod{ + scheduled: true, completed: true, + network: &nadapi.NetworkSelectionElement{Namespace: "namespace", Name: "nad"}, + labels: map[string]string{"vm.kubevirt.io/name": "myvm"}, + }, + }, + expectMACOwnerID: "namespace/myvm", + expectTracked: true, + expectIPRelease: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -656,6 +801,11 @@ func TestPodAllocator_reconcileForNAD(t *testing.T) { kubeMock := &kubemocks.InterfaceOVN{} podNamespaceLister := &v1mocks.PodNamespaceLister{} + if tt.podListerErr != nil { + podNamespaceLister.On("List", mock.AnythingOfType("labels.internalSelector")). + Return(nil, tt.podListerErr).Once() + } + podListerMock.On("Pods", mock.AnythingOfType("string")).Return(podNamespaceLister) var allocated bool @@ -761,6 +911,18 @@ func TestPodAllocator_reconcileForNAD(t *testing.T) { if tt.args.new != nil { new = tt.args.new.getPod(t) podNamespaceLister.On("Get", mock.AnythingOfType("string")).Return(new, nil) + + pods := []*corev1.Pod{new} + if tt.newPodCopyRunning { + cp := new.DeepCopy() + cp.Status.Phase = corev1.PodRunning + cp.UID = "copy" + pods = append(pods, cp) + } + if tt.podListerErr == nil { + podNamespaceLister.On("List", mock.AnythingOfType("labels.internalSelector")). + Return(pods, nil).Once() + } } if tt.tracked { @@ -813,6 +975,9 @@ func TestPodAllocator_reconcileForNAD(t *testing.T) { if tt.expectMACReserve != nil && tt.macRegistry.reservedMAC.String() != tt.expectMACReserve.String() { t.Errorf("expected pod MAC reserved to be %v but it was %v", tt.expectMACReserve, tt.macRegistry.reservedMAC) } + if tt.expectMACRelease != nil && tt.expectMACRelease.String() != tt.macRegistry.releasedMAC.String() { + t.Errorf("expected pod MAC released to be %v but it was %v", tt.expectMACRelease, tt.macRegistry.releasedMAC) + } if tt.expectMACOwnerID != "" && tt.expectMACOwnerID != tt.macRegistry.ownerID { t.Errorf("expected pod MAC owner ID to be %v but it was %v", tt.expectMACOwnerID, tt.macRegistry.ownerID) } diff --git a/go-controller/pkg/kubevirt/pod.go b/go-controller/pkg/kubevirt/pod.go index 8cde9d713e..aa6fa5e79a 100644 --- a/go-controller/pkg/kubevirt/pod.go +++ b/go-controller/pkg/kubevirt/pod.go @@ -11,6 +11,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ktypes "k8s.io/apimachinery/pkg/types" + v1 "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/util/retry" libovsdbclient "github.com/ovn-kubernetes/libovsdb/client" @@ -53,14 +54,22 @@ func IsPodLiveMigratable(pod *corev1.Pod) bool { return ok } +// TODO: remove adapter once all findVMRelatedPods usages transition to use PodLister +type listPodsFn func(namespace string, selector metav1.LabelSelector) ([]*corev1.Pod, error) + // findVMRelatedPods will return pods belong to the same vm annotated at pod and // filter out the one at the function argument func findVMRelatedPods(client *factory.WatchFactory, pod *corev1.Pod) ([]*corev1.Pod, error) { + return findVMRelatedPodsWithListerFn(client.GetPodsBySelector, pod) +} + +func findVMRelatedPodsWithListerFn(listPodsFn listPodsFn, pod *corev1.Pod) ([]*corev1.Pod, error) { vmName, ok := pod.Labels[kubevirtv1.VirtualMachineNameLabel] if !ok { return nil, nil } - vmPods, err := client.GetPodsBySelector(pod.Namespace, metav1.LabelSelector{MatchLabels: map[string]string{kubevirtv1.VirtualMachineNameLabel: vmName}}) + vmLabelSelector := metav1.LabelSelector{MatchLabels: map[string]string{kubevirtv1.VirtualMachineNameLabel: vmName}} + vmPods, err := listPodsFn(pod.Namespace, vmLabelSelector) if err != nil { return nil, err } @@ -149,16 +158,19 @@ func EnsurePodAnnotationForVM(watchFactory *factory.WatchFactory, kube *kube.Kub } // AllVMPodsAreCompleted return true if all the vm pods are completed -func AllVMPodsAreCompleted(client *factory.WatchFactory, pod *corev1.Pod) (bool, error) { - if !IsPodLiveMigratable(pod) { - return false, nil - } - +func AllVMPodsAreCompleted(podLister v1.PodLister, pod *corev1.Pod) (bool, error) { if !util.PodCompleted(pod) { return false, nil } - vmPods, err := findVMRelatedPods(client, pod) + f := func(namespace string, selector metav1.LabelSelector) ([]*corev1.Pod, error) { + s, err := metav1.LabelSelectorAsSelector(&selector) + if err != nil { + return nil, err + } + return podLister.Pods(namespace).List(s) + } + vmPods, err := findVMRelatedPodsWithListerFn(f, pod) if err != nil { return false, fmt.Errorf("failed finding related pods for pod %s/%s when checking if they are completed: %v", pod.Namespace, pod.Name, err) } @@ -241,7 +253,7 @@ func CleanUpLiveMigratablePod(nbClient libovsdbclient.Client, watchFactory *fact return nil } - allVMPodsCompleted, err := AllVMPodsAreCompleted(watchFactory, pod) + allVMPodsCompleted, err := AllVMPodsAreCompleted(watchFactory.PodCoreInformer().Lister(), pod) if err != nil { return fmt.Errorf("failed cleaning up VM when checking if pod is leftover: %v", err) } From eb4789b7e375a40524bcb7e11155be4f68c65fe6 Mon Sep 17 00:00:00 2001 From: Or Mergi Date: Mon, 18 Aug 2025 21:12:51 +0300 Subject: [PATCH 08/15] clustermanager,allocator: Init MAC manager with infra MACs Initialize the pod allocator MAC manager MACs of the network GW and management ports, preventing conflicts with new pods requesting those MACs. The MAC manager is instantiated on primary L2 UDNs with persistent IPs enabled, when EnablePreconfiguredUDNAddresses. The network logical switch has GW (.1) and management (.2) ports. Their MAC address is generated from the IP address. Calculate the GW and management MAC addresses from their IP addresses. Signed-off-by: Or Mergi Co-authored-by: Ram Lavi --- go-controller/pkg/allocator/pod/macs.go | 46 +++++++++++++++++++ .../pkg/clustermanager/pod/allocator.go | 5 ++ 2 files changed, 51 insertions(+) diff --git a/go-controller/pkg/allocator/pod/macs.go b/go-controller/pkg/allocator/pod/macs.go index 1aa23dc486..edb3063243 100644 --- a/go-controller/pkg/allocator/pod/macs.go +++ b/go-controller/pkg/allocator/pod/macs.go @@ -9,9 +9,11 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/klog/v2" + k8snet "k8s.io/utils/net" allocmac "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/mac" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/kubevirt" + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util" ) // macOwner compose the owner identifier reserved for MAC addresses management. @@ -61,3 +63,47 @@ func (allocator *PodAnnotationAllocator) ReleasePodReservedMacAddress(pod *corev return nil } + +// InitializeMACRegistry initializes MAC reservation tracker with MAC addresses in use in the network. +func (allocator *PodAnnotationAllocator) InitializeMACRegistry() error { + networkName := allocator.netInfo.GetNetworkName() + if allocator.macRegistry == nil { + klog.V(5).Infof("No MAC registry defined for network %s, skipping initialization", networkName) + return nil + } + + // reserve MACs used by infra first, to prevent network disruptions to connected pods in case of conflict. + infraMACs := calculateSubnetsInfraMACAddresses(allocator.netInfo) + for owner, mac := range infraMACs { + if err := allocator.macRegistry.Reserve(owner, mac); err != nil { + return fmt.Errorf("failed to reserve infra MAC %q for owner %q on network %q: %w", + mac, owner, networkName, err) + } + klog.V(5).Infof("Reserved MAC %q on initialization, for infra %q on network %q", mac, owner, networkName) + } + + return nil +} + +// calculateSubnetsInfraMACAddresses return map of the network infrastructure mac addresses and owner name. +// It calculates the gateway and management ports MAC addresses from their IP address. +func calculateSubnetsInfraMACAddresses(netInfo util.NetInfo) map[string]net.HardwareAddr { + reservedMACs := map[string]net.HardwareAddr{} + for _, subnet := range netInfo.Subnets() { + if subnet.CIDR == nil { + continue + } + + gwIP := netInfo.GetNodeGatewayIP(subnet.CIDR) + gwMAC := util.IPAddrToHWAddr(gwIP.IP) + gwKey := fmt.Sprintf("gw-v%s", k8snet.IPFamilyOf(gwIP.IP)) + reservedMACs[gwKey] = gwMAC + + mgmtIP := netInfo.GetNodeManagementIP(subnet.CIDR) + mgmtMAC := util.IPAddrToHWAddr(mgmtIP.IP) + mgmtKey := fmt.Sprintf("mgmt-v%s", k8snet.IPFamilyOf(mgmtIP.IP)) + reservedMACs[mgmtKey] = mgmtMAC + } + + return reservedMACs +} diff --git a/go-controller/pkg/clustermanager/pod/allocator.go b/go-controller/pkg/clustermanager/pod/allocator.go index e061532ad6..b9f71a045f 100644 --- a/go-controller/pkg/clustermanager/pod/allocator.go +++ b/go-controller/pkg/clustermanager/pod/allocator.go @@ -100,6 +100,11 @@ func (a *PodAllocator) Init() error { ) } + klog.Infof("Initializing network %s pod annotation allocator MAC registry", a.netInfo.GetNetworkName()) + if err := a.podAnnotationAllocator.InitializeMACRegistry(); err != nil { + return fmt.Errorf("failed to initialize MAC addresses registry: %w", err) + } + return nil } From 89d1696a0eb2a6a964e328764c4d38364b361f71 Mon Sep 17 00:00:00 2001 From: Ram Lavi Date: Thu, 10 Jul 2025 16:27:37 +0300 Subject: [PATCH 09/15] clustermanager,allocator: Init MAC manager with pod MACs Initialize the pod-allocator MAC manager with MACs of existing pods in the network. Preventing unexpected conflicts in scenarios where the control-plane restarts. The MAC manager is instantiated on primary L2 UDNs with persistent IPs enabled, when EnablePreconfiguredUDNAddresses. VMs can have multiple associated pods with the same MAC address (migration scenario). Allow VM associated pods have the same MAC, by composing the owner-id from the associated VM name. Signed-off-by: Ram Lavi --- go-controller/pkg/allocator/pod/macs.go | 69 +++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/go-controller/pkg/allocator/pod/macs.go b/go-controller/pkg/allocator/pod/macs.go index edb3063243..8a6ebb4653 100644 --- a/go-controller/pkg/allocator/pod/macs.go +++ b/go-controller/pkg/allocator/pod/macs.go @@ -8,11 +8,13 @@ import ( kubevirtv1 "kubevirt.io/api/core/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/klog/v2" k8snet "k8s.io/utils/net" allocmac "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/mac" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/kubevirt" + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util" ) @@ -72,6 +74,15 @@ func (allocator *PodAnnotationAllocator) InitializeMACRegistry() error { return nil } + pods, err := allocator.fetchNetworkPods() + if err != nil { + return err + } + podMACs, err := indexMACAddrByPodPrimaryUDN(pods) + if err != nil { + return err + } + // reserve MACs used by infra first, to prevent network disruptions to connected pods in case of conflict. infraMACs := calculateSubnetsInfraMACAddresses(allocator.netInfo) for owner, mac := range infraMACs { @@ -81,6 +92,13 @@ func (allocator *PodAnnotationAllocator) InitializeMACRegistry() error { } klog.V(5).Infof("Reserved MAC %q on initialization, for infra %q on network %q", mac, owner, networkName) } + for owner, mac := range podMACs { + if rerr := allocator.macRegistry.Reserve(owner, mac); rerr != nil { + return fmt.Errorf("failed to reserve pod MAC %q for owner %q on network %q: %w", + mac, owner, networkName, rerr) + } + klog.V(5).Infof("Reserved MAC %q on initialization, for pod %q on network %q", mac, owner, networkName) + } return nil } @@ -107,3 +125,54 @@ func calculateSubnetsInfraMACAddresses(netInfo util.NetInfo) map[string]net.Hard return reservedMACs } + +// fetchNetworkPods fetch running pods in to the network NAD namespaces. +func (allocator *PodAnnotationAllocator) fetchNetworkPods() ([]*corev1.Pod, error) { + var netPods []*corev1.Pod + for _, ns := range allocator.netInfo.GetNADNamespaces() { + pods, err := allocator.podLister.Pods(ns).List(labels.Everything()) + if err != nil { + return nil, fmt.Errorf("failed to list pods for namespace %q: %v", ns, err) + } + for _, pod := range pods { + if pod == nil { + continue + } + if !util.PodRunning(pod) { + continue + } + // Check if pod is being deleted and has no finalizers (about to be disposed) + if util.PodTerminating(pod) && len(pod.Finalizers) == 0 { + continue + } + netPods = append(netPods, pod) + } + } + return netPods, nil +} + +// indexMACAddrByPodPrimaryUDN indexes the MAC address of the primary UDN for each pod. +// It returns a map where keys are the owner ID (composed by macOwner, e.g., "namespace/pod-name") +// and the values are the corresponding MAC addresses. +func indexMACAddrByPodPrimaryUDN(pods []*corev1.Pod) (map[string]net.HardwareAddr, error) { + indexedMACs := map[string]net.HardwareAddr{} + for _, pod := range pods { + podNetworks, err := util.UnmarshalPodAnnotationAllNetworks(pod.Annotations) + if err != nil { + return nil, fmt.Errorf(`failed to unmarshal pod-network annotation "%s/%s": %v`, pod.Namespace, pod.Name, err) + } + for _, network := range podNetworks { + if network.Role != types.NetworkRolePrimary { + // filter out default network and secondary user-defined networks + continue + } + mac, perr := net.ParseMAC(network.MAC) + if perr != nil { + return nil, fmt.Errorf(`failed to parse mac address "%s/%s": %v`, pod.Namespace, pod.Name, perr) + } + indexedMACs[macOwner(pod)] = mac + } + } + + return indexedMACs, nil +} From f46a21c6d5595f6354b74842872abeb7be134279 Mon Sep 17 00:00:00 2001 From: Or Mergi Date: Mon, 18 Aug 2025 11:12:19 +0300 Subject: [PATCH 10/15] e2e,net-seg default-net annot: Test MAC conflict detection Signed-off-by: Or Mergi --- ...segmentation_default_network_annotation.go | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/e2e/network_segmentation_default_network_annotation.go b/test/e2e/network_segmentation_default_network_annotation.go index 4119c03f7e..16280c8e2a 100644 --- a/test/e2e/network_segmentation_default_network_annotation.go +++ b/test/e2e/network_segmentation_default_network_annotation.go @@ -11,7 +11,9 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" e2epod "k8s.io/kubernetes/test/e2e/framework/pod" udnv1 "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/crd/userdefinednetwork/v1" @@ -97,6 +99,33 @@ var _ = Describe("Network Segmentation: Default network multus annotation", feat Expect(netStatus[0].IPs).To(ConsistOf(exposedIPs), "Should have the IPs specified in the default network annotation") Expect(strings.ToLower(netStatus[0].Mac)).To(Equal(strings.ToLower(tc.mac)), "Should have the MAC specified in the default network annotation") + By("Create second pod with default network annotation requesting the same MAC request") + pod2 := e2epod.NewAgnhostPod(f.Namespace.Name, "pod-mac-conflict", nil, nil, nil) + pod2.Annotations = map[string]string{"v1.multus-cni.io/default-network": fmt.Sprintf(`[{"name":"default", "namespace":"ovn-kubernetes", "mac":%q}]`, tc.mac)} + pod2.Spec.Containers[0].Command = []string{"sleep", "infinity"} + pod2, err = f.ClientSet.CoreV1().Pods(f.Namespace.Name).Create(context.Background(), pod2, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + + By("Asserting second pod has event attached reflecting MAC conflict error") + Eventually(func(g Gomega) []corev1.Event { + events, err := f.ClientSet.CoreV1().Events(pod2.Namespace).SearchWithContext(context.Background(), scheme.Scheme, pod2) + g.Expect(err).NotTo(HaveOccurred()) + return events.Items + }).WithTimeout(time.Minute * 1).WithPolling(time.Second * 3).Should(ContainElement(SatisfyAll( + HaveField("Type", "Warning"), + HaveField("Reason", "ErrorAllocatingPod"), + HaveField("Message", ContainSubstring("MAC address already in use")), + ))) + + By("Assert second pod consistently at pending") + Consistently(func(g Gomega) corev1.PodPhase { + pod2Updated, err := f.ClientSet.CoreV1().Pods(f.Namespace.Name).Get(context.Background(), pod2.Name, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + return pod2Updated.Status.Phase + }). + WithTimeout(3 * time.Second). + WithPolling(time.Second). + Should(Equal(corev1.PodPending)) }, Entry("should create the pod with the specified static IP and MAC address", testCase{ From e4835ab88b1157823e47ce60135401871037dcc4 Mon Sep 17 00:00:00 2001 From: Or Mergi Date: Mon, 18 Aug 2025 12:00:40 +0300 Subject: [PATCH 11/15] e2e,kubevirt: Test MAC conflict detection Signed-off-by: Or Mergi --- test/e2e/kubevirt.go | 56 ++++++++++++++++++- ...segmentation_default_network_annotation.go | 8 +-- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/test/e2e/kubevirt.go b/test/e2e/kubevirt.go index c545a1959c..d67ea0e768 100644 --- a/test/e2e/kubevirt.go +++ b/test/e2e/kubevirt.go @@ -41,6 +41,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/util/retry" e2eframework "k8s.io/kubernetes/test/e2e/framework" @@ -2359,7 +2360,7 @@ chpasswd: { expire: False } ) }) - Context("duplicate IP validation", func() { + Context("duplicate addresses validation", func() { var ( cudn *udnv1.ClusterUserDefinedNetwork duplicateIPv4 = "10.128.0.200" // Static IP that will be used by both VMs @@ -2475,5 +2476,58 @@ ethernets: By("Verifying first VM is still running normally") waitForVMReadinessAndVerifyIPs(vm1.Name, staticIPs) }) + + newVMIWithPrimaryIfaceMAC := func(mac string) *kubevirtv1.VirtualMachineInstance { + vm := fedoraWithTestToolingVMI(nil, nil, nil, kubevirtv1.NetworkSource{Pod: &kubevirtv1.PodNetwork{}}, "#", "") + vm.Spec.Domain.Devices.Interfaces[0].Bridge = nil + vm.Spec.Domain.Devices.Interfaces[0].Binding = &kubevirtv1.PluginBinding{Name: "l2bridge"} + vm.Spec.Domain.Devices.Interfaces[0].MacAddress = mac + return vm + } + + It("should fail when creating second VM with duplicate user requested MAC", func() { + const testMAC = "02:a1:b2:c3:d4:e5" + vmi1 := newVMIWithPrimaryIfaceMAC(testMAC) + vm1 := generateVM(vmi1) + createVirtualMachine(vm1) + + By("Asserting VM with static MAC is running as expected") + Eventually(func(g Gomega) []kubevirtv1.VirtualMachineInstanceCondition { + g.Expect(crClient.Get(context.Background(), crclient.ObjectKeyFromObject(vm1), vmi1)).To(Succeed()) + return vmi1.Status.Conditions + }).WithPolling(time.Second).WithTimeout(5 * time.Minute).Should(ContainElement(SatisfyAll( + HaveField("Type", kubevirtv1.VirtualMachineInstanceAgentConnected), + HaveField("Status", corev1.ConditionTrue), + ))) + Expect(crClient.Get(context.Background(), crclient.ObjectKeyFromObject(vm1), vmi1)).To(Succeed()) + Expect(vmi1.Status.Interfaces[0].MAC).To(Equal(testMAC), "vmi status should report the requested mac") + + By("Create second VM requesting the same MAC address") + vmi2 := newVMIWithPrimaryIfaceMAC(testMAC) + vm2 := generateVM(vmi2) + createVirtualMachine(vm2) + + By("Asserting second VM pod has attached event reflecting MAC conflict error") + vm2Selector := fmt.Sprintf("%s=%s", kubevirtv1.VirtualMachineNameLabel, vm2.Name) + Eventually(func(g Gomega) []corev1.Event { + podList, err := fr.ClientSet.CoreV1().Pods(vm2.Namespace).List(context.Background(), metav1.ListOptions{LabelSelector: vm2Selector}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(podList.Items).ToNot(BeEmpty()) + events, err := fr.ClientSet.CoreV1().Events(vm2.Namespace).SearchWithContext(context.Background(), scheme.Scheme, &podList.Items[0]) + g.Expect(err).ToNot(HaveOccurred()) + return events.Items + }).WithTimeout(time.Minute * 1).WithPolling(time.Second * 3).Should(ContainElement(SatisfyAll( + HaveField("Type", "Warning"), + HaveField("Reason", "ErrorAllocatingPod"), + HaveField("Message", ContainSubstring("MAC address already in use")), + ))) + + By("Assert second VM not running") + Expect(crClient.Get(context.Background(), crclient.ObjectKeyFromObject(vm2), vmi2)).To(Succeed()) + Expect(vmi2.Status.Conditions).To(ContainElement(SatisfyAll( + HaveField("Type", kubevirtv1.VirtualMachineInstanceReady), + HaveField("Status", corev1.ConditionFalse), + )), "second VM should not be ready due to MAC conflict") + }) }) }) diff --git a/test/e2e/network_segmentation_default_network_annotation.go b/test/e2e/network_segmentation_default_network_annotation.go index 16280c8e2a..f08229131b 100644 --- a/test/e2e/network_segmentation_default_network_annotation.go +++ b/test/e2e/network_segmentation_default_network_annotation.go @@ -56,11 +56,9 @@ var _ = Describe("Network Segmentation: Default network multus annotation", feat Spec: udnv1.UserDefinedNetworkSpec{ Topology: udnv1.NetworkTopologyLayer2, Layer2: &udnv1.Layer2Config{ - Role: udnv1.NetworkRolePrimary, - Subnets: filterDualStackCIDRs(f.ClientSet, []udnv1.CIDR{ - udnv1.CIDR("103.0.0.0/16"), - udnv1.CIDR("2014:100:200::0/60"), - }), + Role: udnv1.NetworkRolePrimary, + Subnets: filterDualStackCIDRs(f.ClientSet, []udnv1.CIDR{"103.0.0.0/16", "2014:100:200::0/60"}), + IPAM: &udnv1.IPAMConfig{Mode: udnv1.IPAMEnabled, Lifecycle: udnv1.IPAMLifecyclePersistent}, }, }, } From cedfc13eddc117db7b5535615c613b7e51701287 Mon Sep 17 00:00:00 2001 From: Or Mergi Date: Tue, 26 Aug 2025 21:16:20 +0300 Subject: [PATCH 12/15] utils,multinet: GetPodNADToNetworkMappingWithActiveNetwork CUDN support In a scenario of primary CUDN where multiple NAD exist all with the same spec, NetworkInfo.GetNADs return multiple NADs of the selected namespaces. The GetPodNADToNetworkMappingWithActiveNetwork helper, assume the active-network (NetworkInfo{}) consist of single NAD, and return the mapping with the first NAD of the active-network it found. This approach fall short when the given pod is connected to CUDN that span over multiple namespaces, i.e.: active network consist of multiple NADs. The helper return inconsistent mapping where the NAD key doesn't match the pod namespace (NAD of another namespaces). Chagne the helper to find the active-network matching NAD; the NAD that reside at the same namespace as the given pod (matching namespace) Change test to always set an appropriate namespace to the tested pod. Extend the test suite to allow injecting multiple NADs for the active-network, and simulating the CUDN use-case. Signed-off-by: Or Mergi --- .../pkg/ovn/base_network_controller_pods.go | 2 +- go-controller/pkg/ovn/ovn.go | 2 +- go-controller/pkg/util/multi_network.go | 30 ++++++++-- go-controller/pkg/util/multi_network_test.go | 58 ++++++++++++++++++- 4 files changed, 84 insertions(+), 8 deletions(-) diff --git a/go-controller/pkg/ovn/base_network_controller_pods.go b/go-controller/pkg/ovn/base_network_controller_pods.go index 91c092c208..8ac4974c00 100644 --- a/go-controller/pkg/ovn/base_network_controller_pods.go +++ b/go-controller/pkg/ovn/base_network_controller_pods.go @@ -1033,7 +1033,7 @@ func (bnc *BaseNetworkController) allocatesPodAnnotation() bool { func (bnc *BaseNetworkController) shouldReleaseDeletedPod(pod *corev1.Pod, switchName, nad string, podIfAddrs []*net.IPNet) (bool, error) { var err error if !bnc.IsUserDefinedNetwork() && kubevirt.IsPodLiveMigratable(pod) { - allVMPodsAreCompleted, err := kubevirt.AllVMPodsAreCompleted(bnc.watchFactory, pod) + allVMPodsAreCompleted, err := kubevirt.AllVMPodsAreCompleted(bnc.watchFactory.PodCoreInformer().Lister(), pod) if err != nil { return false, err } diff --git a/go-controller/pkg/ovn/ovn.go b/go-controller/pkg/ovn/ovn.go index 280e41eba3..ce227e57ff 100644 --- a/go-controller/pkg/ovn/ovn.go +++ b/go-controller/pkg/ovn/ovn.go @@ -341,7 +341,7 @@ func (oc *DefaultNetworkController) removeRemoteZonePod(pod *corev1.Pod) error { // called for migrations. // https://github.com/ovn-kubernetes/ovn-kubernetes/issues/5627 if kubevirt.IsPodLiveMigratable(pod) { - allVMPodsAreCompleted, err := kubevirt.AllVMPodsAreCompleted(oc.watchFactory, pod) + allVMPodsAreCompleted, err := kubevirt.AllVMPodsAreCompleted(oc.watchFactory.PodCoreInformer().Lister(), pod) if err != nil { return err } diff --git a/go-controller/pkg/util/multi_network.go b/go-controller/pkg/util/multi_network.go index 6a8075f2b9..cf9eb7e6b5 100644 --- a/go-controller/pkg/util/multi_network.go +++ b/go-controller/pkg/util/multi_network.go @@ -16,6 +16,7 @@ import ( "golang.org/x/exp/maps" corev1 "k8s.io/api/core/v1" + k8sapitypes "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" knet "k8s.io/utils/net" @@ -1495,14 +1496,19 @@ func GetPodNADToNetworkMappingWithActiveNetwork(pod *corev1.Pod, nInfo NetInfo, if len(activeNetworkNADs) < 1 { return false, nil, fmt.Errorf("missing NADs at active network %q for namespace %q", activeNetwork.GetNetworkName(), pod.Namespace) } - activeNetworkNADKey := strings.Split(activeNetworkNADs[0], "/") + + activeNADKey := getNADWithNamespace(activeNetworkNADs, pod.Namespace) + if activeNADKey == nil { + return false, nil, fmt.Errorf("no active NAD found for namespace %q", pod.Namespace) + } + if len(networkSelections) == 0 { networkSelections = map[string]*nettypes.NetworkSelectionElement{} } activeNSE := &nettypes.NetworkSelectionElement{ - Namespace: activeNetworkNADKey[0], - Name: activeNetworkNADKey[1], + Namespace: activeNADKey.Namespace, + Name: activeNADKey.Name, } // Feature gate integration: EnablePreconfiguredUDNAddresses controls default network IP/MAC transfer to active network @@ -1531,10 +1537,26 @@ func GetPodNADToNetworkMappingWithActiveNetwork(pod *corev1.Pod, nInfo NetInfo, } } - networkSelections[activeNetworkNADs[0]] = activeNSE + networkSelections[activeNADKey.String()] = activeNSE return true, networkSelections, nil } +// getNADWithNamespace returns the first occurrence of NAD key with the given namespace name. +func getNADWithNamespace(nads []string, targetNamespace string) *k8sapitypes.NamespacedName { + for _, nad := range nads { + nsName := strings.Split(nad, "/") + if len(nsName) != 2 { + continue + } + ns, name := nsName[0], nsName[1] + if ns != targetNamespace { + continue + } + return &k8sapitypes.NamespacedName{Namespace: ns, Name: name} + } + return nil +} + func IsMultiNetworkPoliciesSupportEnabled() bool { return config.OVNKubernetesFeature.EnableMultiNetwork && config.OVNKubernetesFeature.EnableMultiNetworkPolicy } diff --git a/go-controller/pkg/util/multi_network_test.go b/go-controller/pkg/util/multi_network_test.go index 5fee0f9c42..1219669b55 100644 --- a/go-controller/pkg/util/multi_network_test.go +++ b/go-controller/pkg/util/multi_network_test.go @@ -1022,6 +1022,7 @@ func TestGetPodNADToNetworkMappingWithActiveNetwork(t *testing.T) { expectedIsAttachmentRequested bool expectedNetworkSelectionElements map[string]*nadv1.NetworkSelectionElement enablePreconfiguredUDNAddresses bool + injectPrimaryUDNNADs []string } tests := []testConfig{ @@ -1241,6 +1242,53 @@ func TestGetPodNADToNetworkMappingWithActiveNetwork(t *testing.T) { enablePreconfiguredUDNAddresses: true, expectedError: fmt.Errorf(`unexpected default NSE name "unexpected-name", expected "default"`), }, + { + desc: "should fail when no nad of the active network found on the pod namespace", + inputNamespace: "non-existent-ns", + expectedError: fmt.Errorf(`no active NAD found for namespace "non-existent-ns"`), + inputNetConf: &ovncnitypes.NetConf{ + NetConf: cnitypes.NetConf{Name: networkName}, + NADName: GetNADName(namespaceName, attachmentName), + Topology: ovntypes.Layer2Topology, + Role: ovntypes.NetworkRolePrimary, + }, + inputPrimaryUDNConfig: &ovncnitypes.NetConf{ + NetConf: cnitypes.NetConf{Name: networkName}, + Topology: ovntypes.Layer2Topology, + NADName: GetNADName(namespaceName, attachmentName), + Role: ovntypes.NetworkRolePrimary, + }, + }, + { + desc: "primary l2 CUDN (replicated NADs), should return the correct active network according to pod namespace", + inputNetConf: &ovncnitypes.NetConf{ + NetConf: cnitypes.NetConf{Name: "cluster_udn_l2p"}, + NADName: GetNADName("red", "l2p"), + Topology: ovntypes.Layer2Topology, + Role: ovntypes.NetworkRolePrimary, + }, + inputPrimaryUDNConfig: &ovncnitypes.NetConf{ + NetConf: cnitypes.NetConf{Name: "cluster_udn_l2p"}, + NADName: GetNADName("red", "l2p"), + Topology: ovntypes.Layer2Topology, + Role: ovntypes.NetworkRolePrimary, + }, + injectPrimaryUDNNADs: []string{"blue/l2p", "green/l2p"}, + inputNamespace: "blue", + inputPodAnnotations: map[string]string{ + DefNetworkAnnotation: `[{"namespace": "ovn-kubernetes", "name": "default", "ips": ["192.168.0.3/24", "fda6::3/48"], "mac": "aa:bb:cc:dd:ee:ff"}]`, + }, + enablePreconfiguredUDNAddresses: true, + expectedIsAttachmentRequested: true, + expectedNetworkSelectionElements: map[string]*nadv1.NetworkSelectionElement{ + "blue/l2p": { + Name: "l2p", + Namespace: "blue", + IPRequest: []string{"192.168.0.3/24", "fda6::3/48"}, + MacRequest: "aa:bb:cc:dd:ee:ff", + }, + }, + }, { desc: "default-network ips and mac is is ignored for Layer3 topology", @@ -1290,7 +1338,7 @@ func TestGetPodNADToNetworkMappingWithActiveNetwork(t *testing.T) { DefNetworkAnnotation: `[{"foo}`, }, enablePreconfiguredUDNAddresses: true, - expectedError: fmt.Errorf(`failed getting default-network annotation for pod "/test-pod": %w`, fmt.Errorf(`GetK8sPodDefaultNetwork: failed to parse CRD object: parsePodNetworkAnnotation: failed to parse pod Network Attachment Selection Annotation JSON format: unexpected end of JSON input`)), + expectedError: fmt.Errorf(`failed getting default-network annotation for pod "ns1/test-pod": %w`, fmt.Errorf(`GetK8sPodDefaultNetwork: failed to parse CRD object: parsePodNetworkAnnotation: failed to parse pod Network Attachment Selection Annotation JSON format: unexpected end of JSON input`)), }, } for _, test := range tests { @@ -1323,6 +1371,9 @@ func TestGetPodNADToNetworkMappingWithActiveNetwork(t *testing.T) { if test.inputPrimaryUDNConfig.NADName != "" { mutableNetInfo := NewMutableNetInfo(primaryUDNNetInfo) mutableNetInfo.AddNADs(test.inputPrimaryUDNConfig.NADName) + if len(test.injectPrimaryUDNNADs) > 0 { + mutableNetInfo.AddNADs(test.injectPrimaryUDNNADs...) + } primaryUDNNetInfo = mutableNetInfo } } @@ -1330,10 +1381,13 @@ func TestGetPodNADToNetworkMappingWithActiveNetwork(t *testing.T) { pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "test-pod", - Namespace: test.inputNamespace, + Namespace: namespaceName, Annotations: test.inputPodAnnotations, }, } + if test.inputNamespace != "" { + pod.Namespace = test.inputNamespace + } isAttachmentRequested, networkSelectionElements, err := GetPodNADToNetworkMappingWithActiveNetwork( pod, From 31a760d7739d3078f0e125e147fb6bc63d31fdeb Mon Sep 17 00:00:00 2001 From: Enrique Llorente Date: Mon, 6 Oct 2025 12:02:58 +0200 Subject: [PATCH 13/15] kv, e2e: Add --wait to killall Not waiting for `killall` to terminate can cause the Kubevirt console expecter/matcher to incorrectly match the negative case. This occurs because the "Exit 1" string may prematurely appear in the output. Signed-off-by: Enrique Llorente --- test/e2e/kubevirt.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/kubevirt.go b/test/e2e/kubevirt.go index c545a1959c..d70a0684b3 100644 --- a/test/e2e/kubevirt.go +++ b/test/e2e/kubevirt.go @@ -2330,7 +2330,7 @@ chpasswd: { expire: False } checkEastWestIperfTraffic(vmi, testPodsIPs, step) By("Stop iperf3 traffic before force killing vm, so iperf3 server do not get stuck") - output, err = virtClient.RunCommand(vmi, "killall iperf3", 5*time.Second) + output, err = virtClient.RunCommand(vmi, "killall --wait iperf3", 5*time.Second) Expect(err).ToNot(HaveOccurred(), output) step = by(vmi.Name, fmt.Sprintf("Force kill qemu at node %q where VM is running on", vmi.Status.NodeName)) From ad72f1e5cf991672c18f47c6b4bd8172e194a39a Mon Sep 17 00:00:00 2001 From: Patryk Diak Date: Tue, 14 Oct 2025 15:09:06 +0200 Subject: [PATCH 14/15] Fix GARP sending 0.0.0.0 due to incorrect IPv4 byte extraction net.ParseIP() returns 16-byte IPv4-mapped IPv6 format where IPv4 bytes are at the END, not beginning. [4]byte(garp.IP) took wrong bytes. Fixed by calling To4() before, forcing validated creation via NewGARP(). Interface prevents bypassing extracting the correct IPv4 address. Signed-off-by: Patryk Diak --- go-controller/pkg/kubevirt/pod.go | 7 +- .../node/linkmanager/link_network_manager.go | 9 +- go-controller/pkg/util/arp.go | 61 ++++++++++--- test/e2e/egressip.go | 88 +++++++++++++++++++ 4 files changed, 149 insertions(+), 16 deletions(-) diff --git a/go-controller/pkg/kubevirt/pod.go b/go-controller/pkg/kubevirt/pod.go index aa6fa5e79a..429a8f3beb 100644 --- a/go-controller/pkg/kubevirt/pod.go +++ b/go-controller/pkg/kubevirt/pod.go @@ -540,11 +540,10 @@ func (r *DefaultGatewayReconciler) ReconcileIPv4AfterLiveMigration(liveMigration lrpMAC := util.IPAddrToHWAddr(lrpJoinAddress) for _, subnet := range r.netInfo.Subnets() { - gwIP := r.netInfo.GetNodeGatewayIP(subnet.CIDR).IP.To4() - if gwIP == nil { - continue + garp, err := util.NewGARP(r.netInfo.GetNodeGatewayIP(subnet.CIDR).IP, &lrpMAC) + if err != nil { + return fmt.Errorf("failed to create GARP for gateway IP %s: %w", r.netInfo.GetNodeGatewayIP(subnet.CIDR).IP, err) } - garp := util.GARP{IP: gwIP, MAC: &lrpMAC} if err := util.BroadcastGARP(r.interfaceName, garp); err != nil { return err } diff --git a/go-controller/pkg/node/linkmanager/link_network_manager.go b/go-controller/pkg/node/linkmanager/link_network_manager.go index ce047965e5..d188b854df 100644 --- a/go-controller/pkg/node/linkmanager/link_network_manager.go +++ b/go-controller/pkg/node/linkmanager/link_network_manager.go @@ -196,8 +196,13 @@ func (c *Controller) syncLink(link netlink.Link) error { // For IPv4, use arping to try to update other hosts ARP caches, in case this IP was // previously active on another node if addressWanted.IP.To4() != nil { - if err = util.BroadcastGARP(linkName, util.GARP{IP: addressWanted.IP}); err != nil { - klog.Errorf("Failed to send a GARP for IP %s over interface %s: %v", addressWanted.IP.String(), + garp, err := util.NewGARP(addressWanted.IP, nil) + if err != nil { + klog.Errorf("Link manager: failed to create GARP for IP %s: %v", addressWanted.IP.String(), err) + continue + } + if err = util.BroadcastGARP(linkName, garp); err != nil { + klog.Errorf("Link manager: failed to send GARP for IP %s over interface %s: %v", addressWanted.IP.String(), linkName, err) } } diff --git a/go-controller/pkg/util/arp.go b/go-controller/pkg/util/arp.go index d2205eafeb..c9d74bcedf 100644 --- a/go-controller/pkg/util/arp.go +++ b/go-controller/pkg/util/arp.go @@ -6,13 +6,53 @@ import ( "net/netip" "github.com/mdlayher/arp" + + "k8s.io/klog/v2" ) -type GARP struct { - // IP to advertise the MAC address - IP net.IP - // MAC to advertise (optional), default: link mac address - MAC *net.HardwareAddr +// GARP represents a gratuitous ARP request for an IPv4 address. +type GARP interface { + // IP returns the IPv4 address as a net.IP + IP() net.IP + // IPv4 returns the raw 4-byte IPv4 address + IPv4() [net.IPv4len]byte + // MAC returns the MAC address to advertise (nil means use interface MAC) + MAC() *net.HardwareAddr +} + +// garp is the private implementation of GARP +type garp struct { + ip [4]byte + mac *net.HardwareAddr +} + +// NewGARP creates a new GARP with validation that the IP is IPv4. +// Returns error if the IP is not a valid IPv4 address. +// mac can be nil to use the interface's MAC address. +func NewGARP(ip net.IP, mac *net.HardwareAddr) (GARP, error) { + ip4 := ip.To4() + if ip4 == nil { + return nil, fmt.Errorf("GARP only supports IPv4 addresses, got %s (len=%d bytes)", ip.String(), len(ip)) + } + return &garp{ + ip: [4]byte(ip4), + mac: mac, + }, nil +} + +// IP returns the IPv4 address as a net.IP +func (g *garp) IP() net.IP { + return net.IP(g.ip[:]) +} + +// IPv4 returns the raw 4-byte IPv4 address +func (g *garp) IPv4() [4]byte { + return g.ip +} + +// MAC returns the MAC address to advertise +func (g *garp) MAC() *net.HardwareAddr { + return g.mac } // BroadcastGARP send a pair of GARPs with "request" and "reply" operations @@ -20,15 +60,15 @@ type GARP struct { // If "garp.MAC" is not passed the link form "interfaceName" mac will be // advertise func BroadcastGARP(interfaceName string, garp GARP) error { - srcIP := netip.AddrFrom4([4]byte(garp.IP)) - iface, err := net.InterfaceByName(interfaceName) if err != nil { return fmt.Errorf("failed finding interface %s: %v", interfaceName, err) } - if garp.MAC == nil { - garp.MAC = &iface.HardwareAddr + srcIP := netip.AddrFrom4(garp.IPv4()) + mac := garp.MAC() + if mac == nil { + mac = &iface.HardwareAddr } c, err := arp.Dial(iface) @@ -50,7 +90,7 @@ func BroadcastGARP(interfaceName string, garp GARP) error { for _, op := range []arp.Operation{arp.OperationRequest, arp.OperationReply} { // At at GARP the source and target IP should be the same and point to the // the IP we want to reconcile -> https://wiki.wireshark.org/Gratuitous_ARP - p, err := arp.NewPacket(op, *garp.MAC /* srcHw */, srcIP, net.HardwareAddr{0, 0, 0, 0, 0, 0}, srcIP) + p, err := arp.NewPacket(op, *mac /* srcHw */, srcIP, net.HardwareAddr{0, 0, 0, 0, 0, 0}, srcIP) if err != nil { return fmt.Errorf("failed creating %q GARP %+v: %w", op, garp, err) } @@ -60,5 +100,6 @@ func BroadcastGARP(interfaceName string, garp GARP) error { } } + klog.Infof("BroadcastGARP: completed GARP broadcast for IP %s on interface %s with MAC: %s", garp.IP().String(), interfaceName, mac.String()) return nil } diff --git a/test/e2e/egressip.go b/test/e2e/egressip.go index b0264ee433..1afa18ef0e 100644 --- a/test/e2e/egressip.go +++ b/test/e2e/egressip.go @@ -2957,6 +2957,94 @@ spec: "and verify the expected IP, failed for EgressIP %s: %v", egressIPName, err) }) + ginkgo.It("[secondary-host-eip] should send GARP for EgressIP", func() { + if utilnet.IsIPv6(net.ParseIP(egress1Node.nodeIP)) { + ginkgo.Skip("GARP test only supports IPv4") + } + egressIPSecondaryHost := "10.10.10.220" + + // flush any potentially stale MACs and allow GARPs + _, err := infraprovider.Get().ExecK8NodeCommand(secondaryTargetExternalContainer.Name, + []string{"ip", "neigh", "flush", egressIPSecondaryHost}) + gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should flush neighbor cache") + + _, err = infraprovider.Get().ExecK8NodeCommand(secondaryTargetExternalContainer.Name, + []string{"sysctl", "-w", "net.ipv4.conf.all.arp_accept=1"}) + gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should enable arp_accept") + + podNamespace := f.Namespace + labels := map[string]string{"name": f.Namespace.Name} + updateNamespaceLabels(f, podNamespace, labels) + + ginkgo.By("Labeling node as available for egress") + egressNodeAvailabilityHandler := egressNodeAvailabilityHandlerViaLabel{f} + egressNodeAvailabilityHandler.Enable(egress1Node.name) + defer egressNodeAvailabilityHandler.Restore(egress1Node.name) + + _, err = createGenericPodWithLabel(f, pod1Name, egress1Node.name, f.Namespace.Name, []string{"/agnhost", "pause"}, podEgressLabel) + gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should create egress pod") + + egressIPConfig := `apiVersion: k8s.ovn.org/v1 +kind: EgressIP +metadata: + name: ` + egressIPName + ` +spec: + egressIPs: + - ` + egressIPSecondaryHost + ` + podSelector: + matchLabels: + wants: egress + namespaceSelector: + matchLabels: + name: ` + f.Namespace.Name + ` +` + if err := os.WriteFile(egressIPYaml, []byte(egressIPConfig), 0644); err != nil { + framework.Failf("Unable to write CRD config to disk: %v", err) + } + defer func() { + if err := os.Remove(egressIPYaml); err != nil { + framework.Logf("Unable to remove the CRD config from disk: %v", err) + } + }() + e2ekubectl.RunKubectlOrDie("default", "create", "-f", egressIPYaml) + + status := verifyEgressIPStatusLengthEquals(1, nil) + networks, err := providerCtx.GetAttachedNetworks() + gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should get attached networks") + secondaryNetwork, exists := networks.Get(secondaryNetworkName) + gomega.Expect(exists).Should(gomega.BeTrue(), "network %s must exist", secondaryNetworkName) + inf, err := infraprovider.Get().GetK8NodeNetworkInterface(status[0].Node, secondaryNetwork) + gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should have network interface for network %s on instance %s", secondaryNetwork.Name(), egress1Node.name) + + ginkgo.By("Verifying GARP populated neighbor table") + var neighborMAC string + gomega.Eventually(func() bool { + output, err := infraprovider.Get().ExecK8NodeCommand(secondaryTargetExternalContainer.Name, + []string{"ip", "-j", "neigh", "show", egressIPSecondaryHost}) + if err != nil { + framework.Logf("Failed to get neighbor table: %v", err) + return false + } + + var neighbors []IpNeighbor + if err := json.Unmarshal([]byte(output), &neighbors); err != nil { + framework.Logf("Failed to parse neighbor JSON: %v", err) + return false + } + + for _, n := range neighbors { + if n.Lladdr != "" { + neighborMAC = n.Lladdr + framework.Logf("Neighbor entry found for %s -> MAC %s", egressIPSecondaryHost, neighborMAC) + return true + } + } + return false + }, 30*time.Second, 2*time.Second).Should(gomega.BeTrue(), + "Neighbor entry should appear via GARP") + gomega.Expect(neighborMAC).Should(gomega.Equal(inf.MAC), "neighbor entry should have the correct MAC address") + }) + // two pods attached to different namespaces but the same role primary user defined network // One pod is deleted and ensure connectivity for the other pod is ok // The previous pod namespace is deleted and again, ensure connectivity for the other pod is ok From 1e8f2603d03cffb17feeea6129a2e33209f22726 Mon Sep 17 00:00:00 2001 From: Patryk Diak Date: Thu, 16 Oct 2025 10:48:09 +0200 Subject: [PATCH 15/15] Send Unsolicited Neighbor Advertisement for IPv6 EgressIPs IPv6 equivalent of GARP functionality for EgressIP failover. Sends unsolicited NAs when IPv6 addresses are added to secondary host interfaces. Signed-off-by: Patryk Diak --- .../node/linkmanager/link_network_manager.go | 13 +++ go-controller/pkg/util/ndp/na.go | 100 ++++++++++++++++++ test/e2e/egressip.go | 45 +++++--- 3 files changed, 146 insertions(+), 12 deletions(-) create mode 100644 go-controller/pkg/util/ndp/na.go diff --git a/go-controller/pkg/node/linkmanager/link_network_manager.go b/go-controller/pkg/node/linkmanager/link_network_manager.go index d188b854df..d43442e4d8 100644 --- a/go-controller/pkg/node/linkmanager/link_network_manager.go +++ b/go-controller/pkg/node/linkmanager/link_network_manager.go @@ -11,6 +11,7 @@ import ( utilnet "k8s.io/utils/net" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util" + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util/ndp" ) // Gather all suitable interface address + network mask and offer this as a service. @@ -205,6 +206,18 @@ func (c *Controller) syncLink(link netlink.Link) error { klog.Errorf("Link manager: failed to send GARP for IP %s over interface %s: %v", addressWanted.IP.String(), linkName, err) } + } else if addressWanted.IP.To16() != nil { + // For IPv6, send an unsolicited neighbor advertisement to update neighbor caches, in case this IP was + // previously active on another node + na, err := ndp.NewNeighborAdvertisement(addressWanted.IP, nil) + if err != nil { + klog.Errorf("Link manager: failed to create NeighborAdvertisement for IP %s: %v", addressWanted.IP.String(), err) + continue + } + if err = ndp.SendUnsolicitedNeighborAdvertisement(linkName, na); err != nil { + klog.Errorf("Link manager: failed to send an unsolicited neighbor advertisement for IP %s over interface %s: %v", addressWanted.IP.String(), + linkName, err) + } } klog.Infof("Link manager: completed adding address %s to link %s", addressWanted, linkName) } diff --git a/go-controller/pkg/util/ndp/na.go b/go-controller/pkg/util/ndp/na.go new file mode 100644 index 0000000000..2cc45c7eec --- /dev/null +++ b/go-controller/pkg/util/ndp/na.go @@ -0,0 +1,100 @@ +package ndp + +import ( + "fmt" + "net" + "net/netip" + + "github.com/mdlayher/ndp" + + "k8s.io/klog/v2" +) + +// NeighborAdvertisement represents a Neighbor Advertisement for an IPv6 address. +type NeighborAdvertisement interface { + // IP returns the IPv6 address + IP() net.IP + // MAC returns the MAC address to advertise (nil means use interface MAC) + MAC() *net.HardwareAddr +} + +type neighborAdvertisement struct { + ip net.IP + mac *net.HardwareAddr +} + +// NewNeighborAdvertisement creates a new Unsolicited Neighbor Advertisement with validation that the IP is IPv6. +func NewNeighborAdvertisement(ip net.IP, mac *net.HardwareAddr) (NeighborAdvertisement, error) { + // TODO: Can have v6 mapped v4? + if ip.To4() != nil { + return nil, fmt.Errorf("only IPv6 addresses can be used for NeighborAdvertisement, got IPv4 %s", ip.String()) + } + if ip.To16() == nil { + return nil, fmt.Errorf("only IPv6 addresses can be used for NeighborAdvertisement, got %s", ip.String()) + } + + return &neighborAdvertisement{ + ip: ip.To16(), + mac: mac, + }, nil +} + +// IP returns the IPv6 address +func (u *neighborAdvertisement) IP() net.IP { + return u.ip +} + +// MAC returns the MAC address to advertise +func (u *neighborAdvertisement) MAC() *net.HardwareAddr { + return u.mac +} + +// SendUnsolicitedNeighborAdvertisement sends an unsolicited neighbor advertisement for the given IPv6 address. +// If the mac address is not provided it will use the one from the interface. +func SendUnsolicitedNeighborAdvertisement(interfaceName string, na NeighborAdvertisement) error { + iface, err := net.InterfaceByName(interfaceName) + if err != nil { + return fmt.Errorf("failed finding interface %s: %v", interfaceName, err) + } + + targetIP := na.IP() + mac := na.MAC() + if mac == nil { + mac = &iface.HardwareAddr + } + + targetAddr, ok := netip.AddrFromSlice(targetIP) + if !ok { + return fmt.Errorf("failed to convert IP %s to netip.Addr", targetIP.String()) + } + + allNodesMulticast := netip.MustParseAddr("ff02::1") + + // Use the target address as the source for the NA + // This is required for gratuitous NAs to properly update neighbor caches + c, _, err := ndp.Listen(iface, ndp.Addr(na.IP().String())) + if err != nil { + return fmt.Errorf("failed to create NDP connection on %s: %w", interfaceName, err) + } + defer c.Close() + + una := &ndp.NeighborAdvertisement{ + Router: false, + Solicited: false, + Override: true, + TargetAddress: targetAddr, + Options: []ndp.Option{ + &ndp.LinkLayerAddress{ + Direction: ndp.Target, + Addr: *mac, + }, + }, + } + + if err := c.WriteTo(una, nil, allNodesMulticast); err != nil { + return fmt.Errorf("failed to send an unsolicited neighbor advertisement for IP %s over interface %s: %w", targetIP.String(), interfaceName, err) + } + + klog.Infof("Sent an unsolicited neighbor advertisement for IP %s on interface %s with MAC: %s", targetIP.String(), interfaceName, mac.String()) + return nil +} diff --git a/test/e2e/egressip.go b/test/e2e/egressip.go index 1afa18ef0e..536ba78362 100644 --- a/test/e2e/egressip.go +++ b/test/e2e/egressip.go @@ -2957,20 +2957,45 @@ spec: "and verify the expected IP, failed for EgressIP %s: %v", egressIPName, err) }) - ginkgo.It("[secondary-host-eip] should send GARP for EgressIP", func() { - if utilnet.IsIPv6(net.ParseIP(egress1Node.nodeIP)) { - ginkgo.Skip("GARP test only supports IPv4") + ginkgo.It("[secondary-host-eip] should send address advertisements for EgressIP", func() { + if isUserDefinedNetwork(netConfigParams) { + ginkgo.Skip("Unsupported for UDNs") } + egressIPSecondaryHost := "10.10.10.220" + isV6Node := utilnet.IsIPv6(net.ParseIP(egress1Node.nodeIP)) + if isV6Node { + egressIPSecondaryHost = "2001:db8:abcd:1234:c001::" + } // flush any potentially stale MACs and allow GARPs _, err := infraprovider.Get().ExecK8NodeCommand(secondaryTargetExternalContainer.Name, []string{"ip", "neigh", "flush", egressIPSecondaryHost}) gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should flush neighbor cache") - _, err = infraprovider.Get().ExecK8NodeCommand(secondaryTargetExternalContainer.Name, - []string{"sysctl", "-w", "net.ipv4.conf.all.arp_accept=1"}) - gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should enable arp_accept") + networks, err := providerCtx.GetAttachedNetworks() + gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should get attached networks") + secondaryNetwork, exists := networks.Get(secondaryNetworkName) + gomega.Expect(exists).Should(gomega.BeTrue(), "network %s must exist", secondaryNetworkName) + + inf, err := infraprovider.Get().GetK8NodeNetworkInterface(secondaryTargetExternalContainer.Name, secondaryNetwork) + gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should have network interface for network %s on instance %s", secondaryNetwork.Name(), secondaryTargetExternalContainer.Name) + + // The following is required for the test purposes since we are sending and unsolicited advertisement + // for an address that is not tracked already + if !isV6Node { + _, err = infraprovider.Get().ExecK8NodeCommand(secondaryTargetExternalContainer.Name, + []string{"sysctl", "-w", fmt.Sprintf("net.ipv4.conf.%s.arp_accept=1", inf.InfName)}) + gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should enable arp_accept") + } else { + _, err = infraprovider.Get().ExecK8NodeCommand(secondaryTargetExternalContainer.Name, + []string{"sysctl", "-w", fmt.Sprintf("net.ipv6.conf.%s.forwarding=1", inf.InfName)}) + gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should enable forwarding") + + _, err = infraprovider.Get().ExecK8NodeCommand(secondaryTargetExternalContainer.Name, + []string{"sysctl", "-w", fmt.Sprintf("net.ipv6.conf.%s.accept_untracked_na=1", inf.InfName)}) + gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should enable accept_untracked_na") + } podNamespace := f.Namespace labels := map[string]string{"name": f.Namespace.Name} @@ -2990,7 +3015,7 @@ metadata: name: ` + egressIPName + ` spec: egressIPs: - - ` + egressIPSecondaryHost + ` + - "` + egressIPSecondaryHost + `" podSelector: matchLabels: wants: egress @@ -3009,11 +3034,7 @@ spec: e2ekubectl.RunKubectlOrDie("default", "create", "-f", egressIPYaml) status := verifyEgressIPStatusLengthEquals(1, nil) - networks, err := providerCtx.GetAttachedNetworks() - gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should get attached networks") - secondaryNetwork, exists := networks.Get(secondaryNetworkName) - gomega.Expect(exists).Should(gomega.BeTrue(), "network %s must exist", secondaryNetworkName) - inf, err := infraprovider.Get().GetK8NodeNetworkInterface(status[0].Node, secondaryNetwork) + inf, err = infraprovider.Get().GetK8NodeNetworkInterface(status[0].Node, secondaryNetwork) gomega.Expect(err).NotTo(gomega.HaveOccurred(), "should have network interface for network %s on instance %s", secondaryNetwork.Name(), egress1Node.name) ginkgo.By("Verifying GARP populated neighbor table")