From 2233f01943f5bb0c89dc9f8ff8b3ffd4115aadcf Mon Sep 17 00:00:00 2001 From: Vincent <0426vincent@gmail.com> Date: Thu, 7 Nov 2024 10:32:22 -0800 Subject: [PATCH 1/2] Fix: allows users to specify port for demo cluster Signed-off-by: Vincent <0426vincent@gmail.com> --- .../config/subcommand/sandbox/config_flags.go | 1 + .../subcommand/sandbox/config_flags_test.go | 14 ++++++ .../subcommand/sandbox/sandbox_config.go | 17 ++++++- flytectl/cmd/demo/demo.go | 5 -- flytectl/cmd/demo/reload.go | 37 ++------------- flytectl/cmd/demo/start.go | 4 ++ flytectl/pkg/docker/docker_util.go | 14 +++--- flytectl/pkg/docker/docker_util_test.go | 2 +- flytectl/pkg/sandbox/reload.go | 47 +++++++++++++++++++ flytectl/pkg/sandbox/start.go | 8 ++-- 10 files changed, 96 insertions(+), 53 deletions(-) create mode 100644 flytectl/pkg/sandbox/reload.go diff --git a/flytectl/cmd/config/subcommand/sandbox/config_flags.go b/flytectl/cmd/config/subcommand/sandbox/config_flags.go index 32e1423057..e8a7ca25ab 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 sandbox.") return cmdFlags } 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/cmd/config/subcommand/sandbox/sandbox_config.go b/flytectl/cmd/config/subcommand/sandbox/sandbox_config.go index 47ae4918d5..06eade2b7c 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 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..069b90b85e 100644 --- a/flytectl/cmd/demo/start.go +++ b/flytectl/cmd/demo/start.go @@ -20,6 +20,10 @@ 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/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)) } 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 } From 3048b00230aed5d152bae3d8891bb1ac9960fcac Mon Sep 17 00:00:00 2001 From: Vincent <0426vincent@gmail.com> Date: Thu, 7 Nov 2024 11:36:50 -0800 Subject: [PATCH 2/2] Fix: --help layout Signed-off-by: Vincent <0426vincent@gmail.com> --- flytectl/cmd/demo/start.go | 1 + 1 file changed, 1 insertion(+) diff --git a/flytectl/cmd/demo/start.go b/flytectl/cmd/demo/start.go index 069b90b85e..234d203ca3 100644 --- a/flytectl/cmd/demo/start.go +++ b/flytectl/cmd/demo/start.go @@ -22,6 +22,7 @@ Starts the demo cluster without any source code: Starts the demo cluster with different port: :: + flytectl demo start --port 6443 Runs a dev cluster, which only has minio and postgres pod.