Skip to content

OCPBUGS-61487: Add timer for Azure VIP routes reconciliation #5455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ricky-rav wants to merge 2 commits into
base: main
Choose a base branch
from
Open

OCPBUGS-61487: Add timer for Azure VIP routes reconciliation #5455

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
878f96a
Add timer for Azure VIP routes reconciliation
ricky-rav Nov 28, 2025
e4fdefc
Add SyslogIdentifier to GCP routes service
ricky-rav Dec 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 6 additions & 31 deletions templates/master/00-master/azure/files/opt-libexec-openshift-azure-routes-sh.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,11 @@ contents:
if [ -z "${ovnkContainerID}" ]; then
return
fi
echo "Found ovnkube-controller pod... ${ovnkContainerID}"
Copy link
Contributor

@tssurya tssurya Dec 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reason I had added sooo many echo statements was because script was soo raw and we were directly touching the DBs that I was super worried what command we are transaction for history purposes....plus I wasn't a bash expert really :) so wanted to be extra careful with each step I was doing LOL

But I think its been some years now, which hopefully means this is stable, so I'm ok to remove these statements and have less logs. - but also OK with what you did, which is consolidate them into a single line.

systemd logging it every 30 seconds is going to be annoying a bit

local routeVIPsV4=$(crictl exec -i ${ovnkContainerID} ovn-nbctl lr-policy-list ovn_cluster_router | grep "1010" | grep "ip4" | awk '$8{print $8}')
echo "Found v4route vips: ${routeVIPsV4}"
local host=$(hostname)
echo ${host}
for route_vip in ${routeVIPsV4}; do
if [[ ! -v v4vips[${route_vip}] ]] || [[ "${v4vips[${route_vip}]}" = down ]]; then
echo removing stale vip "${route_vip}" for local clients
echo "ovn-nbctl lr-policy-del ovn_cluster_router 1010 inport == rtos-${host} && ip4.dst == ${route_vip}"
echo "removing stale route: ovn-nbctl lr-policy-del ovn_cluster_router 1010 inport == rtos-${host} && ip4.dst == ${route_vip}"
crictl exec -i ${ovnkContainerID} ovn-nbctl lr-policy-del ovn_cluster_router 1010 "inport == \"rtos-${host}\" && ip4.dst == ${route_vip}"
fi
done
Expand All @@ -75,11 +71,9 @@ contents:
fi

local routeVIPsV6=$(crictl exec -i ${ovnkContainerID} ovn-nbctl lr-policy-list ovn_cluster_router | grep "1010" | grep "ip6" | awk '$8{print $8}')
echo "Found v6route vips: ${routeVIPsV6}"
for route_vip in ${routeVIPsV6}; do
if [[ ! -v v6vips[${route_vip}] ]] || [[ "${v6vips[${route_vip}]}" = down ]]; then
echo removing stale vip "${route_vip}" for local clients
echo "ovn-nbctl lr-policy-del ovn_cluster_router 1010 inport == rtos-${host} && ip6.dst == ${route_vip}"
echo "removing stale route: ovn-nbctl lr-policy-del ovn_cluster_router 1010 inport == rtos-${host} && ip6.dst == ${route_vip}"
crictl exec -i ${ovnkContainerID} ovn-nbctl lr-policy-del ovn_cluster_router 1010 "inport == \"rtos-${host}\" && ip6.dst == ${route_vip}"
fi
done
Expand All @@ -101,7 +95,6 @@ contents:
fi
done

echo "synchronizing IPv4 VIPs to (${v4vipset}), IPv6 VIPS to (${v6vipset})"
{
echo "flush chain inet ${TABLE_NAME} ${VIPS_CHAIN}"
if [[ -n "${v4vipset}" ]]; then
Expand All @@ -119,21 +112,13 @@ contents:
echo "OVN-Kubernetes is not running; no routes to add."
return
fi
echo "Found ovnkube-controller pod... ${ovnkContainerID}"
local ovnK8sMp0v4=$(ip -brief address show ovn-k8s-mp0 | awk '{print $3}' | awk -F/ '{print $1}')
echo "Found ovn-k8s-mp0 interface IP ${ovnK8sMp0v4}"
local host=$(hostname)
echo ${host}
for vip in "${!v4vips[@]}"; do
if [[ "${v4vips[${vip}]}" != down ]]; then
echo "ensuring route for ${vip} for internal clients"
local routes=$(crictl exec -i ${ovnkContainerID} ovn-nbctl lr-policy-list ovn_cluster_router | grep "1010" | grep "${vip}" | grep "${ovnK8sMp0v4}")
echo "OVNK Routes on ovn-cluster-router at 1010 priority: $routes"
if [[ "${routes}" == *"${vip}"* ]]; then
echo "Route exists"
else
echo "Route does not exist; creating it..."
echo "ovn-nbctl lr-policy-add ovn_cluster_router 1010 inport == rtos-${host} && ip4.dst == ${vip} reroute ${ovnK8sMp0v4}"
if [[ "${routes}" != *"${vip}"* ]]; then
echo "adding route: ovn-nbctl lr-policy-add ovn_cluster_router 1010 inport == rtos-${host} && ip4.dst == ${vip} reroute ${ovnK8sMp0v4}"
crictl exec -i ${ovnkContainerID} ovn-nbctl lr-policy-add ovn_cluster_router 1010 "inport == \"rtos-${host}\" && ip4.dst == ${vip}" reroute "${ovnK8sMp0v4}"
fi
fi
Expand All @@ -144,18 +129,11 @@ contents:
fi

local ovnK8sMp0v6=$(ip -brief address show ovn-k8s-mp0 | awk '{print $4}' | awk -F/ '{print $1}')
echo "Found ovn-k8s-mp0 interface IP ${ovnK8sMp0v6}"

for vip in "${!v6vips[@]}"; do
if [[ "${v6vips[${vip}]}" != down ]]; then
echo "ensuring route for ${vip} for internal clients"
local routes=$(crictl exec -i ${ovnkContainerID} ovn-nbctl lr-policy-list ovn_cluster_router | grep "1010" | grep "${vip}" | grep "${ovnK8sMp0v6}")
echo "OVNK Routes on ovn-cluster-router at 1010 priority: $routes"
if [[ "${routes}" == *"${vip}"* ]]; then
echo "Route exists"
else
echo "Route does not exist; creating it..."
echo "ovn-nbctl lr-policy-add ovn_cluster_router 1010 inport == rtos-${host} && ip6.dst == ${vip} reroute ${ovnK8sMp0v6}"
if [[ "${routes}" != *"${vip}"* ]]; then
echo "adding route: ovn-nbctl lr-policy-add ovn_cluster_router 1010 inport == rtos-${host} && ip6.dst == ${vip} reroute ${ovnK8sMp0v6}"
crictl exec -i ${ovnkContainerID} ovn-nbctl lr-policy-add ovn_cluster_router 1010 "inport == \"rtos-${host}\" && ip6.dst == ${vip}" reroute "${ovnK8sMp0v6}"
fi
fi
Expand Down Expand Up @@ -195,10 +173,8 @@ contents:
echo "${vip} has upfile and downfile, marking as down"
else
if [[ ${vip} =~ : ]]; then
echo "processing v6 vip ${vip}"
v6vips[${vip}]="${vip}"
else
echo "processing v4 vip ${vip}"
v4vips[${vip}]="${vip}"
fi
fi
Expand All @@ -213,7 +189,6 @@ contents:
sync_rules
remove_stale_routes # needed for OVN-Kubernetes plugin's routingViaHost=false mode
add_routes # needed for OVN-Kubernetes plugin's routingViaHost=false mode
echo "done applying vip rules"
;;
cleanup)
clear_rules
Expand Down
2 changes: 1 addition & 1 deletion templates/master/00-master/azure/units/openshift-azure-routes.service.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ contents: |

[Service]
Type=simple
ExecStart=/bin/bash /opt/libexec/openshift-azure-routes.sh start
ExecStart=/usr/bin/flock --exclusive --nonblock /run/azure-routes.lock /bin/bash /opt/libexec/openshift-azure-routes.sh start
User=root
SyslogIdentifier=openshift-azure-routes
12 changes: 12 additions & 0 deletions templates/master/00-master/azure/units/openshift-azure-routes.timer.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
name: openshift-azure-routes.timer
enabled: true
contents: |
[Unit]
Description=Periodic reconciliation of Azure VIP routes
[Timer]
OnBootSec=30
Copy link
Contributor

@tssurya tssurya Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm does it mean it will run 30sec after bootup as a one time thing? or this is every 30seconds like you said in PR description.. cause isn't it too much if we act on the filechange and run this every 30seconds?
and what does onunitactivesec mean? 30seconds after the unit gets active?

Copy link
Contributor Author

@ricky-rav ricky-rav Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it gets triggered the first time 30 seconds after boot (OnBootSec=30) and then 30 seconds after the service last finished (OnUnitActiveSec=30), repeatedly.

OnUnitActiveSec=30
[Install]
WantedBy=timers.target
1 change: 1 addition & 0 deletions templates/master/00-master/gcp/units/openshift-gcp-routes.service.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ contents: |
ExecStart=/bin/bash /opt/libexec/openshift-gcp-routes.sh start
ExecStopPost=/bin/bash /opt/libexec/openshift-gcp-routes.sh cleanup
User=root
SyslogIdentifier=openshift-gcp-routes
Copy link
Contributor

@tssurya tssurya Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, didn't know about this setting.

RestartSec=30
Restart=always

Expand Down