Merge pull request openstack-k8s-operators#1032 from rabi/ipset_scalein

Cleanup stale ip reservations when scaling in nodeset

pkg/dataplane/const.go

@@ -67,4 +67,7 @@ const (
 
 	//HostnameLabel label for marking secrets to be watched for changes
 	HostnameLabel = "hostname"
+
+	// IPSetOwnershipLabelKey label key to find all IPSets for OpenStackDataPlaneNodeSet
+	IPSetOwnershipLabelKey = "nodeset/name"
 )

pkg/dataplane/ipam.go

@@ -239,6 +239,17 @@ func EnsureDNSData(ctx context.Context, helper *helper.Helper,
 func EnsureIPSets(ctx context.Context, helper *helper.Helper,
 	instance *dataplanev1.OpenStackDataPlaneNodeSet,
 ) (map[string]infranetworkv1.IPSet, bool, error) {
+	// Cleanup the stale reservations first
+	err := cleanupStaleReservations(ctx, helper, instance)
+	if err != nil {
+		util.LogErrorForObject(helper, err, "Could not cleanup stale IP Reservations", instance)
+		instance.Status.Conditions.MarkFalse(
+			dataplanev1.NodeSetIPReservationReadyCondition,
+			condition.ErrorReason, condition.SeverityError,
+			dataplanev1.NodeSetIPReservationReadyErrorMessage,
+			err.Error())
+		return nil, false, err
+	}
 	allIPSets, err := reserveIPs(ctx, helper, instance)
 	if err != nil {
 		instance.Status.Conditions.MarkFalse(
@@ -264,6 +275,44 @@ func EnsureIPSets(ctx context.Context, helper *helper.Helper,
 	return allIPSets, true, nil
 }
 
+// cleanupStaleReservations Cleanup stale ipset reservations
+func cleanupStaleReservations(ctx context.Context, helper *helper.Helper,
+	instance *dataplanev1.OpenStackDataPlaneNodeSet) error {
+	ipSetList := &infranetworkv1.IPSetList{}
+	labelSelectorMap := map[string]string{IPSetOwnershipLabelKey: instance.Name}
+	listOpts := []client.ListOption{
+		client.InNamespace(instance.Namespace),
+		client.MatchingLabels(labelSelectorMap),
+	}
+
+	err := helper.GetClient().List(ctx, ipSetList, listOpts...)
+	if err != nil {
+		return err
+	}
+
+	ipSetsToRemove := []infranetworkv1.IPSet{}
+	// Delete all IPSet for nodes that are not in nodeset
+	for _, ipSet := range ipSetList.Items {
+		found := false
+		for _, node := range instance.Spec.Nodes {
+			if ipSet.Name == node.HostName {
+				found = true
+				break
+			}
+		}
+		if !found {
+			ipSetsToRemove = append(ipSetsToRemove, ipSet)
+		}
+	}
+	for _, ipSet := range ipSetsToRemove {
+		if err := helper.GetClient().Delete(ctx, &ipSet); err != nil {
+			return err
+		}
+	}
+	return nil
+
+}
+
 // reserveIPs Reserves IPs by creating IPSets
 func reserveIPs(ctx context.Context, helper *helper.Helper,
 	instance *dataplanev1.OpenStackDataPlaneNodeSet,
@@ -284,6 +333,7 @@ func reserveIPs(ctx context.Context, helper *helper.Helper,
 	}
 
 	allIPSets := make(map[string]infranetworkv1.IPSet)
+
 	// CreateOrPatch IPSets
 	for nodeName, node := range instance.Spec.Nodes {
 		nets := node.Networks
@@ -300,6 +350,8 @@ func reserveIPs(ctx context.Context, helper *helper.Helper,
 				},
 			}
 			_, err := controllerutil.CreateOrPatch(ctx, helper.GetClient(), ipSet, func() error {
+				ipSet.Labels = util.MergeStringMaps(ipSet.Labels,
+					map[string]string{IPSetOwnershipLabelKey: instance.Name})
 				ipSet.Spec.Networks = nets
 				// Set controller reference to the DataPlaneNode object
 				err := controllerutil.SetControllerReference(
@@ -316,6 +368,5 @@ func reserveIPs(ctx context.Context, helper *helper.Helper,
 			return nil, fmt.Errorf(msg)
 		}
 	}
-
 	return allIPSets, nil
 }

tests/kuttl/tests/dataplane-multinode-nodeset-create-test/00-assert.yaml

@@ -16,15 +16,15 @@ status:
       tenant: edpm-compute-1.tenant.example.com
   allIPs:
     edpm-compute-0:
-      ctlplane: 192.168.122.100
-      internalapi: 172.17.0.100
-      storage: 172.18.0.100
-      tenant: 172.19.0.100
+      ctlplane: 192.168.122.150
+      internalapi: 172.17.0.150
+      storage: 172.18.0.150
+      tenant: 172.19.0.150
     edpm-compute-1:
-      ctlplane: 192.168.122.101
-      internalapi: 172.17.0.101
-      storage: 172.18.0.101
-      tenant: 172.19.0.101
+      ctlplane: 192.168.122.151
+      internalapi: 172.17.0.151
+      storage: 172.18.0.151
+      tenant: 172.19.0.151
   observedGeneration: 1
   ctlplaneSearchDomain: ctlplane.example.com
   conditions:
@@ -76,7 +76,7 @@ status:
     status: "True"
     type: ReservationReady
   reservations:
-  - address: 192.168.122.100
+  - address: 192.168.122.150
     cidr: 192.168.122.0/24
     dnsDomain: ctlplane.example.com
     gateway: 192.168.122.1
@@ -86,21 +86,21 @@ status:
     - destination: 0.0.0.0/0
       nexthop: 192.168.122.1
     subnet: subnet1
-  - address: 172.17.0.100
+  - address: 172.17.0.150
     cidr: 172.17.0.0/24
     dnsDomain: internalapi.example.com
     mtu: 1500
     network: internalapi
     subnet: subnet1
     vlan: 20
-  - address: 172.18.0.100
+  - address: 172.18.0.150
     cidr: 172.18.0.0/24
     dnsDomain: storage.example.com
     mtu: 1500
     network: storage
     subnet: subnet1
     vlan: 21
-  - address: 172.19.0.100
+  - address: 172.19.0.150
     cidr: 172.19.0.0/24
     dnsDomain: tenant.example.com
     mtu: 1500
@@ -127,7 +127,7 @@ status:
     status: "True"
     type: ReservationReady
   reservations:
-  - address: 192.168.122.101
+  - address: 192.168.122.151
     cidr: 192.168.122.0/24
     dnsDomain: ctlplane.example.com
     gateway: 192.168.122.1
@@ -137,21 +137,21 @@ status:
     - destination: 0.0.0.0/0
       nexthop: 192.168.122.1
     subnet: subnet1
-  - address: 172.17.0.101
+  - address: 172.17.0.151
     cidr: 172.17.0.0/24
     dnsDomain: internalapi.example.com
     mtu: 1500
     network: internalapi
     subnet: subnet1
     vlan: 20
-  - address: 172.18.0.101
+  - address: 172.18.0.151
     cidr: 172.18.0.0/24
     dnsDomain: storage.example.com
     mtu: 1500
     network: storage
     subnet: subnet1
     vlan: 21
-  - address: 172.19.0.101
+  - address: 172.19.0.151
     cidr: 172.19.0.0/24
     dnsDomain: tenant.example.com
     mtu: 1500
@@ -168,28 +168,28 @@ spec:
   hosts:
   - hostnames:
     - edpm-compute-0.ctlplane.example.com
-    ip: 192.168.122.100
+    ip: 192.168.122.150
   - hostnames:
     - edpm-compute-0.internalapi.example.com
-    ip: 172.17.0.100
+    ip: 172.17.0.150
   - hostnames:
     - edpm-compute-0.storage.example.com
-    ip: 172.18.0.100
+    ip: 172.18.0.150
   - hostnames:
     - edpm-compute-0.tenant.example.com
-    ip: 172.19.0.100
+    ip: 172.19.0.150
   - hostnames:
     - edpm-compute-1.ctlplane.example.com
-    ip: 192.168.122.101
+    ip: 192.168.122.151
   - hostnames:
     - edpm-compute-1.internalapi.example.com
-    ip: 172.17.0.101
+    ip: 172.17.0.151
   - hostnames:
     - edpm-compute-1.storage.example.com
-    ip: 172.18.0.101
+    ip: 172.18.0.151
   - hostnames:
     - edpm-compute-1.tenant.example.com
-    ip: 172.19.0.101
+    ip: 172.19.0.151
 status:
   conditions:
   - message: Setup complete

tests/kuttl/tests/dataplane-multinode-nodeset-create-test/00-dataplane-create.yaml

@@ -65,32 +65,32 @@ spec:
       - name: ctlplane
         subnetName: subnet1
         defaultRoute: true
-        fixedIP: 192.168.122.101
+        fixedIP: 192.168.122.151
       - name: internalapi
         subnetName: subnet1
-        fixedIP: 172.17.0.101
+        fixedIP: 172.17.0.151
       - name: storage
         subnetName: subnet1
-        fixedIP: 172.18.0.101
+        fixedIP: 172.18.0.151
       - name: tenant
         subnetName: subnet1
-        fixedIP: 172.19.0.101
+        fixedIP: 172.19.0.151
     edpm-compute-0:
       hostName: edpm-compute-0
       networks:
       - name: ctlplane
         subnetName: subnet1
         defaultRoute: true
-        fixedIP: 192.168.122.100
+        fixedIP: 192.168.122.150
       - name: internalapi
         subnetName: subnet1
-        fixedIP: 172.17.0.100
+        fixedIP: 172.17.0.150
       - name: storage
         subnetName: subnet1
-        fixedIP: 172.18.0.100
+        fixedIP: 172.18.0.150
       - name: tenant
         subnetName: subnet1
-        fixedIP: 172.19.0.100
+        fixedIP: 172.19.0.150
   nodeTemplate:
     ansibleSSHPrivateKeySecret: dataplane-ansible-ssh-private-key-secret
     ansible:

tests/kuttl/tests/dataplane-multinode-nodeset-create-test/01-assert.yaml

@@ -0,0 +1,98 @@
+apiVersion: dataplane.openstack.org/v1beta1
+kind: OpenStackDataPlaneNodeSet
+metadata:
+  name: openstack-edpm-multinode
+status:
+  allHostnames:
+    edpm-compute-0:
+      ctlplane: edpm-compute-0.ctlplane.example.com
+      internalapi: edpm-compute-0.internalapi.example.com
+      storage: edpm-compute-0.storage.example.com
+      tenant: edpm-compute-0.tenant.example.com
+  allIPs:
+    edpm-compute-0:
+      ctlplane: 192.168.122.150
+      internalapi: 172.17.0.150
+      storage: 172.18.0.150
+      tenant: 172.19.0.150
+  ctlplaneSearchDomain: ctlplane.example.com
+  conditions:
+  - message: Deployment not started
+    reason: Requested
+    status: "False"
+    type: Ready
+  - message: Deployment not started
+    reason: Requested
+    status: "False"
+    type: DeploymentReady
+  - message: Input data complete
+    reason: Ready
+    status: "True"
+    type: InputReady
+  - message: NodeSetDNSDataReady ready
+    reason: Ready
+    status: "True"
+    type: NodeSetDNSDataReady
+  - message: NodeSetIPReservationReady ready
+    reason: Ready
+    status: "True"
+    type: NodeSetIPReservationReady
+  - message: ServiceAccount created
+    reason: Ready
+    status: "True"
+    type: ServiceAccountReady
+  - message: Setup complete
+    reason: Ready
+    status: "True"
+    type: SetupReady
+---
+apiVersion: network.openstack.org/v1beta1
+kind: IPSet
+metadata:
+  name: edpm-compute-0
+status:
+  conditions:
+  - message: Setup complete
+    reason: Ready
+    status: "True"
+    type: Ready
+  - message: Input data complete
+    reason: Ready
+    status: "True"
+    type: InputReady
+  - message: Reservation successful
+    reason: Ready
+    status: "True"
+    type: ReservationReady
+  reservations:
+  - address: 192.168.122.150
+    cidr: 192.168.122.0/24
+    dnsDomain: ctlplane.example.com
+    gateway: 192.168.122.1
+    mtu: 1500
+    network: ctlplane
+    routes:
+    - destination: 0.0.0.0/0
+      nexthop: 192.168.122.1
+    subnet: subnet1
+  - address: 172.17.0.150
+    cidr: 172.17.0.0/24
+    dnsDomain: internalapi.example.com
+    mtu: 1500
+    network: internalapi
+    subnet: subnet1
+    vlan: 20
+  - address: 172.18.0.150
+    cidr: 172.18.0.0/24
+    dnsDomain: storage.example.com
+    mtu: 1500
+    network: storage
+    subnet: subnet1
+    vlan: 21
+  - address: 172.19.0.150
+    cidr: 172.19.0.0/24
+    dnsDomain: tenant.example.com
+    mtu: 1500
+    network: tenant
+    subnet: subnet1
+    vlan: 22

tests/kuttl/tests/dataplane-multinode-nodeset-create-test/01-dataplane-scale-in.yaml

@@ -0,0 +1,5 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+  - script: |
+      oc patch openstackdataplanenodeset/openstack-edpm-multinode -n openstack-kuttl-tests --type json --patch '[{ "op": "remove", "path": "/spec/nodes/edpm-compute-1" }]'