Repository: flyteorg/flyte (language: python)

Issue:
[Flytectl Feature] flytectl demo cluster - configurable port number
**Problem Description:**

`flytectl demo start` command fails with below error when a developer has `Docker Desktop` which allows us to have a `Kubernetes` local cluster. Kubernetes API server runs on port `6443`. So we have to either turn off local Kubernetes cluster (not a good option) or allow demo cluster to use some other port.

```
{"status":"Status: Downloaded newer image for cr.flyte.org/flyteorg/flyte-sandbox-bundled:sha-1ae254f8683699b68ecddc89d775fc5d39cc3d84"}
🧑‍🏭 booting Flyte-sandbox container
😬 Something went wrong: Failed to start Sandbox container 🐋, Please check your docker client and try again. 
Error: Error response from daemon: driver failed programming external connectivity on endpoint flyte-sandbox (5eaf6609c86e6d818291e2f060e42f8002dfd4e288393da6e01109cfa5a60bd7): listen tcp4 0.0.0.0:6443: bind: address already in use
```

**Requested feature:**
Allow `demo` cluster to use other port preferrably via config file (config.yaml or config-sandbox.yaml or some other file) so that if a developer has some other container / process using the same port, they can change it and quickly get the demo cluster up and running.

**Environment details:**

- Docker Desktop with Kubernetes enabled
- Mac with M1 chip
- Discussion about this issue on slack is [here](https://flyte-org.slack.com/archives/CP2HDHKE1/p1687257726564769)

```
flytectl version  
{
  "App": "flytectl",
  "Build": "0cb2bfa",
  "Version": "0.6.39",
  "BuildTime": "2023-06-20 14:45:22.664715 +0200 CEST m=+0.016518584"
}

```
[Flytectl Feature] flytectl demo cluster - configurable port number
**Problem Description:**

`flytectl demo start` command fails with below error when a developer has `Docker Desktop` which allows us to have a `Kubernetes` local cluster. Kubernetes API server runs on port `6443`. So we have to either turn off local Kubernetes cluster (not a good option) or allow demo cluster to use some other port.

```
{"status":"Status: Downloaded newer image for cr.flyte.org/flyteorg/flyte-sandbox-bundled:sha-1ae254f8683699b68ecddc89d775fc5d39cc3d84"}
🧑‍🏭 booting Flyte-sandbox container
😬 Something went wrong: Failed to start Sandbox container 🐋, Please check your docker client and try again. 
Error: Error response from daemon: driver failed programming external connectivity on endpoint flyte-sandbox (5eaf6609c86e6d818291e2f060e42f8002dfd4e288393da6e01109cfa5a60bd7): listen tcp4 0.0.0.0:6443: bind: address already in use
```

**Requested feature:**
Allow `demo` cluster to use other port preferrably via config file (config.yaml or config-sandbox.yaml or some other file) so that if a developer has some other container / process using the same port, they can change it and quickly get the demo cluster up and running.

**Environment details:**

- Docker Desktop with Kubernetes enabled
- Mac with M1 chip
- Discussion about this issue on slack is [here](https://flyte-org.slack.com/archives/CP2HDHKE1/p1687257726564769)

```
flytectl version  
{
  "App": "flytectl",
  "Build": "0cb2bfa",
  "Version": "0.6.39",
  "BuildTime": "2023-06-20 14:45:22.664715 +0200 CEST m=+0.016518584"
}

```


Discussion / hints:
I do vote in favor of this feature request. Also, `flytectl demo start` seems to insist on serving on `localhost` while I'd need it to serve on `0.0.0.0`, should that also be configurable? 
Hey @jeevb any luck with solving this issue? I can give a try to fix this.
Haven't had a chance to look into this yet. Please feel free to take a crack at it! 🙏 
I can take a look at this feature and will tell you the result soon.
I will create a PR today, I know how to do it.
@Future-Outlier was there a PR for this feature?
> @Future-Outlier was there a PR for this feature?

Nope, you can take it, thank you!
Is there a workaround for this?
This issue seems to be starting when one updates docker / docker desktop version. 
I do vote in favor of this feature request. Also, `flytectl demo start` seems to insist on serving on `localhost` while I'd need it to serve on `0.0.0.0`, should that also be configurable? 
Hey @jeevb any luck with solving this issue? I can give a try to fix this.
Haven't had a chance to look into this yet. Please feel free to take a crack at it! 🙏 
I can take a look at this feature and will tell you the result soon.
I will create a PR today, I know how to do it.
@Future-Outlier was there a PR for this feature?
> @Future-Outlier was there a PR for this feature?

Nope, you can take it, thank you!
Is there a workaround for this?
This issue seems to be starting when one updates docker / docker desktop version. 

Fix patch:
diff --git a/flytectl/cmd/config/subcommand/sandbox/config_flags.go b/flytectl/cmd/config/subcommand/sandbox/config_flags.go
index 32e1423057..4359b5db3c 100755
--- a/flytectl/cmd/config/subcommand/sandbox/config_flags.go
+++ b/flytectl/cmd/config/subcommand/sandbox/config_flags.go
@@ -62,5 +62,6 @@ func (cfg Config) GetPFlagSet(prefix string) *pflag.FlagSet {
 	cmdFlags.BoolVar(&DefaultConfig.Dev, fmt.Sprintf("%v%v", prefix, "dev"), DefaultConfig.Dev, "Optional. Only start minio and postgres in the sandbox.")
 	cmdFlags.BoolVar(&DefaultConfig.DryRun, fmt.Sprintf("%v%v", prefix, "dryRun"), DefaultConfig.DryRun, "Optional. Only print the docker commands to bring up flyte sandbox/demo container.This will still call github api's to get the latest flyte release to use'")
 	cmdFlags.BoolVar(&DefaultConfig.Force, fmt.Sprintf("%v%v", prefix, "force"), DefaultConfig.Force, "Optional. Forcefully delete existing sandbox cluster if it exists.")
+	cmdFlags.StringVar(&DefaultConfig.Port, fmt.Sprintf("%v%v", prefix, "port"), DefaultConfig.Port, "Optional. Specify the port for the Kubernetes in the sandbox.")
 	return cmdFlags
 }
diff --git a/flytectl/cmd/config/subcommand/sandbox/sandbox_config.go b/flytectl/cmd/config/subcommand/sandbox/sandbox_config.go
index 47ae4918d5..a8d097ea46 100644
--- a/flytectl/cmd/config/subcommand/sandbox/sandbox_config.go
+++ b/flytectl/cmd/config/subcommand/sandbox/sandbox_config.go
@@ -1,6 +1,10 @@
 package sandbox
 
-import "github.com/flyteorg/flyte/flytectl/pkg/docker"
+import (
+	"fmt"
+
+	"github.com/flyteorg/flyte/flytectl/pkg/docker"
+)
 
 // Config holds configuration flags for sandbox command.
 type Config struct {
@@ -36,9 +40,18 @@ type Config struct {
 	DryRun bool `json:"dryRun" pflag:",Optional. Only print the docker commands to bring up flyte sandbox/demo container.This will still call github api's to get the latest flyte release to use'"`
 
 	Force bool `json:"force" pflag:",Optional. Forcefully delete existing sandbox cluster if it exists."`
+
+	// Allow user to specify the port for the sandbox
+	Port string `json:"port" pflag:",Optional. Specify the port for the Kubernetes in the sandbox."`
 }
 
 //go:generate pflags Config --default-var DefaultConfig --bind-default-var
 var (
-	DefaultConfig = &Config{}
+	DefaultConfig = &Config{
+		Port: "6443", // Default port for the sandbox
+	}
 )
+
+func (c Config) GetK8sEndpoint() string {
+	return fmt.Sprintf("https://127.0.0.1:%s", c.Port)
+}
diff --git a/flytectl/cmd/demo/demo.go b/flytectl/cmd/demo/demo.go
index 12dcb66fc3..72f0a07ef8 100644
--- a/flytectl/cmd/demo/demo.go
+++ b/flytectl/cmd/demo/demo.go
@@ -6,11 +6,6 @@ import (
 	"github.com/spf13/cobra"
 )
 
-const (
-	flyteNs     = "flyte"
-	K8sEndpoint = "https://127.0.0.1:6443"
-)
-
 // Long descriptions are whitespace sensitive when generating docs using sphinx.
 const (
 	demoShort = `Helps with demo interactions like start, teardown, status, and exec.`
diff --git a/flytectl/cmd/demo/reload.go b/flytectl/cmd/demo/reload.go
index dee3086e35..92f06d77df 100644
--- a/flytectl/cmd/demo/reload.go
+++ b/flytectl/cmd/demo/reload.go
@@ -4,16 +4,14 @@ import (
 	"context"
 	"fmt"
 
+	sandboxCmdConfig "github.com/flyteorg/flyte/flytectl/cmd/config/subcommand/sandbox"
 	cmdCore "github.com/flyteorg/flyte/flytectl/cmd/core"
 	"github.com/flyteorg/flyte/flytectl/pkg/docker"
-	"github.com/flyteorg/flyte/flytectl/pkg/k8s"
-	"github.com/flyteorg/flyte/flytestdlib/logger"
-	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"github.com/flyteorg/flyte/flytectl/pkg/sandbox"
 )
 
 const (
 	internalBootstrapAgent = "flyte-sandbox-bootstrap"
-	labelSelector          = "app.kubernetes.io/name=flyte-binary"
 )
 const (
 	reloadShort = "Power cycle the Flyte executable pod, effectively picking up an updated config."
@@ -73,7 +71,7 @@ func reloadDemoCluster(ctx context.Context, args []string, cmdCtx cmdCore.Comman
 		return err
 	}
 	if useLegacyMethod {
-		return legacyReloadDemoCluster(ctx)
+		return sandbox.LegacyReloadDemoCluster(ctx, sandboxCmdConfig.DefaultConfig)
 	}
 
 	// At this point we know that we are on a modern sandbox, and we can use the
@@ -88,32 +86,3 @@ func reloadDemoCluster(ctx context.Context, args []string, cmdCtx cmdCore.Comman
 
 	return nil
 }
-
-// legacyReloadDemoCluster will kill the flyte binary pod so the new one can pick up a new config file
-func legacyReloadDemoCluster(ctx context.Context) error {
-	k8sClient, err := k8s.GetK8sClient(docker.Kubeconfig, K8sEndpoint)
-	if err != nil {
-		fmt.Println("Could not get K8s client")
-		return err
-	}
-	pi := k8sClient.CoreV1().Pods(flyteNs)
-	podList, err := pi.List(ctx, v1.ListOptions{LabelSelector: labelSelector})
-	if err != nil {
-		fmt.Println("could not list pods")
-		return err
-	}
-	if len(podList.Items) != 1 {
-		return fmt.Errorf("should only have one pod running, %d found, %v", len(podList.Items), podList.Items)
-	}
-	logger.Debugf(ctx, "Found %d pods\n", len(podList.Items))
-	var grace = int64(0)
-	err = pi.Delete(ctx, podList.Items[0].Name, v1.DeleteOptions{
-		GracePeriodSeconds: &grace,
-	})
-	if err != nil {
-		fmt.Printf("Could not delete Flyte pod, old configuration may still be in effect. Err: %s\n", err)
-		return err
-	}
-
-	return nil
-}
diff --git a/flytectl/cmd/demo/start.go b/flytectl/cmd/demo/start.go
index fa3de39101..234d203ca3 100644
--- a/flytectl/cmd/demo/start.go
+++ b/flytectl/cmd/demo/start.go
@@ -20,6 +20,11 @@ Starts the demo cluster without any source code:
 
  flytectl demo start
 
+Starts the demo cluster with different port:
+::
+
+ flytectl demo start --port 6443
+
 Runs a dev cluster, which only has minio and postgres pod.
 ::
 
diff --git a/flytectl/pkg/docker/docker_util.go b/flytectl/pkg/docker/docker_util.go
index f093e3d49a..a495fdc514 100644
--- a/flytectl/pkg/docker/docker_util.go
+++ b/flytectl/pkg/docker/docker_util.go
@@ -134,14 +134,14 @@ func GetSandboxPorts() (map[nat.Port]struct{}, map[nat.Port][]nat.PortBinding, e
 }
 
 // GetDemoPorts will return demo ports
-func GetDemoPorts() (map[nat.Port]struct{}, map[nat.Port][]nat.PortBinding, error) {
+func GetDemoPorts(k8sPort string) (map[nat.Port]struct{}, map[nat.Port][]nat.PortBinding, error) {
 	return nat.ParsePortSpecs([]string{
-		"0.0.0.0:6443:6443",   // K3s API Port
-		"0.0.0.0:30080:30080", // HTTP Port
-		"0.0.0.0:30000:30000", // Registry Port
-		"0.0.0.0:30001:30001", // Postgres Port
-		"0.0.0.0:30002:30002", // Minio API Port (use HTTP port for minio console)
-		"0.0.0.0:30003:30003", // Buildkit Port
+		fmt.Sprintf("0.0.0.0:%s:6443", k8sPort), // K3s API Port
+		"0.0.0.0:30080:30080",                   // HTTP Port
+		"0.0.0.0:30000:30000",                   // Registry Port
+		"0.0.0.0:30001:30001",                   // Postgres Port
+		"0.0.0.0:30002:30002",                   // Minio API Port (use HTTP port for minio console)
+		"0.0.0.0:30003:30003",                   // Buildkit Port
 	})
 }
 
diff --git a/flytectl/pkg/sandbox/reload.go b/flytectl/pkg/sandbox/reload.go
new file mode 100644
index 0000000000..f68b385443
--- /dev/null
+++ b/flytectl/pkg/sandbox/reload.go
@@ -0,0 +1,47 @@
+package sandbox
+
+import (
+	"context"
+	"fmt"
+
+	sandboxCmdConfig "github.com/flyteorg/flyte/flytectl/cmd/config/subcommand/sandbox"
+	"github.com/flyteorg/flyte/flytectl/pkg/docker"
+	"github.com/flyteorg/flyte/flytectl/pkg/k8s"
+	"github.com/flyteorg/flyte/flytestdlib/logger"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+const (
+	flyteNs       = "flyte"
+	labelSelector = "app.kubernetes.io/name=flyte-binary"
+)
+
+// LegacyReloadDemoCluster will kill the flyte binary pod so the new one can pick up a new config file
+func LegacyReloadDemoCluster(ctx context.Context, sandboxConfig *sandboxCmdConfig.Config) error {
+	k8sEndpoint := sandboxConfig.GetK8sEndpoint()
+	k8sClient, err := k8s.GetK8sClient(docker.Kubeconfig, k8sEndpoint)
+	if err != nil {
+		fmt.Println("Could not get K8s client")
+		return err
+	}
+	pi := k8sClient.CoreV1().Pods(flyteNs)
+	podList, err := pi.List(ctx, v1.ListOptions{LabelSelector: labelSelector})
+	if err != nil {
+		fmt.Println("could not list pods")
+		return err
+	}
+	if len(podList.Items) != 1 {
+		return fmt.Errorf("should only have one pod running, %d found, %v", len(podList.Items), podList.Items)
+	}
+	logger.Debugf(ctx, "Found %d pods\n", len(podList.Items))
+	var grace = int64(0)
+	err = pi.Delete(ctx, podList.Items[0].Name, v1.DeleteOptions{
+		GracePeriodSeconds: &grace,
+	})
+	if err != nil {
+		fmt.Printf("Could not delete Flyte pod, old configuration may still be in effect. Err: %s\n", err)
+		return err
+	}
+
+	return nil
+}
diff --git a/flytectl/pkg/sandbox/start.go b/flytectl/pkg/sandbox/start.go
index 6681baf5e1..e638301741 100644
--- a/flytectl/pkg/sandbox/start.go
+++ b/flytectl/pkg/sandbox/start.go
@@ -36,7 +36,6 @@ const (
 	taintEffect          = "NoSchedule"
 	sandboxContextName   = "flyte-sandbox"
 	sandboxDockerContext = "default"
-	K8sEndpoint          = "https://127.0.0.1:6443"
 	sandboxK8sEndpoint   = "https://127.0.0.1:30086"
 	sandboxImageName     = "cr.flyte.org/flyteorg/flyte-sandbox"
 	demoImageName        = "cr.flyte.org/flyteorg/flyte-sandbox-bundled"
@@ -280,12 +279,13 @@ func StartCluster(ctx context.Context, args []string, sandboxConfig *sandboxCmdC
 		return err
 	}
 
+	k8sEndpoint := sandboxConfig.GetK8sEndpoint()
 	if reader != nil {
 		var k8sClient k8s.K8s
 		err = retry.Do(
 			func() error {
 				// This should wait for the kubeconfig file being there.
-				k8sClient, err = k8s.GetK8sClient(docker.Kubeconfig, K8sEndpoint)
+				k8sClient, err = k8s.GetK8sClient(docker.Kubeconfig, k8sEndpoint)
 				return err
 			},
 			retry.Attempts(10),
@@ -299,7 +299,7 @@ func StartCluster(ctx context.Context, args []string, sandboxConfig *sandboxCmdC
 		err = retry.Do(
 			func() error {
 				// Have to get a new client every time because you run into x509 errors if not
-				k8sClient, err = k8s.GetK8sClient(docker.Kubeconfig, K8sEndpoint)
+				k8sClient, err = k8s.GetK8sClient(docker.Kubeconfig, k8sEndpoint)
 				if err != nil {
 					logger.Debugf(ctx, "Error getting K8s client in liveness check %s", err)
 					return err
@@ -398,7 +398,7 @@ func StartClusterForSandbox(ctx context.Context, args []string, sandboxConfig *s
 
 func StartDemoCluster(ctx context.Context, args []string, sandboxConfig *sandboxCmdConfig.Config) error {
 	sandboxImagePrefix := "sha"
-	exposedPorts, portBindings, err := docker.GetDemoPorts()
+	exposedPorts, portBindings, err := docker.GetDemoPorts(sandboxConfig.Port)
 	if err != nil {
 		return err
 	}


Test patch:
diff --git a/flytectl/cmd/config/subcommand/sandbox/config_flags_test.go b/flytectl/cmd/config/subcommand/sandbox/config_flags_test.go
index 8519a75583..436cdad43a 100755
--- a/flytectl/cmd/config/subcommand/sandbox/config_flags_test.go
+++ b/flytectl/cmd/config/subcommand/sandbox/config_flags_test.go
@@ -265,4 +265,18 @@ func TestConfig_SetFlags(t *testing.T) {
 			}
 		})
 	})
+	t.Run("Test_port", func(t *testing.T) {
+
+		t.Run("Override", func(t *testing.T) {
+			testValue := "1"
+
+			cmdFlags.Set("port", testValue)
+			if vString, err := cmdFlags.GetString("port"); err == nil {
+				testDecodeJson_Config(t, fmt.Sprintf("%v", vString), &actual.Port)
+
+			} else {
+				assert.FailNow(t, err.Error())
+			}
+		})
+	})
 }
diff --git a/flytectl/pkg/docker/docker_util_test.go b/flytectl/pkg/docker/docker_util_test.go
index 8decd8824d..a03acab866 100644
--- a/flytectl/pkg/docker/docker_util_test.go
+++ b/flytectl/pkg/docker/docker_util_test.go
@@ -435,7 +435,7 @@ func TestGetOrCreateVolume(t *testing.T) {
 }
 
 func TestDemoPorts(t *testing.T) {
-	_, ports, _ := GetDemoPorts()
+	_, ports, _ := GetDemoPorts("6443")
 	assert.Equal(t, 6, len(ports))
 }