From 725aa9656e9514fd10ae419dbdfe7257baf95004 Mon Sep 17 00:00:00 2001
From: Andy Goldstein <agoldste@redhat.com>
Date: Mon, 3 Aug 2015 15:59:49 -0400
Subject: [PATCH] Correctly error when all port forward binds fail

Fix port forwarding code such that if all local binds fail, an error is
returned instead of waiting for an interrupt.
---
 .../unversioned/portforward/portforward.go    |  8 +++--
 .../portforward/portforward_test.go           | 29 +++++++++++++++++--
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/pkg/client/unversioned/portforward/portforward.go b/pkg/client/unversioned/portforward/portforward.go
index 085e35db2b9..5efc21565de 100644
--- a/pkg/client/unversioned/portforward/portforward.go
+++ b/pkg/client/unversioned/portforward/portforward.go
@@ -161,10 +161,12 @@ func (pf *PortForwarder) forward() error {
 	listenSuccess := false
 	for _, port := range pf.ports {
 		err = pf.listenOnPort(&port)
-		if err != nil {
-			glog.Warningf("Unable to listen on port %d: %v", port, err)
+		switch {
+		case err == nil:
+			listenSuccess = true
+		default:
+			glog.Warningf("Unable to listen on port %d: %v", port.Local, err)
 		}
-		listenSuccess = true
 	}
 
 	if !listenSuccess {
diff --git a/pkg/client/unversioned/portforward/portforward_test.go b/pkg/client/unversioned/portforward/portforward_test.go
index cce3049ab02..0300681e76d 100644
--- a/pkg/client/unversioned/portforward/portforward_test.go
+++ b/pkg/client/unversioned/portforward/portforward_test.go
@@ -310,13 +310,13 @@ func TestForwardPorts(t *testing.T) {
 		},
 		{
 			Upgrader: &fakeUpgrader{conn: newFakeUpgradeConnection()},
-			Ports:    []string{"5000", "6000"},
+			Ports:    []string{"5001", "6000"},
 			Send: map[uint16]string{
-				5000: "abcd",
+				5001: "abcd",
 				6000: "ghij",
 			},
 			Receive: map[uint16]string{
-				5000: "1234",
+				5001: "1234",
 				6000: "5678",
 			},
 		},
@@ -398,3 +398,26 @@ func TestForwardPorts(t *testing.T) {
 	}
 
 }
+
+func TestForwardPortsReturnsErrorWhenAllBindsFailed(t *testing.T) {
+	stopChan1 := make(chan struct{}, 1)
+	defer close(stopChan1)
+
+	pf1, err := New(&client.Request{}, &client.Config{}, []string{"5555"}, stopChan1)
+	if err != nil {
+		t.Fatalf("error creating pf1: %v", err)
+	}
+	pf1.upgrader = &fakeUpgrader{conn: newFakeUpgradeConnection()}
+	go pf1.ForwardPorts()
+	<-pf1.Ready
+
+	stopChan2 := make(chan struct{}, 1)
+	pf2, err := New(&client.Request{}, &client.Config{}, []string{"5555"}, stopChan2)
+	if err != nil {
+		t.Fatalf("error creating pf2: %v", err)
+	}
+	pf2.upgrader = &fakeUpgrader{conn: newFakeUpgradeConnection()}
+	if err := pf2.ForwardPorts(); err == nil {
+		t.Fatal("expected non-nil error for pf2.ForwardPorts")
+	}
+}