Merge pull request #5211 from jcaamano/all-control-plane-tests-with-bgp

Run all control plane tests for bgp lane

Author: tssurya

.github/workflows/test.yml

@@ -464,15 +464,15 @@ jobs:
           - {"target": "network-segmentation", "ha": "noHA", "gateway-mode": "shared", "ipfamily": "ipv4", "disable-snat-multiple-gws": "noSnatGW", "second-bridge": "1br", "ic": "ic-single-node-zones"}
           - {"target": "network-segmentation", "ha": "noHA", "gateway-mode": "shared", "ipfamily": "ipv6", "disable-snat-multiple-gws": "noSnatGW", "second-bridge": "1br", "ic": "ic-single-node-zones"}
           - {"target": "bgp", "ha": "noHA", "gateway-mode": "local",  "ipfamily": "dualstack", "disable-snat-multiple-gws": "snatGW", "second-bridge": "1br", "ic": "ic-single-node-zones", "routeadvertisements": "advertise-default", "network-segmentation": "enable-network-segmentation"}
-          - {"target": "bgp", "ha": "noHA", "gateway-mode": "shared",  "ipfamily": "ipv4", "disable-snat-multiple-gws": "snatGW", "second-bridge": "1br", "ic": "ic-single-node-zones", "routeadvertisements": "advertise-default", "network-segmentation": "enable-network-segmentation"}
+          - {"target": "bgp", "ha": "noHA", "gateway-mode": "shared",  "ipfamily": "dualstack", "disable-snat-multiple-gws": "noSnatGW", "second-bridge": "1br", "ic": "ic-single-node-zones", "routeadvertisements": "advertise-default", "network-segmentation": "enable-network-segmentation"}
           - {"target": "traffic-flow-test-only","ha": "noHA", "gateway-mode": "shared", "ipfamily": "ipv4", "disable-snat-multiple-gws": "noSnatGW", "second-bridge": "1br", "ic": "ic-single-node-zones", "traffic-flow-tests": "1-24", "network-segmentation": "enable-network-segmentation"}
           - {"target": "tools", "ha": "noHA", "gateway-mode": "local", "ipfamily": "dualstack", "disable-snat-multiple-gws": "SnatGW", "second-bridge": "1br", "ic": "ic-single-node-zones", "network-segmentation": "enable-network-segmentation"}
     needs: [ build-pr ]
     env:
       JOB_NAME: "${{ matrix.target }}-${{ matrix.ha }}-${{ matrix.gateway-mode }}-${{ matrix.ipfamily }}-${{ matrix.disable-snat-multiple-gws }}-${{ matrix.second-bridge }}-${{ matrix.ic }}"
       OVN_HYBRID_OVERLAY_ENABLE: ${{ (matrix.target == 'control-plane' || matrix.target == 'control-plane-helm') && (matrix.ipfamily == 'ipv4' || matrix.ipfamily == 'dualstack' ) }}
-      OVN_MULTICAST_ENABLE:  "${{ matrix.target == 'control-plane' || matrix.target == 'control-plane-helm' || matrix.target == 'network-segmentation' }}"
-      OVN_EMPTY_LB_EVENTS: "${{ matrix.target == 'control-plane' || matrix.target == 'control-plane-helm' }}"
+      OVN_MULTICAST_ENABLE:  "${{ matrix.target == 'control-plane' || matrix.target == 'control-plane-helm' || matrix.target == 'network-segmentation' || matrix.target == 'bgp' }}"
+      OVN_EMPTY_LB_EVENTS: "${{ matrix.target == 'control-plane' || matrix.target == 'control-plane-helm' || matrix.target == 'bgp' }}"
       OVN_HA: "${{ matrix.ha == 'HA' }}"
       OVN_DISABLE_SNAT_MULTIPLE_GWS: "${{ matrix.disable-snat-multiple-gws == 'noSnatGW' }}"
       KIND_INSTALL_METALLB: "${{ matrix.target == 'control-plane' || matrix.target == 'control-plane-helm' || matrix.target == 'network-segmentation' }}"
@@ -559,7 +559,7 @@ jobs:
           echo OVN_TEST_EX_GW_NETWORK=xgw >> $GITHUB_ENV
           echo OVN_ENABLE_EX_GW_NETWORK_BRIDGE=true >> $GITHUB_ENV
         fi
-        if [[ "$JOB_NAME" == *"shard-conformance"* ]] && [ "$ADVERTISE_DEFAULT_NETWORK" == "true" ]; then
+        if [ "$ADVERTISE_DEFAULT_NETWORK" == "true" ]; then
           echo "ADVERTISE_DEFAULT_NETWORK=true" >> $GITHUB_ENV
 
           # Use proper variable declaration with default values
@@ -616,7 +616,9 @@ jobs:
     - name: Run Tests
       # e2e tests take ~60 minutes normally, 120 should be more than enough
       # set 3 hours for control-plane tests as these might take a while
-      timeout-minutes: ${{ matrix.target == 'control-plane' && 180 || matrix.target == 'control-plane-helm' && 180 || matrix.target == 'external-gateway' && 180 || 120 }}
+      # give 10m extra to give ginkgo chance to timeout before github so that we
+      # get its output
+      timeout-minutes: ${{ matrix.target == 'bgp' && 190 || matrix.target == 'control-plane' && 190 || matrix.target == 'control-plane-helm' && 190 || matrix.target == 'external-gateway' && 190 || 130 }}
       run: |
         # used by e2e diagnostics package
         export OVN_IMAGE="ovn-daemonset-fedora:pr"
@@ -641,7 +643,7 @@ jobs:
         elif [ "${{ matrix.target }}" == "network-segmentation" ]; then
           make -C test control-plane WHAT="Network Segmentation"
         elif [ "${{ matrix.target }}" == "bgp" ]; then
-          make -C test control-plane WHAT="BGP"
+          make -C test control-plane
         elif [ "${{ matrix.target }}" == "tools" ]; then
           make -C go-controller build
           make -C test tools

contrib/kind-common

@@ -751,6 +751,13 @@ deploy_bgp_external_server() {
     echo "FRR kind network IPv6: ${bgp_network_frr_v6}"
     $OCI_BIN exec bgpserver ip -6 route replace default via "$bgp_network_frr_v6"
   fi
+  # disable the default route to make sure the container only routes accross
+  # directly connected or learnt networks (doing this at the very end since
+  # docker changes the routing table when a new network is connected)
+  docker exec frr ip route delete default
+  docker exec frr ip route
+  docker exec frr ip -6 route delete default
+  docker exec frr ip -6 route
 }
 
 destroy_bgp() {
@@ -817,7 +824,7 @@ EOF
 
   rm -rf "${FRR_TMP_DIR}"
   # Add routes for pod networks dynamically into the github runner for return traffic to pass back
-  if [ -n "${JOB_NAME:-}" ] && [[ "$JOB_NAME" == *"shard-conformance"* ]] && [ "$ADVERTISE_DEFAULT_NETWORK" == "true" ]; then
+  if [ "$ADVERTISE_DEFAULT_NETWORK" = "true" ]; then
     echo "Adding routes for Kubernetes pod networks..."
     NODES=$(kubectl get nodes -o jsonpath='{.items[*].metadata.name}')
     echo "Found nodes: $NODES"
@@ -835,7 +842,7 @@ EOF
         # Add IPv4 route
         if [ -n "$ipv4_subnet" ] && [ -n "$node_ipv4" ]; then
           echo "Adding IPv4 route for $node ($node_ipv4): $ipv4_subnet"
-          sudo ip route add $ipv4_subnet via $node_ipv4
+          sudo ip route replace $ipv4_subnet via $node_ipv4
         fi
       fi
 
@@ -847,7 +854,7 @@ EOF
 
         if [ -n "$ipv6_subnet" ] && [ -n "$node_ipv6" ]; then
           echo "Adding IPv6 route for $node ($node_ipv6): $ipv6_subnet"
-          sudo ip -6 route add $ipv6_subnet via $node_ipv6
+          sudo ip -6 route replace $ipv6_subnet via $node_ipv6
         fi
       fi
     done

go-controller/pkg/node/gateway_shared_intf.go

@@ -1676,7 +1676,7 @@ func flowsForDefaultBridge(bridge *bridgeConfiguration, extraIPs []net.IP) ([]st
 			// at the GR load balancer or switch load balancer. It means the correct port wasn't provided.
 			// nodeCIDR->serviceCIDR traffic flow is internal and it shouldn't be carried to outside the cluster
 			dftFlows = append(dftFlows,
-				fmt.Sprintf("cookie=%s, priority=105, in_port=%s, %s, %s_dst=%s,"+
+				fmt.Sprintf("cookie=%s, priority=115, in_port=%s, %s, %s_dst=%s,"+
 					"actions=drop", defaultOpenFlowCookie, netConfig.ofPortPatch, protoPrefix, protoPrefix, svcCIDR))
 		}
 	}

go-controller/pkg/ovn/master_test.go

@@ -1639,7 +1639,7 @@ var _ = ginkgo.Describe("Default network controller operations", func() {
 			gomega.Expect(err).NotTo(gomega.HaveOccurred())
 
 			ginkgo.By("adding the node becomes possible")
-			gomega.Expect(oc.retryNodes.ResourceHandler.AddResource(&testNode, false)).To(gomega.Succeed())
+			gomega.Eventually(oc.retryNodes.ResourceHandler.AddResource).WithArguments(&testNode, false).Should(gomega.Succeed())
 
 			return nil
 		}

test/scripts/e2e-cp.sh

@@ -33,147 +33,152 @@ queries to the hostNetworked server pod on another node shall work for UDP|\
 ipv4 pod"
 
 SKIPPED_TESTS=""
+skip() {
+  if [ "$SKIPPED_TESTS" != "" ]; then
+  	SKIPPED_TESTS+="|"
+  fi
+  SKIPPED_TESTS+=$*
+}
 
 if [ "$PLATFORM_IPV4_SUPPORT" == true ]; then
-    if  [ "$PLATFORM_IPV6_SUPPORT" == true ]; then
-	# No support for these features in dual-stack yet
-	SKIPPED_TESTS="hybrid.overlay"
-    else
-	# Skip sflow in IPv4 since it's a long test (~5 minutes)
-	# We're validating netflow v5 with an ipv4 cluster, sflow with an ipv6 cluster
-	SKIPPED_TESTS="Should validate flow data of br-int is sent to an external gateway with sflow|ipv6 pod"
-    fi
+  if  [ "$PLATFORM_IPV6_SUPPORT" == true ]; then
+	  # No support for these features in dual-stack yet
+	   skip "hybrid.overlay"
+  else
+	  # Skip sflow in IPv4 since it's a long test (~5 minutes)
+	  # We're validating netflow v5 with an ipv4 cluster, sflow with an ipv6 cluster
+	  skip "Should validate flow data of br-int is sent to an external gateway with sflow|ipv6 pod"
+  fi
 fi
 
 if [ "$PLATFORM_IPV4_SUPPORT" == false ]; then
-  SKIPPED_TESTS+="\[IPv4\]"
+  skip "\[IPv4\]"
 fi
 
 if [ "$OVN_HA" == false ]; then
-  if [ "$SKIPPED_TESTS" != "" ]; then
-  	SKIPPED_TESTS+="|"
-  fi
   # No support for these features in no-ha mode yet
   # TODO streamline the db delete tests
-  SKIPPED_TESTS+="recovering from deleting db files while maintaining connectivity|\
-Should validate connectivity before and after deleting all the db-pods at once in HA mode"
+  skip "recovering from deleting db files while maintaining connectivity"
+  skip "Should validate connectivity before and after deleting all the db-pods at once in HA mode"
 else 
-  if [ "$SKIPPED_TESTS" != "" ]; then
-  	SKIPPED_TESTS+="|"
-  fi
-
-  SKIPPED_TESTS+="Should validate connectivity before and after deleting all the db-pods at once in Non-HA mode|\
-  e2e br-int NetFlow export validation"
+  skip "Should validate connectivity before and after deleting all the db-pods at once in Non-HA mode"
+  skip "e2e br-int NetFlow export validation"
 fi
 
 if [ "$PLATFORM_IPV6_SUPPORT" == true ]; then
-  if [ "$SKIPPED_TESTS" != "" ]; then
-  	SKIPPED_TESTS+="|"
-  fi
   # No support for these tests in IPv6 mode yet
-  SKIPPED_TESTS+=$IPV6_SKIPPED_TESTS
+  skip $IPV6_SKIPPED_TESTS
 fi
 
 if [ "$OVN_DISABLE_SNAT_MULTIPLE_GWS" == false ]; then
-  if [ "$SKIPPED_TESTS" != "" ]; then
-    SKIPPED_TESTS+="|"
-  fi
-  SKIPPED_TESTS+="e2e multiple external gateway stale conntrack entry deletion validation"
+  skip "e2e multiple external gateway stale conntrack entry deletion validation"
 fi
 
 if [ "$OVN_GATEWAY_MODE" == "shared" ]; then
-  if [ "$SKIPPED_TESTS" != "" ]; then
-    SKIPPED_TESTS+="|"
-  fi
-  SKIPPED_TESTS+="Should ensure load balancer service|LGW"
   # See https://github.com/ovn-org/ovn-kubernetes/issues/4138 for details
+  skip "Should ensure load balancer service|LGW"
 fi
 
 if [ "$OVN_GATEWAY_MODE" == "local" ]; then
-  # See https://github.com/ovn-org/ovn-kubernetes/labels/ci-ipv6 for details:
+  # See https://github.com/ovn-org/ovn-kubernetes/labels/ci-ipv6 for details
   if [ "$PLATFORM_IPV6_SUPPORT" == true ]; then
-    if [ "$SKIPPED_TESTS" != "" ]; then
-        SKIPPED_TESTS+="|"
-    fi
-    SKIPPED_TESTS+="Should be allowed by nodeport services|\
-Should successfully create then remove a static pod|\
-Should validate connectivity from a pod to a non-node host address on same node|\
-Should validate connectivity within a namespace of pods on separate nodes|\
-Services"
+    skip "Should be allowed by nodeport services"
+    skip "Should successfully create then remove a static pod"
+    skip "Should validate connectivity from a pod to a non-node host address on same node"
+    skip "Should validate connectivity within a namespace of pods on separate nodes"
+    skip "Services"
   fi
 fi
 
 # skipping the egress ip legacy health check test because it requires two
 # sequenced rollouts of both ovnkube-node and ovnkube-master that take a lot of
 # time.
-SKIPPED_TESTS+="${SKIPPED_TESTS:+|}disabling egress nodes impeding Legacy health check"
+skip "disabling egress nodes impeding Legacy health check"
 
 if [ "$ENABLE_MULTI_NET" != "true" ]; then
-  if [ "$SKIPPED_TESTS" != "" ]; then
-    SKIPPED_TESTS+="|"
-  fi
-  SKIPPED_TESTS+="Multi Homing"
+  skip "Multi Homing"
 fi
 
 if [ "$OVN_NETWORK_QOS_ENABLE" != "true" ]; then
-  if [ "$SKIPPED_TESTS" != "" ]; then
-    SKIPPED_TESTS+="|"
-  fi
-  SKIPPED_TESTS+="e2e NetworkQoS validation"
+  skip "e2e NetworkQoS validation"
 fi
 
 # Only run Node IP/MAC address migration tests if they are explicitly requested
 IP_MIGRATION_TESTS="Node IP and MAC address migration"
 if [[ "${WHAT}" != "${IP_MIGRATION_TESTS}"* ]]; then
-  if [ "$SKIPPED_TESTS" != "" ]; then
-	SKIPPED_TESTS+="|"
-  fi
-  SKIPPED_TESTS+="Node IP and MAC address migration"
+  skip "Node IP and MAC address migration"
 fi
 
 # Only run Multi node zones interconnect tests if they are explicitly requested
 MULTI_NODE_ZONES_TESTS="Multi node zones interconnect"
 if [[ "${WHAT}" != "${MULTI_NODE_ZONES_TESTS}"* ]]; then
-  if [ "$SKIPPED_TESTS" != "" ]; then
-	SKIPPED_TESTS+="|"
-  fi
-  SKIPPED_TESTS+="Multi node zones interconnect"
+  skip "Multi node zones interconnect"
 fi
 
 # Only run external gateway tests if they are explicitly requested
 EXTERNAL_GATEWAY_TESTS="External Gateway"
 if [[ "${WHAT}" != "${EXTERNAL_GATEWAY_TESTS}"* ]]; then
-  if [ "$SKIPPED_TESTS" != "" ]; then
-	SKIPPED_TESTS+="|"
-  fi
-  SKIPPED_TESTS+="External Gateway"
+  skip "External Gateway"
 fi
 
 # Only run kubevirt virtual machines tests if they are explicitly requested
 KV_LIVE_MIGRATION_TESTS="Kubevirt Virtual Machines"
 if [[ "${WHAT}" != "${KV_LIVE_MIGRATION_TESTS}"* ]]; then
-  if [ "$SKIPPED_TESTS" != "" ]; then
-	SKIPPED_TESTS+="|"
-  fi
-  SKIPPED_TESTS+=$KV_LIVE_MIGRATION_TESTS
+  skip $KV_LIVE_MIGRATION_TESTS
 fi
 
 # Only run network segmentation tests if they are explicitly requested
 NETWORK_SEGMENTATION_TESTS="Network Segmentation"
 if [[ "${WHAT}" != "${NETWORK_SEGMENTATION_TESTS}"* ]]; then
-  if [ "$SKIPPED_TESTS" != "" ]; then
-	SKIPPED_TESTS+="|"
-  fi
-  SKIPPED_TESTS+=$NETWORK_SEGMENTATION_TESTS
+  skip $NETWORK_SEGMENTATION_TESTS
 fi
 
-# Only run bgp tests if they are explicitly requested
 BGP_TESTS="BGP"
-if [[ "${WHAT}" != "${BGP_TESTS}"* ]]; then
-  if [ "$SKIPPED_TESTS" != "" ]; then
-	SKIPPED_TESTS+="|"
+if [ "$ENABLE_ROUTE_ADVERTISEMENTS" != true ]; then
+  skip $BGP_TESTS
+else
+  if [ "$ADVERTISE_DEFAULT_NETWORK" = true ]; then
+    # Some test don't work when the default network is advertised, either because
+    # the configuration that the test excercises does not make sense for an advertised network, or
+    # there is some bug or functional gap
+    # call out case by case
+
+    # pod reached from default network through secondary interface, asymetric, configuration does not make sense
+    # TODO: perhaps the secondary network attached pods should not be attached to default network
+    skip "Multi Homing A single pod with an OVN-K secondary network attached to a localnet network mapped to breth0 can be reached by a client pod in the default network on the same node"
+    skip "Multi Homing A single pod with an OVN-K secondary network attached to a localnet network mapped to breth0 can be reached by a client pod in the default network on a different node"
+  
+    # these tests require metallb but the configuration we do for it is not compatible with the configuration we do to advertise the default network
+    # TODO: consolidate configuration
+    skip "Load Balancer Service Tests with MetalLB"
+    skip "EgressService"
+
+    # tests that specifically expect the node SNAT to happen
+    # TODO: expect the pod IP where it makes sense
+    skip "e2e egress firewall policy validation with external containers"
+    skip "e2e egress IP validation Cluster Default Network \[OVN network\] Using different methods to disable a node's availability for egress Should validate the egress IP functionality against remote hosts"
+    skip "e2e egress IP validation Cluster Default Network \[OVN network\] Should validate the egress IP SNAT functionality against host-networked pods"
+    skip "e2e egress IP validation Cluster Default Network Should validate egress IP logic when one pod is managed by more than one egressIP object"
+    skip "e2e egress IP validation Cluster Default Network Should re-assign egress IPs when node readiness / reachability goes down/up"
+    skip "Pod to external server PMTUD when a client ovnk pod targeting an external server is created when tests are run towards the agnhost echo server queries to the hostNetworked server pod on another node shall work for UDP"
+
+    # https://issues.redhat.com/browse/OCPBUGS-55028
+    skip "e2e egress IP validation Cluster Default Network \[secondary-host-eip\]"
+
+    # https://issues.redhat.com/browse/OCPBUGS-50636
+    skip "Services of type NodePort should listen on each host addresses"
+    skip "Services of type NodePort should work on secondary node interfaces for ETP=local and ETP=cluster when backend pods are also served by EgressIP"
+
+    # https://github.com/ovn-kubernetes/ovn-kubernetes/issues/5240
+    skip "e2e control plane test node readiness according to its defaults interface MTU size should get node not ready with a too small MTU"
+
+    # buggy tests that don't work in dual stack mode
+    skip "Service Hairpin SNAT Should ensure service hairpin traffic is NOT SNATed to hairpin masquerade IP; GR LB"
+    skip "Services when a nodePort service targeting a pod with hostNetwork:false is created when tests are run towards the agnhost echo service queries to the nodePort service shall work for TCP"
+    skip "Services when a nodePort service targeting a pod with hostNetwork:true is created when tests are run towards the agnhost echo service queries to the nodePort service shall work for TCP"
+    skip "Services when a nodePort service targeting a pod with hostNetwork:false is created when tests are run towards the agnhost echo service queries to the nodePort service shall work for UDP"
+    skip "Services when a nodePort service targeting a pod with hostNetwork:true is created when tests are run towards the agnhost echo service queries to the nodePort service shall work for UDP"
   fi
-  SKIPPED_TESTS+=$BGP_TESTS
 fi
 
 # setting these is required to make RuntimeClass tests work ... :/
@@ -182,15 +187,20 @@ export KUBE_CONTAINER_RUNTIME_ENDPOINT=unix:///run/containerd/containerd.sock
 export KUBE_CONTAINER_RUNTIME_NAME=containerd
 export NUM_NODES=2
 
-FOCUS=$(echo ${@:1} | sed 's/ /\\s/g')
+FOCUS=$(echo "${@:1}" | sed 's/ /\\s/g')
+
+# Ginkgo test timeout needs to be lower than both github's timeout and go test
+# timeout to be able to get proper Ginkgo output when it happens.
+TEST_TIMEOUT=${TEST_TIMEOUT:-180}
+GO_TEST_TIMEOUT=$((TEST_TIMEOUT + 5))
 
 pushd e2e
 
 go mod download
-go test -test.timeout 180m -v . \
+go test -test.timeout ${GO_TEST_TIMEOUT}m -v . \
         -ginkgo.v \
         -ginkgo.focus ${FOCUS:-.} \
-        -ginkgo.timeout 3h \
+        -ginkgo.timeout ${TEST_TIMEOUT}m \
         -ginkgo.flake-attempts ${FLAKE_ATTEMPTS:-2} \
         -ginkgo.skip="${SKIPPED_TESTS}" \
         -ginkgo.junit-report=${E2E_REPORT_DIR}/junit_${E2E_REPORT_PREFIX}report.xml \

test/scripts/e2e-kind.sh

@@ -200,7 +200,7 @@ fi
 # timeout needs to be lower than github's timeout. Otherwise github terminates
 # the job and doesn't give ginkgo a chance to print status so that we know why
 # the timeout happened.
-TEST_TIMEOUT=${TEST_TIMEOUT:-100m}
+TEST_TIMEOUT=${TEST_TIMEOUT:-120m}
 
 ginkgo --nodes=${NUM_NODES} \
 	--focus=${FOCUS} \