Skip to content

Commit

Permalink
Fix: customize demo cluster port (#5969)
Browse files Browse the repository at this point in the history
* Fix: allows users to specify port for demo cluster

Signed-off-by: Vincent <[email protected]>

* Fix: --help layout

Signed-off-by: Vincent <[email protected]>

* Update flytectl/cmd/config/subcommand/sandbox/sandbox_config.go

Co-authored-by: Kevin Su <[email protected]>
Signed-off-by: 0xffv <[email protected]>

* Fix: regenerate config flags

Signed-off-by: Vincent <[email protected]>

---------

Signed-off-by: Vincent <[email protected]>
Signed-off-by: 0xffv <[email protected]>
Co-authored-by: Kevin Su <[email protected]>
  • Loading branch information
vincent0426 and pingsutw authored Nov 26, 2024
1 parent 172e816 commit 121665d
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 53 deletions.
1 change: 1 addition & 0 deletions flytectl/cmd/config/subcommand/sandbox/config_flags.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions flytectl/cmd/config/subcommand/sandbox/config_flags_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 15 additions & 2 deletions flytectl/cmd/config/subcommand/sandbox/sandbox_config.go
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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)
}
5 changes: 0 additions & 5 deletions flytectl/cmd/demo/demo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.`
Expand Down
37 changes: 3 additions & 34 deletions flytectl/cmd/demo/reload.go
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down Expand Up @@ -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
Expand All @@ -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
}
5 changes: 5 additions & 0 deletions flytectl/cmd/demo/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
::
Expand Down
14 changes: 7 additions & 7 deletions flytectl/pkg/docker/docker_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
}

Expand Down
2 changes: 1 addition & 1 deletion flytectl/pkg/docker/docker_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand Down
47 changes: 47 additions & 0 deletions flytectl/pkg/sandbox/reload.go
Original file line number Diff line number Diff line change
@@ -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
}
8 changes: 4 additions & 4 deletions flytectl/pkg/sandbox/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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),
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 121665d

Please sign in to comment.