Skip to content

Commit

Permalink
iptables: remove port opener
Browse files Browse the repository at this point in the history
  • Loading branch information
@khenidak @aramase
khenidak authored and aramase committed Mar 22, 2022
1 parent f3c511c commit 39234d6
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 121 deletions.
99 changes: 1 addition & 98 deletions pkg/proxy/iptables/proxier.go
View file
Expand Up @@ -189,7 +189,6 @@ type Proxier struct {
mu sync.Mutex // protects the following fields
serviceMap proxy.ServiceMap
endpointsMap proxy.EndpointsMap
portsMap map[utilnet.LocalPort]utilnet.Closeable
nodeLabels map[string]string
// endpointsSynced, endpointSlicesSynced, and servicesSynced are set to true
// when corresponding objects are synced after startup. This is used to avoid
Expand All @@ -209,7 +208,6 @@ type Proxier struct {
localDetector proxyutiliptables.LocalTrafficDetector
hostname string
nodeIP net.IP
portMapper utilnet.PortOpener
recorder record.EventRecorder

serviceHealthServer healthcheck.ServiceHealthServer
Expand Down Expand Up @@ -298,7 +296,6 @@ func NewProxier(ipt utiliptables.Interface,
}

proxier := &Proxier{
portsMap: make(map[utilnet.LocalPort]utilnet.Closeable),
serviceMap: make(proxy.ServiceMap),
serviceChanges: proxy.NewServiceChangeTracker(newServiceInfo, ipFamily, recorder, nil),
endpointsMap: make(proxy.EndpointsMap),
Expand All @@ -311,7 +308,6 @@ func NewProxier(ipt utiliptables.Interface,
localDetector: localDetector,
hostname: hostname,
nodeIP: nodeIP,
portMapper: &utilnet.ListenPortOpener,
recorder: recorder,
serviceHealthServer: serviceHealthServer,
healthzServer: healthzServer,
Expand Down Expand Up @@ -977,9 +973,6 @@ func (proxier *Proxier) syncProxyRules() {
// Accumulate NAT chains to keep.
activeNATChains := map[utiliptables.Chain]bool{} // use a map as a set

// Accumulate the set of local ports that we will be holding open once this update is complete
replacementPortsMap := map[utilnet.LocalPort]utilnet.Closeable{}

// We are creating those slices ones here to avoid memory reallocations
// in every loop. Note that reuse the memory, instead of doing:
// slice = <some new slice>
Expand All @@ -1000,7 +993,6 @@ func (proxier *Proxier) syncProxyRules() {
proxier.endpointChainsNumber += len(proxier.endpointsMap[svcName])
}

localAddrSet := utilproxy.GetLocalAddrSet()
nodeAddresses, err := utilproxy.GetNodeAddresses(proxier.nodePortAddresses, proxier.networkInterfacer)
if err != nil {
klog.ErrorS(err, "Failed to get node ip address matching nodeport cidrs, services with nodeport may not work as intended", "CIDRs", proxier.nodePortAddresses)
Expand All @@ -1014,10 +1006,6 @@ func (proxier *Proxier) syncProxyRules() {
continue
}
isIPv6 := utilnet.IsIPv6(svcInfo.ClusterIP())
localPortIPFamily := utilnet.IPv4
if isIPv6 {
localPortIPFamily = utilnet.IPv6
}
protocol := strings.ToLower(string(svcInfo.Protocol()))
svcNameString := svcInfo.serviceNameString

Expand Down Expand Up @@ -1096,40 +1084,6 @@ func (proxier *Proxier) syncProxyRules() {

// Capture externalIPs.
for _, externalIP := range svcInfo.ExternalIPStrings() {
// If the "external" IP happens to be an IP that is local to this
// machine, hold the local port open so no other process can open it
// (because the socket might open but it would never work).
if (svcInfo.Protocol() != v1.ProtocolSCTP) && localAddrSet.Has(net.ParseIP(externalIP)) {
lp := utilnet.LocalPort{
Description: "externalIP for " + svcNameString,
IP: externalIP,
IPFamily: localPortIPFamily,
Port: svcInfo.Port(),
Protocol: utilnet.Protocol(svcInfo.Protocol()),
}
if proxier.portsMap[lp] != nil {
klog.V(4).InfoS("Port was open before and is still needed", "port", lp.String())
replacementPortsMap[lp] = proxier.portsMap[lp]
} else {
socket, err := proxier.portMapper.OpenLocalPort(&lp)
if err != nil {
msg := fmt.Sprintf("can't open %s, skipping this externalIP: %v", lp.String(), err)

proxier.recorder.Eventf(
&v1.ObjectReference{
Kind: "Node",
Name: proxier.hostname,
UID: types.UID(proxier.hostname),
Namespace: "",
}, v1.EventTypeWarning, err.Error(), msg)
klog.ErrorS(err, "can't open port, skipping externalIP", "port", lp.String())
continue
}
klog.V(2).InfoS("Opened local port", "port", lp.String())
replacementPortsMap[lp] = socket
}
}

if hasEndpoints {
args = append(args[:0],
"-A", string(kubeServicesChain),
Expand Down Expand Up @@ -1252,47 +1206,7 @@ func (proxier *Proxier) syncProxyRules() {
// Capture nodeports. If we had more than 2 rules it might be
// worthwhile to make a new per-service chain for nodeport rules, but
// with just 2 rules it ends up being a waste and a cognitive burden.
if svcInfo.NodePort() != 0 {
// Hold the local port open so no other process can open it
// (because the socket might open but it would never work).
if len(nodeAddresses) == 0 {
continue
}

lps := make([]utilnet.LocalPort, 0)
for address := range nodeAddresses {
lp := utilnet.LocalPort{
Description: "nodePort for " + svcNameString,
IP: address,
IPFamily: localPortIPFamily,
Port: svcInfo.NodePort(),
Protocol: utilnet.Protocol(svcInfo.Protocol()),
}
if utilproxy.IsZeroCIDR(address) {
// Empty IP address means all
lp.IP = ""
lps = append(lps, lp)
// If we encounter a zero CIDR, then there is no point in processing the rest of the addresses.
break
}
lps = append(lps, lp)
}

// For ports on node IPs, open the actual port and hold it.
for _, lp := range lps {
if proxier.portsMap[lp] != nil {
klog.V(4).InfoS("Port was open before and is still needed", "port", lp.String())
replacementPortsMap[lp] = proxier.portsMap[lp]
} else if svcInfo.Protocol() != v1.ProtocolSCTP {
socket, err := proxier.portMapper.OpenLocalPort(&lp)
if err != nil {
klog.ErrorS(err, "can't open port, skipping this nodePort", "port", lp.String())
continue
}
klog.V(2).InfoS("Opened local port", "port", lp.String())
replacementPortsMap[lp] = socket
}
}
if svcInfo.NodePort() != 0 && len(nodeAddresses) != 0 {

if hasEndpoints {
args = append(args[:0],
Expand Down Expand Up @@ -1614,9 +1528,6 @@ func (proxier *Proxier) syncProxyRules() {
if err != nil {
klog.ErrorS(err, "Failed to execute iptables-restore")
metrics.IptablesRestoreFailuresTotal.Inc()
// Revert new local ports.
klog.V(2).InfoS("Closing local ports after iptables-restore failure")
utilproxy.RevertPorts(replacementPortsMap, proxier.portsMap)
return
}
success = true
Expand All @@ -1629,14 +1540,6 @@ func (proxier *Proxier) syncProxyRules() {
}
}

// Close old local ports and save new ones.
for k, v := range proxier.portsMap {
if replacementPortsMap[k] == nil {
v.Close()
}
}
proxier.portsMap = replacementPortsMap

if proxier.healthzServer != nil {
proxier.healthzServer.Updated()
}
Expand Down
23 changes: 0 additions & 23 deletions pkg/proxy/iptables/proxier_test.go
View file
Expand Up @@ -51,7 +51,6 @@ import (
iptablestest "k8s.io/kubernetes/pkg/util/iptables/testing"
"k8s.io/utils/exec"
fakeexec "k8s.io/utils/exec/testing"
utilnet "k8s.io/utils/net"
utilpointer "k8s.io/utils/pointer"
)

Expand Down Expand Up @@ -461,26 +460,6 @@ func TestDeleteEndpointConnectionsIPv6(t *testing.T) {
}
}

// fakeCloseable implements utilproxy.Closeable
type fakeCloseable struct{}

// Close fakes out the close() used by syncProxyRules to release a local port.
func (f *fakeCloseable) Close() error {
return nil
}

// fakePortOpener implements portOpener.
type fakePortOpener struct {
openPorts []*utilnet.LocalPort
}

// OpenLocalPort fakes out the listen() and bind() used by syncProxyRules
// to lock a local port.
func (f *fakePortOpener) OpenLocalPort(lp *utilnet.LocalPort) (utilnet.Closeable, error) {
f.openPorts = append(f.openPorts, lp)
return &fakeCloseable{}, nil
}

const testHostname = "test-hostname"

func NewFakeProxier(ipt utiliptables.Interface, endpointSlicesEnabled bool) *Proxier {
Expand All @@ -501,8 +480,6 @@ func NewFakeProxier(ipt utiliptables.Interface, endpointSlicesEnabled bool) *Pro
masqueradeMark: "0x4000",
localDetector: detectLocal,
hostname: testHostname,
portsMap: make(map[utilnet.LocalPort]utilnet.Closeable),
portMapper: &fakePortOpener{[]*utilnet.LocalPort{}},
serviceHealthServer: healthcheck.NewFakeServiceHealthServer(),
precomputedProbabilities: make([]string, 0, 1001),
iptablesData: bytes.NewBuffer(nil),
Expand Down

0 comments on commit 39234d6

Please sign in to comment.