added test

pkg/fwdport/fwdport.go

@@ -188,13 +188,13 @@ func PortForward(pfo *PortForwardOpts) error {
 	// Waiting until the pod is running
 	pod, err := pfo.StateWaiter.WaitUntilPodRunning(downstreamStopChannel)
 	if err != nil {
-		pfo.Stop()
+		pfo.stopAndShutdown()
 		return err
 	} else if pod == nil {
 		// if err is not nil but pod is nil
 		// mean service deleted but pod is not runnning.
 		// No error, just return
-		pfo.Stop()
+		pfo.stopAndShutdown()
 		return nil
 	}
 
@@ -221,7 +221,7 @@ func PortForward(pfo *PortForwardOpts) error {
 
 	fw, err := pfo.PortForwardHelper.NewOnAddresses(dialerWithPing, address, fwdPorts, pfStopChannel, make(chan struct{}), &p, &p)
 	if err != nil {
-		pfo.Stop()
+		pfo.stopAndShutdown()
 		return err
 	}
 
@@ -238,12 +238,19 @@ func PortForward(pfo *PortForwardOpts) error {
 	return nil
 }
 
+// shutdown removes port-forward from ServiceFwd and removes hosts entries if it's necessary
 func (pfo PortForwardOpts) shutdown() {
 	pfo.ServiceFwd.RemoveServicePodByPort(pfo.String(), pfo.PodPort, true)
 	pfo.HostsOperator.RemoveHosts()
 	pfo.HostsOperator.RemoveInterfaceAlias()
 }
 
+// stopAndShutdown is shortcut for closing all downstream channels and shutdown
+func (pfo PortForwardOpts) stopAndShutdown() {
+	pfo.Stop()
+	pfo.shutdown()
+}
+
 //// BuildHostsParams constructs the basic hostnames for the service
 //// based on the PortForwardOpts configuration
 //func (pfo *PortForwardOpts) BuildHostsParams() {

pkg/fwdport/fwdport_test.go

@@ -42,6 +42,7 @@ func TestPortForward_RemovesItselfFromServiceFwd_AfterPortForwardErr(t *testing.
 		PortForwardHelper: pfHelper,
 		HostsOperator:     hostsOperator,
 	}
+	pfErr := errors.New("pf error")
 
 	pfHelper.EXPECT().RoundTripperFor(gomock.Any()).Return(nil, nil, nil)
 	pfHelper.EXPECT().GetPortForwardRequest(gomock.Any()).Return(nil)
@@ -52,18 +53,79 @@ func TestPortForward_RemovesItselfFromServiceFwd_AfterPortForwardErr(t *testing.
 		NewOnAddresses(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
 		Return(nil, nil)
 
-	pfHelper.EXPECT().ForwardPorts(gomock.Any()).Return(errors.New("pf error"))
+	pfHelper.EXPECT().ForwardPorts(gomock.Any()).Return(pfErr)
 
 	svcFwd.EXPECT().RemoveServicePodByPort(gomock.Eq(pfo.String()), gomock.Eq(pfo.PodPort), gomock.Eq(true))
 	hostsOperator.EXPECT().RemoveHosts().Times(1)
 	hostsOperator.EXPECT().RemoveInterfaceAlias().Times(1)
 
 	err := PortForward(pfo)
 	assert.NotNil(t, err)
+	assert.Equal(t, pfErr, err)
 
-	<- pfo.DoneChan
+	<-pfo.DoneChan
+	assertChannelsClosed(t,
+		assertableChannel{ch:   pfo.DoneChan, name: "DoneChan"},
+	)
 }
 
-func TestPortForward_OnlyClosesDownstreamChannels_WhenExternalSignalReceived(t *testing.T) {
+func TestPortForward_OnlyClosesDownstreamChannels_WhenErrorOnWaitUntilPodRunning(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	svcFwd := NewMockServiceFWD(ctrl)
+	waiter := NewMockPodStateWaiter(ctrl)
+	hostsOperator := NewMockHostsOperator(ctrl)
+
+	pfHelper := NewMockPortForwardHelper(ctrl)
+	pfo := &PortForwardOpts{
+		Out: &fwdpub.Publisher{
+			PublisherName: "Services",
+			Output:        false,
+		},
+		Service:           serviceName,
+		ServiceFwd:        svcFwd,
+		PodName:           podName,
+		PodPort:           "8080",
+		HostFile:          nil,
+		LocalPort:         "8080",
+		Namespace:         namespace,
+		ManualStopChan:    make(chan struct{}),
+		DoneChan:          make(chan struct{}),
+		StateWaiter:       waiter,
+		PortForwardHelper: pfHelper,
+		HostsOperator:     hostsOperator,
+	}
+
+	untilPodRunningErr := errors.New("for example, bad credentials error from clientset")
+
+	pfHelper.EXPECT().RoundTripperFor(gomock.Any()).Return(nil, nil, nil)
+	pfHelper.EXPECT().GetPortForwardRequest(gomock.Any()).Return(nil)
+	hostsOperator.EXPECT().AddHosts().Times(1)
+	waiter.EXPECT().WaitUntilPodRunning(gomock.Any()).Return(nil, untilPodRunningErr)
+	svcFwd.EXPECT().RemoveServicePodByPort(gomock.Eq(pfo.String()), gomock.Eq(pfo.PodPort), gomock.Eq(true))
+	hostsOperator.EXPECT().RemoveHosts().Times(1)
+	hostsOperator.EXPECT().RemoveInterfaceAlias().Times(1)
+
+	err := PortForward(pfo)
+	assert.NotNil(t, err)
+	assert.Equal(t, untilPodRunningErr, err)
+
+	<-pfo.DoneChan
+	assertChannelsClosed(t,
+		assertableChannel{ch:   pfo.DoneChan, name: "DoneChan"},
+		assertableChannel{ch: pfo.ManualStopChan, name: "ManualStopChan"},
+	)
+}
+
+func assertChannelsClosed(t *testing.T, channels ...assertableChannel) {
+	for _, assertableCh := range channels {
+		_, open := <-assertableCh.ch
+		assert.False(t, open, "%s must be closed", assertableCh.name)
+	}
+}
 
+type assertableChannel struct {
+	ch chan struct{}
+	name string
 }