odo dev: handle port forwarding after pod restart (#5885)

* Move port forwarding to reconcile loop

* Add integration test

* Fix dependencies on cli/dev + remove message to restart odo dev after devfile.yaml changes

* Restart port forwarding when endpoints are modified in Devfile

* Fix setNamespace

* Fix componentName

* Move message back to CLI

* add doc

* Move port forwarding after starting application

* Remove unnecessary dependency to State module

* Fix err

pkg/dev/dev.go

@@ -5,6 +5,9 @@ import (
 	"io"
 
 	"github.com/redhat-developer/odo/pkg/envinfo"
+	"github.com/redhat-developer/odo/pkg/kclient"
+	"github.com/redhat-developer/odo/pkg/portForward"
+	"github.com/redhat-developer/odo/pkg/preference"
 
 	"github.com/devfile/library/pkg/devfile/parser"
 	"k8s.io/klog/v2"
@@ -16,14 +19,25 @@ import (
 )
 
 type DevClient struct {
-	watchClient watch.Client
+	kubernetesClient  kclient.ClientInterface
+	prefClient        preference.Client
+	portForwardClient portForward.Client
+	watchClient       watch.Client
 }
 
 var _ Client = (*DevClient)(nil)
 
-func NewDevClient(watchClient watch.Client) *DevClient {
+func NewDevClient(
+	kubernetesClient kclient.ClientInterface,
+	prefClient preference.Client,
+	portForwardClient portForward.Client,
+	watchClient watch.Client,
+) *DevClient {
 	return &DevClient{
-		watchClient: watchClient,
+		kubernetesClient:  kubernetesClient,
+		prefClient:        prefClient,
+		portForwardClient: portForwardClient,
+		watchClient:       watchClient,
 	}
 }
 
@@ -35,9 +49,13 @@ func (o *DevClient) Start(
 	debug bool,
 	buildCommand string,
 	runCommand string,
+	randomPorts bool,
+	errOut io.Writer,
 ) error {
 	klog.V(4).Infoln("Creating new adapter")
-	adapter, err := adapters.NewComponentAdapter(devfileObj.GetMetadataName(), path, "app", devfileObj, platformContext)
+	adapter, err := adapters.NewComponentAdapter(
+		o.kubernetesClient, o.prefClient, o.portForwardClient,
+		devfileObj.GetMetadataName(), path, "app", devfileObj, platformContext, randomPorts, errOut)
 	if err != nil {
 		return err
 	}

pkg/dev/interface.go

@@ -25,6 +25,8 @@ type Client interface {
 		debug bool,
 		buildCommand string,
 		runCommand string,
+		randomPorts bool,
+		errOut io.Writer,
 	) error
 
 	// Watch watches for any changes to the files under path while ignoring the files/directories in ignorePaths.

pkg/devfile/adapters/helper.go

@@ -2,15 +2,29 @@ package adapters
 
 import (
 	"errors"
+	"io"
+
 	devfileParser "github.com/devfile/library/pkg/devfile/parser"
 	"github.com/redhat-developer/odo/pkg/devfile/adapters/common"
 	"github.com/redhat-developer/odo/pkg/devfile/adapters/kubernetes"
 	"github.com/redhat-developer/odo/pkg/kclient"
+	"github.com/redhat-developer/odo/pkg/portForward"
 	"github.com/redhat-developer/odo/pkg/preference"
 )
 
 // NewComponentAdapter returns a Devfile adapter for the targeted platform
-func NewComponentAdapter(componentName string, context string, appName string, devObj devfileParser.DevfileObj, platformContext interface{}) (common.ComponentAdapter, error) {
+func NewComponentAdapter(
+	kubernetesClient kclient.ClientInterface,
+	prefClient preference.Client,
+	portForwardClient portForward.Client,
+	componentName string,
+	context string,
+	appName string,
+	devObj devfileParser.DevfileObj,
+	platformContext interface{},
+	randomPorts bool,
+	errOut io.Writer,
+) (common.ComponentAdapter, error) {
 
 	adapterContext := common.AdapterContext{
 		ComponentName: componentName,
@@ -23,30 +37,35 @@ func NewComponentAdapter(componentName string, context string, appName string, d
 	if !ok {
 		return nil, errors.New("error retrieving context for Kubernetes")
 	}
-	return createKubernetesAdapter(adapterContext, kc.Namespace)
+	return createKubernetesAdapter(adapterContext, kubernetesClient, prefClient, portForwardClient, kc.Namespace, randomPorts, errOut)
 
 }
 
-func createKubernetesAdapter(adapterContext common.AdapterContext, namespace string) (common.ComponentAdapter, error) {
-	client, err := kclient.New()
-	if err != nil {
-		return nil, err
-	}
-	prefClient, err := preference.NewClient()
-	if err != nil {
-		return nil, err
-	}
-
-	// If a namespace was passed in
+func createKubernetesAdapter(
+	adapterContext common.AdapterContext,
+	kubernetesClient kclient.ClientInterface,
+	prefClient preference.Client,
+	portForwardClient portForward.Client,
+	namespace string,
+	randomPorts bool,
+	errOut io.Writer,
+) (common.ComponentAdapter, error) {
 	if namespace != "" {
-		client.Namespace = namespace
+		kubernetesClient.SetNamespace(namespace)
 	}
-	return newKubernetesAdapter(adapterContext, client, prefClient)
+	return newKubernetesAdapter(adapterContext, kubernetesClient, prefClient, portForwardClient, randomPorts, errOut)
 }
 
-func newKubernetesAdapter(adapterContext common.AdapterContext, client kclient.ClientInterface, prefClient preference.Client) (common.ComponentAdapter, error) {
+func newKubernetesAdapter(
+	adapterContext common.AdapterContext,
+	client kclient.ClientInterface,
+	prefClient preference.Client,
+	portForwardClient portForward.Client,
+	randomPorts bool,
+	errOut io.Writer,
+) (common.ComponentAdapter, error) {
 	// Feed the common metadata to the platform-specific adapter
-	kubernetesAdapter := kubernetes.New(adapterContext, client, prefClient)
+	kubernetesAdapter := kubernetes.New(adapterContext, client, prefClient, portForwardClient, randomPorts, errOut)
 
 	return kubernetesAdapter, nil
 }

pkg/devfile/adapters/helper_test.go

@@ -1,6 +1,7 @@
 package adapters
 
 import (
+	"os"
 	"reflect"
 	"testing"
 
@@ -49,7 +50,7 @@ func TestNewPlatformAdapter(t *testing.T) {
 				Devfile:       devObj,
 			}
 			fkclient, _ := kclient.FakeNew()
-			adapter, err := newKubernetesAdapter(adapterContext, fkclient, nil)
+			adapter, err := newKubernetesAdapter(adapterContext, fkclient, nil, nil, false, os.Stdout)
 			if err != nil {
 				t.Errorf("unexpected error: '%v'", err)
 			}

pkg/devfile/adapters/kubernetes/adapter.go

@@ -2,8 +2,10 @@ package kubernetes
 
 import (
 	"fmt"
+	"io"
 
 	"github.com/redhat-developer/odo/pkg/kclient"
+	"github.com/redhat-developer/odo/pkg/portForward"
 	"github.com/redhat-developer/odo/pkg/preference"
 
 	"github.com/redhat-developer/odo/pkg/devfile/adapters/common"
@@ -22,9 +24,16 @@ type KubernetesContext struct {
 }
 
 // New instantiates a kubernetes adapter
-func New(adapterContext common.AdapterContext, client kclient.ClientInterface, prefClient preference.Client) Adapter {
-
-	compAdapter := component.New(adapterContext, client, prefClient)
+func New(
+	adapterContext common.AdapterContext,
+	client kclient.ClientInterface,
+	prefClient preference.Client,
+	portForwardClient portForward.Client,
+	randomPorts bool,
+	errOut io.Writer,
+) Adapter {
+
+	compAdapter := component.New(adapterContext, client, prefClient, portForwardClient, randomPorts, errOut)
 
 	return Adapter{
 		componentAdapter: &compAdapter,

pkg/devfile/adapters/kubernetes/component/adapter.go

@@ -20,6 +20,7 @@ import (
 	"github.com/redhat-developer/odo/pkg/libdevfile"
 	"github.com/redhat-developer/odo/pkg/log"
 	"github.com/redhat-developer/odo/pkg/machineoutput"
+	"github.com/redhat-developer/odo/pkg/portForward"
 	"github.com/redhat-developer/odo/pkg/preference"
 	"github.com/redhat-developer/odo/pkg/service"
 	storagepkg "github.com/redhat-developer/odo/pkg/storage"
@@ -37,12 +38,22 @@ import (
 )
 
 // New instantiates a component adapter
-func New(adapterContext common.AdapterContext, kubeClient kclient.ClientInterface, prefClient preference.Client) Adapter {
+func New(
+	adapterContext common.AdapterContext,
+	kubeClient kclient.ClientInterface,
+	prefClient preference.Client,
+	portForwardClient portForward.Client,
+	randomPorts bool,
+	errOut io.Writer,
+) Adapter {
 	return Adapter{
-		kubeClient:     kubeClient,
-		prefClient:     prefClient,
-		AdapterContext: adapterContext,
-		logger:         machineoutput.NewMachineEventLoggingClient(),
+		kubeClient:        kubeClient,
+		prefClient:        prefClient,
+		portForwardClient: portForwardClient,
+		AdapterContext:    adapterContext,
+		logger:            machineoutput.NewMachineEventLoggingClient(),
+		randomPorts:       randomPorts,
+		errOut:            errOut,
 	}
 }
 
@@ -75,8 +86,9 @@ func (a *Adapter) ComponentInfo(command devfilev1.Command) (common.ComponentInfo
 
 // Adapter is a component adapter implementation for Kubernetes
 type Adapter struct {
-	kubeClient kclient.ClientInterface
-	prefClient preference.Client
+	kubeClient        kclient.ClientInterface
+	prefClient        preference.Client
+	portForwardClient portForward.Client
 
 	common.AdapterContext
 	logger machineoutput.MachineEventLoggingClient
@@ -87,6 +99,9 @@ type Adapter struct {
 	devfileDebugPort int
 	pod              *corev1.Pod
 	deployment       *appsv1.Deployment
+
+	randomPorts bool
+	errOut      io.Writer
 }
 
 var _ sync.SyncClient = (*Adapter)(nil)
@@ -369,9 +384,21 @@ func (a Adapter) Push(parameters common.PushParameters) (err error) {
 			}
 		}
 		err = libdevfile.ExecuteCommandByNameAndKind(a.Devfile, cmdName, cmdKind, &cmdHandler, false)
+		if err != nil {
+			return err
+		}
+	}
+
+	if podChanged {
+		a.portForwardClient.StopPortForwarding()
 	}
 
-	return err
+	err = a.portForwardClient.StartPortForwarding(a.Devfile, a.ComponentName, a.randomPorts, a.errOut)
+	if err != nil {
+		return fmt.Errorf("fail starting the port forwarding: %w", err)
+	}
+
+	return nil
 }
 
 func (a *Adapter) createOrUpdateComponent(componentExists bool, ei envinfo.EnvSpecificInfo, isMainStorageEphemeral bool) (err error) {

pkg/devfile/adapters/kubernetes/component/adapter_test.go

@@ -3,6 +3,7 @@ package component
 import (
 	"encoding/json"
 	"errors"
+	"os"
 	"reflect"
 	"testing"
 	"time"
@@ -134,7 +135,7 @@ func TestCreateOrUpdateComponent(t *testing.T) {
 				Name:    testComponentName,
 				AppName: testAppName,
 			})
-			componentAdapter := New(adapterCtx, fkclient, nil)
+			componentAdapter := New(adapterCtx, fkclient, nil, nil, false, os.Stdout)
 			err := componentAdapter.createOrUpdateComponent(tt.running, tt.envInfo, false)
 
 			// Checks for unexpected error cases
@@ -347,7 +348,7 @@ func TestDoesComponentExist(t *testing.T) {
 			})
 
 			// DoesComponentExist requires an already started component, so start it.
-			componentAdapter := New(adapterCtx, fkclient, nil)
+			componentAdapter := New(adapterCtx, fkclient, nil, nil, false, os.Stdout)
 			err := componentAdapter.createOrUpdateComponent(false, tt.envInfo, false)
 
 			// Checks for unexpected error cases
@@ -443,7 +444,7 @@ func TestWaitAndGetComponentPod(t *testing.T) {
 			ctrl := gomock.NewController(t)
 			prefClient := preference.NewMockClient(ctrl)
 			prefClient.EXPECT().GetPushTimeout().Return(100 * time.Second)
-			componentAdapter := New(adapterCtx, fkclient, prefClient)
+			componentAdapter := New(adapterCtx, fkclient, prefClient, nil, false, os.Stdout)
 			_, err := componentAdapter.getPod(false)
 
 			// Checks for unexpected error cases

pkg/kclient/interface.go

@@ -117,7 +117,7 @@ type ClientInterface interface {
 	// SetupPortForwarding creates port-forwarding for the pod on the port pairs provided in the
 	// ["<localhost-port>":"<remote-pod-port>"] format. errOut is used by the client-go library to output any errors
 	// encountered while the port-forwarding is running
-	SetupPortForwarding(pod *corev1.Pod, portPairs []string, out io.Writer, errOut io.Writer) error
+	SetupPortForwarding(pod *corev1.Pod, portPairs []string, out io.Writer, errOut io.Writer, stopChan chan struct{}) error
 
 	// projects.go
 	CreateNewProject(projectName string, wait bool) error