From cdce2d0ef98b9f46d8efa50de2c8f3ccd5af8bd0 Mon Sep 17 00:00:00 2001 From: Vallery Lancey Date: Wed, 3 Apr 2019 15:27:21 -0700 Subject: [PATCH 1/6] Removed cleanup for non-current kube-proxy modes in newProxyServer() --- cmd/kube-proxy/app/server_others.go | 30 +--------------------------- cmd/kube-proxy/app/server_windows.go | 2 -- 2 files changed, 1 insertion(+), 31 deletions(-) diff --git a/cmd/kube-proxy/app/server_others.go b/cmd/kube-proxy/app/server_others.go index d8bd8614e78..84d5ea0d07d 100644 --- a/cmd/kube-proxy/app/server_others.go +++ b/cmd/kube-proxy/app/server_others.go @@ -53,13 +53,12 @@ import ( // NewProxyServer returns a new ProxyServer. func NewProxyServer(o *Options) (*ProxyServer, error) { - return newProxyServer(o.config, o.CleanupAndExit, o.CleanupIPVS, o.scheme, o.master) + return newProxyServer(o.config, o.CleanupAndExit, o.scheme, o.master) } func newProxyServer( config *proxyconfigapi.KubeProxyConfiguration, cleanupAndExit bool, - cleanupIPVS bool, scheme *runtime.Scheme, master string) (*ProxyServer, error) { @@ -174,17 +173,6 @@ func newProxyServer( proxier = proxierIPTables serviceEventHandler = proxierIPTables endpointsEventHandler = proxierIPTables - // No turning back. Remove artifacts that might still exist from the userspace Proxier. - klog.V(0).Info("Tearing down inactive rules.") - // TODO this has side effects that should only happen when Run() is invoked. - userspace.CleanupLeftovers(iptInterface) - // IPVS Proxier will generate some iptables rules, need to clean them before switching to other proxy mode. - // Besides, ipvs proxier will create some ipvs rules as well. Because there is no way to tell if a given - // ipvs rule is created by IPVS proxier or not. Users should explicitly specify `--clean-ipvs=true` to flush - // all ipvs rules when kube-proxy start up. Users do this operation should be with caution. - if canUseIPVS { - ipvs.CleanupLeftovers(ipvsInterface, iptInterface, ipsetInterface, cleanupIPVS) - } } else if proxyMode == proxyModeIPVS { klog.V(0).Info("Using ipvs Proxier.") proxierIPVS, err := ipvs.NewProxier( @@ -214,10 +202,6 @@ func newProxyServer( proxier = proxierIPVS serviceEventHandler = proxierIPVS endpointsEventHandler = proxierIPVS - klog.V(0).Info("Tearing down inactive rules.") - // TODO this has side effects that should only happen when Run() is invoked. - userspace.CleanupLeftovers(iptInterface) - iptables.CleanupLeftovers(iptInterface) } else { klog.V(0).Info("Using userspace Proxier.") // This is a proxy.LoadBalancer which NewProxier needs but has methods we don't need for @@ -243,18 +227,6 @@ func newProxyServer( } serviceEventHandler = proxierUserspace proxier = proxierUserspace - - // Remove artifacts from the iptables and ipvs Proxier, if not on Windows. - klog.V(0).Info("Tearing down inactive rules.") - // TODO this has side effects that should only happen when Run() is invoked. - iptables.CleanupLeftovers(iptInterface) - // IPVS Proxier will generate some iptables rules, need to clean them before switching to other proxy mode. - // Besides, ipvs proxier will create some ipvs rules as well. Because there is no way to tell if a given - // ipvs rule is created by IPVS proxier or not. Users should explicitly specify `--clean-ipvs=true` to flush - // all ipvs rules when kube-proxy start up. Users do this operation should be with caution. - if canUseIPVS { - ipvs.CleanupLeftovers(ipvsInterface, iptInterface, ipsetInterface, cleanupIPVS) - } } iptInterface.AddReloadFunc(proxier.Sync) diff --git a/cmd/kube-proxy/app/server_windows.go b/cmd/kube-proxy/app/server_windows.go index 5aba6cf665a..596796cbbe1 100644 --- a/cmd/kube-proxy/app/server_windows.go +++ b/cmd/kube-proxy/app/server_windows.go @@ -144,8 +144,6 @@ func newProxyServer(config *proxyconfigapi.KubeProxyConfiguration, cleanupAndExi } proxier = proxierUserspace serviceEventHandler = proxierUserspace - klog.V(0).Info("Tearing down pure-winkernel proxy rules.") - winkernel.CleanupLeftovers() } return &ProxyServer{ From 4f1bb2bd2fc5b792e8b63986257d2f9bee1f96cf Mon Sep 17 00:00:00 2001 From: Vallery Lancey Date: Thu, 4 Apr 2019 12:51:36 -0700 Subject: [PATCH 2/6] Depricated --cleanup-ipvs flag in kube-proxy --- cmd/kube-proxy/app/server.go | 10 +++------- pkg/proxy/ipvs/proxier.go | 25 ++++++++++++------------- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/cmd/kube-proxy/app/server.go b/cmd/kube-proxy/app/server.go index 51f1a2e63f8..15a0256a8ea 100644 --- a/cmd/kube-proxy/app/server.go +++ b/cmd/kube-proxy/app/server.go @@ -100,10 +100,8 @@ type Options struct { ConfigFile string // WriteConfigTo is the path where the default configuration will be written. WriteConfigTo string - // CleanupAndExit, when true, makes the proxy server clean up iptables rules, then exit. + // CleanupAndExit, when true, makes the proxy server clean up iptables, ipvs, etc rules, then exit. CleanupAndExit bool - // CleanupIPVS, when true, makes the proxy server clean up ipvs rules before running. - CleanupIPVS bool // WindowsService should be set to true if kube-proxy is running as a service on Windows. // Its corresponding flag only gets registered in Windows builds WindowsService bool @@ -143,7 +141,7 @@ func (o *Options) AddFlags(fs *pflag.FlagSet) { fs.BoolVar(&o.CleanupAndExit, "cleanup-iptables", o.CleanupAndExit, "If true cleanup iptables and ipvs rules and exit.") fs.MarkDeprecated("cleanup-iptables", "This flag is replaced by --cleanup.") fs.BoolVar(&o.CleanupAndExit, "cleanup", o.CleanupAndExit, "If true cleanup iptables and ipvs rules and exit.") - fs.BoolVar(&o.CleanupIPVS, "cleanup-ipvs", o.CleanupIPVS, "If true make kube-proxy cleanup ipvs rules before running. Default is true") + fs.MarkDeprecated("cleanup-ipvs", "This flag is replaced by --cleanup.") // All flags below here are deprecated and will eventually be removed. @@ -204,7 +202,6 @@ func NewOptions() *Options { metricsPort: ports.ProxyStatusPort, scheme: scheme.Scheme, codecs: scheme.Codecs, - CleanupIPVS: true, errCh: make(chan error), } } @@ -502,7 +499,6 @@ type ProxyServer struct { ProxyMode string NodeRef *v1.ObjectReference CleanupAndExit bool - CleanupIPVS bool MetricsBindAddress string EnableProfiling bool OOMScoreAdj *int32 @@ -559,7 +555,7 @@ func (s *ProxyServer) Run() error { if s.CleanupAndExit { encounteredError := userspace.CleanupLeftovers(s.IptInterface) encounteredError = iptables.CleanupLeftovers(s.IptInterface) || encounteredError - encounteredError = ipvs.CleanupLeftovers(s.IpvsInterface, s.IptInterface, s.IpsetInterface, s.CleanupIPVS) || encounteredError + encounteredError = ipvs.CleanupLeftovers(s.IpvsInterface, s.IptInterface, s.IpsetInterface) || encounteredError if encounteredError { return errors.New("encountered an error while tearing down rules") } diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index 64742868557..9c88ab0d088 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -595,22 +595,21 @@ func cleanupIptablesLeftovers(ipt utiliptables.Interface) (encounteredError bool } // CleanupLeftovers clean up all ipvs and iptables rules created by ipvs Proxier. -func CleanupLeftovers(ipvs utilipvs.Interface, ipt utiliptables.Interface, ipset utilipset.Interface, cleanupIPVS bool) (encounteredError bool) { - if cleanupIPVS { - // Return immediately when ipvs interface is nil - Probably initialization failed in somewhere. - if ipvs == nil { - return true - } - encounteredError = false - err := ipvs.Flush() - if err != nil { - klog.Errorf("Error flushing IPVS rules: %v", err) - encounteredError = true - } +func CleanupLeftovers(ipvs utilipvs.Interface, ipt utiliptables.Interface, ipset utilipset.Interface) (encounteredError bool) { + // Return immediately when ipvs interface is nil - Probably initialization failed in somewhere. + if ipvs == nil { + return true } + encounteredError = false + err := ipvs.Flush() + if err != nil { + klog.Errorf("Error flushing IPVS rules: %v", err) + encounteredError = true + } + // Delete dummy interface created by ipvs Proxier. nl := NewNetLinkHandle(false) - err := nl.DeleteDummyDevice(DefaultDummyDevice) + err = nl.DeleteDummyDevice(DefaultDummyDevice) if err != nil { klog.Errorf("Error deleting dummy device %s created by IPVS proxier: %v", DefaultDummyDevice, err) encounteredError = true From 29ba1b03724ccdaf7f476792a94b73e7a3bd2522 Mon Sep 17 00:00:00 2001 From: Vallery Lancey Date: Thu, 4 Apr 2019 13:09:15 -0700 Subject: [PATCH 3/6] Fixed old function signature in kube-proxy tests. --- pkg/proxy/ipvs/proxier_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/proxy/ipvs/proxier_test.go b/pkg/proxy/ipvs/proxier_test.go index 3862124d47f..5c37aad5c9d 100644 --- a/pkg/proxy/ipvs/proxier_test.go +++ b/pkg/proxy/ipvs/proxier_test.go @@ -267,7 +267,7 @@ func TestCleanupLeftovers(t *testing.T) { fp.syncProxyRules() // test cleanup left over - if CleanupLeftovers(ipvs, ipt, ipset, true) { + if CleanupLeftovers(ipvs, ipt, ipset) { t.Errorf("Cleanup leftovers failed") } } From eff9b4036e279513fc0c7a113c05a03271161e82 Mon Sep 17 00:00:00 2001 From: Vallery Lancey Date: Thu, 4 Apr 2019 16:59:05 -0700 Subject: [PATCH 4/6] Revert "Deprecated --cleanup-ipvs flag in kube-proxy" This reverts commit 4f1bb2bd2fc5b792e8b63986257d2f9bee1f96cf. --- cmd/kube-proxy/app/server.go | 10 +++++++--- pkg/proxy/ipvs/proxier.go | 25 +++++++++++++------------ 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/cmd/kube-proxy/app/server.go b/cmd/kube-proxy/app/server.go index 15a0256a8ea..51f1a2e63f8 100644 --- a/cmd/kube-proxy/app/server.go +++ b/cmd/kube-proxy/app/server.go @@ -100,8 +100,10 @@ type Options struct { ConfigFile string // WriteConfigTo is the path where the default configuration will be written. WriteConfigTo string - // CleanupAndExit, when true, makes the proxy server clean up iptables, ipvs, etc rules, then exit. + // CleanupAndExit, when true, makes the proxy server clean up iptables rules, then exit. CleanupAndExit bool + // CleanupIPVS, when true, makes the proxy server clean up ipvs rules before running. + CleanupIPVS bool // WindowsService should be set to true if kube-proxy is running as a service on Windows. // Its corresponding flag only gets registered in Windows builds WindowsService bool @@ -141,7 +143,7 @@ func (o *Options) AddFlags(fs *pflag.FlagSet) { fs.BoolVar(&o.CleanupAndExit, "cleanup-iptables", o.CleanupAndExit, "If true cleanup iptables and ipvs rules and exit.") fs.MarkDeprecated("cleanup-iptables", "This flag is replaced by --cleanup.") fs.BoolVar(&o.CleanupAndExit, "cleanup", o.CleanupAndExit, "If true cleanup iptables and ipvs rules and exit.") - fs.MarkDeprecated("cleanup-ipvs", "This flag is replaced by --cleanup.") + fs.BoolVar(&o.CleanupIPVS, "cleanup-ipvs", o.CleanupIPVS, "If true make kube-proxy cleanup ipvs rules before running. Default is true") // All flags below here are deprecated and will eventually be removed. @@ -202,6 +204,7 @@ func NewOptions() *Options { metricsPort: ports.ProxyStatusPort, scheme: scheme.Scheme, codecs: scheme.Codecs, + CleanupIPVS: true, errCh: make(chan error), } } @@ -499,6 +502,7 @@ type ProxyServer struct { ProxyMode string NodeRef *v1.ObjectReference CleanupAndExit bool + CleanupIPVS bool MetricsBindAddress string EnableProfiling bool OOMScoreAdj *int32 @@ -555,7 +559,7 @@ func (s *ProxyServer) Run() error { if s.CleanupAndExit { encounteredError := userspace.CleanupLeftovers(s.IptInterface) encounteredError = iptables.CleanupLeftovers(s.IptInterface) || encounteredError - encounteredError = ipvs.CleanupLeftovers(s.IpvsInterface, s.IptInterface, s.IpsetInterface) || encounteredError + encounteredError = ipvs.CleanupLeftovers(s.IpvsInterface, s.IptInterface, s.IpsetInterface, s.CleanupIPVS) || encounteredError if encounteredError { return errors.New("encountered an error while tearing down rules") } diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index 9c88ab0d088..64742868557 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -595,21 +595,22 @@ func cleanupIptablesLeftovers(ipt utiliptables.Interface) (encounteredError bool } // CleanupLeftovers clean up all ipvs and iptables rules created by ipvs Proxier. -func CleanupLeftovers(ipvs utilipvs.Interface, ipt utiliptables.Interface, ipset utilipset.Interface) (encounteredError bool) { - // Return immediately when ipvs interface is nil - Probably initialization failed in somewhere. - if ipvs == nil { - return true +func CleanupLeftovers(ipvs utilipvs.Interface, ipt utiliptables.Interface, ipset utilipset.Interface, cleanupIPVS bool) (encounteredError bool) { + if cleanupIPVS { + // Return immediately when ipvs interface is nil - Probably initialization failed in somewhere. + if ipvs == nil { + return true + } + encounteredError = false + err := ipvs.Flush() + if err != nil { + klog.Errorf("Error flushing IPVS rules: %v", err) + encounteredError = true + } } - encounteredError = false - err := ipvs.Flush() - if err != nil { - klog.Errorf("Error flushing IPVS rules: %v", err) - encounteredError = true - } - // Delete dummy interface created by ipvs Proxier. nl := NewNetLinkHandle(false) - err = nl.DeleteDummyDevice(DefaultDummyDevice) + err := nl.DeleteDummyDevice(DefaultDummyDevice) if err != nil { klog.Errorf("Error deleting dummy device %s created by IPVS proxier: %v", DefaultDummyDevice, err) encounteredError = true From a12e98a3b83a59ef4628c079f07fa483ea3ec52b Mon Sep 17 00:00:00 2001 From: Vallery Lancey Date: Thu, 4 Apr 2019 17:09:39 -0700 Subject: [PATCH 5/6] Revert "Fixed old function signature in kube-proxy tests." This reverts commit 29ba1b03724ccdaf7f476792a94b73e7a3bd2522. --- pkg/proxy/ipvs/proxier_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/proxy/ipvs/proxier_test.go b/pkg/proxy/ipvs/proxier_test.go index 5c37aad5c9d..3862124d47f 100644 --- a/pkg/proxy/ipvs/proxier_test.go +++ b/pkg/proxy/ipvs/proxier_test.go @@ -267,7 +267,7 @@ func TestCleanupLeftovers(t *testing.T) { fp.syncProxyRules() // test cleanup left over - if CleanupLeftovers(ipvs, ipt, ipset) { + if CleanupLeftovers(ipvs, ipt, ipset, true) { t.Errorf("Cleanup leftovers failed") } } From 3c9989258a695fd9cbd543edb0e3d8ed45daafd9 Mon Sep 17 00:00:00 2001 From: Vallery Lancey Date: Thu, 4 Apr 2019 18:23:08 -0700 Subject: [PATCH 6/6] Fixed --cleanup-ipvs help text --- cmd/kube-proxy/app/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/kube-proxy/app/server.go b/cmd/kube-proxy/app/server.go index 51f1a2e63f8..3e42db51d1d 100644 --- a/cmd/kube-proxy/app/server.go +++ b/cmd/kube-proxy/app/server.go @@ -143,7 +143,7 @@ func (o *Options) AddFlags(fs *pflag.FlagSet) { fs.BoolVar(&o.CleanupAndExit, "cleanup-iptables", o.CleanupAndExit, "If true cleanup iptables and ipvs rules and exit.") fs.MarkDeprecated("cleanup-iptables", "This flag is replaced by --cleanup.") fs.BoolVar(&o.CleanupAndExit, "cleanup", o.CleanupAndExit, "If true cleanup iptables and ipvs rules and exit.") - fs.BoolVar(&o.CleanupIPVS, "cleanup-ipvs", o.CleanupIPVS, "If true make kube-proxy cleanup ipvs rules before running. Default is true") + fs.BoolVar(&o.CleanupIPVS, "cleanup-ipvs", o.CleanupIPVS, "If true and --cleanup is specified, kube-proxy will also flush IPVS rules, in addition to normal cleanup.") // All flags below here are deprecated and will eventually be removed.