Revert GoogleContainerTools#3263

It seems that GoogleContainerTools#3263 makes `skaffold run` freeze
when port forwarding in on.

This reverts commit 647b3c3.
This reverts commit 2fbed4e.
This reverts commit 0abb700.

Signed-off-by: David Gageot <david@gageot.net>

cmd/skaffold/app/cmd/flags.go

@@ -209,7 +209,7 @@ var FlagRegistry = []Flag{
 		Value:         &opts.PortForward.Enabled,
 		DefValue:      false,
 		FlagAddMethod: "BoolVar",
-		DefinedOn:     []string{"dev", "debug", "run"},
+		DefinedOn:     []string{"dev", "debug"},
 	},
 	{
 		Name:          "status-check",

docs/content/en/docs/references/cli/_index.md

@@ -701,7 +701,6 @@ Options:
   -n, --namespace='': Run deployments in the specified namespace
       --no-prune=false: Skip removing images and containers built by Skaffold
       --no-prune-children=false: Skip removing layers reused by Skaffold
-      --port-forward=false: Port-forward exposed container ports within pods
   -p, --profile=[]: Activate profiles by name
       --render-only=false: Print rendered Kubernetes manifests instead of deploying them
       --rpc-http-port=50052: tcp port to expose event REST API over HTTP
@@ -735,7 +734,6 @@ Env vars:
 * `SKAFFOLD_NAMESPACE` (same as `--namespace`)
 * `SKAFFOLD_NO_PRUNE` (same as `--no-prune`)
 * `SKAFFOLD_NO_PRUNE_CHILDREN` (same as `--no-prune-children`)
-* `SKAFFOLD_PORT_FORWARD` (same as `--port-forward`)
 * `SKAFFOLD_PROFILE` (same as `--profile`)
 * `SKAFFOLD_RENDER_ONLY` (same as `--render-only`)
 * `SKAFFOLD_RPC_HTTP_PORT` (same as `--rpc-http-port`)

integration/port_forward_test.go

@@ -55,29 +55,10 @@ func TestPortForward(t *testing.T) {
 	portforward.WhiteBoxPortForwardCycle(t, kubectlCLI, ns.Name)
 }
 
-func TestRunPortForward(t *testing.T) {
-	if testing.Short() || RunOnGCP() {
-		t.Skip("skipping kind integration test")
-	}
-
-	ns, _, deleteNs := SetupNamespace(t)
-	defer deleteNs()
-
-	rpcAddr := randomPort()
-	stop := skaffold.Run("--port-forward", "--rpc-port", rpcAddr, "--enable-rpc").InDir("examples/microservices").InNs(ns.Name).RunBackground(t)
-	defer stop()
-
-	_, entries, shutdown := apiEvents(t, rpcAddr)
-	defer shutdown()
-
-	address, localPort := getLocalPortFromPortForwardEvent(t, entries, "leeroy-app", "service", ns.Name)
-	assertResponseFromPort(t, address, localPort, constants.LeeroyAppResponse)
-}
-
-// TestDevPortForwardDeletePod tests that port forwarding works
+// TestPortForwardDeletePod tests that port forwarding works
 // as expected. Then, the test force deletes a pod,
 // and tests that the pod eventually comes up at the same port again.
-func TestDevPortForwardDeletePod(t *testing.T) {
+func TestPortForwardDeletePod(t *testing.T) {
 	if testing.Short() || RunOnGCP() {
 		t.Skip("skipping kind integration test")
 	}

pkg/skaffold/runner/build_deploy.go

@@ -30,7 +30,6 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 )
 
@@ -78,7 +77,7 @@ func (r *SkaffoldRunner) BuildAndTest(ctx context.Context, out io.Writer, artifa
 
 // DeployAndLog deploys a list of already built artifacts and optionally show the logs.
 func (r *SkaffoldRunner) DeployAndLog(ctx context.Context, out io.Writer, artifacts []build.Artifact) error {
-	if !r.runCtx.Opts.Tail && !r.runCtx.Opts.PortForward.Enabled {
+	if !r.runCtx.Opts.Tail {
 		return r.Deploy(ctx, out, artifacts)
 	}
 
@@ -88,31 +87,19 @@ func (r *SkaffoldRunner) DeployAndLog(ctx context.Context, out io.Writer, artifa
 		r.podSelector.Add(artifact.Tag)
 	}
 
-	if err := r.Deploy(ctx, out, artifacts); err != nil {
-		return err
-	}
+	logger := r.newLoggerForImages(out, imageNames)
+	defer logger.Stop()
 
-	if r.runCtx.Opts.Tail {
-		logger := r.newLoggerForImages(out, imageNames)
-		defer logger.Stop()
+	// Logs should be retrieve up to just before the deploy
+	logger.SetSince(time.Now())
 
-		// Logs should be retrieve up to just before the deploy
-		logger.SetSince(time.Now())
-
-		// Start printing the logs after deploy is finished
-		if err := logger.Start(ctx); err != nil {
-			return errors.Wrap(err, "starting logger")
-		}
+	if err := r.Deploy(ctx, out, artifacts); err != nil {
+		return err
 	}
 
-	if r.runCtx.Opts.PortForward.Enabled {
-		kubectlCLI := kubectl.NewFromRunContext(r.runCtx)
-		r.createForwarder(out, kubectlCLI)
-		defer r.forwarderManager.Stop()
-
-		if err := r.forwarderManager.Start(ctx); err != nil {
-			logrus.Warnln("Error starting port forwarding:", err)
-		}
+	// Start printing the logs after deploy is finished
+	if err := logger.Start(ctx); err != nil {
+		return errors.Wrap(err, "starting logger")
 	}
 
 	<-ctx.Done()