Repository: kubernetes-sigs/apiserver-network-proxy
Pull request #603 (2024-03-31 04:28:36)

Trim duplicate code (related to default route backend storage).
DefaultBackendStorage.defaultRouteAgentIDs is duplicate with DefaultRouteBackendManager (.backends).


Patch:
diff --git a/pkg/server/backend_manager.go b/pkg/server/backend_manager.go
index 52c3af11..0dbe5150 100644
--- a/pkg/server/backend_manager.go
+++ b/pkg/server/backend_manager.go
@@ -247,9 +247,7 @@ type DefaultBackendStorage struct {
 	// randomly pick a key from a map (in this case, the backends) in
 	// Golang.
 	agentIDs []string
-	// defaultRouteAgentIDs tracks the agents that have claimed the default route.
-	defaultRouteAgentIDs []string
-	random               *rand.Rand
+	random   *rand.Rand
 	// idTypes contains the valid identifier types for this
 	// DefaultBackendStorage. The DefaultBackendStorage may only tolerate certain
 	// types of identifiers when associating to a specific BackendManager,
@@ -309,9 +307,6 @@ func (s *DefaultBackendStorage) addBackend(identifier string, idType header.Iden
 	s.backends[identifier] = []Backend{backend}
 	metrics.Metrics.SetBackendCount(len(s.backends))
 	s.agentIDs = append(s.agentIDs, identifier)
-	if idType == header.DefaultRoute {
-		s.defaultRouteAgentIDs = append(s.defaultRouteAgentIDs, identifier)
-	}
 }
 
 // removeBackend removes a backend.
@@ -347,14 +342,6 @@ func (s *DefaultBackendStorage) removeBackend(identifier string, idType header.I
 				break
 			}
 		}
-		if idType == header.DefaultRoute {
-			for i := range s.defaultRouteAgentIDs {
-				if s.defaultRouteAgentIDs[i] == identifier {
-					s.defaultRouteAgentIDs = append(s.defaultRouteAgentIDs[:i], s.defaultRouteAgentIDs[i+1:]...)
-					break
-				}
-			}
-		}
 	}
 	if !found {
 		klog.V(1).InfoS("Could not find connection matching identifier to remove", "agentID", identifier, "idType", idType)


Test patch:
diff --git a/pkg/server/backend_manager_test.go b/pkg/server/backend_manager_test.go
index 1c90cae8..783a8b1a 100644
--- a/pkg/server/backend_manager_test.go
+++ b/pkg/server/backend_manager_test.go
@@ -121,16 +121,12 @@ func TestDefaultBackendManager_AddRemoveBackends(t *testing.T) {
 	p.RemoveBackend(backend1)
 	expectedBackends := make(map[string][]Backend)
 	expectedAgentIDs := []string{}
-	expectedDefaultRouteAgentIDs := []string(nil)
 	if e, a := expectedBackends, p.backends; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %v, got %v", e, a)
 	}
 	if e, a := expectedAgentIDs, p.agentIDs; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %v, got %v", e, a)
 	}
-	if e, a := expectedDefaultRouteAgentIDs, p.defaultRouteAgentIDs; !reflect.DeepEqual(e, a) {
-		t.Errorf("expected %v, got %v", e, a)
-	}
 
 	p = NewDefaultBackendManager()
 	p.AddBackend(backend1)
@@ -148,16 +144,12 @@ func TestDefaultBackendManager_AddRemoveBackends(t *testing.T) {
 		"agent3": {backend3},
 	}
 	expectedAgentIDs = []string{"agent1", "agent3"}
-	expectedDefaultRouteAgentIDs = []string(nil)
 	if e, a := expectedBackends, p.backends; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %v, got %v", e, a)
 	}
 	if e, a := expectedAgentIDs, p.agentIDs; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %v, got %v", e, a)
 	}
-	if e, a := expectedDefaultRouteAgentIDs, p.defaultRouteAgentIDs; !reflect.DeepEqual(e, a) {
-		t.Errorf("expected %v, got %v", e, a)
-	}
 }
 
 func TestDefaultRouteBackendManager_AddRemoveBackends(t *testing.T) {
@@ -176,16 +168,12 @@ func TestDefaultRouteBackendManager_AddRemoveBackends(t *testing.T) {
 	p.RemoveBackend(backend1)
 	expectedBackends := make(map[string][]Backend)
 	expectedAgentIDs := []string{}
-	expectedDefaultRouteAgentIDs := []string{}
 	if e, a := expectedBackends, p.backends; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %v, got %v", e, a)
 	}
 	if e, a := expectedAgentIDs, p.agentIDs; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %v, got %v", e, a)
 	}
-	if e, a := expectedDefaultRouteAgentIDs, p.defaultRouteAgentIDs; !reflect.DeepEqual(e, a) {
-		t.Errorf("expected %v, got %v", e, a)
-	}
 
 	p = NewDefaultRouteBackendManager()
 	p.AddBackend(backend1)
@@ -204,7 +192,6 @@ func TestDefaultRouteBackendManager_AddRemoveBackends(t *testing.T) {
 		"agent3": {backend3},
 	}
 	expectedAgentIDs = []string{"agent1", "agent3"}
-	expectedDefaultRouteAgentIDs = []string{"agent1", "agent3"}
 
 	if e, a := expectedBackends, p.backends; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %v, got %v", e, a)
@@ -212,9 +199,6 @@ func TestDefaultRouteBackendManager_AddRemoveBackends(t *testing.T) {
 	if e, a := expectedAgentIDs, p.agentIDs; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %v, got %v", e, a)
 	}
-	if e, a := expectedDefaultRouteAgentIDs, p.defaultRouteAgentIDs; !reflect.DeepEqual(e, a) {
-		t.Errorf("expected %v, got %v", e, a)
-	}
 }
 
 func TestDestHostBackendManager_AddRemoveBackends(t *testing.T) {
@@ -233,16 +217,12 @@ func TestDestHostBackendManager_AddRemoveBackends(t *testing.T) {
 	p.RemoveBackend(backend1)
 	expectedBackends := make(map[string][]Backend)
 	expectedAgentIDs := []string{}
-	expectedDefaultRouteAgentIDs := []string(nil)
 	if e, a := expectedBackends, p.backends; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %v, got %v", e, a)
 	}
 	if e, a := expectedAgentIDs, p.agentIDs; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %v, got %v", e, a)
 	}
-	if e, a := expectedDefaultRouteAgentIDs, p.defaultRouteAgentIDs; !reflect.DeepEqual(e, a) {
-		t.Errorf("expected %v, got %v", e, a)
-	}
 
 	p = NewDestHostBackendManager()
 	p.AddBackend(backend1)
@@ -266,9 +246,6 @@ func TestDestHostBackendManager_AddRemoveBackends(t *testing.T) {
 	if e, a := expectedAgentIDs, p.agentIDs; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %v, got %v", e, a)
 	}
-	if e, a := expectedDefaultRouteAgentIDs, p.defaultRouteAgentIDs; !reflect.DeepEqual(e, a) {
-		t.Errorf("expected %v, got %v", e, a)
-	}
 
 	p.AddBackend(backend2)
 	p.AddBackend(backend3)
@@ -291,7 +268,6 @@ func TestDestHostBackendManager_AddRemoveBackends(t *testing.T) {
 		"::",
 		"node2.mydomain.com",
 	}
-	expectedDefaultRouteAgentIDs = []string(nil)
 
 	if e, a := expectedBackends, p.backends; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %v, got %v", e, a)
@@ -299,9 +275,6 @@ func TestDestHostBackendManager_AddRemoveBackends(t *testing.T) {
 	if e, a := expectedAgentIDs, p.agentIDs; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %v, got %v", e, a)
 	}
-	if e, a := expectedDefaultRouteAgentIDs, p.defaultRouteAgentIDs; !reflect.DeepEqual(e, a) {
-		t.Errorf("expected %v, got %v", e, a)
-	}
 
 	p.RemoveBackend(backend2)
 	p.RemoveBackend(backend1)
@@ -323,9 +296,6 @@ func TestDestHostBackendManager_AddRemoveBackends(t *testing.T) {
 	if e, a := expectedAgentIDs, p.agentIDs; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %v, got %v", e, a)
 	}
-	if e, a := expectedDefaultRouteAgentIDs, p.defaultRouteAgentIDs; !reflect.DeepEqual(e, a) {
-		t.Errorf("expected %v, got %v", e, a)
-	}
 
 	p.RemoveBackend(backend3)
 	expectedBackends = map[string][]Backend{}
@@ -337,9 +307,6 @@ func TestDestHostBackendManager_AddRemoveBackends(t *testing.T) {
 	if e, a := expectedAgentIDs, p.agentIDs; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %v, got %v", e, a)
 	}
-	if e, a := expectedDefaultRouteAgentIDs, p.defaultRouteAgentIDs; !reflect.DeepEqual(e, a) {
-		t.Errorf("expected %v, got %v", e, a)
-	}
 }
 
 func TestDestHostBackendManager_WithDuplicateIdents(t *testing.T) {
@@ -374,7 +341,6 @@ func TestDestHostBackendManager_WithDuplicateIdents(t *testing.T) {
 		"::",
 		"node2.mydomain.com",
 	}
-	expectedDefaultRouteAgentIDs := []string(nil)
 
 	if e, a := expectedBackends, p.backends; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %v, got %v", e, a)
@@ -382,9 +348,6 @@ func TestDestHostBackendManager_WithDuplicateIdents(t *testing.T) {
 	if e, a := expectedAgentIDs, p.agentIDs; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %v, got %v", e, a)
 	}
-	if e, a := expectedDefaultRouteAgentIDs, p.defaultRouteAgentIDs; !reflect.DeepEqual(e, a) {
-		t.Errorf("expected %v, got %v", e, a)
-	}
 
 	p.RemoveBackend(backend1)
 	p.RemoveBackend(backend3)
@@ -408,9 +371,6 @@ func TestDestHostBackendManager_WithDuplicateIdents(t *testing.T) {
 	if e, a := expectedAgentIDs, p.agentIDs; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %v, got %v", e, a)
 	}
-	if e, a := expectedDefaultRouteAgentIDs, p.defaultRouteAgentIDs; !reflect.DeepEqual(e, a) {
-		t.Errorf("expected %v, got %v", e, a)
-	}
 
 	p.RemoveBackend(backend2)
 	expectedBackends = map[string][]Backend{}
@@ -422,7 +382,4 @@ func TestDestHostBackendManager_WithDuplicateIdents(t *testing.T) {
 	if e, a := expectedAgentIDs, p.agentIDs; !reflect.DeepEqual(e, a) {
 		t.Errorf("expected %v, got %v", e, a)
 	}
-	if e, a := expectedDefaultRouteAgentIDs, p.defaultRouteAgentIDs; !reflect.DeepEqual(e, a) {
-		t.Errorf("expected %v, got %v", e, a)
-	}
 }