Skip to content

fix: delete all ips before deleting a subnet #5775

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
+31 −11
Draft

fix: delete all ips before deleting a subnet #5775

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
a49a60a
fix: delete all IPs before deleting the subnets
tallquist10 Oct 6, 2025
2bcc17d
update version to 1.16.0 in all spots
tallquist10 Oct 7, 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
2 changes: 1 addition & 1 deletion VERSION
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
v1.15.0
v1.16.0
4 changes: 2 additions & 2 deletions charts/kube-ovn-v2/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ apiVersion: v2
name: kube-ovn-v2
description: Helm chart for Kube-OVN
type: application
version: 1.15.0
appVersion: "1.15.0"
version: 1.16.0
appVersion: "1.16.0"
kubeVersion: ">= 1.29.0-0"
2 changes: 1 addition & 1 deletion charts/kube-ovn-v2/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ Get IPs of master nodes from values
{{- $imageVersion := (index $ds.spec.template.spec.containers 0).image | splitList ":" | last | trimPrefix "v" -}}
{{- $versionRegex := `^(?P<major>0|[1-9]\d*)\.(?P<minor>0|[1-9]\d*)\.(?P<patch>0|[1-9]\d*)` -}}
{{- if and (ne $newChartVersion $chartVersion) (regexMatch $versionRegex $imageVersion) -}}
{{- if regexFind $versionRegex $imageVersion | semverCompare ">= 1.15.0" -}}
{{- if regexFind $versionRegex $imageVersion | semverCompare ">= 1.16.0" -}}
25.03
{{- else if regexFind $versionRegex $imageVersion | semverCompare ">= 1.13.0" -}}
24.03
Expand Down
2 changes: 1 addition & 1 deletion charts/kube-ovn-v2/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ natGw:
repository: docker.io/kubeovn/kube-ovn
# -- Image tag.
# @section -- NAT gateways configuration
tag: v1.15.0
tag: v1.16.0
# -- Image pull policy.
# @section -- NAT gateways configuration
pullPolicy: IfNotPresent
Expand Down
4 changes: 2 additions & 2 deletions charts/kube-ovn/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
version: v1.15.0
version: v1.16.0

# This is the version number of the application being deployed. This version number should be
# incremented each time you make changes to the application. Versions are not expected to
# follow Semantic Versioning. They should reflect the version the application is using.
# It is recommended to use it with quotes.
appVersion: "1.15.0"
appVersion: "1.16.0"

kubeVersion: ">= 1.29.0-0"
2 changes: 1 addition & 1 deletion charts/kube-ovn/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ Number of master nodes
{{- $imageVersion := (index $ds.spec.template.spec.containers 0).image | splitList ":" | last | trimPrefix "v" -}}
{{- $versionRegex := `^(?P<major>0|[1-9]\d*)\.(?P<minor>0|[1-9]\d*)\.(?P<patch>0|[1-9]\d*)` -}}
{{- if and (ne $newChartVersion $chartVersion) (regexMatch $versionRegex $imageVersion) -}}
{{- if regexFind $versionRegex $imageVersion | semverCompare ">= 1.15.0" -}}
{{- if regexFind $versionRegex $imageVersion | semverCompare ">= 1.16.0" -}}
25.03
{{- else if regexFind $versionRegex $imageVersion | semverCompare ">= 1.13.0" -}}
24.03
Expand Down
2 changes: 1 addition & 1 deletion charts/kube-ovn/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ global:
kubeovn:
repository: kube-ovn
vpcRepository: vpc-nat-gateway
tag: v1.15.0
tag: v1.16.0
support_arm: true
thirdparty: true

Expand Down
2 changes: 1 addition & 1 deletion dist/images/install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ CNI_BIN_DIR="/opt/cni/bin"

REGISTRY="docker.io/kubeovn"
VPC_NAT_IMAGE="vpc-nat-gateway"
VERSION="v1.15.0"
VERSION="v1.16.0"
IMAGE_PULL_POLICY="IfNotPresent"
POD_CIDR="10.16.0.0/16" # Do NOT overlap with NODE/SVC/JOIN CIDR
POD_GATEWAY="10.16.0.1"
Expand Down
22 changes: 21 additions & 1 deletion pkg/controller/subnet.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1008,8 +1008,28 @@ func (c *Controller) handleDeleteSubnet(subnet *kubeovnv1.Subnet) error {
return err
}

err := c.handleDeleteLogicalSwitch(subnet.Name)
klog.Infof("cleaning up IPs for subnet %s", subnet.Name)
ips, err := c.ipsLister.List(labels.Everything())
if err != nil {
klog.Errorf("failed to list ips: %v", err)
return err
}
for _, ip := range ips {
if ip.Spec.Subnet == subnet.Name {
// Delete with foreground propagation to wait for finalizer cleanup
deletePolicy := metav1.DeletePropagationForeground
if err := c.config.KubeOvnClient.KubeovnV1().IPs().Delete(
context.Background(),
ip.Name,
metav1.DeleteOptions{PropagationPolicy: &deletePolicy},
); err != nil && !k8serrors.IsNotFound(err) {
klog.Errorf("failed to delete ip %s: %v", ip.Name, err)
return err
Copy link
Collaborator

Choose a reason for hiding this comment

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

how about just logging error and continue?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, sorry about this confusion. I will put together a more thorough PR description today. Basically, the issue is that IPs aren't getting deleted before their subnets, so when the controller goes to remove the finalizer, and the subnet doesn't exist, it fails and doesn't remove the finalizer, and it never proceeds. I forgot I made a draft PR with my changes, and this will not be the final version.

}
}
}

if err := c.handleDeleteLogicalSwitch(subnet.Name); err != nil {
klog.Errorf("failed to delete logical switch %s %v", subnet.Name, err)
return err
}
Expand Down