add ValidateDelete method in virtualIPStrategy to prevent vip deletion

Author: Rohit-0505

client-go/openapi/zz_generated.openapi.go

@@ -77,10 +77,10 @@ func (r *NetworkInterfaceEphemeralVirtualIPReconciler) ephemeralNetworkInterface
 	annotations.SetDefaultEphemeralManagedBy(virtualIP)
 	if ephemeral.VirtualIPTemplate.Spec.ReclaimPolicy != networkingv1alpha1.ReclaimPolicyTypeRetain {
 		_ = ctrl.SetControllerReference(nic, virtualIP, r.Scheme())
-		virtualIP.Spec.TargetRef = &commonv1alpha1.LocalUIDReference{
-			Name: nic.Name,
-			UID:  nic.UID,
-		}
+	}
+	virtualIP.Spec.TargetRef = &commonv1alpha1.LocalUIDReference{
+		Name: nic.Name,
+		UID:  nic.UID,
 	}
 	res[virtualIPName] = virtualIP
 	return res

internal/controllers/networking/networkinterface_ephemeralvirtualip_controller.go

@@ -78,8 +78,8 @@ var _ = Describe("NetworkInterfaceEphemeralVirtualIP", func() {
 		})),
 		))
 	})
-	It("should verify ownerRef is set for ephemeral virtual IPs based on ReclaimPolicy", func(ctx SpecContext) {
-		By("creating a network interface with an ephemeral virtual IP")
+	It("should verify ownerRef is updated based on ReclaimPolicyType for ephemeral virtualIP", func(ctx SpecContext) {
+		By("creating a network interface with an ephemeral virtual IP having ReclaimPolicyType Retain")
 		vipSrc := networkingv1alpha1.VirtualIPSource{
 			Ephemeral: &networkingv1alpha1.EphemeralVirtualIPSource{
 				VirtualIPTemplate: &networkingv1alpha1.VirtualIPTemplateSpec{
@@ -110,28 +110,103 @@ var _ = Describe("NetworkInterfaceEphemeralVirtualIP", func() {
 		}
 		Expect(k8sClient.Create(ctx, nic)).To(Succeed())
 
-		By("waiting for the virtual IP to exist")
+		By("waiting for the virtual IP to exist with empty OwnerRef")
 		vip := &networkingv1alpha1.VirtualIP{
 			ObjectMeta: metav1.ObjectMeta{
 				Namespace: ns.Name,
 				Name:      networkingv1alpha1.NetworkInterfaceVirtualIPName(nic.Name, vipSrc),
 			},
 		}
-		Eventually(Get(vip)).Should(Succeed())
-		By("Verifying OwnerRef is not set for ephemeral virtualIP when reclaim policy is retain")
-		Eventually(Object(vip)).Should(HaveField("ObjectMeta.OwnerReferences", BeEmpty()))
+		Eventually(Object(vip)).Should(SatisfyAll(
+			HaveField("ObjectMeta.OwnerReferences", BeEmpty()),
+			HaveField("Spec", networkingv1alpha1.VirtualIPSpec{
+				Type:     networkingv1alpha1.VirtualIPTypePublic,
+				IPFamily: corev1.IPv4Protocol,
+				TargetRef: &commonv1alpha1.LocalUIDReference{
+					Name: nic.Name,
+					UID:  nic.UID,
+				},
+			}),
+		))
 
-		By("Updating reclaim policy to delete")
+		By("Updating reclaim policy to Delete")
 		baseNic := nic.DeepCopy()
 		nic.Spec.VirtualIP.Ephemeral.VirtualIPTemplate.Spec.ReclaimPolicy = networkingv1alpha1.ReclaimPolicyTypeDelete
 		Expect(k8sClient.Patch(ctx, nic, client.MergeFrom(baseNic))).To(Succeed())
-		By("Verifying OwnerRef is updated for ephemeral virtualIP")
+
+		By("Verifying ephemeral virtualIP is updated with OwnerRef after updating the ReclaimPolicyTypeDelete")
 		Eventually(Object(vip)).Should(HaveField("ObjectMeta.OwnerReferences", ConsistOf(MatchFields(IgnoreExtras, Fields{
 			"APIVersion": Equal(networkingv1alpha1.SchemeGroupVersion.String()),
 			"Kind":       Equal("NetworkInterface"),
 			"Name":       Equal(nic.Name),
 		})),
 		))
+
+	})
+
+	It("should verify ephemeral virutalIP is not deleted having ReclaimPolicyType Retain with nic deletion", func(ctx SpecContext) {
+		By("creating a network interface with an ephemeral virtual IP")
+		vipSrc := networkingv1alpha1.VirtualIPSource{
+			Ephemeral: &networkingv1alpha1.EphemeralVirtualIPSource{
+				VirtualIPTemplate: &networkingv1alpha1.VirtualIPTemplateSpec{
+					Spec: networkingv1alpha1.EphemeralVirtualIPSpec{
+						ReclaimPolicy: networkingv1alpha1.ReclaimPolicyTypeRetain,
+						VirtualIPSpec: networkingv1alpha1.VirtualIPSpec{
+							Type:     networkingv1alpha1.VirtualIPTypePublic,
+							IPFamily: corev1.IPv4Protocol,
+						},
+					},
+				},
+			},
+		}
+		nic := &networkingv1alpha1.NetworkInterface{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace:    ns.Name,
+				GenerateName: "nic-",
+			},
+			Spec: networkingv1alpha1.NetworkInterfaceSpec{
+				NetworkRef: corev1.LocalObjectReference{Name: "my-network"},
+				IPs: []networkingv1alpha1.IPSource{
+					{
+						Value: commonv1alpha1.MustParseNewIP("10.0.0.1"),
+					},
+				},
+				VirtualIP: &vipSrc,
+			},
+		}
+		Expect(k8sClient.Create(ctx, nic)).To(Succeed())
+
+		By("waiting for the virtual IP to exist")
+		vip := &networkingv1alpha1.VirtualIP{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: ns.Name,
+				Name:      networkingv1alpha1.NetworkInterfaceVirtualIPName(nic.Name, vipSrc),
+			},
+		}
+		Eventually(Object(vip)).Should(SatisfyAll(
+			HaveField("ObjectMeta.OwnerReferences", BeEmpty()),
+			HaveField("Spec", networkingv1alpha1.VirtualIPSpec{
+				Type:     networkingv1alpha1.VirtualIPTypePublic,
+				IPFamily: corev1.IPv4Protocol,
+				TargetRef: &commonv1alpha1.LocalUIDReference{
+					Name: nic.Name,
+					UID:  nic.UID,
+				},
+			}),
+		))
+
+		By("deleting nic")
+		Expect(k8sClient.Delete(ctx, nic)).To(Succeed())
+
+		By("ensuring the nic is deleted")
+		nicKey := client.ObjectKey{Namespace: ns.Name, Name: nic.Name}
+		err := k8sClient.Get(ctx, nicKey, nic)
+		Expect(apierrors.IsNotFound(err)).To(BeTrue())
+
+		By("ensuring the virtualIP still exists after the nic is deleted")
+		vipKey := client.ObjectKey{Namespace: ns.Name, Name: vip.Name}
+		Expect(k8sClient.Get(ctx, vipKey, vip)).To(Succeed())
+
 	})
 
 	It("should delete undesired virtual IPs for a network interface", func(ctx SpecContext) {

internal/controllers/networking/networkinterface_ephemeralvirtualip_controller_test.go

@@ -19,10 +19,10 @@ var _ = Describe("VirtualIPReleaseReconciler", func() {
 
 	It("should release virtual IPs whose owner is gone", func(ctx SpecContext) {
 		By("creating a virtual IP referencing an owner that does not exist")
-		nic := &networkingv1alpha1.VirtualIP{
+		vip := &networkingv1alpha1.VirtualIP{
 			ObjectMeta: metav1.ObjectMeta{
 				Namespace:    ns.Name,
-				GenerateName: "nic-",
+				GenerateName: "vip-",
 			},
 			Spec: networkingv1alpha1.VirtualIPSpec{
 				Type:     networkingv1alpha1.VirtualIPTypePublic,
@@ -33,9 +33,9 @@ var _ = Describe("VirtualIPReleaseReconciler", func() {
 				},
 			},
 		}
-		Expect(k8sClient.Create(ctx, nic)).To(Succeed())
+		Expect(k8sClient.Create(ctx, vip)).To(Succeed())
 
 		By("waiting for the virtual IP to be released")
-		Eventually(Object(nic)).Should(HaveField("Spec.TargetRef", BeNil()))
+		Eventually(Object(vip)).Should(HaveField("Spec.TargetRef", BeNil()))
 	})
 })

internal/controllers/networking/virtualip_release_controller_test.go

@@ -83,6 +83,19 @@ func (virtualIPStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Ob
 	return validation.ValidateVirtualIPUpdate(newVirtualIP, oldVirtualIP)
 }
 
+func (virtualIPStrategy) ValidateDelete(ctx context.Context, obj runtime.Object) field.ErrorList {
+	virtualIP := obj.(*networking.VirtualIP)
+	var allerr field.ErrorList
+	// Check if the VirtualIP has a TargetRef set
+	if virtualIP.Spec.TargetRef != nil {
+		// Return a validation error preventing deletion
+		err := field.Forbidden(field.NewPath("spec", "targetRef"), "deletion is not allowed while TargetRef is set")
+		allerr = append(allerr, err)
+	}
+	return allerr
+
+}
+
 func (virtualIPStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string {
 	return nil
 }