Merge pull request #2907 from jcaamano/master

Bug 2036113: configure-ovs: cleanup leftovers from previous run

templates/common/_base/files/configure-ovs-network.yaml

@@ -172,6 +172,7 @@ contents:
 
       # create bridge
       if ! nmcli connection show "$bridge_name" &> /dev/null; then
+        ovs-vsctl --timeout=30 --if-exists del-br "$bridge_name"
         nmcli c add type ovs-bridge \
             con-name "$bridge_name" \
             conn.interface "$bridge_name" \
@@ -183,10 +184,12 @@ contents:
 
       # find default port to add to bridge
       if ! nmcli connection show "$default_port_name" &> /dev/null; then
+        ovs-vsctl --timeout=30 --if-exists del-port "$bridge_name" ${iface}
         nmcli c add type ovs-port conn.interface ${iface} master "$bridge_name" con-name "$default_port_name"
       fi
 
       if ! nmcli connection show "$ovs_port" &> /dev/null; then
+        ovs-vsctl --timeout=30 --if-exists del-port "$bridge_name" "$bridge_name"
         nmcli c add type ovs-port conn.interface "$bridge_name" master "$bridge_name" con-name "$ovs_port"
       fi
 
@@ -226,6 +229,7 @@ contents:
 
       # use ${extra_phys_args[@]+"${extra_phys_args[@]}"} instead of ${extra_phys_args[@]} to be compatible with bash 4.2 in RHEL7.9
       if ! nmcli connection show "$bridge_interface_name" &> /dev/null; then
+        ovs-vsctl --timeout=30 --if-exists destroy interface ${iface}
         nmcli c add type ${iface_type} conn.interface ${iface} master "$default_port_name" con-name "$bridge_interface_name" \
         connection.autoconnect-priority 100 802-3-ethernet.mtu ${iface_mtu} ${extra_phys_args[@]+"${extra_phys_args[@]}"}
       fi
@@ -238,6 +242,7 @@ contents:
       replace_connection_master $iface $new_conn
 
       if ! nmcli connection show "$ovs_interface" &> /dev/null; then
+        ovs-vsctl --timeout=30 --if-exists destroy interface "$bridge_name"
         if nmcli --fields ipv4.method,ipv6.method conn show $old_conn | grep manual; then
           echo "Static IP addressing detected on default gateway connection: ${old_conn}"
           # find and copy the old connection to get the address settings
@@ -368,6 +373,21 @@ contents:
       fi
     }
 
+    # Removes all configuration and reloads NM if necessary
+    rollback_nm() {
+      # This will be set to 1 if remove_all_ovn_bridges actually changes anything
+      nm_conn_files_removed=0
+
+      # Revert changes made by /usr/local/bin/configure-ovs.sh during SDN migration.
+      remove_all_ovn_bridges
+      
+      # Reload only if we removed connection profiles
+      if [ $nm_conn_files_removed -eq 1 ]; then
+        echo "OVS configuration was cleaned up, will reload NetworkManager"
+        reload_nm
+      fi
+    }
+
     # Activates a NM connection profile
     activate_nm_conn() {
       local conn="$1"
@@ -386,8 +406,9 @@ contents:
 
     # Used to print network state
     print_state() {
-      echo "Current connection, interface and routing state:"
-      nmcli -g all c show
+      echo "Current device, connection, interface and routing state:"
+      nmcli -g all device | grep -v unmanaged
+      nmcli -g all connection
       ip -d address show
       ip route show
       ip -6 route show
@@ -411,8 +432,7 @@ contents:
 
       # attempt to restore the previous network state
       echo "Attempting to restore previous configuration..."
-      remove_all_ovn_bridges
-      reload_nm
+      rollback_nm
       print_state
 
       exit $e
@@ -458,9 +478,8 @@ contents:
       # in any changes that have possibly been applied in the standard
       # configuration sources
       if [ ! -f /run/configure-ovs-boot-done ]; then
-        echo "Running on boot, restoring to previous configuration..."
-        remove_all_ovn_bridges
-        reload_nm
+        echo "Running on boot, restoring previous configuration before proceeding..."
+        rollback_nm
         print_state
       fi
       touch /run/configure-ovs-boot-done
@@ -488,13 +507,18 @@ contents:
 
       extra_bridge_file='/etc/ovnk/extra_bridge'
 
-      # Some deployments uses a temporary solution where br-ex is moved out from the default gateway interface
-      # and bound to a different nic. If that is the case, we rollback (https://github.com/trozet/openshift-ovn-migration).
-      if [ "$iface" != "br-ex" ] && [  -f "$extra_bridge_file" ] && nmcli connection show br-ex &> /dev/null; then
-        echo "Default gateway is not bridge but bridge exists, reverting"
-        remove_all_ovn_bridges
-        reload_nm
-        print_state
+      if [ "$iface" != "br-ex" ]; then
+        # Default gateway is not br-ex.
+        # Some deployments use a temporary solution where br-ex is moved out from the default gateway interface
+        # and bound to a different nic (https://github.com/trozet/openshift-ovn-migration).
+        # This is now supported through an extra bridge if requested. If that is the case, we rollback.
+        # We also rollback if it looks like we need to configure things, just in case there are any leftovers
+        # from previous attempts.
+        if [ -f "$extra_bridge_file" ] || [ -z "$(nmcli connection show --active br-ex 2> /dev/null)" ]; then
+          echo "Bridge br-ex is not active, restoring previous configuration before proceeding..."
+          rollback_nm
+          print_state
+        fi
       fi
 
       convert_to_bridge "$iface" "br-ex" "phys0"
@@ -525,17 +549,8 @@ contents:
         activate_nm_conn ovs-if-br-ex1
       fi
     elif [ "$1" == "OpenShiftSDN" ]; then
-      # This will be set to 1 if remove_all_ovn_bridges actually changes anything
-      nm_conn_files_removed=0
-
       # Revert changes made by /usr/local/bin/configure-ovs.sh during SDN migration.
-      remove_all_ovn_bridges
-      
-      # Reload only if we removed connection profiles
-      if [ $nm_conn_files_removed -eq 1 ]; then
-        echo "OVS configuration was cleaned up, will reload NetworkManager"
-        reload_nm
-      fi
+      rollback_nm
       
       # Remove bridges created by ovn-kubernetes
       ovs-vsctl --timeout=30 --if-exists del-br br-int -- --if-exists del-br br-local