From de36967c7e79812ba6f00f6cf90a38e191028e9f Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Thu, 14 May 2015 16:03:30 -0400 Subject: [PATCH] Proxier should return typed errors --- cmd/kube-proxy/app/server.go | 10 +++--- pkg/proxy/proxier.go | 50 ++++++++++++++++---------- pkg/proxy/proxier_test.go | 70 ++++++++++++++++++++++++++++-------- 3 files changed, 92 insertions(+), 38 deletions(-) diff --git a/cmd/kube-proxy/app/server.go b/cmd/kube-proxy/app/server.go index 08e166d0adb..2113955d82b 100644 --- a/cmd/kube-proxy/app/server.go +++ b/cmd/kube-proxy/app/server.go @@ -76,14 +76,14 @@ func (s *ProxyServer) AddFlags(fs *pflag.FlagSet) { func (s *ProxyServer) Run(_ []string) error { // TODO(vmarmol): Use container config for this. if err := util.ApplyOomScoreAdj(0, s.OOMScoreAdj); err != nil { - glog.Info(err) + glog.V(2).Info(err) } // Run in its own container. if err := util.RunInResourceContainer(s.ResourceContainer); err != nil { glog.Warningf("Failed to start in resource-only container %q: %v", s.ResourceContainer, err) } else { - glog.Infof("Running in resource-only container %q", s.ResourceContainer) + glog.V(2).Infof("Running in resource-only container %q", s.ResourceContainer) } serviceConfig := config.NewServiceConfig() @@ -94,9 +94,9 @@ func (s *ProxyServer) Run(_ []string) error { protocol = iptables.ProtocolIpv6 } loadBalancer := proxy.NewLoadBalancerRR() - proxier := proxy.NewProxier(loadBalancer, net.IP(s.BindAddress), iptables.New(exec.New(), protocol)) - if proxier == nil { - glog.Fatalf("failed to create proxier, aborting") + proxier, err := proxy.NewProxier(loadBalancer, net.IP(s.BindAddress), iptables.New(exec.New(), protocol)) + if err != nil { + glog.Fatalf("Unable to create proxer: %v", err) } // Wire proxier to handle changes to services diff --git a/pkg/proxy/proxier.go b/pkg/proxy/proxier.go index 00410e74c17..5022a6f4c05 100644 --- a/pkg/proxy/proxier.go +++ b/pkg/proxy/proxier.go @@ -319,38 +319,50 @@ type Proxier struct { hostIP net.IP } +var ( + // ErrProxyOnLocalhost is returned by NewProxier if the user requests a proxier on + // the loopback address. May be checked for by callers of NewProxier to know whether + // the caller provided invalid input. + ErrProxyOnLocalhost = fmt.Errorf("cannot proxy on localhost") +) + +// IsProxyLocked returns true if the proxy could not acquire the lock on iptables. +func IsProxyLocked(err error) bool { + return strings.Contains(err.Error(), "holding the xtables lock") +} + // NewProxier returns a new Proxier given a LoadBalancer and an address on // which to listen. Because of the iptables logic, It is assumed that there -// is only a single Proxier active on a machine. -func NewProxier(loadBalancer LoadBalancer, listenIP net.IP, iptables iptables.Interface) *Proxier { +// is only a single Proxier active on a machine. An error will be returned if +// the proxier cannot be started due to an invalid ListenIP (loopback) or +// if iptables fails to update or acquire the initial lock. Once a proxier is +// created, it will keep iptables up to date in the background and will not +// terminate if a particular iptables call fails. +func NewProxier(loadBalancer LoadBalancer, listenIP net.IP, iptables iptables.Interface) (*Proxier, error) { if listenIP.Equal(localhostIPv4) || listenIP.Equal(localhostIPv6) { - glog.Errorf("Can't proxy only on localhost - iptables can't do it") - return nil + return nil, ErrProxyOnLocalhost } hostIP, err := util.ChooseHostInterface() if err != nil { - glog.Errorf("Failed to select a host interface: %v", err) - return nil + return nil, fmt.Errorf("failed to select a host interface: %v", err) } - glog.Infof("Setting Proxy IP to %v", hostIP) + + glog.V(2).Infof("Setting proxy IP to %v and initializing iptables", hostIP) return createProxier(loadBalancer, listenIP, iptables, hostIP) } -func createProxier(loadBalancer LoadBalancer, listenIP net.IP, iptables iptables.Interface, hostIP net.IP) *Proxier { - glog.Infof("Initializing iptables") +func createProxier(loadBalancer LoadBalancer, listenIP net.IP, iptables iptables.Interface, hostIP net.IP) (*Proxier, error) { // Clean up old messes. Ignore erors. iptablesDeleteOld(iptables) // Set up the iptables foundations we need. if err := iptablesInit(iptables); err != nil { - glog.Errorf("Failed to initialize iptables: %v", err) - return nil + return nil, fmt.Errorf("failed to initialize iptables: %v", err) } // Flush old iptables rules (since the bound ports will be invalid after a restart). // When OnUpdate() is first called, the rules will be recreated. if err := iptablesFlush(iptables); err != nil { - glog.Errorf("Failed to flush iptables: %v", err) - return nil + return nil, fmt.Errorf("failed to flush iptables: %v", err) } return &Proxier{ loadBalancer: loadBalancer, @@ -358,7 +370,7 @@ func createProxier(loadBalancer LoadBalancer, listenIP net.IP, iptables iptables listenIP: listenIP, iptables: iptables, hostIP: hostIP, - } + }, nil } // The periodic interval for checking the state of things. @@ -370,7 +382,7 @@ func (proxier *Proxier) SyncLoop() { defer t.Stop() for { <-t.C - glog.V(3).Infof("Periodic sync") + glog.V(6).Infof("Periodic sync") if err := iptablesInit(proxier.iptables); err != nil { glog.Errorf("Failed to ensure iptables: %v", err) } @@ -453,7 +465,7 @@ func (proxier *Proxier) addServiceOnPort(service ServicePortName, protocol api.P } proxier.setServiceInfo(service, si) - glog.V(1).Infof("Proxying for service %q on %s port %d", service, protocol, portNum) + glog.V(2).Infof("Proxying for service %q on %s port %d", service, protocol, portNum) go func(service ServicePortName, proxier *Proxier) { defer util.HandleCrash() atomic.AddInt32(&proxier.numProxyLoops, 1) @@ -592,7 +604,7 @@ func (proxier *Proxier) openOnePortal(portalIP net.IP, portalPort int, protocol return err } if !existed { - glog.Infof("Opened iptables from-containers portal for service %q on %s %s:%d", name, protocol, portalIP, portalPort) + glog.V(3).Infof("Opened iptables from-containers portal for service %q on %s %s:%d", name, protocol, portalIP, portalPort) } // Handle traffic from the host. @@ -603,7 +615,7 @@ func (proxier *Proxier) openOnePortal(portalIP net.IP, portalPort int, protocol return err } if !existed { - glog.Infof("Opened iptables from-host portal for service %q on %s %s:%d", name, protocol, portalIP, portalPort) + glog.V(3).Infof("Opened iptables from-host portal for service %q on %s %s:%d", name, protocol, portalIP, portalPort) } return nil } @@ -615,7 +627,7 @@ func (proxier *Proxier) closePortal(service ServicePortName, info *serviceInfo) el = append(el, proxier.closeOnePortal(net.ParseIP(publicIP), info.portalPort, info.protocol, proxier.listenIP, info.proxyPort, service)...) } if len(el) == 0 { - glog.Infof("Closed iptables portals for service %q", service) + glog.V(3).Infof("Closed iptables portals for service %q", service) } else { glog.Errorf("Some errors closing iptables portals for service %q", service) } diff --git a/pkg/proxy/proxier_test.go b/pkg/proxy/proxier_test.go index 32100e5a17f..7e9c52039f2 100644 --- a/pkg/proxy/proxier_test.go +++ b/pkg/proxy/proxier_test.go @@ -206,7 +206,10 @@ func TestTCPProxy(t *testing.T) { }, }) - p := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + p, err := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + if err != nil { + t.Fatal(err) + } waitForNumProxyLoops(t, p, 0) svcInfo, err := p.addServiceOnPort(service, "TCP", 0, time.Second) @@ -230,7 +233,10 @@ func TestUDPProxy(t *testing.T) { }, }) - p := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + p, err := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + if err != nil { + t.Fatal(err) + } waitForNumProxyLoops(t, p, 0) svcInfo, err := p.addServiceOnPort(service, "UDP", 0, time.Second) @@ -259,7 +265,10 @@ func TestMultiPortProxy(t *testing.T) { }}, }}) - p := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + p, err := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + if err != nil { + t.Fatal(err) + } waitForNumProxyLoops(t, p, 0) svcInfoP, err := p.addServiceOnPort(serviceP, "TCP", 0, time.Second) @@ -283,7 +292,10 @@ func TestMultiPortOnUpdate(t *testing.T) { serviceQ := ServicePortName{types.NamespacedName{"testnamespace", "echo"}, "q"} serviceX := ServicePortName{types.NamespacedName{"testnamespace", "echo"}, "x"} - p := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + p, err := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + if err != nil { + t.Fatal(err) + } waitForNumProxyLoops(t, p, 0) p.OnUpdate([]api.Service{{ @@ -343,7 +355,10 @@ func TestTCPProxyStop(t *testing.T) { }, }) - p := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + p, err := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + if err != nil { + t.Fatal(err) + } waitForNumProxyLoops(t, p, 0) svcInfo, err := p.addServiceOnPort(service, "TCP", 0, time.Second) @@ -378,7 +393,10 @@ func TestUDPProxyStop(t *testing.T) { }, }) - p := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + p, err := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + if err != nil { + t.Fatal(err) + } waitForNumProxyLoops(t, p, 0) svcInfo, err := p.addServiceOnPort(service, "UDP", 0, time.Second) @@ -413,7 +431,10 @@ func TestTCPProxyUpdateDelete(t *testing.T) { }, }) - p := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + p, err := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + if err != nil { + t.Fatal(err) + } waitForNumProxyLoops(t, p, 0) svcInfo, err := p.addServiceOnPort(service, "TCP", 0, time.Second) @@ -447,7 +468,10 @@ func TestUDPProxyUpdateDelete(t *testing.T) { }, }) - p := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + p, err := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + if err != nil { + t.Fatal(err) + } waitForNumProxyLoops(t, p, 0) svcInfo, err := p.addServiceOnPort(service, "UDP", 0, time.Second) @@ -481,7 +505,10 @@ func TestTCPProxyUpdateDeleteUpdate(t *testing.T) { }, }) - p := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + p, err := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + if err != nil { + t.Fatal(err) + } waitForNumProxyLoops(t, p, 0) svcInfo, err := p.addServiceOnPort(service, "TCP", 0, time.Second) @@ -530,7 +557,10 @@ func TestUDPProxyUpdateDeleteUpdate(t *testing.T) { }, }) - p := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + p, err := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + if err != nil { + t.Fatal(err) + } waitForNumProxyLoops(t, p, 0) svcInfo, err := p.addServiceOnPort(service, "UDP", 0, time.Second) @@ -579,7 +609,10 @@ func TestTCPProxyUpdatePort(t *testing.T) { }, }) - p := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + p, err := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + if err != nil { + t.Fatal(err) + } waitForNumProxyLoops(t, p, 0) svcInfo, err := p.addServiceOnPort(service, "TCP", 0, time.Second) @@ -624,7 +657,10 @@ func TestUDPProxyUpdatePort(t *testing.T) { }, }) - p := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + p, err := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + if err != nil { + t.Fatal(err) + } waitForNumProxyLoops(t, p, 0) svcInfo, err := p.addServiceOnPort(service, "UDP", 0, time.Second) @@ -666,7 +702,10 @@ func TestProxyUpdatePublicIPs(t *testing.T) { }, }) - p := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + p, err := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + if err != nil { + t.Fatal(err) + } waitForNumProxyLoops(t, p, 0) svcInfo, err := p.addServiceOnPort(service, "TCP", 0, time.Second) @@ -715,7 +754,10 @@ func TestProxyUpdatePortal(t *testing.T) { }, }) - p := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + p, err := createProxier(lb, net.ParseIP("0.0.0.0"), &fakeIptables{}, net.ParseIP("127.0.0.1")) + if err != nil { + t.Fatal(err) + } waitForNumProxyLoops(t, p, 0) svcInfo, err := p.addServiceOnPort(service, "TCP", 0, time.Second)