refactor: unregister logic for route registry

Resolves: cloudfoundry#468
Co-Authored-By: Clemens Hoffmann <[email protected]>

Author: maxmoehl

src/code.cloudfoundry.org/gorouter/registry/registry.go

@@ -172,43 +172,57 @@ func (r *RouteRegistry) Unregister(uri route.Uri, endpoint *route.Endpoint) {
 		return
 	}
 
-	r.unregister(uri, endpoint)
+	endpointRemoved, routeRemoved := r.unregister(uri, endpoint)
 
 	r.reporter.CaptureUnregistryMessage(endpoint)
 
+	// WARNING: this only works because there are only info log statements after this point. If you
+	// extend this function, this probably needs to be refactored.
+	if !r.logger.Enabled(context.Background(), slog.LevelInfo) {
+		return
+	}
+
+	if endpointRemoved {
+		r.logger.Info("endpoint-unregistered", buildSlogAttrs(uri, endpoint)...)
+	} else {
+		r.logger.Info("endpoint-not-unregistered", buildSlogAttrs(uri, endpoint)...)
+	}
+
+	if routeRemoved {
+		r.logger.Info("route-unregistered", slog.Any("uri", uri))
+	} else {
+		r.logger.Info("route-not-unregistered", slog.Any("uri", uri))
+	}
 }
 
-func (r *RouteRegistry) unregister(uri route.Uri, endpoint *route.Endpoint) {
+func (r *RouteRegistry) unregister(uri route.Uri, endpoint *route.Endpoint) (endpointRemoved, routeRemoved bool) {
 	r.Lock()
 	defer r.Unlock()
 
 	uri = uri.RouteKey()
 
 	pool := r.byURI.Find(uri)
-	if pool != nil {
-		endpointRemoved := pool.Remove(endpoint)
-		if endpointRemoved {
-			if r.logger.Enabled(context.Background(), slog.LevelInfo) {
-				r.logger.Info("endpoint-unregistered", buildSlogAttrs(uri, endpoint)...)
-			}
-		} else {
-			if r.logger.Enabled(context.Background(), slog.LevelInfo) {
-				r.logger.Info("endpoint-not-unregistered", buildSlogAttrs(uri, endpoint)...)
-			}
-		}
+	if pool == nil {
+		return false, false
+	}
 
-		if pool.IsEmpty() {
-			if r.EmptyPoolResponseCode503 && r.EmptyPoolTimeout > 0 {
-				if time.Since(pool.LastUpdated()) > r.EmptyPoolTimeout {
-					r.byURI.Delete(uri)
-					r.logger.Info("route-unregistered", slog.Any("uri", uri))
-				}
-			} else {
-				r.byURI.Delete(uri)
-				r.logger.Info("route-unregistered", slog.Any("uri", uri))
-			}
-		}
+	endpointRemoved = pool.Remove(endpoint)
+	if !endpointRemoved {
+		return false, false
 	}
+
+	if !pool.IsEmpty() {
+		return true, false
+	}
+
+	// If we have empty pool responses, the timeout for empty pools is greater than zero and the
+	// timeout of this pool has not yet expired, don't remove it yet.
+	if r.EmptyPoolResponseCode503 && r.EmptyPoolTimeout > 0 && time.Since(pool.LastUpdated()) <= r.EmptyPoolTimeout {
+		return true, false
+	}
+
+	r.byURI.Delete(uri)
+	return true, true
 }
 
 func (r *RouteRegistry) Lookup(uri route.Uri) *route.EndpointPool {

src/code.cloudfoundry.org/gorouter/registry/registry_test.go

@@ -650,7 +650,12 @@ var _ = Describe("RouteRegistry", func() {
 			Context("EmptyPoolResponseCode503 is true and EmptyPoolTimeout greater than 0", func() {
 				JustBeforeEach(func() {
 					r.EmptyPoolResponseCode503 = true
-					r.EmptyPoolTimeout = 5 * time.Second
+					r.EmptyPoolTimeout = 1 * time.Second
+					r.StartPruningCycle()
+				})
+
+				JustAfterEach(func() {
+					r.StopPruningCycle()
 				})
 
 				It("Removes the route after EmptyPoolTimeout period of time is passed", func() {
@@ -659,10 +664,7 @@ var _ = Describe("RouteRegistry", func() {
 
 					r.Unregister("bar", barEndpoint)
 					Expect(r.NumUris()).To(Equal(1))
-					time.Sleep(r.EmptyPoolTimeout)
-					r.Unregister("bar", barEndpoint)
-					Expect(r.NumUris()).To(Equal(0))
-
+					Eventually(r.NumUris).WithTimeout(r.EmptyPoolTimeout + time.Second).Should(Equal(0))
 				})
 			})