From 5af5a2ac7d40a4670ed2789c92cef36c43549eb0 Mon Sep 17 00:00:00 2001 From: jornshen Date: Mon, 11 Jan 2021 10:18:20 +0800 Subject: [PATCH] migrate proxy.UpdateServiceMap to be a method of ServiceMap --- pkg/proxy/iptables/proxier.go | 2 +- pkg/proxy/iptables/proxier_test.go | 20 ++++++++++---------- pkg/proxy/ipvs/proxier.go | 2 +- pkg/proxy/ipvs/proxier_test.go | 16 ++++++++-------- pkg/proxy/service.go | 8 ++++---- pkg/proxy/service_test.go | 18 +++++++++--------- pkg/proxy/winkernel/proxier.go | 2 +- 7 files changed, 34 insertions(+), 34 deletions(-) diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 615baa8ed76..a3ba549e89e 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -846,7 +846,7 @@ func (proxier *Proxier) syncProxyRules() { // We assume that if this was called, we really want to sync them, // even if nothing changed in the meantime. In other words, callers are // responsible for detecting no-op changes and not calling this function. - serviceUpdateResult := proxy.UpdateServiceMap(proxier.serviceMap, proxier.serviceChanges) + serviceUpdateResult := proxier.serviceMap.Update(proxier.serviceChanges) endpointUpdateResult := proxier.endpointsMap.Update(proxier.endpointsChanges) staleServices := serviceUpdateResult.UDPStaleClusterIP diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index 0cae52809dd..aaebb517236 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/pkg/proxy/iptables/proxier_test.go @@ -252,7 +252,7 @@ func TestDeleteEndpointConnectionsIPv4(t *testing.T) { }), ) - proxy.UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + fp.serviceMap.Update(fp.serviceChanges) } // Run the test cases @@ -395,7 +395,7 @@ func TestDeleteEndpointConnectionsIPv6(t *testing.T) { }), ) - proxy.UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + fp.serviceMap.Update(fp.serviceChanges) } // Run the test cases @@ -1416,7 +1416,7 @@ func TestBuildServiceMapAddRemove(t *testing.T) { for i := range services { fp.OnServiceAdd(services[i]) } - result := proxy.UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result := fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 10 { t.Errorf("expected service map length 10, got %v", fp.serviceMap) } @@ -1449,7 +1449,7 @@ func TestBuildServiceMapAddRemove(t *testing.T) { fp.OnServiceDelete(services[2]) fp.OnServiceDelete(services[3]) - result = proxy.UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result = fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 1 { t.Errorf("expected service map length 1, got %v", fp.serviceMap) } @@ -1489,7 +1489,7 @@ func TestBuildServiceMapServiceHeadless(t *testing.T) { ) // Headless service should be ignored - result := proxy.UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result := fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 0 { t.Errorf("expected service map length 0, got %d", len(fp.serviceMap)) } @@ -1517,7 +1517,7 @@ func TestBuildServiceMapServiceTypeExternalName(t *testing.T) { }), ) - result := proxy.UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result := fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 0 { t.Errorf("expected service map length 0, got %v", fp.serviceMap) } @@ -1557,7 +1557,7 @@ func TestBuildServiceMapServiceUpdate(t *testing.T) { fp.OnServiceAdd(servicev1) - result := proxy.UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result := fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 2 { t.Errorf("expected service map length 2, got %v", fp.serviceMap) } @@ -1571,7 +1571,7 @@ func TestBuildServiceMapServiceUpdate(t *testing.T) { // Change service to load-balancer fp.OnServiceUpdate(servicev1, servicev2) - result = proxy.UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result = fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 2 { t.Errorf("expected service map length 2, got %v", fp.serviceMap) } @@ -1585,7 +1585,7 @@ func TestBuildServiceMapServiceUpdate(t *testing.T) { // No change; make sure the service map stays the same and there are // no health-check changes fp.OnServiceUpdate(servicev2, servicev2) - result = proxy.UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result = fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 2 { t.Errorf("expected service map length 2, got %v", fp.serviceMap) } @@ -1598,7 +1598,7 @@ func TestBuildServiceMapServiceUpdate(t *testing.T) { // And back to ClusterIP fp.OnServiceUpdate(servicev2, servicev1) - result = proxy.UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result = fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 2 { t.Errorf("expected service map length 2, got %v", fp.serviceMap) } diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index d5f281ed5b0..5cdc9ffd873 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -1056,7 +1056,7 @@ func (proxier *Proxier) syncProxyRules() { // We assume that if this was called, we really want to sync them, // even if nothing changed in the meantime. In other words, callers are // responsible for detecting no-op changes and not calling this function. - serviceUpdateResult := proxy.UpdateServiceMap(proxier.serviceMap, proxier.serviceChanges) + serviceUpdateResult := proxier.serviceMap.Update(proxier.serviceChanges) endpointUpdateResult := proxier.endpointsMap.Update(proxier.endpointsChanges) staleServices := serviceUpdateResult.UDPStaleClusterIP diff --git a/pkg/proxy/ipvs/proxier_test.go b/pkg/proxy/ipvs/proxier_test.go index e3d1cd5cd22..8b7c602a1e3 100644 --- a/pkg/proxy/ipvs/proxier_test.go +++ b/pkg/proxy/ipvs/proxier_test.go @@ -2251,7 +2251,7 @@ func TestBuildServiceMapAddRemove(t *testing.T) { for i := range services { fp.OnServiceAdd(services[i]) } - result := proxy.UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result := fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 12 { t.Errorf("expected service map length 12, got %v", fp.serviceMap) } @@ -2284,7 +2284,7 @@ func TestBuildServiceMapAddRemove(t *testing.T) { fp.OnServiceDelete(services[2]) fp.OnServiceDelete(services[3]) - result = proxy.UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result = fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 1 { t.Errorf("expected service map length 1, got %v", fp.serviceMap) } @@ -2331,7 +2331,7 @@ func TestBuildServiceMapServiceHeadless(t *testing.T) { ) // Headless service should be ignored - result := proxy.UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result := fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 0 { t.Errorf("expected service map length 0, got %d", len(fp.serviceMap)) } @@ -2361,7 +2361,7 @@ func TestBuildServiceMapServiceTypeExternalName(t *testing.T) { }), ) - result := proxy.UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result := fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 0 { t.Errorf("expected service map length 0, got %v", fp.serviceMap) } @@ -2403,7 +2403,7 @@ func TestBuildServiceMapServiceUpdate(t *testing.T) { fp.OnServiceAdd(servicev1) - result := proxy.UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result := fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 2 { t.Errorf("expected service map length 2, got %v", fp.serviceMap) } @@ -2417,7 +2417,7 @@ func TestBuildServiceMapServiceUpdate(t *testing.T) { // Change service to load-balancer fp.OnServiceUpdate(servicev1, servicev2) - result = proxy.UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result = fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 2 { t.Errorf("expected service map length 2, got %v", fp.serviceMap) } @@ -2431,7 +2431,7 @@ func TestBuildServiceMapServiceUpdate(t *testing.T) { // No change; make sure the service map stays the same and there are // no health-check changes fp.OnServiceUpdate(servicev2, servicev2) - result = proxy.UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result = fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 2 { t.Errorf("expected service map length 2, got %v", fp.serviceMap) } @@ -2444,7 +2444,7 @@ func TestBuildServiceMapServiceUpdate(t *testing.T) { // And back to ClusterIP fp.OnServiceUpdate(servicev2, servicev1) - result = proxy.UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result = fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 2 { t.Errorf("expected service map length 2, got %v", fp.serviceMap) } diff --git a/pkg/proxy/service.go b/pkg/proxy/service.go index 84c97124857..32b1d0a74eb 100644 --- a/pkg/proxy/service.go +++ b/pkg/proxy/service.go @@ -292,15 +292,15 @@ type UpdateServiceMapResult struct { UDPStaleClusterIP sets.String } -// UpdateServiceMap updates ServiceMap based on the given changes. -func UpdateServiceMap(serviceMap ServiceMap, changes *ServiceChangeTracker) (result UpdateServiceMapResult) { +// Update updates ServiceMap base on the given changes. +func (sm ServiceMap) Update(changes *ServiceChangeTracker) (result UpdateServiceMapResult) { result.UDPStaleClusterIP = sets.NewString() - serviceMap.apply(changes, result.UDPStaleClusterIP) + sm.apply(changes, result.UDPStaleClusterIP) // TODO: If this will appear to be computationally expensive, consider // computing this incrementally similarly to serviceMap. result.HCServiceNodePorts = make(map[types.NamespacedName]uint16) - for svcPortName, info := range serviceMap { + for svcPortName, info := range sm { if info.HealthCheckNodePort() != 0 { result.HCServiceNodePorts[svcPortName.NamespacedName] = uint16(info.HealthCheckNodePort()) } diff --git a/pkg/proxy/service_test.go b/pkg/proxy/service_test.go index 0329b8ace31..ebf4002c787 100644 --- a/pkg/proxy/service_test.go +++ b/pkg/proxy/service_test.go @@ -538,7 +538,7 @@ func (fake *FakeProxier) deleteService(service *v1.Service) { fake.serviceChanges.Update(service, nil) } -func TestUpdateServiceMapHeadless(t *testing.T) { +func TestServiceMapUpdateHeadless(t *testing.T) { fp := newFakeProxier(v1.IPv4Protocol) makeServiceMap(fp, @@ -554,7 +554,7 @@ func TestUpdateServiceMapHeadless(t *testing.T) { ) // Headless service should be ignored - result := UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result := fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 0 { t.Errorf("expected service map length 0, got %d", len(fp.serviceMap)) } @@ -581,7 +581,7 @@ func TestUpdateServiceTypeExternalName(t *testing.T) { }), ) - result := UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result := fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 0 { t.Errorf("expected service map length 0, got %v", fp.serviceMap) } @@ -641,7 +641,7 @@ func TestBuildServiceMapAddRemove(t *testing.T) { for i := range services { fp.addService(services[i]) } - result := UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result := fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 8 { t.Errorf("expected service map length 2, got %v", fp.serviceMap) } @@ -674,7 +674,7 @@ func TestBuildServiceMapAddRemove(t *testing.T) { fp.deleteService(services[2]) fp.deleteService(services[3]) - result = UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result = fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 1 { t.Errorf("expected service map length 1, got %v", fp.serviceMap) } @@ -723,7 +723,7 @@ func TestBuildServiceMapServiceUpdate(t *testing.T) { fp.addService(servicev1) - result := UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result := fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 2 { t.Errorf("expected service map length 2, got %v", fp.serviceMap) } @@ -737,7 +737,7 @@ func TestBuildServiceMapServiceUpdate(t *testing.T) { // Change service to load-balancer fp.updateService(servicev1, servicev2) - result = UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result = fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 2 { t.Errorf("expected service map length 2, got %v", fp.serviceMap) } @@ -751,7 +751,7 @@ func TestBuildServiceMapServiceUpdate(t *testing.T) { // No change; make sure the service map stays the same and there are // no health-check changes fp.updateService(servicev2, servicev2) - result = UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result = fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 2 { t.Errorf("expected service map length 2, got %v", fp.serviceMap) } @@ -764,7 +764,7 @@ func TestBuildServiceMapServiceUpdate(t *testing.T) { // And back to ClusterIP fp.updateService(servicev2, servicev1) - result = UpdateServiceMap(fp.serviceMap, fp.serviceChanges) + result = fp.serviceMap.Update(fp.serviceChanges) if len(fp.serviceMap) != 2 { t.Errorf("expected service map length 2, got %v", fp.serviceMap) } diff --git a/pkg/proxy/winkernel/proxier.go b/pkg/proxy/winkernel/proxier.go index 47b449e3714..dcd6489d2ed 100644 --- a/pkg/proxy/winkernel/proxier.go +++ b/pkg/proxy/winkernel/proxier.go @@ -941,7 +941,7 @@ func (proxier *Proxier) syncProxyRules() { // We assume that if this was called, we really want to sync them, // even if nothing changed in the meantime. In other words, callers are // responsible for detecting no-op changes and not calling this function. - serviceUpdateResult := proxy.UpdateServiceMap(proxier.serviceMap, proxier.serviceChanges) + serviceUpdateResult := proxier.serviceMap.Update(proxier.serviceChanges) endpointUpdateResult := proxier.endpointsMap.Update(proxier.endpointsChanges) staleServices := serviceUpdateResult.UDPStaleClusterIP