From 168602e597e5b9b06a67159316c33f69e04dd0a3 Mon Sep 17 00:00:00 2001 From: Pavithra Ramesh Date: Fri, 25 Jan 2019 11:05:18 -0800 Subject: [PATCH 1/3] Clear conntrack entries for externalIP When an endpoint is deleted, the conntrack entries are cleared for clusterIP but not for externalIP of the service. This change adds that step. --- pkg/proxy/iptables/proxier.go | 6 ++++++ pkg/proxy/ipvs/proxier.go | 6 ++++++ pkg/proxy/service.go | 7 ++++++- pkg/proxy/types.go | 4 +++- 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 1fb994be32f..568df17377a 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -611,6 +611,12 @@ func (proxier *Proxier) deleteEndpointConnections(connectionMap []proxy.ServiceE if err != nil { klog.Errorf("Failed to delete %s endpoint connections, error: %v", epSvcPair.ServicePortName.String(), err) } + for _, extIP := range svcInfo.ExternalIPStrings() { + err := conntrack.ClearEntriesForNAT(proxier.exec, extIP, endpointIP, v1.ProtocolUDP) + if err != nil { + klog.Errorf("Failed to delete %s endpoint connections for externalIP %s, error: %v", epSvcPair.ServicePortName.String(), extIP, err) + } + } } } } diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index 20fd8acca5e..4cf32068f03 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -1475,6 +1475,12 @@ func (proxier *Proxier) deleteEndpointConnections(connectionMap []proxy.ServiceE if err != nil { klog.Errorf("Failed to delete %s endpoint connections, error: %v", epSvcPair.ServicePortName.String(), err) } + for _, extIP := range svcInfo.ExternalIPStrings() { + err := conntrack.ClearEntriesForNAT(proxier.exec, extIP, endpointIP, v1.ProtocolUDP) + if err != nil { + klog.Errorf("Failed to delete %s endpoint connections for externalIP %s, error: %v", epSvcPair.ServicePortName.String(), extIP, err) + } + } } } } diff --git a/pkg/proxy/service.go b/pkg/proxy/service.go index 8386e62c0e2..bbada270a83 100644 --- a/pkg/proxy/service.go +++ b/pkg/proxy/service.go @@ -25,7 +25,7 @@ import ( "k8s.io/klog" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" @@ -74,6 +74,11 @@ func (info *BaseServiceInfo) GetHealthCheckNodePort() int { return info.HealthCheckNodePort } +// ExternalIPStrings is part of ServicePort interface. +func (info *BaseServiceInfo) ExternalIPStrings() []string { + return info.ExternalIPs +} + func (sct *ServiceChangeTracker) newBaseServiceInfo(port *v1.ServicePort, service *v1.Service) *BaseServiceInfo { onlyNodeLocalEndpoints := false if apiservice.RequestsOnlyLocalTraffic(service) { diff --git a/pkg/proxy/types.go b/pkg/proxy/types.go index f38937068c8..736c3295beb 100644 --- a/pkg/proxy/types.go +++ b/pkg/proxy/types.go @@ -19,7 +19,7 @@ package proxy import ( "fmt" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" ) @@ -50,6 +50,8 @@ type ServicePort interface { String() string // ClusterIPString returns service cluster IP in string format. ClusterIPString() string + // ExternalIPStrings returns service ExternalIPs as a string array. + ExternalIPStrings() []string // GetProtocol returns service protocol. GetProtocol() v1.Protocol // GetHealthCheckNodePort returns service health check node port if present. If return 0, it means not present. From cd2d33eaa3ab5afcf95bca4fa3b67e630ac17ac5 Mon Sep 17 00:00:00 2001 From: Pavithra Ramesh Date: Fri, 25 Jan 2019 11:17:31 -0800 Subject: [PATCH 2/3] fix import --- pkg/proxy/service.go | 2 +- pkg/proxy/types.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/proxy/service.go b/pkg/proxy/service.go index 099eb6495a4..9135312cd73 100644 --- a/pkg/proxy/service.go +++ b/pkg/proxy/service.go @@ -25,7 +25,7 @@ import ( "k8s.io/klog" - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" diff --git a/pkg/proxy/types.go b/pkg/proxy/types.go index 13c147bf3d4..c8b2ee18770 100644 --- a/pkg/proxy/types.go +++ b/pkg/proxy/types.go @@ -19,7 +19,7 @@ package proxy import ( "fmt" - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" ) From 24d3ab83dcb97c57eba65fe7b0c0b1c458a7ca82 Mon Sep 17 00:00:00 2001 From: Pavithra Ramesh Date: Wed, 13 Feb 2019 09:55:31 -0800 Subject: [PATCH 3/3] Remove conntrack entries from loadbalancer ip too. --- pkg/proxy/iptables/proxier.go | 6 ++++++ pkg/proxy/ipvs/proxier.go | 6 ++++++ pkg/proxy/service.go | 10 ++++++++++ pkg/proxy/types.go | 2 ++ 4 files changed, 24 insertions(+) diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 4eebe6cc738..e423acc6edb 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -619,6 +619,12 @@ func (proxier *Proxier) deleteEndpointConnections(connectionMap []proxy.ServiceE klog.Errorf("Failed to delete %s endpoint connections for externalIP %s, error: %v", epSvcPair.ServicePortName.String(), extIP, err) } } + for _, lbIP := range svcInfo.LoadBalancerIPStrings() { + err := conntrack.ClearEntriesForNAT(proxier.exec, lbIP, endpointIP, v1.ProtocolUDP) + if err != nil { + klog.Errorf("Failed to delete %s endpoint connections for LoabBalancerIP %s, error: %v", epSvcPair.ServicePortName.String(), lbIP, err) + } + } } } } diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index 94fabbb0215..92062168ebd 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -1499,6 +1499,12 @@ func (proxier *Proxier) deleteEndpointConnections(connectionMap []proxy.ServiceE klog.Errorf("Failed to delete %s endpoint connections for externalIP %s, error: %v", epSvcPair.ServicePortName.String(), extIP, err) } } + for _, lbIP := range svcInfo.LoadBalancerIPStrings() { + err := conntrack.ClearEntriesForNAT(proxier.exec, lbIP, endpointIP, v1.ProtocolUDP) + if err != nil { + klog.Errorf("Failed to delete %s endpoint connections for LoabBalancerIP %s, error: %v", epSvcPair.ServicePortName.String(), lbIP, err) + } + } } } } diff --git a/pkg/proxy/service.go b/pkg/proxy/service.go index 9135312cd73..1cb2f425d24 100644 --- a/pkg/proxy/service.go +++ b/pkg/proxy/service.go @@ -78,11 +78,21 @@ func (info *BaseServiceInfo) GetHealthCheckNodePort() int { func (info *BaseServiceInfo) GetNodePort() int { return info.NodePort } + // ExternalIPStrings is part of ServicePort interface. func (info *BaseServiceInfo) ExternalIPStrings() []string { return info.ExternalIPs } +// LoadBalancerIPStrings is part of ServicePort interface. +func (info *BaseServiceInfo) LoadBalancerIPStrings() []string { + var ips []string + for _, ing := range info.LoadBalancerStatus.Ingress { + ips = append(ips, ing.IP) + } + return ips +} + func (sct *ServiceChangeTracker) newBaseServiceInfo(port *v1.ServicePort, service *v1.Service) *BaseServiceInfo { onlyNodeLocalEndpoints := false if apiservice.RequestsOnlyLocalTraffic(service) { diff --git a/pkg/proxy/types.go b/pkg/proxy/types.go index c8b2ee18770..b7d8b1654a9 100644 --- a/pkg/proxy/types.go +++ b/pkg/proxy/types.go @@ -52,6 +52,8 @@ type ServicePort interface { ClusterIPString() string // ExternalIPStrings returns service ExternalIPs as a string array. ExternalIPStrings() []string + // LoadBalancerIPStrings returns service LoadBalancerIPs as a string array. + LoadBalancerIPStrings() []string // GetProtocol returns service protocol. GetProtocol() v1.Protocol // GetHealthCheckNodePort returns service health check node port if present. If return 0, it means not present.