Clean up Endpoint kind of ResourceExport (#3652)

If Service is deleted after it's exported successfully, Endpoint kind of ResourceExport is not deleted. Fix this bug to delete both Service and Endpoint kinds of ResourceExport when a Service is deleted but ServiceExport is still kept. Signed-off-by: Lan Luo <luola@vmware.com>
antrea-io · Apr 26, 2022 · be1116e · be1116e
1 parent 052987a
commit be1116e
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 15 deletions.
diff --git a/multicluster/controllers/multicluster/serviceexport_controller.go b/multicluster/controllers/multicluster/serviceexport_controller.go
@@ -185,41 +185,43 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	r.leaderNamespace = remoteCluster.GetNamespace()
 	r.leaderClusterID = string(remoteCluster.GetClusterID())
 
-	svc := &corev1.Service{}
-	if err := r.Client.Get(ctx, req.NamespacedName, &svcExport); err != nil {
-		klog.V(2).ErrorS(err, "Unable to fetch ServiceExport", "serviceexport", req.String())
-		if !apierrors.IsNotFound(err) {
-			return ctrl.Result{}, err
-		}
+	cleanup := func() error {
 		if svcInstalled {
 			err = r.handleServiceDeleteEvent(ctx, req, remoteCluster)
 			if err != nil {
-				return ctrl.Result{}, err
+				return err
 			}
 			r.installedSvcs.Delete(svcObj)
 		}
-
 		if epInstalled {
 			err = r.handleEndpointDeleteEvent(ctx, req, remoteCluster)
 			if err != nil {
-				return ctrl.Result{}, err
+				return err
 			}
 			r.installedEps.Delete(epObj)
 		}
+		return nil
+	}
+
+	if err := r.Client.Get(ctx, req.NamespacedName, &svcExport); err != nil {
+		if !apierrors.IsNotFound(err) {
+			klog.ErrorS(err, "Unable to fetch ServiceExport", "serviceexport", req.String())
+			return ctrl.Result{}, err
+		}
+		if err := cleanup(); err != nil {
+			return ctrl.Result{}, err
+		}
 		return ctrl.Result{}, nil
 	}
 
 	// if corresponding Service doesn't exist, update ServiceExport's status reason to not_found_service,
 	// and clean up remote ResourceExport if it's an installed Service.
+	svc := &corev1.Service{}
 	err = r.Client.Get(ctx, req.NamespacedName, svc)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
-			if svcInstalled {
-				err = r.handleServiceDeleteEvent(ctx, req, remoteCluster)
-				if err != nil {
-					return ctrl.Result{}, err
-				}
-				r.installedSvcs.Delete(svcObj)
+			if err := cleanup(); err != nil {
+				return ctrl.Result{}, err
 			}
 			err = r.updateSvcExportStatus(ctx, req, notFound)
 			if err != nil {

diff --git a/multicluster/test/integration/serviceexport_controller_test.go b/multicluster/test/integration/serviceexport_controller_test.go
@@ -230,4 +230,69 @@ var _ = Describe("ServiceExport controller", func() {
 			return apierrors.IsNotFound(err)
 		}, timeout, interval).Should(BeTrue())
 	})
+
+	It("Should update existing ServiceExport status when corresponding Service is removed", func() {
+		By("By deleting a Service")
+		svc := &corev1.Service{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "nginx-svc-deleted",
+				Namespace: testNamespace,
+			},
+			Spec: svcSpec,
+		}
+		ep := &corev1.Endpoints{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "nginx-svc-deleted",
+				Namespace: testNamespace,
+			},
+			Subsets: []corev1.EndpointSubset{
+				{
+					Addresses: []corev1.EndpointAddress{
+						{
+							IP:       "192.168.170.11",
+							Hostname: "pod1",
+						},
+					},
+					Ports: epPorts,
+				},
+			},
+		}
+		svcResExportName := LocalClusterID + "-" + svc.Namespace + "-" + svc.Name + "-service"
+		epResExportName := LocalClusterID + "-" + ep.Namespace + "-" + ep.Name + "-endpoints"
+
+		svcExportDeletedService := &k8smcsv1alpha1.ServiceExport{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "nginx-svc-deleted",
+				Namespace: testNamespace,
+			},
+		}
+		Expect(k8sClient.Create(ctx, svc)).Should(Succeed())
+		Expect(k8sClient.Create(ctx, ep)).Should(Succeed())
+		Expect(k8sClient.Create(ctx, svcExportDeletedService)).Should(Succeed())
+		time.Sleep(2 * time.Second)
+
+		err := k8sClient.Delete(ctx, svc)
+		Expect(err).ToNot(HaveOccurred())
+		time.Sleep(2 * time.Second)
+
+		latestsvcExportDeletedService := &k8smcsv1alpha1.ServiceExport{}
+		err = k8sClient.Get(ctx, types.NamespacedName{
+			Namespace: svcExportDeletedService.Namespace,
+			Name:      svcExportDeletedService.Name,
+		}, latestsvcExportDeletedService)
+		Expect(err).ToNot(HaveOccurred())
+		conditions := latestsvcExportDeletedService.Status.Conditions
+		Expect(len(conditions)).Should(Equal(1))
+		Expect(*conditions[0].Message).Should(Equal("the Service does not exist"))
+
+		resExp := &mcsv1alpha1.ResourceExport{}
+		Eventually(func() bool {
+			err = k8sClient.Get(ctx, types.NamespacedName{Namespace: LeaderNamespace, Name: svcResExportName}, resExp)
+			return apierrors.IsNotFound(err)
+		}, timeout, interval).Should(BeTrue())
+		Eventually(func() bool {
+			err = k8sClient.Get(ctx, types.NamespacedName{Namespace: LeaderNamespace, Name: epResExportName}, resExp)
+			return apierrors.IsNotFound(err)
+		}, timeout, interval).Should(BeTrue())
+	})
 })